Skip to content

refactor: Centralize trait utilities#820

Merged
allenporter merged 1 commit intoPython-roborock:mainfrom
allenporter:centralize-trait-utilities
Apr 26, 2026
Merged

refactor: Centralize trait utilities#820
allenporter merged 1 commit intoPython-roborock:mainfrom
allenporter:centralize-trait-utilities

Conversation

@allenporter
Copy link
Copy Markdown
Contributor

@allenporter allenporter commented Apr 26, 2026

This extracts the core trait listener and data converter mechanisms into a shared module to eliminate duplication and establish a cleaner base for upcoming protocol features. This will be used by the upcoming v1 streaming updates feature.

Pulled out from #799 as a separate PR

@allenporter allenporter marked this pull request as ready for review April 26, 2026 21:18
Copilot AI review requested due to automatic review settings April 26, 2026 21:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors trait infrastructure by moving the trait update listener and DPS-to-dataclass conversion logic into a shared module, reducing duplication and preparing for upcoming protocol/streaming work.

Changes:

  • Generalize DpsDataConverter and TraitUpdateListener into roborock/devices/traits/common.py.
  • Update Q10 status trait to import shared utilities from the centralized module.
  • Refresh module docstring to reflect broader “device traits” scope.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
roborock/devices/traits/common.py Centralizes and generalizes trait utilities; updates docs and typing around DPS conversion.
roborock/devices/traits/b01/q10/status.py Switches imports to use the new centralized trait utilities module.
Comments suppressed due to low confidence (2)

roborock/devices/traits/common.py:44

  • TDps is currently declared as TypeVar(..., bound=int), but existing callers (e.g. B01 Q10) use B01_Q10_DP keys, which are RoborockModeEnum (a StrEnum), not int. This bound makes the shared converter’s type contract misleading and may block correct generic usage for enum-keyed DPS maps; consider widening the bound (e.g., to Hashable) or removing it so both int- and enum-keyed DPS dictionaries are supported.
    roborock/devices/traits/common.py:24
  • The update-lifecycle docstring example says the transport layer decodes DPS into a dict like {"101": 80} (string keys), but for B01 Q10 the protocol layer converts DPS keys to B01_Q10_DP enums (see roborock/protocols/b01_q10_protocol.py). Consider rewording to reflect that decoded DPS keys are normalized (e.g., int for A01/B01-Q7, B01_Q10_DP for B01-Q10) so trait code knows what key type to expect.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@allenporter allenporter merged commit d125afb into Python-roborock:main Apr 26, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants