-
Notifications
You must be signed in to change notification settings - Fork 52
feat: Add a DnD trait and fix bugs in the rpc channels #471
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
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| """Module for Roborock V1 devices. | ||
|
|
||
| This interface is experimental and subject to breaking changes without notice | ||
| until the API is stable. | ||
| """ | ||
|
|
||
| import logging | ||
|
|
||
| from roborock.containers import DnDTimer | ||
| from roborock.devices.v1_rpc_channel import V1RpcChannel | ||
| from roborock.roborock_typing import RoborockCommand | ||
|
|
||
| from .trait import Trait | ||
|
|
||
| _LOGGER = logging.getLogger(__name__) | ||
|
|
||
| __all__ = [ | ||
| "DoNotDisturbTrait", | ||
| ] | ||
|
|
||
|
|
||
| class DoNotDisturbTrait(Trait): | ||
| """Trait for managing Do Not Disturb (DND) settings on Roborock devices.""" | ||
|
|
||
| name = "do_not_disturb" | ||
|
|
||
| def __init__(self, rpc_channel: V1RpcChannel) -> None: | ||
| """Initialize the DoNotDisturbTrait.""" | ||
| self._rpc_channel = rpc_channel | ||
|
|
||
| async def get_dnd_timer(self) -> DnDTimer: | ||
| """Get the current Do Not Disturb (DND) timer settings of the device.""" | ||
| return await self._rpc_channel.send_command(RoborockCommand.GET_DND_TIMER, response_type=DnDTimer) | ||
|
|
||
| async def set_dnd_timer(self, dnd_timer: DnDTimer) -> None: | ||
| """Set the Do Not Disturb (DND) timer settings of the device.""" | ||
| await self._rpc_channel.send_command(RoborockCommand.SET_DND_TIMER, params=dnd_timer.as_dict()) | ||
|
|
||
| async def clear_dnd_timer(self) -> None: | ||
| """Clear the Do Not Disturb (DND) timer settings of the device.""" | ||
| await self._rpc_channel.send_command(RoborockCommand.CLOSE_DND_TIMER) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| """Tests for device traits.""" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| """Tests for the DoNotDisturbTrait class.""" | ||
|
|
||
| from unittest.mock import AsyncMock | ||
|
|
||
| import pytest | ||
|
|
||
| from roborock.containers import DnDTimer | ||
| from roborock.devices.traits.dnd import DoNotDisturbTrait | ||
| from roborock.devices.v1_rpc_channel import V1RpcChannel | ||
| from roborock.roborock_typing import RoborockCommand | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def mock_rpc_channel() -> AsyncMock: | ||
| """Create a mock RPC channel.""" | ||
| mock_channel = AsyncMock(spec=V1RpcChannel) | ||
| # Ensure send_command is an AsyncMock that returns awaitable coroutines | ||
| mock_channel.send_command = AsyncMock() | ||
| return mock_channel | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def dnd_trait(mock_rpc_channel: AsyncMock) -> DoNotDisturbTrait: | ||
| """Create a DoNotDisturbTrait instance with mocked dependencies.""" | ||
| return DoNotDisturbTrait(mock_rpc_channel) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def sample_dnd_timer() -> DnDTimer: | ||
| """Create a sample DnDTimer for testing.""" | ||
| return DnDTimer( | ||
| start_hour=22, | ||
| start_minute=0, | ||
| end_hour=8, | ||
| end_minute=0, | ||
| enabled=1, | ||
| ) | ||
|
|
||
|
|
||
| def test_trait_name(dnd_trait: DoNotDisturbTrait) -> None: | ||
| """Test that the trait has the correct name.""" | ||
| assert dnd_trait.name == "do_not_disturb" | ||
|
|
||
|
|
||
| async def test_get_dnd_timer_success( | ||
| dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock, sample_dnd_timer: DnDTimer | ||
| ) -> None: | ||
| """Test successfully getting DnD timer settings.""" | ||
| # Setup mock to return the sample DnD timer | ||
| mock_rpc_channel.send_command.return_value = sample_dnd_timer | ||
|
|
||
| # Call the method | ||
| result = await dnd_trait.get_dnd_timer() | ||
|
|
||
| # Verify the result | ||
| assert result == sample_dnd_timer | ||
| assert result.start_hour == 22 | ||
| assert result.start_minute == 0 | ||
| assert result.end_hour == 8 | ||
| assert result.end_minute == 0 | ||
| assert result.enabled == 1 | ||
|
|
||
| # Verify the RPC call was made correctly | ||
| mock_rpc_channel.send_command.assert_called_once_with(RoborockCommand.GET_DND_TIMER, response_type=DnDTimer) | ||
|
|
||
|
|
||
| async def test_get_dnd_timer_disabled(dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock) -> None: | ||
| """Test getting DnD timer when it's disabled.""" | ||
| disabled_timer = DnDTimer( | ||
| start_hour=22, | ||
| start_minute=0, | ||
| end_hour=8, | ||
| end_minute=0, | ||
| enabled=0, | ||
| ) | ||
| mock_rpc_channel.send_command.return_value = disabled_timer | ||
|
|
||
| result = await dnd_trait.get_dnd_timer() | ||
|
|
||
| assert result.enabled == 0 | ||
| mock_rpc_channel.send_command.assert_called_once_with(RoborockCommand.GET_DND_TIMER, response_type=DnDTimer) | ||
|
|
||
|
|
||
| async def test_set_dnd_timer_success( | ||
| dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock, sample_dnd_timer: DnDTimer | ||
| ) -> None: | ||
| """Test successfully setting DnD timer settings.""" | ||
| # Call the method | ||
| await dnd_trait.set_dnd_timer(sample_dnd_timer) | ||
|
|
||
| # Verify the RPC call was made correctly with dataclass converted to dict | ||
|
|
||
| expected_params = { | ||
| "startHour": 22, | ||
| "startMinute": 0, | ||
| "endHour": 8, | ||
| "endMinute": 0, | ||
| "enabled": 1, | ||
| } | ||
| mock_rpc_channel.send_command.assert_called_once_with(RoborockCommand.SET_DND_TIMER, params=expected_params) | ||
|
|
||
|
|
||
| async def test_clear_dnd_timer_success(dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock) -> None: | ||
| """Test successfully clearing DnD timer settings.""" | ||
| # Call the method | ||
| await dnd_trait.clear_dnd_timer() | ||
|
|
||
| # Verify the RPC call was made correctly | ||
| mock_rpc_channel.send_command.assert_called_once_with(RoborockCommand.CLOSE_DND_TIMER) | ||
|
|
||
|
|
||
| async def test_get_dnd_timer_propagates_exception(dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock) -> None: | ||
| """Test that exceptions from RPC channel are propagated in get_dnd_timer.""" | ||
| from roborock.exceptions import RoborockException | ||
|
|
||
| # Setup mock to raise an exception | ||
| mock_rpc_channel.send_command.side_effect = RoborockException("Communication error") | ||
|
|
||
| # Verify the exception is propagated | ||
| with pytest.raises(RoborockException, match="Communication error"): | ||
| await dnd_trait.get_dnd_timer() | ||
|
|
||
|
|
||
| async def test_set_dnd_timer_propagates_exception( | ||
| dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock, sample_dnd_timer: DnDTimer | ||
| ) -> None: | ||
| """Test that exceptions from RPC channel are propagated in set_dnd_timer.""" | ||
| from roborock.exceptions import RoborockException | ||
|
|
||
| # Setup mock to raise an exception | ||
| mock_rpc_channel.send_command.side_effect = RoborockException("Communication error") | ||
|
|
||
| # Verify the exception is propagated | ||
| with pytest.raises(RoborockException, match="Communication error"): | ||
| await dnd_trait.set_dnd_timer(sample_dnd_timer) | ||
|
|
||
|
|
||
| async def test_clear_dnd_timer_propagates_exception(dnd_trait: DoNotDisturbTrait, mock_rpc_channel: AsyncMock) -> None: | ||
| """Test that exceptions from RPC channel are propagated in clear_dnd_timer.""" | ||
| from roborock.exceptions import RoborockException | ||
|
|
||
| # Setup mock to raise an exception | ||
| mock_rpc_channel.send_command.side_effect = RoborockException("Communication error") | ||
|
|
||
| # Verify the exception is propagated | ||
| with pytest.raises(RoborockException, match="Communication error"): | ||
| await dnd_trait.clear_dnd_timer() |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Zooming out a bit, where do you see the data from get dnd timer being stored? In the device object? Still in the trait somehow?
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.
I don't know yet?
My thinking is on this next step is: the current set of commands isn't complex enough to have any real use cases, so I want to start adding them in. The whole trait syntax is not good yet, and needs to be rewritten, with a few examples. I was only thinking in the context of adding traits to the CLI for now.
One question i'm wondering if is if data needs to be stored at all here? but yeah i think it probably will.
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.
I think we will need data stored somewhere for simplicity, but i'm open to other solutions.
There could be cases where one trait has multiple entities relying on it, so we don't want to call update() for each entity.
But 100% fine with what you're saying for now, fine with punting this down the road.