feat: Q10 (B01/ss07) clean-record history trait#857
Conversation
Q10 clean history is push-driven over dpCleanRecord (DP 52), unlike the Q7's
synchronous get_record_list RPC. Adds:
- Q10CleanRecord: parses the 12-field op:list underscore string (raw retained),
with crash-safe enum accessors .scope/.work/.result/.started_by (IntEnums that
return None for an unmapped code, never raising like the YX from_code path)
- CleanHistoryTrait: refresh() sends {op:list}; update_from_dps decodes both the
full list and the single op:notify clean-finished push (upsert by record_id,
newest-first); registered as a read-model trait in the dispatch loop
Field layout is the device app's own (setHoldData), cross-confirmed by ioBroker's
Q10CleanRecordService. Tests cover decode, enum labels + unmapped-safety, and the
list/notify paths.
andrewlyeats
force-pushed
the
feat/q10-clean-history
branch
from
80994f3 to
80b0bd6
Compare
allenporter
left a comment
allenporter
left a comment
There was a problem hiding this comment.
Thank you @andrewlyeats this is fantastic. I have some code structure related comments, though overall look forward to merging this feature.
| _E = TypeVar("_E", bound=IntEnum) | ||
|
|
||
|
|
||
| def _safe_clean_enum(enum_cls: type[_E], value: int | None) -> _E | None: |
There was a problem hiding this comment.
Can you take a look at the enum definitions in code_mappings and use them above? I believe it will give you equivalent functionality to what is needed here, but i didn't look really closely.
|
|
||
| The device returns each record as an underscore-delimited string in the ``data`` | ||
| list of a ``{"op": "list"}`` query. The field names and meanings here follow the | ||
| device's own app, which splits the string into these 12 values. The ``*_len`` |
There was a problem hiding this comment.
don't need to mention device app here, just say its how the format works
| """Length of the saved path blob for this record (0 = none stored).""" | ||
| virtual_len: int | None = None | ||
| """Length of the saved virtual-restriction blob for this record (0 = none stored).""" | ||
| clean_mode: int | None = None |
There was a problem hiding this comment.
Once you update the fields above it should be possible to just directly add the enum types here and RoborockBase should parse them correctly, so you don't need the additional enum property method converters.
| params={str(B01_Q10_DP.CLEAN_RECORD.code): {"op": "list"}}, | ||
| ) | ||
|
|
||
| def update_from_dps(self, decoded_dps: dict[B01_Q10_DP, Any]) -> None: |
There was a problem hiding this comment.
Can you review the converter pattern used in other objects and extract separate objects for parsing the data vs updating state on the object? I realize this is more complex since it supports a merge vs an update, but i do think we still want to separate concerns a little bit more here. The trait can manage merge vs update still.
We can simplify this so we don't need two copies of the code that rebuilds the list, sorts it, and notifies, since that can happen in one place. So i think that would be:
- define a format for the notification
- have a class for parsing the dps to that object
- the trait then will get the object, decide if its merging or updating (single if statement branch), then sort (one impementation) and notify (one implementation).
My gut says the string conversion logic belongs in the converter /trait and not in the container.
|
|
||
| The device returns each record as an underscore-delimited string in the ``data`` | ||
| list of a ``{"op": "list"}`` query. The field names and meanings here follow the | ||
| device's own app, which splits the string into these 12 values. The ``*_len`` |
There was a problem hiding this comment.
Can you add some full payload examples in /tests/protocols/testdata/b01_q10_protocol? it serves as nice documentation of strings we actually see in practice from the device, since it can be a little harder to understand the full payload in unit tests
2 participants
Implements #767 for B01/ss07 (Q10).
Adds a
CleanHistoryTraitthat fetches and decodes the Q10's clean-record history. Unlike the Q7 (which exposes a synchronousget_record_listRPC), the Q10 is push-driven, so the trait queries and then decodes the device's reply on the subscribe stream.What it does
refresh()sends{"op": "list"}fordpCleanRecord(DP 52) overdpCommon; the device publishes its record list back, andupdate_from_dps()decodes each underscore-delimited record into aQ10CleanRecord(12 fields: id, start time, clean time, area, the three blob lengths, clean mode, work mode, result, start method, dust-collection count).op:"notify"— the single clean-finished push the device emits when a clean ends — is decoded the same way and upserted by record id, so the history stays current without re-polling.scope/work/result/started_by) are layered on as crash-safe properties that returnNonefor an unmapped code rather than raising. The raw ints stay authoritative, so the label layer can be kept, renamed, or dropped without touching the data.Validation
op:"list"returns and decodes 25 records on the device's own reply topic; the end-of-cleanop:"notify"push was captured on a genuine whole-apartment completion and decoded + upserted correctly.Q10CleanRecordService, in addition to the ss07 app's own record parser.Honest caveats (one device observed)
2and start-method0(remote). They're documented as app-parser-sourced and left unmapped rather than guessed — the raw int still surfaces, and the enums returnNonefor them.map_len > 0gate) is intentionally out of scope here — a natural follow-up.Happy to adjust naming/structure to fit the trait conventions, and to share capture fixtures if useful.