From b13dbecec05ddff7874d2ecf87959ac473c0e2ad Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Wed, 2 Sep 2026 15:05:18 +0100 Subject: [PATCH] Mapping design: reconcile the doc with what landed, and index it from CLAUDE.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The design landed in #111 and the month since has built four of its ten items, so the doc described work as pending that is in main and attributed two of it to the wrong task. Each work-breakdown item now names its Voro task and takes that task's state as its own, and the doc is registered as Voro doc 1 so the derivation is queryable from either end (`voro doc show 1`). Corrections the review found: - Item 8 (candidate zone editing) was credited to task 339. That is the preview pane; the editor is task 346. Stage 3's third bullet carried the same mistake. - Items 4, 7 and 8 read as unbuilt. 7 and 8 are in main (PR #100, #101), and 4 is in review (PR #113) — which also moved where it runs: a workstation beside `bag_replay`, not the fleet box, until item 5 gives a builder standing to upload. Its invocation is `map-build -- --bag `. - Stage 3's "see the candidate" described an overlay on the promote picker. What shipped is a `review` mode that stands the operations panes down, because two canvases is the confusion the pane exists to remove. The substantive finding is stage 2's step 5. It was written on zone/v0's vocabulary/binding split — names stable, coordinates rebuilt per revision — and made carry-forward a re-binding of names onto new geometry. Task 625 settled that the other way on 2026-09-02: a zone is a coordinate in the floor's frame, a fact about the building, so a rebuild moves no zone and the same-frame half of the step is void rather than easier. The step keeps what survives — matching new rooms to existing zones, proposing the leftovers, the report the review pane reads — and states the reversal rather than quietly rewriting around it, since 625 has not landed. Task 345 is parked behind 625 and carries the rescoping; its acceptance also still named `aliases` and `kinds`, which task 491 retired. CLAUDE.md gets a short section beside the fleet design references pointing at the doc: the four stages, the two rules that do most of the work, and where the state of the work lives. Verified: `mkdocs build --strict` passes (the doc was already in the nav from #111); pre-commit clean on both files. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_018MSorjZAVQsDoXfmJsJwDt --- CLAUDE.md | 19 +++++ docs/design/mapping-pipeline.md | 129 +++++++++++++++++++------------- 2 files changed, 98 insertions(+), 50 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8880ce8..0e87c0a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -570,6 +570,25 @@ Milestone Ms of `docs/design/fleet.md`: how the two **non-robot** machines are b Everything that is only meaningful relative to one mapped place — the Nav2 map pair, the slam_toolbox posegraph, and named zones — lives together as a **site bundle** under `~/.mote/sites//floors//`, managed by `mote_bringup/sites.py` (CLI: `pixi run site`, docs in the module docstring). A floor is one SLAM session (one map frame); a site groups floors sharing a location. `~/.mote/active.yaml` selects the active site/floor per robot; launch files resolve the map (`nav2_launch.py`, `robot_launch.py`) and zones (`tasks_launch.py`) from it at launch time (zones fall back to the committed default). `MOTE_HOME` overrides `~/.mote` for tests/experiments. What a revision must *contain* — and how it validates, packs and travels — is `mote_bringup/bundle.py` (ROS-free, shared with the fleet server; see the map registry section above). Map artifacts are immutable **revisions** under `floors//maps//`, published by atomically flipping the `floors//map` symlink once the revision is complete — a half-written save or interrupted transfer is never visible, and `site use-map ` rolls back. `save-map` stores the posegraph alongside the map so mapping can be *continued* in the same frame later (extend, don't remap — remapping breaks zone coordinates). Mapping runs also record the `mapping` rosbag stream by default (`mapping_launch.py record:=true`; the sim passes false), and `save-map` stamps the session's bag into the revision's `meta.yaml` for provenance (`site info` shows it). Zones get their geometry three ways, and only the first needs a robot: `pixi run save-zone [--note TEXT]` captures the pose the robot is standing at (the one way that also measures an approach heading), `segment-map` reads room outlines off a saved map, and the fleet dashboard's zone editor places and drags them on a candidate revision. A zone is a named pose (a fetch waypoint or a `goto ` target) that may optionally carry an area **footprint** — a taught `--radius` circle, or a `polygon` outline that follows the actual room walls — so it reads as a room and answers "am I in it"; `site info` shows the zone/footprint counts and how many names the binding carries no geometry for. A floor's zones are **two files, not one** (zone/v0): `vocabulary.yaml` holds what the places are called and `binding.yaml` holds where geometry says they are — stored apart because only the names are portable off the map frame they were measured in; the binding travels inside the revision that names that frame. A legacy combined `zones.yaml` is still read and is migrated the first time anything writes. See "Fleet: the zone vocabulary/binding split" and "Fleet: zones are place-names". Maps are saved as PNG (map_server reads it natively; browsers can render it directly). `save-map` automatically runs an FFT structure-extraction **cleaning pass** (`mote_bringup/map_cleanup`, `sites._promote_cleaned`): it keeps the untouched map_saver output as `map_raw.png` and promotes the decluttered image to the served `map.png` (plus a `diagnostics.png`), so navigation always consumes the cleaned map while the raw is retained for provenance/audit. The `map.yaml` frame is identical for both, so zones/localization are unaffected; a cleaning failure falls back to serving the raw. The posegraph belongs to the raw map — mapping continuation extends from raw, never the cleaned image. **Zones no longer have to be taught one at a time**: `pixi run segment-map` (`map_cleanup/room_segmentation.py`, the ROSE² second stage the declutter pass left open) carves a saved map's free space into rooms and proposes one polygon zone per room, `--write` merging them into the floor's zones for the operator to rename — additive over zones already bound (a candidate covering an already-footprinted zone is dropped as named, so re-running is a byte-identical no-op) and written at floor level, never into the immutable map revision. A proposed room is anchored `derived`, not `taught`: it was read off a map by an algorithm, which is what tells an operator later that a re-map invalidates it. The method is one physical assumption — a doorway is narrow — applied to a grid the wall lines cut into faces: faces merge wherever their shared boundary has a clear span wider than a door, so it is indifferent to room size where a distance-transform threshold is not. Two consequences: a **corridor network is not proposed at all** (a footprint is a single outline, so a region encircling a block of rooms would claim them; those are dropped, taking with them any room wrongly absorbed into the corridor), and the geometry is **Manhattan after rotation** — an arbitrarily rotated map frame is fine, a building with wings at 30° to each other is not. Scored against ground-truth room rectangles on the sim ladder by `pixi run segment-eval` (30/33 mapped hospital rooms, 10/10 office, 1/1 mote, **zero merges**, unchanged with the map turned 17° or -31°); results in `docs/tuning/2026-07-27-room-segmentation.md`. +## Mapping: the pipeline (design) + +`docs/design/mapping-pipeline.md` — **mapping as a build process**, written +from the 2026-08-02 flat-mapping session, where the best map this project has +had required leaving the paved road at every stage. The bag is the source, the +map revision is a build artifact, the parameters are the toolchain, and +promotion is a release; the robot's live map is scaffolding for navigation +during capture and is never the deliverable. Four stages on three machines: +capture (robot) → build (off-board: solve, align, declutter, segment, carry +zones forward, validate and score, package) → review (operator, dashboard) → +distribute (M4, unchanged). Two rules do most of the work. **The live config is +never deliberately hobbled** — optimisation beyond best-known-good belongs in +the build, where the output is scored and reviewed before it can reach a robot. +And **a build gates on what it can measure and reports the rest**: an assertion +the measurement cannot see is worse than no assertion, because it is believed. +It builds on the site bundle (Sites, above), the M4 registry, and the +declutter/segmentation passes, and it moves none of them. The work breakdown at +the end of the doc names its Voro task per item and is the state of the work. + ## Drive path (who gets the wheels) `DiffDriveController` has exactly one publisher: **`twist_mux`**, started with the base by `twist_mux_launch.py` (included by `mote_launch.py` and, so the sim base matches, by `sim_launch.py`). Nav2's `controller_server`/`behavior_server` publish `/cmd_vel_nav` (priority 10), everything a human drives with publishes `/cmd_vel_teleop_stamped` (priority 100) — `twist_relay` for the Foxglove panel, `pixi run teleop`, the RViz teleop panel in `mote.rviz`, and `pixi run explore` (it stands in for a human driver) — and the mux forwards one of them to `/diff_drive_controller/cmd_vel`, whose name is deliberately unchanged so bags, the benchmark and the sim smoke test still watch the command the wheels got. Table in `config/twist_mux.yaml`; rationale and measurements in `mote_bringup/README.md` "Drive path". It is **adopted, not built** (`ros-jazzy-twist-mux` 4.5.0, which supports `TwistStamped` via a `use_stamped` that defaults true — the parameter is not declared by the node, so setting it in the launch would be a no-op that read like a setting): a first-party mux would have been a Python node in a 20 Hz path, the thing #73 just removed from a 50 Hz one. Three decisions are load-bearing. **Teleop overrides Nav2, it does not cancel it** — cancelling from the drive path would wire velocity arbitration into the action layer, and a nudge to straighten the robot in a doorway would destroy a fetch mission. **The teleop input's timeout (1.0 s) is deliberately longer than the controller's `cmd_vel_timeout` (0.5 s)**, so after the operator's last command the wheels halt before Nav2 regains the topic: a takeover always ends with a stopped robot, never a handback mid-motion (measured 1.00–1.05 s of silence; pre-emption itself ~50 ms). Invert those two numbers and the property vanishes silently, so `test_twist_mux.py` holds the files together and `test_twist_mux_arbitration.py` measures a real mux. **The deadman is unchanged**: twist_mux publishes only from an input callback, with no timer and no stored last command, so every source stopping still means the drive topic stopping — asserted, not assumed, because a mux that re-published would turn "the link dropped" into "the robot keeps going". To hold autonomy off entirely there is a twist_mux **lock** on `/pause_navigation` (`std_msgs/Bool`, priority 50 — masks navigation, not teleop; a Publish panel in the shipped layout sends it), with one consequence worth knowing: a goal held off the wheels while the robot stands still fails Nav2's own `SimpleProgressChecker` after ~10 s, so a long pause ends the task. Cost is one process and **one DDS participant**, putting the robot stack at ~26 of 33. There is still **no `cancel` command in the task layer** — `task_server` accepts only `fetch`/`goto` — so "cancel the task first" was never actually possible; the pause lock is what an operator has. diff --git a/docs/design/mapping-pipeline.md b/docs/design/mapping-pipeline.md index 99464ef..1081664 100644 --- a/docs/design/mapping-pipeline.md +++ b/docs/design/mapping-pipeline.md @@ -12,9 +12,11 @@ at the end is sized so each item becomes one dispatchable task. > its validator (`mote_bringup/mote_bringup/sites.py`, > `mote_bringup/mote_bringup/bundle.py`), the map registry and its promotion > flow (`docs/design/fleet.md` M4, `mote_fleet/server/bundle_store.py`), the -> zone vocabulary split (names travel, coordinates do not — fleet-api.md), the -> declutter/segmentation passes (`mote_bringup/mote_bringup/map_cleanup/`), and -> the lockstep replay harness (task 295, `mote_simulation/tools/bag_replay/`). +> zone records the floor carries (task 625 collapses zone/v0's +> vocabulary/binding split into one floor-level document — see stage 2, step 5), +> the declutter/segmentation passes (`mote_bringup/mote_bringup/map_cleanup/`), +> and the lockstep replay harness (task 295, +> `mote_simulation/tools/bag_replay/`). > None of those move. What this design changes is **what happens between them, > on which machine, and under whose control**. @@ -120,8 +122,9 @@ mapping session. ### Stage 2 — Build (fleet box) Trigger: a new bag arriving, or an operator asking for a rebuild of an old bag -under new params. One orchestrator (`pixi run map-build `) chains what was -done by hand on 2026-08-02: +under new params. One orchestrator (`pixi run map-build -- --bag `, task +343) chains what was done by hand on 2026-08-02. It runs on a workstation +today and moves to the fleet box with item 5: 1. **Solve** — lockstep replay of the whole bag under the committed *build* params (`slam_toolbox_build_params.yaml`: the live file plus documented @@ -155,13 +158,24 @@ done by hand on 2026-08-02: 3. **Declutter** — the FFT structure pass with prominence-based peak picking (task 337); no hand thresholds. 4. **Segment** — room polygons from the cleaned map, as today. -5. **Carry forward the vocabulary** — the previous revision's zone names, - aliases, kinds and taught poses re-bind onto the new geometry: same-frame - rebuilds by containment (a named zone whose pose lands inside a proposed - room claims it); new frames get proposed matches for the operator to - confirm in review. Placeholder names are minted only for genuinely new - rooms. (This is the vocabulary/binding split doing work: names are the - stable half, coordinates are rebuilt.) +5. **Carry forward the zones** — a rebuild must not cost the operator a + renaming session, so the floor's existing zones survive it and only + genuinely new rooms are minted as placeholders. A segmented room containing + an existing zone's pose takes that zone's name; the leftovers are proposed + for the operator to confirm in review, never auto-accepted. + + > **This step was written on a premise that has since been settled the + > other way** (task 625, 2026-09-02). The draft above rested on the zone/v0 + > vocabulary/binding split — names are the stable half, coordinates are + > rebuilt per revision — and so made carry-forward a *re-binding* of names + > onto new geometry. Under floor-level zones a zone is a coordinate in the + > floor's frame, a fact about the building: the kitchen does not move, a + > map is an estimate registered into that frame, and a rebuild moves no + > zone at all. That deletes the same-frame half of this step outright and + > turns the cross-frame half into a question about the *map's* registration + > rather than the names'. What survives is the matching of new rooms to + > existing zones, and the report the review pane reads. Item 6 carries the + > rescoping. 6. **Validate + score** — `bundle.validate`, then the truth-free metrics (loop drift when the trajectory closes, explored area, speckle, wall thickness) diffed against the current canonical revision. Regressions @@ -182,14 +196,18 @@ this stage touches a robot or the canonical map. The operator decision point, and the place the 2026-08-02 session had nothing. Requirements, in priority order: -- **See the candidate.** Selecting a candidate in the promote picker renders - *that revision's* map in the pane (route for a candidate's `map.png`; task - 339), clearly marked as a preview, with a one-click return to canonical. -- **See the zones on it.** The candidate's zones draw over the preview exactly - as canonical zones draw today (circle, polygon, waypoint cross), plus the - carry-forward report: which names re-bound automatically, which are proposed - matches, which rooms are new placeholders. -- **Edit before promoting.** *Built* (task 339's review pane, +- **See the candidate.** *Built* (task 339): a `review` pane draws the + revision under review from its own `map.png`, `map.json` and `zones.json`. + It is a mode rather than a column — opening it stands the operations panes + down at every width, because a canonical canvas with robots on it beside a + candidate one without is the confusion the pane exists to remove. +- **See the zones on it.** *Built* with the pane: the candidate's zones draw + over it exactly as canonical zones draw on the operations map (circle, + polygon, waypoint cross), and the read reports whether they came from the + revision or were inherited from the floor. Still to do: the carry-forward + report — which rooms matched an existing zone, which are proposed, which are + new placeholders (item 6). +- **Edit before promoting.** *Built* (task 346, in that pane — `server/ui/zone_editor.mjs`): rename a zone, write its note, adjust or delete a polygon, and place a pose by clicking the map (`⌖`), which is what replaced driving to a goto target — `save-zone` remains for poses that need a real @@ -218,9 +236,9 @@ rsync path is demoted to a bench tool. | | | |---|---| -| **Stays** | Site bundle layout + `bundle.validate` both ends; M4 registry, promotion, announcement, pull; zone vocabulary split; live-mapping stack for capture; lockstep harness | +| **Stays** | Site bundle layout + `bundle.validate` both ends; M4 registry, promotion, announcement, pull; live-mapping stack for capture; lockstep harness | | **Changes** | Live SLAM params become best-known-good (chain 15 → 10 now — task 335 — and whatever proves out next); `save-map`/`publish-map` demoted to bench tools; promote picker becomes a review surface | -| **New** | Bag sync + retention (sync-then-prune); build orchestrator + committed build params; an orientation estimator the alignment step can be gated on; build identity for candidate upload; vocabulary carry-forward; candidate preview/edit UI; build report | +| **New** | Bag sync + retention (sync-then-prune); build orchestrator + committed build params; an orientation estimator the alignment step can be gated on; build identity for candidate upload; zone carry-forward; candidate preview/edit UI; build report | ## Decisions @@ -233,8 +251,8 @@ rsync path is demoted to a bench tool. a timestamp. (Operator decision, 2026-08-02.) - **The bag store is the system of record for mapping sessions**; the robot is never the sole holder of one. -- **Zones' human half survives rebuilds.** Renaming a flat's rooms is done - once, not per revision. +- **Zone names survive rebuilds.** Renaming a flat's rooms is done once, not + per revision. - **A build gates on what it can measure, and reports the rest.** Every gate names a threshold its own measurement resolves, demonstrated on a real map rather than a synthetic one; anything softer than that is review evidence @@ -265,36 +283,47 @@ rsync path is demoted to a bench tool. ## Work breakdown -Sized so each is one dispatchable task; existing tasks noted. +Sized so each is one dispatchable task. Each names its Voro task, and the +state of that task is the state of the item. -1. **Live params to best-known-good** — task 335 (chain 15 → 10, comment - rewrite). Unchanged by this design; the divergence note lands with item 3. -2. **Bag sync + sync-then-prune** — robot uploads mapping bags to the fleet - box on session end; pruner trims only confirmed-synced bags; bag store - beside the registry with checksums. -3. **Build params file + divergence note** — landed (#106): +1. **Live params to best-known-good** — task 335, ready (chain 15 → 10, + comment rewrite). Unchanged by this design; the divergence note landed with + item 3. +2. **Bag sync + sync-then-prune** — task 341, ready. The robot uploads mapping + bags to the fleet box on session end; the pruner trims only confirmed-synced + bags; a bag store beside the registry, with checksums. +3. **Build params file + divergence note** — task 342, landed (PR #106): `slam_toolbox_build_params.yaml` beside the live file, one divergent key, held to it by `test_slam_build_params.py`. It also records what was measured and rejected, so the same sweep is not run twice. -4. **`map-build` orchestrator** — the stage-2 chain as one command on the - fleet box, emitting a validated candidate + build report. Depends on the - lockstep harness landing (task 295 / PR 91), prominence picking (337), and - item 10 for the alignment step's gate. -5. **Build identity** — the registry accepts candidate uploads from a - credentialed builder, not only enrolled robots; audit rows name it. -6. **Vocabulary carry-forward** — same-frame rebinding by containment + - new-frame proposals, emitted into the candidate with a carry-forward - report. -7. **Candidate preview** — task 339 (see the map you promote), extended with - the zones overlay. -8. **Candidate zone editing** — *done* (task 339): rename, note, polygon and - click-to-place on the candidate from the dashboard, deriving a new - candidate, audited, inert until promotion. What remains is accepting or - rejecting a carry-forward match, which needs step 6. -9. **Build report on the review page** — stage-2 scoring diff rendered beside - the promote picker. -10. **An orientation estimator the alignment step can be gated on** — a - primitive that resolves a change of 0.25° on a real map, with the +4. **`map-build` orchestrator** — task 343, in review (PR #113). The stage-2 + chain as one command, emitting a validated candidate and a build report. It + runs on a workstation rather than the fleet box, beside `bag_replay` and + excluded from the robot sync; the fleet box gets it when item 5 gives a + builder standing to upload. Steps 1, 3–7 are built; step 2 waits on item 10. + Its evidence run — the 2026-08-02 bag rebuilt against the promoted + revision as baseline — is banked with that PR as + `docs/tuning/2026-09-01-map-build.md`. +5. **Build identity** — task 344, ready. The registry accepts candidate uploads + from a credentialed builder, not only from enrolled robots; audit rows name + it. +6. **Zone carry-forward** — task 345 (still titled "zone vocabulary + carry-forward"), parked behind task 625, which **rescopes it** — see the + note under stage 2, step 5. Under floor-level zones the same-frame half of + this item is void — the zones did not move — and what remains is matching + newly segmented rooms against the floor's existing zones, proposing the + leftovers, and emitting the report the review pane reads. +7. **Candidate preview** — task 339, landed (PR #100): the review pane renders + the revision under review, its own map and its own zones, marked as a + candidate and separate from the operations map. +8. **Candidate zone editing** — task 346, landed (PR #101): rename, note, + polygon and click-to-place on the candidate from the dashboard, deriving a + new candidate, audited, inert until promotion. What remains is accepting or + rejecting a carry-forward match, which needs item 6. +9. **Build report on the review page** — task 347, parked behind item 4. The + stage-2 scoring diff rendered beside the promote button. +10. **An orientation estimator the alignment step can be gated on** — task 615, + ready. A primitive that resolves a change of 0.25° on a real map, with the validation table `wall_rotation` never had (a known rotation applied to a real solved map, not a synthetic outline), plus the tile spread that says whether the map has one wall grid at all. Blocks item 4's step 2.