Skip to content

feat: parse getRTK for mower devices - #1587

Open
Beennnn wants to merge 2 commits into
DeebotUniverse:devfrom
Beennnn:feat/parse-getrtk-mower
Open

Beennnn wants to merge 2 commits into
DeebotUniverse:devfrom
Beennnn:feat/parse-getrtk-mower

Conversation

@Beennnn

@Beennnn Beennnn commented May 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Parses the response of the existing getRTK cloud command and notifies a new RtkEvent. The Ecovacs Home app already shows these counters under the GOAT's RTK settings screen but the library currently ignores the payload.

Mapping (sample below comes from a real GOAT A1600 RTK on firmware 1.15.13):

App JSON field Event field
Usable satellites (common) observations.baseSolnSvs / roverSolnSvs base_satellites_used / rover_satellites_used
Reference station — discovered rtks[0].star base_stations[0].satellites_visible
Rover — discovered observations.roverSvs rover_satellites_visible
Signal strength observations.roverSignalScore rover_signal_score (0–100)

Sample payload:

{
  "result": 0,
  "rtks": [{"sn": "908276", "star": 30, "state": 0, "mode": 0,
            "version": "...,QD302 1.3.8,...,QD302 1.3.1"}],
  "observations": {
    "solStat": 0, "poseType": 50,
    "roverId": "908336", "roverSvs": 35, "roverSolnSvs": 30,
    "roverSignalRate": 44, "roverSignalScore": 90, "roverOcclusionRate": 8,
    "baseStnId": "\"1544\"", "baseSolnSvs": 29,
    "baseSignalRate": 45, "baseSignalScore": 94, "baseOcclusionRate": 24
  }
}

Changes

  • deebot_client/events/rtk.pyRtkEvent + RtkBaseStation dataclasses
  • deebot_client/commands/json/rtk.pyGetRtk command
  • deebot_client/capabilities.pyCapabilities.rtk (optional CapabilityEvent[RtkEvent])
  • deebot_client/hardware/xmp9ds.py — wires rtk=CapabilityEvent(RtkEvent, [GetRtk()]) for the GOAT A1600 RTK
  • tests/commands/json/test_rtk.py — full-payload + no-base-station cases

Other devices stay unaffected (rtk defaults to None).

Test

uv run pytest tests/ — 707 passed (705 existing + 2 new).

@codecov

codecov Bot commented May 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.07%. Comparing base (7fb6b38) to head (3554628).
⚠️ Report is 1 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #1587      +/-   ##
==========================================
+ Coverage   95.02%   95.07%   +0.04%     
==========================================
  Files         159      161       +2     
  Lines        6234     6297      +63     
  Branches      353      354       +1     
==========================================
+ Hits         5924     5987      +63     
  Misses        248      248              
  Partials       62       62              

☔ 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 May 9, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 226 untouched benchmarks


Comparing Beennnn:feat/parse-getrtk-mower (3554628) with dev (7fb6b38)

Open in CodSpeed

@Beennnn

Beennnn commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Rebased on top of dev (was 27 commits behind). No conflicts, suite still 694/694 green locally.

Quick recap so it's easy to pick up:

  • Parses the existing getRTK cloud command and emits a new RtkEvent with the counters the Ecovacs Home app already shows on the GOAT's RTK settings screen (baseSolnSvs, roverSolnSvs, etc.).
  • Wired into the 4 mower hardware models; vacuums are untouched.
  • Tests cover the full payload mapping including the rare states (no fix / dgps only / fix-only) seen on my A1600 RTK fw 1.15.13.

Happy to address any follow-up — let me know if anything needs a tweak.


event_bus.notify(
RtkEvent(
rover_serial_number=str(observations.get("roverId", "")),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we really need all these information?
I don't know if they are useful to show

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair question — I don't have strong signal from the field yet on which of these get consumed vs ignored. The subset I'd argue is genuinely useful right now:

  • solution_status + pose_type — the two "is the position trustworthy?" knobs. When the GOAT loses RTK lock (roverSolnSvs drops, solStat flips off 4), the mowing gets erratic. Users need to see this.
  • rover_satellites_used / base_satellites_used — direct "signal quality" proxy visible in the Ecovacs Home app already.
  • signal_score + occlusion_rate — degrees of the same signal question. Nice-to-have, somewhat redundant with satellites_used.
  • rover_serial_number / base_station_id / base_stations[] — identifiers, mostly diagnostic.

Happy to trim to just the first two groups (solution_status/pose_type + satellites_used) if you'd rather keep the surface minimal. I lean toward keeping everything since the parsing cost is identical and downstream consumers can pick what they want, but I don't feel strongly.

Beennnn and others added 2 commits July 1, 2026 08:54
Parses the cloud response of getRTK so consumers can read RTK
status (rover/base satellite counts, signal scores, occlusion
rates, base station SN/firmware) instead of only seeing it in the
official Ecovacs Home app.

Wired into the GOAT A1600 RTK (xmp9ds) capability tree as an
optional CapabilityEvent on Capabilities.rtk; other devices stay
unaffected (rtk defaults to None).

Sample payload comes from a real GOAT A1600 RTK (firmware 1.15.13);
tests cover the full payload and the no-base-station edge case.
Addresses Codecov patch coverage on this PR (was 96.83%, 2 uncovered
lines flagged in deebot_client/commands/json/rtk.py per report #4413587767):

- test_GetRtk_missing_observations_returns_analyse: covers the early
  `if not observations` return that defers to the analyse fallback when
  the device omits the field (RTK lock loss / early boot).
- test_GetRtk_skips_non_dict_rtks_entries: covers the
  `if isinstance(r, dict)` filter in the `rtks` list comprehension so a
  stray string/None in the list doesn't drop the whole event.

Full suite still green (696 tests, 11 docker deselected).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Beennnn
Beennnn force-pushed the feat/parse-getrtk-mower branch from d771dbc to 3554628 Compare July 1, 2026 06:55
@Beennnn

Beennnn commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Hi @edenhaus, gentle ping on this one 🙂
To move it forward, I'd love your call on the open question about the RTK fields. I'm happy to go either way:

  • Full set (current PR): all 8 values (rover/base satellites visible + used, signal score, occlusion rate). Occlusion-rate fields already default to disabled to keep the entity list quiet.
  • Minimal set: I can trim to just the satellite counts + signal score and drop occlusion entirely, if you'd prefer a leaner surface.

Whichever you pick, I'll push the change right away. Just flag "full" or "minimal" and I'll finalize.
Context for the urgency: the downstream Home Assistant core PR (home-assistant/core#170229) that consumes RtkEvent was auto-closed as stale while waiting on this one, so getting #1587 landed (and into a release) is what unblocks re-opening it. Thanks for taking a look!

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.

2 participants