-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
Add own coordinator to devolo_home_network #128159
Conversation
Hey there @2Fake, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PARALLEL_UPDATES one is a blocker because the integration is platinum. afaiw, the parallel_updates is also used with service calls
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Putting to draft for the parallel updates |
I had to add a pylint disable to the device_tracker.py (is this rule new?). Moving the entity to the entity module as suggested would more or less empty the device_tracker.py and hide stuff, that belongs into the device_tracker.py IMHO. |
class DevoloScannerEntity( # pylint: disable=hass-enforce-class-module | ||
CoordinatorEntity[DevoloDataUpdateCoordinator[list[ConnectedStationInfo]]], | ||
ScannerEntity, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ScannerEntity
should take precedence over CoordinatorEntity
, and so pylint disable shouldn't be needed.
Is this branch up-to-date with latest pylint plugin?
class DevoloScannerEntity( # pylint: disable=hass-enforce-class-module | |
CoordinatorEntity[DevoloDataUpdateCoordinator[list[ConnectedStationInfo]]], | |
ScannerEntity, | |
class DevoloScannerEntity( | |
CoordinatorEntity[DevoloDataUpdateCoordinator[list[ConnectedStationInfo]]], | |
ScannerEntity, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Obviously not ... I did a rebase and now it is working 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is weird - it's now causing issue in CI - but I can't replicate locally.
Could you please add a follow-up PR with the pylint disable and an extra comment linking to pylint-dev/pylint#9138 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: #129429
aebda11
to
bfbeb14
Compare
Proposed change
The fix I tried in #122847 doesn't work. It seems I have to coordinate the coordinators, which I try by introducing an own coordinator that is tied to a semaphore.
Type of change
Additional information
Checklist
ruff format homeassistant tests
)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest
.requirements_all.txt
.Updated by running
python3 -m script.gen_requirements_all
.To help with the load of incoming pull requests: