Roborock Q10 (B01) part 1 vacuum entity#163112
Roborock Q10 (B01) part 1 vacuum entity#163112lboue wants to merge 52 commits intohome-assistant:devfrom
Conversation
|
Hey there @Lash-L, @allenporter, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
lboue
changed the title
…ockB01SelectEntity
| base_url=entry.data[CONF_BASE_URL], | ||
| ) | ||
| cache = CacheStore(hass, entry.entry_id) | ||
| try: |
There was a problem hiding this comment.
Changes to init should probably be in a separate PR. Also the approach for error message handling by string is not following the existing design patterns, and having some separate exception types if needed would be more appropriate. (e.g. raising a specific exception when rate limited)
There was a problem hiding this comment.
Even adding RoborockB01Q10UpdateCoordinator?
There was a problem hiding this comment.
No, adding the coordinator is required, and is OK. I just meant everything else: Changing prefer_cache or changing the exception handling behavior are independent of adding a new device I believe.
…trations and simplifying data checks
…peed refactor, restore Q7 service stubs, compile translations, all tests passing
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
homeassistant/components/roborock/vacuum.py:497
- Missing
await self.coordinator.async_refresh()call afterasync_return_to_basecommand. All other state-changing commands in this class (async_start, async_pause, async_stop, async_locate, async_set_fan_speed) call async_refresh() after executing their commands to update the coordinator's state. This method should follow the same pattern for consistency.
async def async_return_to_base(self, **kwargs: Any) -> None:
"""Send vacuum back to base."""
try:
await self.coordinator.api.return_to_dock()
except RoborockException as err:
raise HomeAssistantError(
translation_domain=DOMAIN,
translation_key="command_failed",
translation_placeholders={
"command": "return_to_dock",
},
) from err
|
I cleaned up and moved the code concerning Q7 to another PR. |
| class RoborockB01Q10UpdateCoordinator(DataUpdateCoordinator[dict[B01_Q10_DP, Any]]): | ||
| """Coordinator for B01 Q10 devices.""" | ||
|
|
||
| config_entry: RoborockConfigEntry | ||
|
|
||
| def __init__( | ||
| self, | ||
| hass: HomeAssistant, | ||
| config_entry: RoborockConfigEntry, | ||
| device: RoborockDevice, | ||
| api: Q7PropertiesApi, | ||
| api: Q10PropertiesApi, | ||
| ) -> None: | ||
| """Initialize.""" | ||
| super().__init__(hass, config_entry, device) | ||
| super().__init__( | ||
| hass, | ||
| _LOGGER, | ||
| config_entry=config_entry, | ||
| name=DOMAIN, | ||
| update_interval=A01_UPDATE_INTERVAL, | ||
| ) | ||
| self._device = device | ||
| self.device_info = DeviceInfo( | ||
| name=device.name, | ||
| identifiers={(DOMAIN, device.duid)}, | ||
| manufacturer="Roborock", | ||
| model=device.product.model, | ||
| sw_version=device.device_info.fv, | ||
| ) | ||
| self.api = api | ||
| self.request_protocols: list[RoborockB01Props] = [ | ||
| RoborockB01Props.STATUS, | ||
| RoborockB01Props.MAIN_BRUSH, | ||
| RoborockB01Props.SIDE_BRUSH, | ||
| RoborockB01Props.DUST_BAG_USED, | ||
| RoborockB01Props.MOP_LIFE, | ||
| RoborockB01Props.MAIN_SENSOR, | ||
| RoborockB01Props.CLEANING_TIME, | ||
| RoborockB01Props.REAL_CLEAN_TIME, | ||
| RoborockB01Props.HYPA, | ||
| RoborockB01Props.WIND, | ||
| RoborockB01Props.WATER, | ||
| RoborockB01Props.MODE, | ||
| ] | ||
|
|
||
| @cached_property | ||
| def duid(self) -> str: | ||
| """Get the unique id of the device as specified by Roborock.""" | ||
| return self._device.duid | ||
|
|
||
| @cached_property | ||
| def duid_slug(self) -> str: | ||
| """Get the slug of the duid.""" | ||
| return slugify(self.duid) | ||
|
|
||
| @property | ||
| def device(self) -> RoborockDevice: | ||
| """Get the RoborockDevice.""" | ||
| return self._device | ||
|
|
||
| async def _async_update_data( | ||
| self, | ||
| ) -> B01Props: | ||
| ) -> Any: | ||
| try: | ||
| data = await self.api.query_values(self.request_protocols) | ||
| await self.api.refresh() | ||
| except RoborockException as ex: | ||
| _LOGGER.debug("Failed to update Q7 data: %s", ex) | ||
| _LOGGER.debug("Failed to update Q10 data: %s", ex) | ||
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="update_data_fail", | ||
| ) from ex | ||
| if data is None: | ||
| raise UpdateFailed( | ||
| translation_domain=DOMAIN, | ||
| translation_key="update_data_fail", | ||
| ) | ||
| return data | ||
| return self.api.status |
There was a problem hiding this comment.
The coordinator's generic type parameter DataUpdateCoordinator[dict[B01_Q10_DP, Any]] doesn't match what _async_update_data actually returns. The method returns self.api.status (line 625), which based on the test code (lines 1328-1335 in test_vacuum.py) is a status trait object with a .data attribute, not a raw dictionary. This mismatch could cause type checking issues. Consider either:
- Using a more specific type like
DataUpdateCoordinator[Q10Status]if that's the actual type, or - Returning
self.api.status.datainstead ofself.api.statusto match the declared dict type.
Note that Q10 entities access data via self.coordinator.api.status rather than self.coordinator.data, which is unconventional but intentional for the Q10's push-based MQTT updates.
|
|
||
| class RoborockB01Q7UpdateCoordinator(RoborockDataUpdateCoordinatorB01): | ||
| """Coordinator for B01 Q7 devices.""" | ||
| class RoborockB01Q10UpdateCoordinator(DataUpdateCoordinator[dict[B01_Q10_DP, Any]]): |
There was a problem hiding this comment.
This no longer works with B01_Q10_DP
| translation_key="update_data_fail", | ||
| ) | ||
| return data | ||
| return self.api.status |
There was a problem hiding this comment.
So big picture I think what will happen is:
- coordinator sends polls to the device "hey, please refresh"
- we won't know when they arrive, so here at this point we may have stale data. that is, the coordinator likely won't be able to hold any state data so its data type should probably just be
None - the trait itself will notify listeners when data arrives as you have done with the vacuum entity. it will update its own state when that happens
- the coordinator + entity already has a method for handling state updates on the coordinated entity. that probably needs to be overridden to be a no-op since the coordinator wont know when new data is available
So this needs a couple more changes
| @property | ||
| def activity(self) -> VacuumActivity | None: | ||
| """Return the status of the vacuum cleaner.""" | ||
| if self.coordinator.data.status is not None: |
There was a problem hiding this comment.
seems like this doesn't need to change in this PR
| }, | ||
| ) from err | ||
|
|
||
| async def get_maps(self) -> ServiceResponse: |
| ) | ||
| _attr_translation_key = DOMAIN | ||
| _attr_name = None | ||
| _attr_fan_speed_list = [ |
There was a problem hiding this comment.
Do you know what "CLOSE" is or if its ever returned?
There was a problem hiding this comment.
I don't understand the connection between the selected lines and the message about CLOSE.
| # Simulate MQTT data update | ||
| status_trait.update_from_dps({B01_Q10_DP.STATUS: q10_trait._current_status}) | ||
|
|
||
| q10_trait.start_clean = AsyncMock(side_effect=start_clean_side_effect) |
There was a problem hiding this comment.
Q10PropertiesApi does not have any of these methods
| initial_data[B01_Q10_DP.STATUS] = q10_trait._current_status | ||
| status_trait.update_from_dps(initial_data) | ||
|
|
||
| q10_trait.start = AsyncMock(side_effect=start_side_effect) |
There was a problem hiding this comment.
in the device: "start" will subscribe, but it won't request anything explicitly. Is an update automatically sent or do we need to query first?
| device.device_id = "q10_s5_plus_duid" | ||
| device.name = "Roborock Q10 S5+" | ||
| device.model = "roborock.vacuum.q10" | ||
| device.product_id = "q10_product_id" |
There was a problem hiding this comment.
Many of these fields do not actually exist on RoborockDevice. Please use FakeDevice like the other tests, populated from home data (device data and product data).
|
|
||
| # Add B01 trait for Q10 | ||
| device.b01_properties = create_b01_q10_trait() | ||
| device.status_trait = create_b01_q10_trait() |
There was a problem hiding this comment.
status_trat is not a thing on device
| device.b01_properties = create_b01_q10_trait() | ||
| device.status_trait = create_b01_q10_trait() | ||
|
|
||
| # Mock async methods |
There was a problem hiding this comment.
I don't think these two things exist either
Co-authored-by: Allen Porter <allen.porter@gmail.com>
|
Closing this in favor of #165624 |
Proposed change
Add Roborock Q10 (B01) vacuum support, including Q10-specific coordinator handling, entity features (vacuum, sensors, selects), and command mapping.
Update error handling for unsupported map services, add translations, and expand test coverage for Q10 behavior and helper mappings.
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: