chore: add design documentation and copilot instructions#607
chore: add design documentation and copilot instructions#607allenporter wants to merge 2 commits intoPython-roborock:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive design documentation for the python-roborock library to support upcoming architectural simplifications. The documentation was AI-generated using GitHub Copilot and Gemini 3 Pro.
- Adds
DESIGN.mddocumenting the library's architecture, communication layers, and protocol details - Adds
.github/copilot-instructions.mdwith coding standards and project-specific guidance for GitHub Copilot - Provides detailed information on device discovery, channels, protocol versions, and encryption schemes
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| roborock/devices/DESIGN.md | Comprehensive architecture documentation covering device discovery, communication channels, protocol versions, and data flow |
| .github/copilot-instructions.md | GitHub Copilot configuration with project overview, tech stack, coding standards, and examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
roborock/devices/DESIGN.md
Outdated
|
|
||
| #### MQTT Authentication | ||
| The connection to the Roborock MQTT broker requires specific credentials derived from the user's `rriot` data (obtained during login): | ||
| * **Username**: Derived from `MD5(rriot.u + ":" + rriot.k)`. |
There was a problem hiding this comment.
The username derivation is incomplete. According to the actual implementation in roborock/protocol.py line 464, the username is derived from MD5(rriot.u + ':' + rriot.k)[2:10] (characters 2-10 of the MD5 hash, not the full hash).
| * **Username**: Derived from `MD5(rriot.u + ":" + rriot.k)`. | |
| * **Username**: Derived from characters 2-10 of `MD5(rriot.u + ":" + rriot.k)` (i.e., `MD5(rriot.u + ":" + rriot.k)[2:10]`). |
roborock/devices/DESIGN.md
Outdated
| #### MQTT Authentication | ||
| The connection to the Roborock MQTT broker requires specific credentials derived from the user's `rriot` data (obtained during login): | ||
| * **Username**: Derived from `MD5(rriot.u + ":" + rriot.k)`. | ||
| * **Password**: Derived from `MD5(rriot.s + ":" + rriot.k)`. |
There was a problem hiding this comment.
The password derivation is incomplete. According to the actual implementation in roborock/protocol.py line 465, the password is derived from MD5(rriot.s + ':' + rriot.k)[16:] (characters 16 onwards of the MD5 hash, not the full hash).
| * **Password**: Derived from `MD5(rriot.s + ":" + rriot.k)`. | |
| * **Password**: Derived from the last 16 characters of `MD5(rriot.s + ":" + rriot.k)`, i.e., `MD5(rriot.s + ":" + rriot.k)[16:]`. |
roborock/devices/DESIGN.md
Outdated
| * **A01 / B01**: | ||
| * **Encryption**: AES-CBC. | ||
| * **IV**: Derived from `MD5(random + HASH)`. | ||
| * These are typically used by newer camera-equipped models (e.g., S7 MaxV, Zeo). |
There was a problem hiding this comment.
[nitpick] Corrected spelling of 'Zeo' to 'Zeo' (likely meant to be 'Zero' or a specific model name). Please verify the correct model name.
| * These are typically used by newer camera-equipped models (e.g., S7 MaxV, Zeo). | |
| * These are typically used by newer camera-equipped models (e.g., S7 MaxV, Q7 MaxV). |
.github/copilot-instructions.md
Outdated
| 3. **Create Trait**: For v1, create a class inheriting from your data model and `V1TraitMixin`. | ||
| * Set the `command` class variable to your `RoborockCommand`. | ||
| * Add methods to perform actions using `self.rpc_channel.send_command()`. | ||
| 4. **Register Trait**: Add the trait to `PropertiesApi` in `roborock/devices/traits/v1/__init__.py`. |
There was a problem hiding this comment.
The instruction about 'registering' the trait in PropertiesApi is misleading. Based on the actual code in roborock/devices/traits/v1/__init__.py, traits are added as dataclass fields to PropertiesApi, not 'registered'. The initialization happens in the __init__ method. Consider clarifying this to say 'Add the trait as a field to the PropertiesApi dataclass'.
| 4. **Register Trait**: Add the trait to `PropertiesApi` in `roborock/devices/traits/v1/__init__.py`. | |
| 4. **Add Trait**: Add the trait as a field to the `PropertiesApi` dataclass in `roborock/devices/traits/v1/__init__.py`. |
.github/copilot-instructions.md
Outdated
| @@ -0,0 +1,117 @@ | |||
| # GitHub Copilot Instructions for python-roborock | |||
There was a problem hiding this comment.
I wish a standard existed for this for all AI tools...
There was a problem hiding this comment.
The standard I am aware of is to have them all reference each others files.
There was a problem hiding this comment.
I think stating that we use semantic commit tags would be helpful as well.
.github/copilot-instructions.md
Outdated
| ### 1. Typing | ||
| * **Strict Typing**: All functions and methods must have type hints. | ||
| * **Generics**: Use `list[str]` instead of `List[str]`, `dict[str, Any]` instead of `Dict[str, Any]`, etc. | ||
| * **Optional**: Use `str | None` instead of `Optional[str]`. |
There was a problem hiding this comment.
for what it is worth, i prefer this to be the case
.github/copilot-instructions.md
Outdated
| ### 3. Documentation | ||
| * **Docstrings**: Use Google-style docstrings for all modules, classes, and functions. | ||
| * **Comments**: Comment complex logic, especially protocol parsing and encryption details. |
There was a problem hiding this comment.
Worth stating that we use pydoc?
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.
2 participants
I would like to do some further simplification of the device channels since there are currently too many unnecessary layers of abstraction, but wanted to first document what currently exists.
These are AI generated with co-pilot and Gemini 3 Pro.