Conversation
Mower firmwares (observed: GOAT A1600 RTK fw 1.15.13) push spontaneous
``onMapTrace`` messages whose body schema is completely different from
the existing ``GetMapTrace`` response:
{
"header": {"fwVer": "1.15.13", ...},
"body": {"data": {
"mid": "...", "batid": "...", "serial": "1",
"index": "0", "type": "4",
"info": "<base64 of LZMA1 compressed JSON>",
"infoSize": 3455
}}
}
The compressed ``info`` field, once decompressed, is a JSON list of
trajectory groups: ``[[group_id, "0;x1,y1;x2,y2;...;", "0;x,y;..."], ...]``
with negative-and-positive integer coordinates (relative to a map origin).
This adds a dedicated ``OnMapTrace`` message handler that:
1. Detects the new format via the presence of ``info``.
2. Decompresses via the existing Rust ``decompress_base64_data`` helper
(which already handles the firmware's trimmed 9-byte LZMA header).
3. Parses the JSON, drops the leading ``"0"`` anchor of each segment,
and concatenates the remaining points across groups.
4. Notifies ``MapTraceEvent`` using the firmware ``serial`` as ``start``
so the ``Map`` Rust helper does not clear the trace on every push.
Registered alongside the other JSON map messages so it is dispatched
*before* the legacy ``getMapTrace`` fallback (which still serves vacuum
firmwares unchanged).
Tests:
- Happy paths (single group, multi-group, multi-segment).
- ``info`` missing → ANALYSE (defer to legacy handler).
- Empty groups → ANALYSE (no event emitted).
- Corrupt ``info`` (invalid base64, too short, decompresses to non-JSON) → ANALYSE (no exception escapes).
- ``serial`` propagates as ``MapTraceEvent.start``.
- Full suite: 705/705 pass, no regression.
Refs:
- DeebotUniverse#1376 (Disable getMapTrace for Goat) — this PR
is the proper alternative: instead of disabling, the message is now
parsed and surfaces as a usable trajectory.
- Companion to DeebotUniverse#1565 (skip legacy fallback for mowers) and DeebotUniverse#1566
(warn-once rate limit). DeebotUniverse#1565 still serves as a safety net for any
remaining unhandled map messages on mowers.
Address review feedback from edenhaus on DeebotUniverse#1567: - Replace runtime LZMA encoding in tests with pre-computed static base64 strings. Test inputs are now constants, not computed at test time. - Remove mid/batid from debug log message to satisfy CodeQL "clear-text logging of sensitive information" alert. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Mowers don't expose the regular map capability used by vacuums; their trajectory only comes through MapTraceEvent. Move accumulation, FIFO cap and SVG rendering into the library so consumers only forward the event payload and read back an SVG.
- json → orjson per TID251 (banned import) - mower_trace: rename for-loop var to avoid PLW2901 reassignment - restore `except (TypeError, ValueError):` parens (ruff format had stripped them, breaking Python 3 syntax) - imports reorganised by ruff - ruff format applied to test_on_map_trace.py and the map __init__.py 8 unit tests for OnMapTrace still pass. Addresses CI 'Run prek checks' fail.
ruff-format 0.15.11 (pinned in .pre-commit-config.yaml) incorrectly rewrites `except (TypeError, ValueError):` to `except TypeError, ValueError:` which is invalid Python 3 syntax. Confirmed reproducible locally with `uvx ruff@0.15.11 format --diff`. Newer ruff releases are fine. Wrap the offending block with `# fmt: off` / `# fmt: on` so prek doesn't strip the tuple parens. The semantics (single int() call, two distinct exception types caught) are unchanged.
…onMapTrace Address review feedback from @monsivar on DeebotUniverse#1567 based on real O1200 LiDAR Pro MQTT captures. 1. Reassemble chunked LZMA streams before decompression. Firmware paginates one contiguous stream across `index` values; only `index=0` carries the LZMA header. Previously each chunk was treated as standalone, which only worked for single-chunk captures. Now buffered per `(mid, batid, serial, type)` and decompressed once `len(decoded) >= infoSize`. 2. Stop using `serial` as `MapTraceEvent.start`. Real captures reuse the same serial across different batid values; the previous behaviour bypassed the `Map` Rust helper's reset logic by accident. Use a stable non-zero constant (always ≠ 0 so the Map helper appends rather than clearing). 3. Add `MowerMapTraceEvent` to preserve `group_id` and per-segment boundaries from the firmware payload (zone / layer / cycle semantics). The legacy `MapTraceEvent` with a flattened compatibility string is still emitted for the Rust `Map` renderer; structure-aware consumers subscribe to the new event instead. 4. Static maps and live traces stay distinct concepts. The handler now emits one event per concept rather than collapsing groups across cycles into a single polyline. Safety: - Per-key buffer cap (512 kB) drops runaway buffers when a firmware never sends the final chunk. - Total buffer cap (2 MB) bounds memory across in-flight cycles; oldest cycle is evicted on overflow. - Out-of-order chunk arrivals reassemble in sorted index order, so the LZMA header always lands first. - A second `index=0` for an already-completed-or-started key resets just that key's buffer (firmware retry). Tests: - 13 OnMapTrace tests (single/multi group, chunk reassembly, out-of-order arrivals, per-key cap, fresh-cycle reset, independent concurrent keys, corrupt payload paths). - 3 new MowerMapTrace.add_groups tests for structured consumption. - Full suite: 711 passed (11 docker-marked tests skipped). Refs: DeebotUniverse#1567 review comment 4826137614 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeQL flagged the log message in OnMapTrace as 'Clear-text logging of sensitive information' because the formatted tuple included mid/batid. Same class of issue as the earlier d88f690 fix on this branch — only the cap value is needed for diagnosis, the device identifiers are not. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A new test asserts that get_message(name, static) returns the right handler per device class. With the OnMapTrace handler registered for mowers (xmp9ds), the expected value flips from None to OnMapTrace. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per @edenhaus review on mower_trace.py:3 ("we should use the map capability … instead of creating a workaround just for the traces"), the SVG accumulator/renderer is out of scope for this PR. Kept in this PR: - The onMapTrace message handler that stops the 217k-warning log storm - The MowerMapTraceEvent structure that preserves group/segment semantics (needed by any future consumer, whether the map capability or a downstream renderer) - The compatibility MapTraceEvent flat projection so today's Rust Map renderer keeps working for vacuums Removed: - deebot_client/mower_trace.py (MowerMapTrace accumulator + SVG renderer) - tests/test_mower_trace.py (12 tests for the renderer) The full-map capability for mowers (onMI / onArI / getAreaSet static map + zone registration) is the right home for rendering. That work is tracked separately with @monsivar sharing sanitised MQTT captures + a Python viewer for anchor-matching validation — planned as a follow-up PR once this parser lands. Suite: 705/705 pass (was 717, minus the 12 removed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses Codecov patch coverage report (was 87.43% patch on this PR): - test_OnMapTrace_parse_groups_handles_malformed_input: exercises the non-list-group / empty-group / non-string-segment / invalid-point-token skip branches inside _parse_groups. Also invalid-JSON and non-list-root guard paths. - test_OnMapTrace_evict_to_make_room_drops_oldest_by_total_bytes: covers the while _total() + incoming > _MAX_TOTAL_BYTES loop that per-key eviction alone doesn't hit. - test_OnMapTrace_evict_to_make_room_caps_keys_tracked: covers the _MAX_KEYS_TRACKED eviction path. Suite: 708 pass (was 705, +3 targeted tests). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Downstream consumers can re-check assumptions about the leading marker and the x,y split as the format is reverse-engineered further. Per monsivar's format notes on DeebotUniverse#1567.
…h paths Both branches (except KeyError/TypeError/ValueError on envelope fields, same on int(infoSize)) were flagged by codecov/patch on d521cca as the missing coverage delta on DeebotUniverse#1567.
This was referenced Aug 23, 2026
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #1788 +/- ##
==========================================
+ Coverage 95.02% 95.75% +0.72%
==========================================
Files 159 164 +5
Lines 6234 7000 +766
Branches 353 472 +119
==========================================
+ Hits 5924 6703 +779
+ Misses 248 202 -46
- Partials 62 95 +33 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
monsivar
force-pushed
the
feature/goat-map-work-areas
branch
from
August 24, 2026 05:53
099fa1b to
512af5e
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked draft — not ready for merge. Depends on #1782, transitively on #1567.
The diff against
devis currently cumulative because the dependency branches live in the contributor fork. It will be reduced as the preceding PRs land.Summary
This PR implements the read-only work-area layer from step 2 of #1785 for the GOAT O1200 LiDAR Pro.
It combines:
onArIwork-area geometry;getAreaSet type="ar"area IDs and user-visible names;onMImap coordinate frame.The output uses the same shared mower representation as #1567 and #1782:
MowerMapTraceGroup → MowerMapTraceSegment → pointsThe original firmware geometry record remains preserved as
raw.onArIgeometryThe observed complete
onArI type=0representation is handled as a chunked snapshot:serialdefines the expected chunk count;0..serial-1sequence;infoSize.The persistent work-area geometry is carried in layer
"1".Each observed area record contains an area ID, a local start coordinate and the same eight-direction RLE path representation used by the static map. The observed O1200 step size remains 50 coordinate units and is not generalized to all mower models.
getAreaSetmetadatagetAreaSet type="ar"supplies the persistent area metadata used to associate geometry with user-visible names.The observed rows contain:
Empty names are valid.
The response envelope
aidis not interpreted as a work-area ID.AreaSet framing
Controlled O1200 captures showed that the AreaSet envelope
infoSizeis not the decompressed payload length.The parser therefore:
infoSizeas positive opaque metadata;The fixture includes a real observed mismatch where envelope
infoSize=286while the internal/decompressed size is140.This behavior is intentionally limited to AreaSet and is not generalized to
onMIoronArI.Area registration
Work-area polygons are not stored at their final position in the main-map coordinate frame.
Registration therefore does not use model-specific fixed offsets.
For each work area the implementation:
onMIboundary;No scale or rotation is introduced by this registration step.
Public model
The resulting snapshot is exposed as:
MowerWorkAreanamegeometryMowerWorkAreasEventmidareasstep_sizegeometry.group_idpreserves the work-area ID while its points are registered into the static-map coordinate frame.Safety
Parsing and registration are fail-closed.
Malformed or incomplete chunk sets, invalid Base64/LZMA framing, invalid RLE data, incompatible map IDs, excessive sizes, missing metadata, or ambiguous registration do not produce a misleading work-area snapshot.
Scope
This PR intentionally does not implement:
Mapcapability;onMapTrack;Dependencies and follow-up
onMImain-map geometry.onArI + getAreaSetwork areas and registration.Mapcapability and Rust SVG renderer.This also supersedes the map-zone parsing direction of #1774. That PR should remain untouched until this draft has been reviewed.
Validation
The rebased implementation was validated locally with the relevant
onMI, work-area and message-routing tests:67 passed