Add DataUpdateCoordinator to bmw_connected_drive #67003
Conversation
|
Hey there @gerard33, mind taking a look at this pull request as it has been labeled with an integration ( |
rikroe
force-pushed
the
bmw-updatecoordinator
branch
from
9400b17
to
1851055
Compare
rikroe
force-pushed
the
bmw-updatecoordinator
branch
from
1851055
to
2ca61cb
Compare
rikroe
force-pushed
the
bmw-updatecoordinator
branch
2 times, most recently
from
5020047
to
a445592
Compare
rikroe
force-pushed
the
bmw-updatecoordinator
branch
from
5a540e8
to
2bd8d50
Compare
|
Just to be clear: The desired way of refreshing I find it adds quite some verbosity, but I'll happily adhere if thats the desired path! Didn't change EDIT: Based on your input, I revisited and refactored the binary sensors - here it does make it much clearer imo. |
| "door_lock_state": vehicle_state.door_lock_state.value, | ||
| "last_update_reason": vehicle_state.last_update_reason, |
There was a problem hiding this comment.
For a future PR since this is existing:
It looks like these should be sensor entities
https://developers.home-assistant.io/docs/core/entity/?_highlight=extra_state_attributes
There was a problem hiding this comment.
I know, sorry. It regularly comes up but haven't had the chance to go give them a good clean up.
There was a problem hiding this comment.
Thanks for the reminder, thats on my todo list once the bigger backend changes are done!
|
Added the breaking change paragraph + now logging a warning. Also created a docs PR. |
|
|
||
|
|
||
| class BMWConnectedDriveBaseEntity(Entity): | ||
| class BMWConnectedDriveBaseEntity(CoordinatorEntity, Entity): |
There was a problem hiding this comment.
We don't need to inherit from Entity. The CoordinatorEntity already does that.
| """Fetch data from BMW.""" | ||
| try: | ||
| async with async_timeout.timeout(15): | ||
| if isinstance(self.account, ConnectedDriveAccount): |
There was a problem hiding this comment.
It's faster to check for None than using isinstance. Please invert the check.
| @property | ||
| def extra_state_attributes(self) -> dict: | ||
| """Return entity specific state attributes.""" | ||
| return dict(self._attrs, **{ATTR_DIRECTION: self.vehicle.status.gps_heading}) |
There was a problem hiding this comment.
return {**self._attrs, ATTR_DIRECTION: self.vehicle.status.gps_heading}Or now in Python 3.9 we can do:
return self._attrs | {ATTR_DIRECTION: self.vehicle.status.gps_heading}| state = getattr(self._vehicle.status, self.entity_description.key) | ||
| return cast(StateType, self.entity_description.value(state, self.hass)) | ||
| @callback | ||
| def _handle_coordinator_update(self) -> None: |
There was a problem hiding this comment.
As we can see it's often more succinct to implement the entity state property than overriding _handle_coordinator_update in coordinator entities.
Breaking change
The
button.<your_vehicle_refreshentity is deprecated and will be removed in a future version. Please use thehomeassistant.update_entityservice with any BMW entity to force-refresh all platforms from the BMW API.Proposed change
Adds a
DataUpdateCoordinatortobmw_connected_drive.This leads to drastically reduced API calls, as before each platform would call the API twice every 5 minutes. Now, only the coordinator does the required two requests.
Main changes:
setup_account()(i.e. all library-related code) from__init__.pytocoordinator.pyBMWConnectedDriveAccountwithBMWDataUpdateCoordinatorin all platformsRemove allupdate()methods in platforms and switch to propertiesupdate()methods in platforms with__handle_coordinator_update()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: