Add Q10 remote trait#813
Conversation
Added commands enum to b01_q10_code_mappings.py Created pytest file
Well, i cant tell
There was a problem hiding this comment.
Pull request overview
Adds a new “remote control” movement trait for B01/Q10 devices and wires it into the existing Q10 properties API, with accompanying unit tests to validate the emitted MQTT DPS payloads.
Changes:
- Introduce
RemoteTraitwith forward/left/right/stop/exit commands sent viadpCommon. - Add
RemoteCommandenum values to the B01/Q10 code mappings. - Expose the new trait on
Q10PropertiesApiand add tests validating published DPS payloads.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
roborock/devices/traits/b01/q10/remote.py |
New trait that sends remote-control movement commands via CommandTrait. |
roborock/devices/traits/b01/q10/__init__.py |
Wires RemoteTrait into Q10PropertiesApi as q10_api.remote. |
roborock/data/b01_q10/b01_q10_code_mappings.py |
Adds RemoteCommand enum used by the new trait. |
tests/devices/traits/b01/q10/test_remote.py |
Adds async parameterized tests validating the outgoing MQTT payload structure for each remote command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
||
| class RemoteTrait: | ||
| """Trait for sending vacuum commands. |
There was a problem hiding this comment.
RemoteTrait docstring says it is a "Trait for sending vacuum commands", but this trait is for remote-control movement commands. This is misleading for readers and API consumers; update the docstring to describe remote-control commands instead of vacuum commands.
| """Trait for sending vacuum commands. | |
| """Trait for sending remote-control commands. |
| command_fn: Callable[[RemoteTrait], Awaitable[None]], | ||
| expected_payload: dict[str, Any], | ||
| ) -> None: | ||
| """Test sending a remote start command.""" |
There was a problem hiding this comment.
The test docstring says "Test sending a remote start command" but the parametrization covers multiple remote commands (forward/left/right/stop/exit). Please update the docstring to reflect what the test is actually validating.
| """Test sending a remote start command.""" | |
| """Test sending remote commands with the expected payloads.""" |
|
Looks pretty good. Mind addressing the nits from copilot? |
| INTERVAL_60 = "interval_60", 60 | ||
|
|
||
|
|
||
| class RemoteCommand(Enum): |
There was a problem hiding this comment.
Please use RoborockEnum or IntEnum
| FORWARD = 0 | ||
| LEFT = 2 | ||
| RIGHT = 3 | ||
| STOP = 4 | ||
| EXIT = 5 |
There was a problem hiding this comment.
What is 1? Does that exist?
| """Stop last moving command or start remote control.""" | ||
| await self._send_remote(RemoteCommand.STOP) | ||
|
|
||
| async def exit(self) -> None: |
There was a problem hiding this comment.
Let's name this exit_remote
| """Turn right.""" | ||
| await self._send_remote(RemoteCommand.RIGHT) | ||
|
|
||
| async def stop(self) -> None: |
There was a problem hiding this comment.
Stop both starts and stops the control?
If it uses the same REmoteCommand thats okay, but can we split it up into two different functions? one start() and one stop()?
|
Thanks @Kraina68512 I left a few comments, should be fast, mind fixing them? I will be faster on the merge afterwards. |
I really sorry for opening a new pull request, but i must
So, I updated the tests and checked it. should be fine now