chore: Update devices documentation with design details#633
chore: Update devices documentation with design details#633allenporter merged 7 commits intoPython-roborock:mainfrom
Conversation
This documents the current code base as is with a higher level overview. This may deserve a better home after we've removed the original API, but seems Ok fr now.
There was a problem hiding this comment.
Pull request overview
This PR updates the devices module documentation with comprehensive design details generated by Claude AI. The documentation now includes architectural diagrams, connection flow sequences, and detailed explanations of the library's layered design approach.
Key changes:
- Added architecture overview with Mermaid diagrams showing the library's layered structure
- Documented connection flows and RPC patterns with sequence diagrams
- Expanded design section with class diagrams and implementation details
- Corrected several typos in existing documentation (e.g., "unfieid" → "unified", missing "buffer" word)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
roborock/devices/README.md
Outdated
| ### Local connection | ||
|
|
||
| - We can use the `ip` from the `NetworkingInfo` to find the device | ||
| - The local connection is preferred to for improved latency and reducing load on the cloud servers to avoid rate limiting. |
There was a problem hiding this comment.
The phrase "preferred to for improved latency" is missing a word. Should be "preferred for improved latency" (remove "to").
| - The local connection is preferred to for improved latency and reducing load on the cloud servers to avoid rate limiting. | |
| - The local connection is preferred for improved latency and reducing load on the cloud servers to avoid rate limiting. |
roborock/devices/README.md
Outdated
| The `MqttSession` manages a shared MQTT connection for all devices: | ||
|
|
||
| - **Subscription Pooling**: Multiple callbacks can subscribe to the same topic | ||
| - **Idle Timeout**: Keeps subscriptions alive for 10 seconds after the last callback unsubscribes |
There was a problem hiding this comment.
Inconsistency: The description states idle timeout is "60 seconds" on line 224, but later on line 319 it says "10 seconds". These values should be consistent throughout the documentation.
| - **Idle Timeout**: Keeps subscriptions alive for 10 seconds after the last callback unsubscribes | |
| - **Idle Timeout**: Keeps subscriptions alive for 60 seconds after the last callback unsubscribes |
| 1. **Home Data**: Cached on disk, refreshed periodically | ||
| 2. **Network Info**: Cached for 12 hours | ||
| 3. **Device Capabilities**: Detected once and cached | ||
| 4. **MQTT Subscriptions**: Kept alive for 60 seconds (idle timeout) |
There was a problem hiding this comment.
Inconsistency: Line 224 states subscriptions are kept alive for "60 seconds (or idle timeout)" but line 482 says "60 seconds (idle timeout)". The first phrasing suggests these are different values while the second equates them. Additionally, line 319 mentions "10 seconds" for idle timeout. This needs clarification for accuracy.
| ### Testing | ||
|
|
||
| The test suite uses mocking extensively to avoid real devices: |
There was a problem hiding this comment.
Can you talk more about our testing structure. i.e. the different sub packages
There was a problem hiding this comment.
Done. FWIW i did not specifically consider how AI will use this file or do any sort of attempt to make it easier for AI to follow.
2 participants
This uses Claude to update the design overview based on reviewing the code and packages. Overall, it is pretty good.