chore: refactor v1 rpc channels#609
Merged
allenporter merged 7 commits intoPython-roborock:mainfrom Nov 29, 2025
Merged
Conversation
This merges the two packages v1_channel and v1_rpc_channel since they are very closely related. The differences between the RPC types are now handled with `RpcStrategy` (encoding, decoding, which channel, health checking, etc). The "PayloadEncodedV1RpcChannel" is now `_send_rpc` which accepts the rpc strategy. The `V1RpcChannel` interface is moved to the `v1_protocol` module and now has a single implementation that handles everyting (the `PickFirstAvailable` logic as well as the response parsing). Overall the code is now less generalized, but is probably easier to understand since its all in a single place. Notably, all the rpc code was already tested via the v1_channel interface.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the V1 RPC channel implementation by consolidating the v1_rpc_channel and v1_channel modules. The refactoring introduces a strategy pattern via RpcStrategy to handle different transport methods (MQTT, local) with their specific encoding, decoding, and health checking requirements.
Key Changes:
- Moved
V1RpcChannelProtocol fromv1_rpc_channel.pytov1_protocol.pyfor better organization - Introduced
RpcStrategydataclass to encapsulate transport-specific configuration (encoder, decoder, channel, health manager) - Consolidated RPC logic into a single
RpcChannelimplementation with automatic fallback between transports - Removed separate channel factory functions in favor of dynamic strategy creation methods
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
roborock/protocols/v1_protocol.py |
Added V1RpcChannel Protocol definition with overloaded send_command method to support both typed and untyped responses |
roborock/devices/v1_channel.py |
Introduced RpcStrategy and RpcChannel classes; consolidated all RPC logic including transport selection, encoding/decoding, and error handling with automatic fallback |
roborock/devices/v1_rpc_channel.py |
Deleted entire file - functionality merged into v1_channel.py |
roborock/devices/mqtt_channel.py |
Fixed docstring from "V1Channel" to "MQTT channel" for accuracy |
roborock/devices/traits/v1/common.py |
Updated import to use V1RpcChannel from v1_protocol module |
roborock/devices/traits/v1/__init__.py |
Updated import to use V1RpcChannel from v1_protocol module |
tests/devices/test_v1_device.py |
Updated import for decode_rpc_response from v1_protocol module |
tests/devices/test_v1_channel.py |
Updated mock patch location for create_map_response_decoder; updated test expectations to reflect new fallback behavior; changed test commands from CHANGE_SOUND_VOLUME to GET_STATUS to match test fixtures |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lash-L
approved these changes
Nov 29, 2025
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This merges the two packages v1_channel and v1_rpc_channel since they are very closely related. The differences between the RPC types are now handled with
RpcStrategy(encoding, decoding, which channel, health checking, etc). The "PayloadEncodedV1RpcChannel" is now_send_rpcwhich accepts the rpc strategy.The
V1RpcChannelinterface is moved to thev1_protocolmodule and now has a single implementation that handles everyting (thePickFirstAvailablelogic as well as the response parsing).Overall the code is now less generalized, but is probably easier to understand since its all in a single place. Notably, all the rpc code was already tested via the v1_channel interface.