Skip to content

Commit 2368bef

Browse files
committed
fix: Ensure traits to always reflect the the status of commands
This changes the contract for traits that have commands that mutate state to always ensure they reflect the latest device state after the command completes.
1 parent 3296a3a commit 2368bef

File tree

10 files changed

+98
-11
lines changed

10 files changed

+98
-11
lines changed

roborock/devices/traits/v1/child_lock.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ def is_on(self) -> bool:
1919
async def enable(self) -> None:
2020
"""Enable the child lock."""
2121
await self.rpc_channel.send_command(RoborockCommand.SET_CHILD_LOCK_STATUS, params={_STATUS_PARAM: 1})
22+
self.lock_status = 1
2223

2324
async def disable(self) -> None:
2425
"""Disable the child lock."""
2526
await self.rpc_channel.send_command(RoborockCommand.SET_CHILD_LOCK_STATUS, params={_STATUS_PARAM: 0})
27+
self.lock_status = 0

roborock/devices/traits/v1/common.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,14 @@ class V1TraitMixin(ABC):
2828
Each trait subclass must define a class variable `command` that specifies
2929
the RoborockCommand used to fetch the trait data from the device. The
3030
`refresh()` method can be called to update the contents of the trait data
31-
from the device. A trait can also support additional commands for updating
32-
state associated with the trait.
31+
from the device.
32+
33+
A trait can also support additional commands for updating state associated
34+
with the trait. It is expected that a trait will update it's own internal
35+
state either reflecting the change optimistically or by refreshing the
36+
trait state from the device. In cases where one trait caches data that is
37+
also represented in another trait, it is the responsibility of the caller
38+
to ensure that both traits are refreshed as needed to keep them in sync.
3339
3440
The traits typically subclass RoborockBase to provide serialization
3541
and deserialization functionality, but this is not strictly required.

roborock/devices/traits/v1/consumeable.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,4 @@ class ConsumableTrait(Consumable, common.V1TraitMixin):
4545
async def reset_consumable(self, consumable: ConsumableAttribute) -> None:
4646
"""Reset a specific consumable attribute on the device."""
4747
await self.rpc_channel.send_command(RoborockCommand.RESET_CONSUMABLE, params=[consumable.value])
48+
await self.refresh()

roborock/devices/traits/v1/do_not_disturb.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,22 @@ def is_on(self) -> bool:
1818
async def set_dnd_timer(self, dnd_timer: DnDTimer) -> None:
1919
"""Set the Do Not Disturb (DND) timer settings of the device."""
2020
await self.rpc_channel.send_command(RoborockCommand.SET_DND_TIMER, params=dnd_timer.as_list())
21+
await self.refresh()
2122

2223
async def clear_dnd_timer(self) -> None:
2324
"""Clear the Do Not Disturb (DND) timer settings of the device."""
2425
await self.rpc_channel.send_command(RoborockCommand.CLOSE_DND_TIMER)
26+
await self.refresh()
2527

2628
async def enable(self) -> None:
2729
"""Set the Do Not Disturb (DND) timer settings of the device."""
2830
await self.rpc_channel.send_command(
2931
RoborockCommand.SET_DND_TIMER,
3032
params=self.as_list(),
3133
)
34+
self.enabled = 1
3235

3336
async def disable(self) -> None:
3437
"""Disable the Do Not Disturb (DND) timer settings of the device."""
3538
await self.rpc_channel.send_command(RoborockCommand.CLOSE_DND_TIMER)
39+
self.enabled = 0

roborock/devices/traits/v1/flow_led_status.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ def is_on(self) -> bool:
1919
async def enable(self) -> None:
2020
"""Enable the Flow LED status."""
2121
await self.rpc_channel.send_command(RoborockCommand.SET_FLOW_LED_STATUS, params={_STATUS_PARAM: 1})
22+
self.status = 1
2223

2324
async def disable(self) -> None:
2425
"""Disable the Flow LED status."""
2526
await self.rpc_channel.send_command(RoborockCommand.SET_FLOW_LED_STATUS, params={_STATUS_PARAM: 0})
27+
self.status = 0

roborock/devices/traits/v1/led_status.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@ def is_on(self) -> bool:
1919
async def enable(self) -> None:
2020
"""Enable the LED status."""
2121
await self.rpc_channel.send_command(RoborockCommand.SET_LED_STATUS, params=[1])
22+
self.status = 1
2223

2324
async def disable(self) -> None:
2425
"""Disable the LED status."""
2526
await self.rpc_channel.send_command(RoborockCommand.SET_LED_STATUS, params=[0])
27+
self.status = 0
2628

2729
@classmethod
2830
def _parse_type_response(cls, response: V1ResponseData) -> LedStatus:

roborock/devices/traits/v1/valley_electricity_timer.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,19 @@ async def set_timer(self, timer: ValleyElectricityTimer) -> None:
2323
async def clear_timer(self) -> None:
2424
"""Clear the Valley Electricity Timer settings of the device."""
2525
await self.rpc_channel.send_command(RoborockCommand.CLOSE_VALLEY_ELECTRICITY_TIMER)
26+
await self.refresh()
2627

2728
async def enable(self) -> None:
2829
"""Enable the Valley Electricity Timer settings of the device."""
2930
await self.rpc_channel.send_command(
3031
RoborockCommand.SET_VALLEY_ELECTRICITY_TIMER,
3132
params=self.as_list(),
3233
)
34+
self.enabled = 1
3335

3436
async def disable(self) -> None:
3537
"""Disable the Valley Electricity Timer settings of the device."""
3638
await self.rpc_channel.send_command(
3739
RoborockCommand.CLOSE_VALLEY_ELECTRICITY_TIMER,
3840
)
41+
self.enabled = 0

roborock/devices/traits/v1/volume.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,4 @@ class SoundVolumeTrait(SoundVolume, common.V1TraitMixin):
2424
async def set_volume(self, volume: int) -> None:
2525
"""Set the sound volume of the device."""
2626
await self.rpc_channel.send_command(RoborockCommand.CHANGE_SOUND_VOLUME, params=[volume])
27+
self.volume = volume

tests/devices/traits/v1/test_consumable.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
"""Tests for the DoNotDisturbTrait class."""
22

3-
from unittest.mock import AsyncMock
3+
from unittest.mock import AsyncMock, call
44

55
import pytest
66

@@ -60,11 +60,29 @@ async def test_reset_consumable_data(
6060
reset_param: str,
6161
) -> None:
6262
"""Test successfully resetting consumable data."""
63+
mock_rpc_channel.send_command.side_effect = [
64+
{}, # Response for RESET_CONSUMABLE
65+
# Response for GET_CONSUMABLE after reset
66+
{
67+
"main_brush_work_time": 5555,
68+
"side_brush_work_time": 6666,
69+
"filter_work_time": 7777,
70+
"filter_element_work_time": 8888,
71+
"sensor_dirty_time": 9999,
72+
},
73+
]
74+
6375
# Call the method
6476
await consumable_trait.reset_consumable(consumable)
6577

6678
# Verify the RPC call was made correctly with expected parameters
67-
mock_rpc_channel.send_command.assert_called_once_with(RoborockCommand.RESET_CONSUMABLE, params=[reset_param])
68-
69-
70-
#
79+
assert mock_rpc_channel.send_command.mock_calls == [
80+
call(RoborockCommand.RESET_CONSUMABLE, params=[reset_param]),
81+
call(RoborockCommand.GET_CONSUMABLE),
82+
]
83+
# Verify the consumable data was refreshed correctly
84+
assert consumable_trait.main_brush_work_time == 5555
85+
assert consumable_trait.side_brush_work_time == 6666
86+
assert consumable_trait.filter_work_time == 7777
87+
assert consumable_trait.filter_element_work_time == 8888
88+
assert consumable_trait.sensor_dirty_time == 9999

tests/devices/traits/v1/test_dnd.py

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
"""Tests for the DoNotDisturbTrait class."""
22

3-
from unittest.mock import AsyncMock
3+
from unittest.mock import AsyncMock, call
44

55
import pytest
66

77
from roborock.data import DnDTimer
88
from roborock.devices.device import RoborockDevice
99
from roborock.devices.traits.v1.do_not_disturb import DoNotDisturbTrait
10+
from roborock.exceptions import RoborockException
1011
from roborock.roborock_typing import RoborockCommand
1112

1213

@@ -74,27 +75,74 @@ async def test_set_dnd_timer_success(
7475
dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock, sample_dnd_timer: DnDTimer
7576
) -> None:
7677
"""Test successfully setting DnD timer settings."""
78+
mock_rpc_channel.send_command.side_effect = [
79+
# Response for SET_DND_TIMER
80+
{},
81+
# Response for GET_DND_TIMER after updating
82+
{
83+
"startHour": 22,
84+
"startMinute": 0,
85+
"endHour": 8,
86+
"endMinute": 0,
87+
"enabled": 1,
88+
},
89+
]
90+
7791
# Call the method
7892
await dnd_trait.set_dnd_timer(sample_dnd_timer)
7993

8094
# Verify the RPC call was made correctly with dataclass converted to dict
8195

8296
expected_params = [22, 0, 8, 0]
83-
mock_rpc_channel.send_command.assert_called_once_with(RoborockCommand.SET_DND_TIMER, params=expected_params)
97+
mock_rpc_channel.send_command.mock_calls = [
98+
call(RoborockCommand.SET_DND_TIMER, params=expected_params),
99+
call(RoborockCommand.GET_DND_TIMER),
100+
]
101+
102+
# Verify the trait state is updated
103+
assert dnd_trait.enabled == 1
104+
assert dnd_trait.is_on
105+
assert dnd_trait.start_hour == 22
106+
assert dnd_trait.start_minute == 0
107+
assert dnd_trait.end_hour == 8
108+
assert dnd_trait.end_minute == 0
84109

85110

86111
async def test_clear_dnd_timer_success(dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock) -> None:
87112
"""Test successfully clearing DnD timer settings."""
113+
mock_rpc_channel.send_command.side_effect = [
114+
# Response for CLOSE_DND_TIMER
115+
{},
116+
# Response for GET_DND_TIMER after clearing
117+
{
118+
"startHour": 0,
119+
"startMinute": 0,
120+
"endHour": 0,
121+
"endMinute": 0,
122+
"enabled": 0,
123+
},
124+
]
125+
88126
# Call the method
89127
await dnd_trait.clear_dnd_timer()
90128

91129
# Verify the RPC call was made correctly
92-
mock_rpc_channel.send_command.assert_called_once_with(RoborockCommand.CLOSE_DND_TIMER)
130+
mock_rpc_channel.send_command.mock_calls = [
131+
call(RoborockCommand.CLOSE_DND_TIMER),
132+
call(RoborockCommand.GET_DND_TIMER),
133+
]
134+
135+
# Verify the trait state is updated
136+
assert dnd_trait.enabled == 0
137+
assert not dnd_trait.is_on
138+
assert dnd_trait.start_hour == 0
139+
assert dnd_trait.start_minute == 0
140+
assert dnd_trait.end_hour == 0
141+
assert dnd_trait.end_minute == 0
93142

94143

95144
async def test_get_dnd_timer_propagates_exception(dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock) -> None:
96145
"""Test that exceptions from RPC channel are propagated in get_dnd_timer."""
97-
from roborock.exceptions import RoborockException
98146

99147
# Setup mock to raise an exception
100148
mock_rpc_channel.send_command.side_effect = RoborockException("Communication error")

0 commit comments

Comments
 (0)