Skip to content

Refactor command lookup per device - #1772

Open
monsivar wants to merge 18 commits into
DeebotUniverse:devfrom
monsivar:refactor/device-command-lookup
Open

monsivar wants to merge 18 commits into
DeebotUniverse:devfrom
monsivar:refactor/device-command-lookup

Conversation

@monsivar

Copy link
Copy Markdown
Contributor

Summary

Refactor command lookup so command classes can be resolved from the capabilities configured for a specific device, instead of relying only on the global command-name dictionaries.

This is a prerequisite for supporting different command implementations that use the same command name on different device families.

Motivation

This addresses the architectural blocker discussed in #1624.

The current global command lookup requires command names to be unique. That prevents, for example, registering two device-specific command classes that both use the same MQTT/API command name.

This PR adds a device-specific lookup while retaining the existing global registries as fallbacks for backwards compatibility.

Changes

Device capabilities

  • Extract command classes configured in the capability tree.
  • Store a per-device command lookup in Capabilities.
  • Add Capabilities.get_command(name).
  • Preserve the first/primary configured command when multiple capabilities use the same command name.
  • Keep commands hidden behind lambdas or otherwise not directly discoverable working through the existing global fallback.

Legacy message lookup

  • Pass the device's capabilities into JSON legacy message lookup.
  • Resolve the corresponding command from the device first.
  • Fall back to the existing global COMMANDS lookup when no device-specific command is available.

MQTT P2P lookup

  • Resolve P2P command classes from the subscribed device's capabilities first.
  • Use the receiver device for P2P requests (q).
  • Use the sender device for P2P responses (p).
  • Fall back to the existing global P2P command registry when the device does not configure the command.
  • Do not fall back to another global implementation when the device explicitly configures the same command name with a non-P2P command.

Tests

Added coverage for:

  • command lookup being specific to device capabilities
  • different command classes sharing the same command name
  • device-specific legacy message lookup
  • device-specific MQTT P2P command lookup
  • global MQTT P2P fallback
  • preventing an incorrect global P2P fallback for an explicitly configured device command
  • correct P2P device routing:
    • request → receiver device
    • response → sender device

Follow-up

Once this prerequisite is accepted, device families can safely select different command implementations with the same command name, including the mower clean command work discussed in #1624.

@codecov

codecov Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.56098% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.33%. Comparing base (5453eba) to head (2d50596).

Files with missing lines Patch % Lines
deebot_client/messages/json/__init__.py 85.71% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1772      +/-   ##
==========================================
+ Coverage   96.26%   96.33%   +0.06%     
==========================================
  Files         161      161              
  Lines        6399     6434      +35     
  Branches      368      379      +11     
==========================================
+ Hits         6160     6198      +38     
+ Misses        172      170       -2     
+ Partials       67       66       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed

codspeed Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Merging this PR will degrade performance by 30.42%

❌ 73 regressed benchmarks
✅ 153 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Simulation test_onStationState_analyse[1-additional_content1] 5.2 ms 7.7 ms -33.36%
❌ Simulation test_onMapSetV2[199390082-vw] 5.2 ms 7.8 ms -33.21%
❌ Simulation test_onStationState_analyse[1-additional_content0] 5.2 ms 7.8 ms -33.2%
❌ Simulation test_onStationState_analyse[2-additional_content3] 5.2 ms 7.7 ms -33.19%
❌ Simulation test_onMapInfo_V2[2-5-False] 5.2 ms 7.7 ms -33.17%
❌ Simulation test_onStationState_analyse[1-additional_content2] 5.2 ms 7.7 ms -33.12%
❌ Simulation test_onMapInfo_V2[0-1-False] 5.2 ms 7.7 ms -33.08%
❌ Simulation test_onCachedMapInfo_no_using_map[0-expected_events1] 5.2 ms 7.8 ms -32.85%
❌ Simulation test_onAutoEmpty[False-None-None] 5.4 ms 8 ms -32.68%
❌ Simulation test_onMapSetV2[199390082-ar] 5.3 ms 7.9 ms -32.59%
❌ Simulation test_onGpsPos[139.691706-35.689487] 5.4 ms 8 ms -32.51%
❌ Simulation test_onAutoEmpty[False-10-10] 5.4 ms 8 ms -32.49%
❌ Simulation test_onAutoEmpty[True-10-10] 5.4 ms 8 ms -32.49%
❌ Simulation test_onStationState[1-additional_content1-1] 5.4 ms 8 ms -32.48%
❌ Simulation test_onMapSetV2[199390082-mw] 5.3 ms 7.9 ms -32.47%
❌ Simulation test_onAutoEmpty[False-auto-auto] 5.4 ms 8 ms -32.46%
❌ Simulation test_onAutoEmpty[False-smart-smart] 5.4 ms 8 ms -32.45%
❌ Simulation test_onStats[data1-expected1] 5.4 ms 7.9 ms -32.44%
❌ Simulation test_onMapInfo_V2[1-1-True] 5.4 ms 8 ms -32.43%
❌ Simulation test_onAutoEmpty[True-auto-auto] 5.4 ms 8 ms -32.39%
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing monsivar:refactor/device-command-lookup (2d50596) with dev (5453eba)

Open in CodSpeed

@monsivar
monsivar marked this pull request as ready for review August 21, 2026 10:49
@monsivar

Copy link
Copy Markdown
Contributor Author

@edenhaus This PR addresses the per-device command lookup prerequisite you mentioned in #1624.

All CI checks are passing. When you have time, I’d appreciate a review.

This branch has not been deployed

No deployments
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.

1 participant