Skip to content

Create miele devices dynamically #143804

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

Draft
wants to merge 4 commits into
base: dev
Choose a base branch
from

Conversation

astrandb
Copy link
Contributor

If new devices appear in the cloud account, they are automatically added to the integration.

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

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:
  • Link to developer documentation pull request:
  • Link to frontend pull request:

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:

@home-assistant
Copy link

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.

@home-assistant home-assistant bot marked this pull request as draft April 28, 2025 06:46
@astrandb astrandb marked this pull request as ready for review April 28, 2025 09:11
@home-assistant home-assistant bot requested a review from zweckj April 28, 2025 09:11
if new_devices:
self.known_devices.update(new_devices)
for callback in self.new_device_callbacks:
callback(
Copy link
Member

Choose a reason for hiding this comment

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

I think we should test those (e.g. that really all entities get added automatically and we didn't miss/mess up a platform). Set up the integration, add a device, let the freezer tick, then check we have the new devices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did a quick test that all intended platforms are loaded by the default data fixture. To add an extra device after a delay needs some more thinking and I don't think I can make that before beta cut.

Copy link
Member

Choose a reason for hiding this comment

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

Is it really that complicated? Don't you just need to add a device in the get_devices fixture dict (e.g. by copying one of those in there and changing the name&id) after integration setup, then let the freezer tick?

Copy link
Contributor Author

@astrandb astrandb Apr 28, 2025

Choose a reason for hiding this comment

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

It might not be complicated, but it is beyond my comfort zone... And I have some other commitments away from my desk tomorrow.

You mean that I first load default 4_devices.json and then after setup the next get_devices() should get another file (5_devices.json)? Can you point me at an example?

Copy link
Member

Choose a reason for hiding this comment

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

I don’t think you'll even need a full fixture file just basically get_devices.return_value["deviceB"] = get_devices.return_value["deviceA"] and then adapt its identifier to make sure we have unique ids. I think I'm doing something like this in tedee

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am looking into this PR now. There is a race problem in that coordinator.data must be complete with new data before kicking off the platform callbacks. It works fine directly after setup but not when I find added devices within async_update_data(). I have to find a way to move the hunt for new devices a bit later in the chain.

Copy link
Member

Choose a reason for hiding this comment

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

you can just write to self.data during async_update_data if that helps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made it work nice and clean IRL. I have issues with the tests though. If I run a single test with pytest ... -k test_function, it works as intended, but when I run a whole file or all of the tests for the integration it will fail. It seems to be related to the loading of alternate fixture files. It works for a single test, but the alternate is not loaded if other tests have been run before.

@zweckj
Copy link
Member

zweckj commented Apr 28, 2025

There are conflicts and I was talking about the dynamic part: If you add an additional device after the initial setup, still all entities for that new device get added correctly after the initial setup

@zweckj zweckj marked this pull request as draft April 28, 2025 13:57
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.

2 participants