Skip to content

Commit dab740f

Browse files
committed
refactor: remove trait update listeners and centralize data conversion into dedicated converter classes
1 parent d11ee2a commit dab740f

21 files changed

+276
-263
lines changed

roborock/devices/traits/v1/child_lock.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class ChildLockTrait(ChildLockStatus, common.V1TraitMixin, common.RoborockSwitch
99
"""Trait for controlling the child lock of a Roborock device."""
1010

1111
command = RoborockCommand.GET_CHILD_LOCK_STATUS
12-
converter = common.V1TraitDataConverter(ChildLockStatus)
12+
converter = common.DefaultConverter(ChildLockStatus)
1313
requires_feature = "is_set_child_supported"
1414

1515
@property
Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import logging
2-
from typing import Self
32

4-
from roborock.data import CleanRecord, CleanSummaryWithDetail
3+
from roborock.data import CleanRecord, CleanSummaryWithDetail, RoborockBase
54
from roborock.devices.traits.v1 import common
65
from roborock.roborock_typing import RoborockCommand
76
from roborock.util import unpack_list
@@ -10,53 +9,29 @@
109

1110

1211
class CleanSummaryConverter(common.V1TraitDataConverter):
12+
"""Converter for CleanSumary objects."""
1313

14-
15-
16-
@classmethod
17-
def _convert_to_dict(cls, response: common.V1ResponseData) -> Self:
14+
def convert(self, response: common.V1ResponseData) -> RoborockBase:
1815
"""Parse the response from the device into a CleanSummary."""
1916
if isinstance(response, dict):
20-
return cls.from_dict(response)
17+
return CleanSummaryWithDetail.from_dict(response)
2118
elif isinstance(response, list):
2219
clean_time, clean_area, clean_count, records = unpack_list(response, 4)
23-
return cls(
20+
return CleanSummaryWithDetail(
2421
clean_time=clean_time,
2522
clean_area=clean_area,
2623
clean_count=clean_count,
2724
records=records,
2825
)
2926
elif isinstance(response, int):
30-
return cls(clean_time=response)
27+
return CleanSummaryWithDetail(clean_time=response)
3128
raise ValueError(f"Unexpected clean summary format: {response!r}")
3229

3330

34-
class CleanSummaryTrait(CleanSummaryWithDetail, common.V1TraitMixin):
35-
"""Trait for managing the clean summary of Roborock devices."""
36-
37-
command = RoborockCommand.GET_CLEAN_SUMMARY
38-
converter = common.V1TraitDataConverter(CleanSummaryWithDetail)
39-
40-
async def refresh(self) -> None:
41-
"""Refresh the clean summary data and last clean record.
42-
43-
Assumes that the clean summary has already been fetched.
44-
"""
45-
await super().refresh()
46-
if not self.records:
47-
_LOGGER.debug("No clean records available in clean summary.")
48-
self.last_clean_record = None
49-
return
50-
last_record_id = self.records[0]
51-
self.last_clean_record = await self.get_clean_record(last_record_id)
52-
53-
async def get_clean_record(self, record_id: int) -> CleanRecord:
54-
"""Load a specific clean record by ID."""
55-
response = await self.rpc_channel.send_command(RoborockCommand.GET_CLEAN_RECORD, params=[record_id])
56-
return self._parse_clean_record_response(response)
31+
class CleanRecordConverter(common.V1TraitDataConverter):
32+
"""Convert server responses to a CleanRecord."""
5733

58-
@classmethod
59-
def _parse_clean_record_response(cls, response: common.V1ResponseData) -> CleanRecord:
34+
def convert(self, response: common.V1ResponseData) -> CleanRecord:
6035
"""Parse the response from the device into a CleanRecord."""
6136
if isinstance(response, list) and len(response) == 1:
6237
response = response[0]
@@ -87,3 +62,29 @@ def _parse_clean_record_response(cls, response: common.V1ResponseData) -> CleanR
8762
begin, end, duration, area = unpack_list(response, 4)
8863
return CleanRecord(begin=begin, end=end, duration=duration, area=area)
8964
raise ValueError(f"Unexpected clean record format: {response!r}")
65+
66+
67+
class CleanSummaryTrait(CleanSummaryWithDetail, common.V1TraitMixin):
68+
"""Trait for managing the clean summary of Roborock devices."""
69+
70+
command = RoborockCommand.GET_CLEAN_SUMMARY
71+
converter = CleanSummaryConverter()
72+
clean_record_converter = CleanRecordConverter()
73+
74+
async def refresh(self) -> None:
75+
"""Refresh the clean summary data and last clean record.
76+
77+
Assumes that the clean summary has already been fetched.
78+
"""
79+
await super().refresh()
80+
if not self.records:
81+
_LOGGER.debug("No clean records available in clean summary.")
82+
self.last_clean_record = None
83+
return
84+
last_record_id = self.records[0]
85+
self.last_clean_record = await self.get_clean_record(last_record_id)
86+
87+
async def get_clean_record(self, record_id: int) -> CleanRecord:
88+
"""Load a specific clean record by ID."""
89+
response = await self.rpc_channel.send_command(RoborockCommand.GET_CLEAN_RECORD, params=[record_id])
90+
return self.clean_record_converter.convert(response)

roborock/devices/traits/v1/common.py

Lines changed: 44 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -5,49 +5,27 @@
55

66
import logging
77
from abc import ABC, abstractmethod
8-
from dataclasses import dataclass, fields
9-
from typing import Any, ClassVar, Self
8+
from dataclasses import fields
9+
from typing import ClassVar
1010

11-
from roborock.callbacks import CallbackList
1211
from roborock.data import RoborockBase
1312
from roborock.protocols.v1_protocol import V1RpcChannel
1413
from roborock.roborock_typing import RoborockCommand
15-
from roborock.devices.traits.common import TraitDataConverter, TraitUpdateListener
1614

1715
_LOGGER = logging.getLogger(__name__)
1816

1917
V1ResponseData = dict | list | int | str
2018

19+
2120
class V1TraitDataConverter:
22-
"""Utility to handle the transformation and merging of data into models.
21+
"""Converts responses to RoborockBase objects."""
2322

24-
This has some special handling for v1 responses, then uses the common data
25-
converters for updating the target objects.
26-
"""
27-
28-
def __init__(self, dataclass_type: type[RoborockBase]):
29-
"""Initialize the converter for a specific RoborockBase-derived class."""
30-
self._dataclass_type = dataclass_type
31-
self._converter = TraitDataConverter(dataclass_type)
23+
@abstractmethod
24+
def convert(self, response: V1ResponseData) -> RoborockBase:
25+
"""Convert the values to a dict that can be parsed as a RoborockBase."""
3226

33-
def update_from_data(self, target: RoborockBase, data: V1ResponseData) -> None:
34-
"""Update the target object from a dictionary of raw values.
35-
36-
Returns True if any values were updated.
37-
"""
38-
response = self._convert_to_dict(data)
39-
return self._converter.update_from_dict(target, response)
40-
41-
def _convert_to_dict(self, response: V1ResponseData) -> None:
42-
"""Convert the values to a dict that can be parsed as a RoborockBase.
43-
44-
Subclasses can override to implement custom parsing logic
45-
"""
46-
if isinstance(response, list):
47-
response = response[0]
48-
if not isinstance(response, dict):
49-
raise ValueError(f"Unexpected {self._dataclass_type.__name__} response format: {response!r}")
50-
return response
27+
def __repr__(self) -> str:
28+
return self.__class__.__name__
5129

5230

5331
class V1TraitMixin(ABC):
@@ -74,13 +52,11 @@ class V1TraitMixin(ABC):
7452
"""
7553

7654
command: ClassVar[RoborockCommand]
77-
converter: ClassVar[V1TraitDataConverter]
55+
converter: V1TraitDataConverter
7856

7957
def __init__(self) -> None:
8058
"""Initialize the V1TraitMixin."""
8159
self._rpc_channel = None
82-
self._update_listener = TraitUpdateListener(_LOGGER)
83-
8460

8561
@property
8662
def rpc_channel(self) -> V1RpcChannel:
@@ -92,36 +68,43 @@ def rpc_channel(self) -> V1RpcChannel:
9268
async def refresh(self) -> None:
9369
"""Refresh the contents of this trait."""
9470
response = await self.rpc_channel.send_command(self.command)
95-
self._update_data(response)
71+
new_data = self.converter.convert(response)
72+
# Mixin doesn't know its type
73+
merge_object_values(self, new_data) # type: ignore[arg-type]
9674

97-
def _update_data(self, response: Any) -> None:
98-
"""Applies the new data to the object and notifies listeners.
99-
100-
This can be overridden by subclasses to change the behavior, but it is
101-
preferred to just use the `converter`.
102-
"""
103-
if self.converter.update_from_data(self, response):
104-
self._update_listener._notify_update()
10575

106-
def add_update_listener(self, callback: Callable[[], None]) -> Callable[[], None]:
107-
"""Register a callback when the trait has been updated.
76+
def merge_object_values(target: RoborockBase, new_object: RoborockBase) -> bool:
77+
"""Update the target object with set fields in new_object."""
78+
updated = False
79+
for field in fields(new_object):
80+
old_value = getattr(target, field.name, None)
81+
new_value = getattr(new_object, field.name, None)
82+
if new_value != old_value:
83+
setattr(target, field.name, new_value)
84+
updated = True
85+
return updated
10886

109-
Returns a callable to remove the listener.
110-
"""
111-
self._update_listener.add_update_listener(callback)
11287

88+
class DefaultConverter(V1TraitDataConverter):
89+
"""Converts responses to RoborockBase objects."""
90+
91+
def __init__(self, dataclass_type: type[RoborockBase]) -> None:
92+
"""Initialize the converter."""
93+
self._dataclass_type = dataclass_type
11394

114-
def _get_value_field(clazz: type[V1TraitMixin]) -> str:
115-
"""Get the name of the field marked as the main value of the RoborockValueBase."""
116-
value_fields = [field.name for field in fields(clazz) if field.metadata.get("roborock_value", False)]
117-
if len(value_fields) != 1:
118-
raise ValueError(
119-
f"RoborockValueBase subclass {clazz} must have exactly one field marked as roborock_value, "
120-
f" but found: {value_fields}"
121-
)
122-
return value_fields[0]
95+
def convert(self, response: V1ResponseData) -> RoborockBase:
96+
"""Convert the values to a dict that can be parsed as a RoborockBase.
12397
124-
class SingleValueConverter(V1TraitDataConverter):
98+
Subclasses can override to implement custom parsing logic
99+
"""
100+
if isinstance(response, list):
101+
response = response[0]
102+
if not isinstance(response, dict):
103+
raise ValueError(f"Unexpected {self._dataclass_type.__name__} response format: {response!r}")
104+
return self._dataclass_type.from_dict(response)
105+
106+
107+
class SingleValueConverter(DefaultConverter):
125108
"""Base class for traits that represent a single value.
126109
127110
This class is intended to be subclassed by traits that represent a single
@@ -135,15 +118,14 @@ def __init__(self, dataclass_type: type[RoborockBase], value_field: str) -> None
135118
super().__init__(dataclass_type)
136119
self._value_field = value_field
137120

138-
@classmethod
139-
def _parse_response(cls, response: V1ResponseData) -> Self:
121+
def convert(self, response: V1ResponseData) -> RoborockBase:
140122
"""Parse the response from the device into a RoborockValueBase."""
141123
if isinstance(response, list):
142124
response = response[0]
143125
if not isinstance(response, int):
144126
raise ValueError(f"Unexpected response format: {response!r}")
145-
value_field = _get_value_field(cls)
146-
return cls(**{self.__value_field: response})
127+
return super().convert({self._value_field: response})
128+
147129

148130
class RoborockSwitchBase(ABC):
149131
"""Base class for traits that represent a boolean switch."""

roborock/devices/traits/v1/consumeable.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class ConsumableTrait(Consumable, common.V1TraitMixin):
4141
"""
4242

4343
command = RoborockCommand.GET_CONSUMABLE
44-
converter = common.V1TraitDataConverter(Consumable)
44+
converter = common.DefaultConverter(Consumable)
4545

4646
def __init__(self) -> None:
4747
common.V1TraitMixin.__init__(self)

roborock/devices/traits/v1/device_features.py

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,36 @@
88
from roborock.roborock_typing import RoborockCommand
99

1010

11+
class DeviceTraitsConverter(common.V1TraitDataConverter):
12+
"""Converter for CleanSumary objects."""
13+
14+
def __init__(self, product: HomeDataProduct) -> None:
15+
"""Initialize CleanSummaryConverter."""
16+
self._product = product
17+
18+
def convert(self, response: common.V1ResponseData) -> DeviceFeatures:
19+
"""Parse the response from the device into a MapContentTrait instance."""
20+
if not isinstance(response, list):
21+
raise ValueError(f"Unexpected AppInitStatus response format: {type(response)}")
22+
app_status = AppInitStatus.from_dict(response[0])
23+
return DeviceFeatures.from_feature_flags(
24+
new_feature_info=app_status.new_feature_info,
25+
new_feature_info_str=app_status.new_feature_info_str,
26+
feature_info=app_status.feature_info,
27+
product_nickname=self._product.product_nickname,
28+
)
29+
30+
1131
class DeviceFeaturesTrait(DeviceFeatures, common.V1TraitMixin):
1232
"""Trait for managing supported features on Roborock devices."""
1333

1434
command = RoborockCommand.APP_GET_INIT_STATUS
15-
converter = common.V1TraitDataConverter(DeviceFeatures)
1635

1736
def __init__(self, product: HomeDataProduct, device_cache: DeviceCache) -> None: # pylint: disable=super-init-not-called
1837
"""Initialize DeviceFeaturesTrait."""
1938
common.V1TraitMixin.__init__(self)
39+
self.converter = DeviceTraitsConverter(product)
2040
self._product = product
21-
self._nickname = product.product_nickname
2241
self._device_cache = device_cache
2342
# All fields of DeviceFeatures are required. Initialize them to False
2443
# so we have some known state.
@@ -56,21 +75,9 @@ async def refresh(self) -> None:
5675
"""
5776
cache_data = await self._device_cache.get()
5877
if cache_data.device_features is not None:
59-
self._update_trait_values(cache_data.device_features)
78+
common.merge_object_values(self, cache_data.device_features)
6079
return
6180
# Save cached device features
6281
await super().refresh()
6382
cache_data.device_features = self
6483
await self._device_cache.set(cache_data)
65-
66-
def _parse_response(self, response: common.V1ResponseData) -> DeviceFeatures:
67-
"""Parse the response from the device into a MapContentTrait instance."""
68-
if not isinstance(response, list):
69-
raise ValueError(f"Unexpected AppInitStatus response format: {type(response)}")
70-
app_status = AppInitStatus.from_dict(response[0])
71-
return DeviceFeatures.from_feature_flags(
72-
new_feature_info=app_status.new_feature_info,
73-
new_feature_info_str=app_status.new_feature_info_str,
74-
feature_info=app_status.feature_info,
75-
product_nickname=self._nickname,
76-
)

roborock/devices/traits/v1/do_not_disturb.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class DoNotDisturbTrait(DnDTimer, common.V1TraitMixin, common.RoborockSwitchBase
99
"""Trait for managing Do Not Disturb (DND) settings on Roborock devices."""
1010

1111
command = RoborockCommand.GET_DND_TIMER
12-
converter = common.V1TraitDataConverter(DnDTimer)
12+
converter = common.DefaultConverter(DnDTimer)
1313

1414
@property
1515
def is_on(self) -> bool:

roborock/devices/traits/v1/dust_collection_mode.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@ class DustCollectionModeTrait(DustCollectionMode, common.V1TraitMixin):
1010
"""Trait for dust collection mode."""
1111

1212
command = RoborockCommand.GET_DUST_COLLECTION_MODE
13-
converter = common.V1TraitDataConverter(DustCollectionMode)
13+
converter = common.DefaultConverter(DustCollectionMode)
1414
requires_dock_type = is_valid_dock

roborock/devices/traits/v1/flow_led_status.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ class FlowLedStatusTrait(FlowLedStatus, common.V1TraitMixin, common.RoborockSwit
99
"""Trait for controlling the Flow LED status of a Roborock device."""
1010

1111
command = RoborockCommand.GET_FLOW_LED_STATUS
12-
converter = common.V1TraitDataConverter(FlowLedStatus)
12+
converter = common.DefaultConverter(FlowLedStatus)
1313
requires_feature = "is_flow_led_setting_supported"
1414

1515
@property

roborock/devices/traits/v1/home.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class HomeTrait(RoborockBase, common.V1TraitMixin):
4141
"""Trait that represents a full view of the home layout."""
4242

4343
command = RoborockCommand.GET_MAP_V1 # This is not used
44-
converter = common.V1TraitDataConverter(RoborockBase)
44+
converter = common.DefaultConverter(RoborockBase) # Not used
4545

4646
def __init__(
4747
self,

0 commit comments

Comments
 (0)