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
Use DataUpdateCoordinator for mikrotik
#72954
Use DataUpdateCoordinator for mikrotik
#72954
Conversation
…rotik-dataupdatecoordinator
Appreciate getting this merged if there are no other comments. |
I'm a little worried about the performance impact here as DataUpdateCoordinator calls back all entities each update so you'll go from writing state for a single entity via the signal to writing state for every entity every time there is an update. That might be a performance problem. |
Currently the signal is causing all entities to update. So by changing to the DataUpdateCoordinator we are not changing this. |
That does seem to be the case. I read So by changing to the DataUpdateCoordinator we are not changing this.
Do you have a way of knowing which ones actually changed so you could only signal the ones that have changed? We use something like this to only signal/callback entities that have changes to avoid lots of state writes when there are many entities |
Let me look into it and see if it is possible. |
I tried checking other integrations that have |
async def async_update(self): | ||
"""Update Mikrotik devices information.""" | ||
await self.hass.async_add_executor_job(self._mk_data.update) | ||
async_dispatcher_send(self.hass, self.signal_update) | ||
await self.hass.async_add_executor_job(self._mk_data.update_devices) | ||
|
||
async def async_setup(self): |
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.
In a future PR it would be good to move to into __init__.py
since it makes it harder to see what is going on at startup with having it on the coordinator object
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.
Please address the comments in a new PR. Thanks!
Breaking change
Proposed change
Use DataUpdateCoordinator for
mikrotik
Type of change
Additional information
Checklist
black --fast 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
..coveragerc
.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: