Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Flick Electric API #133475

Open
wants to merge 25 commits into
base: dev
Choose a base branch
from

Conversation

ZephireNZ
Copy link
Contributor

Proposed change

Updates integration for flick_electric due to downstream API being turned off.

ZephireNZ/PyFlick#2

The main breaking change here is that we now have multiple accounts/services returned, so the config flow has been updated to take this into account. I have also added a migration so that users will be prompted to select an account when more than one exists.

This also comes with some QoL improvements to the integration:

  • Use Data Coordinator
  • Use typed ConfigEntry
  • Added a reauth flow

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

Copy link
Member

@joostlek joostlek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it was draft, I did a review.

Some remarks that we want to have in:

  • We want to have tests for the entry migration. They are quite delicate, so we want it to do what we want
  • Config flow requires 100% coverage

homeassistant/components/flick_electric/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/flick_electric/sensor.py Outdated Show resolved Hide resolved
Comment on lines 50 to 57
@property
def extra_state_attributes(self):
def extra_state_attributes(self) -> dict[str, Any] | None:
"""Return the state attributes."""
return self._attributes

async def async_update(self) -> None:
"""Get the Flick Pricing data from the web service."""
if self._price and self._price.end_at >= utcnow():
return # Power price data is still valid

async with asyncio.timeout(60):
self._price = await self._api.getPricing()

_LOGGER.debug("Pricing data: %s", self._price)

self._attributes[ATTR_START_AT] = self._price.start_at
self._attributes[ATTR_END_AT] = self._price.end_at
for component in self._price.components:
if component.charge_setter not in ATTR_COMPONENTS:
_LOGGER.warning("Found unknown component: %s", component.charge_setter)
continue

self._attributes[component.charge_setter] = float(component.value)
return {
ATTR_START_AT: self.coordinator.data.start_at,
ATTR_END_AT: self.coordinator.data.end_at,
# TODO: Components
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another remark that's not for this PR, but ideally we try to split out attributes into their own entities or use action responses for things that return large amounts of data.

@joostlek
Copy link
Member

Did you also discuss when the old API would be removed? We might do another patch release before the weekend, otherwise the next release would be 3 januari

@ZephireNZ
Copy link
Contributor Author

Did you also discuss when the old API would be removed? We might do another patch release before the weekend, otherwise the next release would be 3 januari

From the email thread:

We'll be decommissioning the old endpoints before the end of the year

I am almost there in terms of the library itself, just need to see the structure of the "components" section, I've asked James to help with a test account with active services (unfortunately the one provided was disconnected quite a while ago) 😊

@joostlek
Copy link
Member

Check! Let's try to get this in swiftly. Feel free to ping me on discord if you have questions or remarks!

@ZephireNZ ZephireNZ changed the title Feature/flick new api Update Flick Electric API Dec 18, 2024
@ZephireNZ ZephireNZ marked this pull request as ready for review December 20, 2024 04:22
@ZephireNZ
Copy link
Contributor Author

ZephireNZ commented Dec 20, 2024

@joostlek this is now ready for review 😊

I still haven't been able to get a full end-to-end test done, hoping James can get me some UAT credentials which are active.

However I am feeling fairly confident my changes should be functional based on the sample API responses he has sent me.

@ZephireNZ
Copy link
Contributor Author

ZephireNZ commented Dec 20, 2024

@joostlek FYI James confirmed Jan release should be fine so don't worry too much about getting it in with the last minor release 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FlickElectric integration about to stop working
2 participants