-
-
Notifications
You must be signed in to change notification settings - Fork 597
Add additional sensors and settings to Roborock vacuums #1543
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
Changes from all commits
c808adf
fc8c7b4
a63270a
b040a7c
bc09260
c766026
383de8b
92dae6e
444f8ed
2c3b4b3
f7cbb95
43e7a3e
33faf82
c32b24a
7488ea9
32d0028
73fa561
60e6d70
d995d26
a4e6dc4
5cc7664
b3ecd75
a164846
0c0a617
68a4fb4
00dfae9
4d9ca0e
6c8bb0e
e490cd4
298a485
5fb7f9e
f80a13e
39d9781
fd49e94
142dfb0
4485c79
6f21e31
8c3eae4
1b009d5
bef9197
a86d7cc
cd86f24
46c2b80
f53edc3
28f3340
65037f9
edc42d4
6068336
e47a9de
cdc140e
c051028
6ad0e45
14a7d6e
2f5f3b3
ee90954
758e185
252c1b6
de9dac9
180dd5c
cc7d4ea
83e7e90
ac58bf3
f2fed51
1c69cc5
f1f3a62
998ec16
a5a0e3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,8 @@ def __init__( | |
| self.token: Optional[str] = token | ||
| self._model: Optional[str] = model | ||
| self._info: Optional[DeviceInfo] = None | ||
| self._status: Optional[DeviceStatus] = None | ||
| self._buttons: Optional[List[ButtonDescriptor]] = None | ||
| self._actions: Optional[Dict[str, ActionDescriptor]] = None | ||
| timeout = timeout if timeout is not None else self.timeout | ||
| self._protocol = MiIOProtocol( | ||
|
|
@@ -250,6 +252,14 @@ def status(self) -> DeviceStatus: | |
| """Return device status.""" | ||
| raise NotImplementedError() | ||
|
|
||
|
|
||
| def cached_status(self) -> DeviceStatus: | ||
| """Return device status from cache.""" | ||
| if self._status is None: | ||
| self._status = self.status() | ||
|
|
||
| return self._status | ||
|
|
||
| def actions(self) -> Dict[str, ActionDescriptor]: | ||
| """Return device actions.""" | ||
| if self._actions is None: | ||
|
|
@@ -263,8 +273,10 @@ def actions(self) -> Dict[str, ActionDescriptor]: | |
| return self._actions | ||
|
|
||
| def settings(self) -> Dict[str, SettingDescriptor]: | ||
| """Return device settings.""" | ||
| settings = self.status().settings() | ||
| """Return list of settings.""" | ||
| settings = ( | ||
| self.cached_status().settings() | ||
| ) # NOTE that this already does IO so schould be run in executer job in HA | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my note about below about doing the binding during the init. I suppose the best place to hook this is to happen is in the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can move all the binding/IO (status retrieving, choices list retrieving etc) to the Do you want me to move it?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or we could raise an error when sensors/buttons/settings is accesed when cache is still empty, maybe that is nicer since you then know that the sensors/buttons/settings will not do IO, and therefore do not have to be run in a executor job... |
||
| for setting in settings.values(): | ||
| # TODO: Bind setter methods, this should probably done only once during init. | ||
| if setting.setter is None: | ||
|
|
@@ -292,9 +304,8 @@ def settings(self) -> Dict[str, SettingDescriptor]: | |
| return settings | ||
|
|
||
| def sensors(self) -> Dict[str, SensorDescriptor]: | ||
| """Return device sensors.""" | ||
| # TODO: the latest status should be cached and re-used by all meta information getters | ||
| sensors = self.status().sensors() | ||
| """Return sensors.""" | ||
| sensors = self.cached_status().sensors() | ||
| return sensors | ||
|
|
||
| def __repr__(self): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| from croniter import croniter | ||
| from pytz import BaseTzInfo | ||
|
|
||
| from miio.descriptors import SensorDescriptor | ||
| from miio.device import DeviceStatus | ||
| from miio.devicestatus import sensor, setting | ||
| from miio.interfaces.vacuuminterface import VacuumDeviceStatus, VacuumState | ||
|
|
@@ -136,7 +137,9 @@ def map_name_dict(self) -> Dict[str, int]: | |
| class VacuumStatus(VacuumDeviceStatus): | ||
| """Container for status reports from the vacuum.""" | ||
|
|
||
| def __init__(self, data: Dict[str, Any]) -> None: | ||
| def __init__( | ||
| self, data: Dict[str, Any], multi_maps: Optional[MultiMapList] = None | ||
| ) -> None: | ||
| # {'result': [{'state': 8, 'dnd_enabled': 1, 'clean_time': 0, | ||
| # 'msg_ver': 4, 'map_present': 1, 'error_code': 0, 'in_cleaning': 0, | ||
| # 'clean_area': 0, 'battery': 100, 'fan_power': 20, 'msg_seq': 320}], | ||
|
|
@@ -177,6 +180,7 @@ def __init__(self, data: Dict[str, Any]) -> None: | |
| # 'water_shortage_status': 0, 'dock_type': 0, 'dust_collection_status': 0, | ||
| # 'auto_dust_collection': 1, 'mop_mode': 300, 'debug_mode': 0}] | ||
| self.data = data | ||
| self._multi_maps = multi_maps | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should avoid doing this, and do it another way around (i.e., the multimap container could expose whatever is necessary based on the status container). Does that sound reasonable to you?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not get what you mean.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So basically I don't like the idea of injecting the information to the main status container but I'm currently too tired to think for a proper solution.. Let's remove the multi map parts from this PR–as it's a very large PR already–and figure a nicer solution on a separate discussion. |
||
|
|
||
| @property | ||
| @sensor("State code", entity_category="diagnostic", enabled_default=False) | ||
|
|
@@ -337,6 +341,14 @@ def current_map_id(self) -> int: | |
| """ | ||
| return int((self.data["map_status"] + 1) / 4 - 1) | ||
|
|
||
| @property | ||
| def map_name(self) -> str: | ||
| """The name of the current map with regards to the multi map feature.""" | ||
| if self._multi_maps is None: | ||
| return str(self.multi_map_id) | ||
|
|
||
| return self._multi_maps.map_list[self.multi_map_id]["name"] | ||
|
|
||
| @property | ||
| def in_zone_cleaning(self) -> bool: | ||
| """Return True if the vacuum is in zone cleaning mode.""" | ||
|
|
@@ -494,10 +506,15 @@ def dust_collection_count(self) -> Optional[int]: | |
| class CleaningDetails(DeviceStatus): | ||
| """Contains details about a specific cleaning run.""" | ||
|
|
||
| def __init__(self, data: Union[List[Any], Dict[str, Any]]) -> None: | ||
| def __init__( | ||
| self, | ||
| data: Union[List[Any], Dict[str, Any]], | ||
| multi_maps: Optional[MultiMapList] = None, | ||
| ) -> None: | ||
| # start, end, duration, area, unk, complete | ||
| # { "result": [ [ 1488347071, 1488347123, 16, 0, 0, 0 ] ], "id": 1 } | ||
| # newer models return a dict | ||
| self._multi_maps = multi_maps | ||
| if isinstance(data, list): | ||
| self.data = { | ||
| "begin": data[0], | ||
|
|
@@ -560,6 +577,15 @@ def map_id(self) -> int: | |
| """Map id used (multi map feature) during the cleaning run.""" | ||
| return self.data.get("map_flag", 0) | ||
|
|
||
| @property | ||
| @sensor("Last clean map name", icon="mdi:floor-plan", entity_category="diagnostic") | ||
| def map_name(self) -> str: | ||
| """The name of the map used (multi map feature) during the cleaning run.""" | ||
| if self._multi_maps is None: | ||
| return str(self.multi_map_id) | ||
|
|
||
| return self._multi_maps.map_list[self.multi_map_id]["name"] | ||
|
|
||
| @property | ||
| def error_code(self) -> int: | ||
| """Error code.""" | ||
|
|
@@ -579,6 +605,58 @@ def complete(self) -> bool: | |
| return self.data["complete"] == 1 | ||
|
|
||
|
|
||
| class FloorCleanDetails(DeviceStatus): | ||
| """Contains details about a last cleaning run per floor.""" | ||
|
|
||
| def __init__(self, data: Dict[str, Any]) -> None: | ||
| self.data = data | ||
|
|
||
| for map_id in self.data: | ||
| if self.data[map_id] is None: | ||
| setattr(self, f"CleanDetails_{map_id}", None) | ||
| setattr(self, f"start_{map_id}", None) | ||
| continue | ||
| setattr(self, f"CleanDetails_{map_id}", self.data[map_id]) | ||
| setattr(self, f"start_{map_id}", self.data[map_id].start) | ||
|
Comment on lines
+614
to
+620
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks rather messy and hard to follow through the setattr "magic", I think we can need to find a better solution for doing this. See my comment above wrt multiroom support. |
||
|
|
||
| def __repr__(self): | ||
|
|
||
| s = f"<{self.__class__.__name__}" | ||
|
Comment on lines
+622
to
+624
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We shouldn't have a need for extra magic, but a proper solution that leverages the existing repr. |
||
| for map_id in self.data: | ||
| name = f"CleanDetails_{map_id}" | ||
| value = getattr(self, name) | ||
| s += f" {name}={value}" | ||
|
|
||
| name = f"start_{map_id}" | ||
| value = getattr(self, name) | ||
| s += f" {name}={value}" | ||
|
|
||
| for name, embedded in self._embedded.items(): | ||
| s += f" {name}={repr(embedded)}" | ||
|
|
||
| s += ">" | ||
| return s | ||
|
|
||
| def sensors(self) -> Dict[str, SensorDescriptor]: | ||
| """Return the dict of sensors exposed by the status container.""" | ||
| self._sensors = {} # type: ignore[attr-defined] | ||
|
|
||
| for map_id in self.data: | ||
| self._sensors[f"start_{map_id}"] = SensorDescriptor( | ||
| id=f"FloorCleanDetails.start_{map_id}", | ||
| property=f"start_{map_id}", | ||
| name=f"Floor {map_id} clean start", | ||
| type="sensor", | ||
| extras={ | ||
| "icon": "mdi:clock-time-twelve", | ||
| "device_class": "timestamp", | ||
| "entity_category": "diagnostic", | ||
| }, | ||
| ) | ||
|
|
||
| return self._sensors | ||
|
|
||
|
|
||
| class ConsumableStatus(DeviceStatus): | ||
| """Container for consumable status information, including information about brushes | ||
| and duration until they should be changed. The methods returning time left are based | ||
|
|
||
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.
Let's not add caching on this PR as it isn't directly relevant to the improvements for the roborock vacuums, and it will require some more investigation on what is the best approach to implement the functionality you are aiming to fix with this.