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

Convert Anova to cloud push #109508

Merged
merged 28 commits into from
May 8, 2024

Conversation

Lash-L
Copy link
Contributor

@Lash-L Lash-L commented Feb 3, 2024

Breaking change

The State and Mode entities for Anova devices have been changed to match the new protocol messaging. Any automations based on these entities will need to be updated.

Proposed change

This changes Anova to use websockets instead of the old REST api. It should allow support for a bunch of different devices including the Anova Oven.

Package update has to be included with this PR and not done separately as this is a major internal refactor.
Lash-L/anova_wifi@v0.10.0...v0.12.0

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.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

homeassistant/components/anova/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/anova/__init__.py Show resolved Hide resolved
homeassistant/components/anova/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/anova/coordinator.py Outdated Show resolved Hide resolved
homeassistant/components/anova/sensor.py Outdated Show resolved Hide resolved
@@ -49,11 +52,15 @@ class AnovaSensorEntityDescription(
AnovaSensorEntityDescription(
key="state",
translation_key="state",
device_class=SensorDeviceClass.ENUM,
options=[state.name for state in AnovaState],
Copy link
Member

Choose a reason for hiding this comment

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

What should we do in the case of "no_state"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That should be fine I think. "no_state" should get turned into "No state" through translations. It's why I use name instead of value.

I create the stateby doing AnovaState(response_from_api) - when state is "" - it sets state = AnovaState.no_state

Then the value is saved to AnovaUpdateSensor as the state.name

Comment on lines 120 to 122
if f"{DOMAIN}.{SENSOR_DOMAIN}_{sensor.unique_id}" in ent_registry.entities:
# If the entity has been added before - we know it is supported.
valid_entities.add(sensor)
Copy link
Member

Choose a reason for hiding this comment

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

Why do we do this?

Why don't remove the non working ones and just setup like normal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it's less about them not working. This is just a means of adding the existing entities even when the device is offline. Since the entities will exist, when the websocket ends up sending update data - the sensors should automatically update.

Basically - if we have added an entity before - it is supported and we should add it again, even if the device is offline.

homeassistant/components/anova/sensor.py Outdated Show resolved Hide resolved
homeassistant/components/anova/sensor.py Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft February 3, 2024 15:09
@home-assistant
Copy link

home-assistant bot commented Feb 3, 2024

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@Lash-L Lash-L marked this pull request as ready for review February 3, 2024 18:24
homeassistant/components/anova/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/anova/sensor.py Show resolved Hide resolved
homeassistant/components/anova/sensor.py Show resolved Hide resolved
tests/components/anova/test_init.py Outdated Show resolved Hide resolved
tests/components/anova/test_sensor.py Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft February 13, 2024 09:46
@Lash-L Lash-L marked this pull request as ready for review February 26, 2024 23:12
@Lash-L
Copy link
Contributor Author

Lash-L commented Apr 9, 2024

merging to attempt to fix failed test

homeassistant/components/anova/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/anova/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/anova/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/anova/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/anova/config_flow.py Outdated Show resolved Hide resolved
tests/components/anova/conftest.py Outdated Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft April 24, 2024 17:59
Lash-L and others added 2 commits April 24, 2024 16:07
Co-authored-by: Erik Montnemery <erik@montnemery.com>
@Lash-L Lash-L marked this pull request as ready for review April 25, 2024 01:26
Copy link
Contributor

@emontnemery emontnemery left a comment

Choose a reason for hiding this comment

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

Just fix the check for api.websocket_handler, then this PR is good to go 👍

@home-assistant home-assistant bot marked this pull request as draft April 25, 2024 06:45
@Lash-L
Copy link
Contributor Author

Lash-L commented Apr 25, 2024

Just fix the check for api.websocket_handler, then this PR is good to go 👍

Done! Also gave it a small version bump to do a bit more error catching in ws creation.

Thank you for the review!

@Lash-L Lash-L marked this pull request as ready for review April 25, 2024 13:06
@emontnemery emontnemery dismissed stale reviews from edenhaus and joostlek May 8, 2024 12:44

Stale

@emontnemery emontnemery merged commit 22bc11f into home-assistant:dev May 8, 2024
34 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Anova precision cooker not found
4 participants