From df9ea3d4f8afb052ff89e618f6945e52d8edd4df Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Tue, 1 Sep 2026 18:20:20 +0100 Subject: [PATCH] Zones are no longer taught only by driving: unbound remedies, editor anchor provenance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The zone layer was written when a zone got geometry one way: a robot was driven there and `save-zone` captured the pose. `segment-map` and the dashboard's zone editor have since added two more, and M4 made a binding travel inside a map revision, so a robot can hold geometry for a floor it has never entered. Five behaviours and about sixty prose sites still said otherwise. **`unbound` is a fact about the revision, not about where the robot has been.** The goto/fetch refusal, `task_server`'s boot warning, `site info` and `bundle.validate`'s warning each said the remedy was to drive there. They now name the mechanism — no geometry for this name in the binding this robot holds — and all three remedies: place it in the dashboard's zone editor and promote, pull the revision that binds it, or drive there and `save-zone`, which is the one that also measures an approach heading. `site info` puts the remedy on its own line under the count, since a tally and three sentences do not belong in one parenthesis. **Geometry an operator places is no longer recorded as one a robot drove to.** `zone.anchor()` defaulted to `taught` and the editor sent no anchor at all, so every click, drag and reshape was stored as a measurement — in the one field that exists to tell a later reader whether to trust a coordinate after the map changes. The editor now stamps zone/v0's `external` (`EDITOR_ANCHOR`, through `reanchored`) on every zone whose geometry it moves, and leaves an untouched one carrying what it arrived with, so a `segment-map` room stays `derived` until somebody reshapes it. `external` is zone/v0's closest fit rather than an exact one — the spec glosses it as an off-platform localisation system — but the enum is closed, and the alternatives were recording a click as a measurement or as an algorithm's output. A successor revision should carry a method for it (#616). The browser says only the method and `by: zone-editor`; `_stamp_anchor` fills in `at` from the server's clock and rewrites `by` to name the operator holding the token, which is the half a browser cannot be trusted for. Carrying it through took two fixes: `bundle.parse_zones` dropped `anchor` outright, and `zone.split` stamped `taught` over whatever arrived — between them the editor's own output was relabelled on the way back in. `read_anchor` validates one at the parse, so a method outside the four is a 422 rather than a stored claim nothing afterwards can distinguish from a real one. The snap rule loses its `taught` justification with it: nothing re-snaps a coordinate the operator did not touch, whoever put it there, and a dragged pose does snap because a drag is a fresh coordinate with the map's precision and no more. The prose sweep replaces "taught" used as a synonym for "bound" throughout, and keeps it where it means the anchor method or a floor written before a migration. Verified: `pixi run lint` clean; 749 passed / 10 skipped across `mote_fleet/test`, `mote_bringup/test` and `mote_tasks/test` (the skips are the JSON Schema conformance set — `jsonschema` is not in the env); 97 node UI tests. Driven in a real browser against `fleet-ui-check`: editing `pickup`'s pose and saving stored `method: external, by: fleet-ui-check (zone-editor)` with a server timestamp, while untouched `dropoff` and `home` kept `taught`; the review pane showed the reworded validator warning and the new inherited- zones tooltip. The goto refusal and `site info` output were read back directly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UUbZKWZ1coJyJAiaqdVvaS --- CLAUDE.md | 42 +++++++-- README.md | 2 +- docs/design/fleet.md | 4 +- docs/design/mapping-pipeline.md | 27 +++--- docs/fleet/README.md | 35 ++++--- docs/fleet/fleet-api.md | 24 +++-- docs/fleet/m4-verification.md | 6 +- docs/getting-started.md | 2 +- docs/robot/sites.md | 32 ++++--- mote_arm/README.md | 2 +- mote_arm/mote_arm/poses.py | 2 +- mote_bringup/mote_bringup/bundle.py | 38 ++++++-- .../mote_bringup/map_cleanup/README.md | 6 +- .../mote_bringup/map_cleanup/rooms_cli.py | 2 +- mote_bringup/mote_bringup/sites.py | 25 +++-- mote_bringup/mote_bringup/spec/zone.py | 92 +++++++++++++++---- mote_bringup/test/test_bundle.py | 37 +++++++- mote_bringup/test/test_sites.py | 2 +- mote_bringup/tools/icp_excursions.py | 2 +- mote_fleet/mote_fleet/mapsync.py | 2 +- mote_fleet/server/bundle_store.py | 40 +++++++- mote_fleet/server/fleet_server.py | 4 +- mote_fleet/server/ui/app.mjs | 8 +- mote_fleet/server/ui/index.html | 5 +- mote_fleet/server/ui/map.mjs | 4 +- mote_fleet/server/ui/review.mjs | 6 +- mote_fleet/server/ui/style.css | 2 +- mote_fleet/server/ui/zone_editor.mjs | 32 +++++-- mote_fleet/test/fake_robots.py | 2 +- mote_fleet/test/test_e2e_map_registry.py | 2 +- mote_fleet/test/test_map_registry.py | 6 +- mote_fleet/test/test_mapsync.py | 6 +- mote_fleet/test/test_zone_edit.py | 45 +++++++++ mote_fleet/test/test_zone_vocabulary.py | 4 +- mote_fleet/test/ui_test.mjs | 51 +++++++++- mote_tasks/README.md | 55 ++++++----- mote_tasks/config/zones.default.yaml | 2 +- mote_tasks/mote_tasks/save_zone.py | 33 ++++--- mote_tasks/mote_tasks/task_server.py | 24 ++--- mote_tasks/mote_tasks/zones.py | 73 ++++++++------- mote_tasks/test/test_segmented_zones.py | 4 +- mote_tasks/test/test_zone_split.py | 29 +++--- mote_tasks/test/test_zones.py | 18 ++-- pixi.toml | 2 +- 44 files changed, 598 insertions(+), 243 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index f222ebe..4429a2f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -300,13 +300,33 @@ differently every time. Three consequences: a *body* drag snaps its delta rather than each vertex (`snapDelta`), so a room traced onto its walls keeps its shape, and it is measured from the grab rather than accumulated per move, which would drift the zone behind the pointer by whatever each rounding threw away; the -outline `add zone` invents starts on the grid, while the **pose stays where it -was taught** — that number was measured by driving a robot there; and nothing -re-snaps a coordinate the operator did not touch. A pose **typed** into the +outline `add zone` invents starts on the grid; and **nothing re-snaps a +coordinate the operator did not touch**, whoever put it there — a pose a robot +measured by driving to it, and one an earlier edit placed, are alike in being +already recorded. Drag that pose and it snaps, because a drag is a fresh +coordinate with the map's precision and no more. A pose **typed** into the details column is not snapped either, for the same reason shift-drag is not: a number somebody wrote is already the number they meant, and the grid exists to stop a *drag* claiming precision the map does not have. +**An edit says who placed it.** zone/v0's `anchor.method` is how a later reader +decides whether to trust a coordinate once the map moves, and the three ways +geometry reaches a floor answer differently: `taught` was measured by driving a +robot there, `derived` was read off a map by `segment-map`, and a click is +neither. The editor stamps `external` (`EDITOR_ANCHOR`, through `reanchored` in +`zone_editor.mjs`) on every zone whose geometry it moves — pose, vertex or body — and leaves an +untouched one carrying whatever it arrived with, so a `segment-map` room stays +`derived` until someone reshapes it. `external` is zone/v0's closest fit rather +than an exact one (the spec glosses it as an off-platform localisation system); +the enum is closed, and the alternatives were recording a click as a +measurement or as an algorithm's output. A successor revision should carry a +method for it (#616). The browser says only the method and `by: zone-editor`; +`bundle_store._stamp_anchor` fills in `at` and which operator held the token, +because a browser's clock is the operator's laptop. The anchor then travels: the +editor packs it into the derived candidate's `zones.yaml`, `bundle.parse_zones` +carries it through the reader instead of dropping it, and `zone.split` honours +it instead of stamping `taught` over the top. + **Geometry is a property, not a type**, so the row says `point , ` or `area · corners` and there is no kind to declare. `add zone` makes an area (a square at the view centre, to drag onto the walls) and `⌖` gives an outline a @@ -408,9 +428,15 @@ leak would be a plausible-looking coordinate rather than a crash. Tests assert it by walking the whole document for geometry-shaped keys. **What it buys is a distinction the robot could not draw.** A name in the -vocabulary that this robot has never been taught now resolves `unbound` — -"I know that place, nobody has driven me there" — where before it was -`unknown_name`, which sent an operator hunting for a typo that was not there. +vocabulary that the binding this robot holds carries no geometry for now +resolves `unbound` — "I know that place, nothing has said where it is" — where +before it was `unknown_name`, which sent an operator hunting for a typo that +was not there. `unbound` is a fact about the revision, not about where the +robot has been: the promoted revision may bind it for nobody, this robot may be +running an older one, or it may be a name nothing has ever placed. So the +refusal names all three remedies — place it in the dashboard's zone editor and +promote, pull the revision that binds it, or drive there and `save-zone` — and +not just the last. `task_server` loads with `zones.load_floor` (vocabulary ∪ binding) rather than `load_zones` (bound only) for exactly that. A binding whose name the vocabulary does not carry is a **local extension**: usable here, never advertised, because @@ -542,7 +568,7 @@ Milestone Ms of `docs/design/fleet.md`: how the two **non-robot** machines are b ## Sites (maps & zones) -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 are taught by driving there and running `pixi run save-zone [--note TEXT]`, not by editing YAML; 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 this robot has *not* been taught. A floor's zones are **two files, not one** (zone/v0): `vocabulary.yaml` holds what the places are called and `binding.yaml` holds where this robot believes they are — taught together, stored apart, because only the names are portable off this robot. 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 hand-taught zones (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`. +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`. ## Drive path (who gets the wheels) @@ -626,7 +652,7 @@ Home for camera-derived perception. Runs on the robot (feeds Nav2), so unlike `m The task layer: py_trees behaviour trees on top of Nav2 (synced to the Pi). py_trees is a pixi *PyPI* dependency (not packaged on robostack/conda-forge); the ROS glue is first-party and small — no py_trees_ros. Contains: - `capabilities.py` — what this robot can be asked to do, as a capability/v0 document: `goto` with `{target}` and `fetch` with `{target, destination}`, both standard-registry keys with the registry's own property names. See "Fleet: adopting spec v0" above. - `task_server.py` — node hosting the mission trees. Three `std_msgs/String` topics carrying JSON: it publishes its capability set on `task/capabilities` (latched), takes mission/v0 commands on `task/command` and answers with mission/v0 statuses on `task/status`. A command names a capability key and carries a typed `input` validated against that capability's `input_schema`; the failures it publishes are typed (`unknown_capability`, `invalid_input`, `busy`, `precondition`, `unresolved_zone`, and on the way out `obstructed`/`unreachable`/`timeout`/`internal`). **It owns the lane** — one mission at a time, rejected with `busy` naming the holder — and evaluates the blocking preconditions before accepting. Zone names → map poses come from a zones YAML resolved via Sites (active floor, then legacy `~/.mote/zones.yaml`, then the committed `config/zones.default.yaml` whose poses match `mote_world.sdf`; in the sim, `sim_launch.py` passes the loaded world's own zones file instead). `save_zone` (`pixi run save-zone [--radius R]`) teaches zones from the live robot pose; `mission.py` (`ros2 run mote_tasks mission`) dispatches one from a terminal, which is what a JSON seam took away from `ros2 topic pub`. -- `zones.py` — the one named-place concept: `load_zones` returns `{name: Zone(name, pose, footprint)}` from the `zones:` section. A **zone** is a pose the robot navigates to (both fetch waypoints *and* goto targets resolve against this single table); it *optionally* carries an area **footprint** — a `radius` circle or a `polygon` of explicit vertices — that's just optional metadata, not a second type. `zones.containing(zones, x, y)` is the "which zone am I in" membership query (nearest-pose first) over zones that have a footprint; `goto` itself only needs the pose. A polygon may be concave (ray-cast membership, so an L-shaped ward or a corridor stretch works where a circle can only under- or over-cover: the hospital wards are 4.7x5.6 m, of which a `radius: 1.5` circle claimed 7.1 m² of 26.5 m²), wins over a `radius` if a zone carries both, and — since polygons come from post-processing a map rather than from driving — may omit `x`/`y`, in which case the loader derives a pose guaranteed to lie inside the outline. `save-zone` therefore preserves an existing footprint when it re-teaches a pose; `--radius` is the explicit way to replace one. Polygon zones no longer have to be hand-written either: `pixi run segment-map` proposes one per room of a saved map (see Sites). A `Zone` also carries the **vocabulary** half — `note` and `navigable` (zone/v0; see "Fleet: zones are place-names") — both optional, so no existing `zones.yaml` needed rewriting and a zone that says nothing but its name is somewhere a robot may drive to. Three consequences here. `resolve(zones, query)` is what `goto`/`fetch` match on: the name exactly, then case-insensitively and whitespace-normalised, which is what makes `store room` typeable; both then refuse a **non-navigable** zone rather than driving to it — `fetch` explicitly, because falling through to its label branch would send the detector hunting for an object called "server room". `load_zones` **refuses a vocabulary with a collision** (two zones answering one query), because loading it would resolve `goto` by dict order — silently, once per boot, differently after an edit; the rules live once in `mote_bringup.bundle` (`zone_term`, `ambiguities`, `check_vocabulary`) so the robot, `save-map` and the fleet server cannot disagree about what a vocabulary means. And `append_zone` carries the vocabulary through a re-teach and bumps `vocabulary_revision`: a better coordinate is not a rename. +- `zones.py` — the one named-place concept: `load_zones` returns `{name: Zone(name, pose, footprint)}` from the `zones:` section. A **zone** is a pose the robot navigates to (both fetch waypoints *and* goto targets resolve against this single table); it *optionally* carries an area **footprint** — a `radius` circle or a `polygon` of explicit vertices — that's just optional metadata, not a second type. `zones.containing(zones, x, y)` is the "which zone am I in" membership query (nearest-pose first) over zones that have a footprint; `goto` itself only needs the pose. A polygon may be concave (ray-cast membership, so an L-shaped ward or a corridor stretch works where a circle can only under- or over-cover: the hospital wards are 4.7x5.6 m, of which a `radius: 1.5` circle claimed 7.1 m² of 26.5 m²), wins over a `radius` if a zone carries both, and — since polygons come from post-processing a map or from the dashboard's editor rather than from driving — may omit `x`/`y`, in which case the loader derives a pose guaranteed to lie inside the outline. `save-zone` therefore preserves an existing footprint when it re-teaches a pose; `--radius` is the explicit way to replace one. Polygon zones no longer have to be hand-written either: `pixi run segment-map` proposes one per room of a saved map (see Sites). A `Zone` also carries the **vocabulary** half — `note` and `navigable` (zone/v0; see "Fleet: zones are place-names") — both optional, so no existing `zones.yaml` needed rewriting and a zone that says nothing but its name is somewhere a robot may drive to. Three consequences here. `resolve(zones, query)` is what `goto`/`fetch` match on: the name exactly, then case-insensitively and whitespace-normalised, which is what makes `store room` typeable; both then refuse a **non-navigable** zone rather than driving to it — `fetch` explicitly, because falling through to its label branch would send the detector hunting for an object called "server room". `load_zones` **refuses a vocabulary with a collision** (two zones answering one query), because loading it would resolve `goto` by dict order — silently, once per boot, differently after an edit; the rules live once in `mote_bringup.bundle` (`zone_term`, `ambiguities`, `check_vocabulary`) so the robot, `save-map` and the fleet server cannot disagree about what a vocabulary means. And `append_zone` carries the vocabulary through a re-teach and bumps `vocabulary_revision`: a better coordinate is not a rename. - `behaviours/` — `DriveTo` (Nav2 NavigateToPose action client as a behaviour; cancels in-flight goals on preemption), `AcquireObject` (label missions: publishes the label to `detect/labels`, waits for a matching `detected_objects` detection, writes a standoff goal — 0.4 m short of the object, facing it — to `object_pose`; zone missions pass through), and `TimedStub` (placeholder pick/place until the SO-101 arm is actuated). - `trees/` — `common.py` (shared `WaitForTask` + the `task` blackboard key), `fetch.py` (wait → acquire object → drive to object → pick stub → drive to drop → place stub; blackboard keys `task`/`object_pose`/`object_label`/`drop_pose` are the seam between the command grammar and perception), and `goto.py` (wait → drive to the zone's pose; success == Nav2 success). - `test/` — mock-`navigate_to_pose` tree ticks (`test_fetch_tree.py`, `test_fetch_object.py` against a mock detector, `test_goto_tree.py`) plus pure parser/loader tests (`test_parse_command.py`, `test_goto_command.py`, `test_zones.py` — which covers zone footprints and `containing`), no Gazebo/Nav2 needed, run by `pixi run test`. diff --git a/README.md b/README.md index e232d2a..8cddd70 100644 --- a/README.md +++ b/README.md @@ -173,7 +173,7 @@ Maps live in **site bundles** under `~/.mote/sites/`. SLAM, Nav2, and teleop.** The longer version of all of the above — provisioning a clean Pi, what a site -bundle holds, the systemd services, teaching zones and running missions — is +bundle holds, the systemd services, naming zones and running missions — is [Getting started](https://clachdev.github.io/Mote/getting-started/) on the docs site. diff --git a/docs/design/fleet.md b/docs/design/fleet.md index 2fbbaa1..aa6c416 100644 --- a/docs/design/fleet.md +++ b/docs/design/fleet.md @@ -571,7 +571,7 @@ duplicated in two places — one small refactor to factor out at M4.) **Conflict: two robots map the same floor.** Do **not** auto-merge. A map frame's origin is *"an accident of where SLAM started, so zones/map/posegraph must live and travel together"* (`sites.py` docstring); silently merging two frames breaks every -taught zone coordinate. Instead the server keeps both as **candidate revisions**, +bound zone coordinate. Instead the server keeps both as **candidate revisions**, and an operator **promotes** one to canonical — the same `site use-map` semantics, centralised. Nothing is lost, nothing is silently merged, and the loser is retained for audit (matching the raw-map-retention ethos already in `save-map`). @@ -1013,7 +1013,7 @@ M7 (security hardening) : cross-cutting, folds into each; can start after M0 must not leave a floor half-promoted — the server re-announces every floor at startup, which repairs it; and zones travel *inside* the revision and replace the floor's on install, because a different session's map makes previously - taught zones wrong. The one thing the milestone does not do is make a pulled + bound zones wrong. The one thing the milestone does not do is make a pulled map take effect live: `map_server` reads its map at startup, so a flip lands on the next bringup, and health now carries the revision each robot is actually running so the gap is visible. diff --git a/docs/design/mapping-pipeline.md b/docs/design/mapping-pipeline.md index ab5e9c7..99464ef 100644 --- a/docs/design/mapping-pipeline.md +++ b/docs/design/mapping-pipeline.md @@ -47,8 +47,9 @@ road at nearly every stage. whose design assumes the robot built the map. Distribution order ran backwards (robot first, registry second). - Zones: `segment-map` proposed seven placeholder rooms; renaming is manual; - the one taught pose (`office`) was invalidated by the new frame and must be - re-taught by driving to it. The candidate was published with placeholder + the one taught pose (`office`) was invalidated by the new frame and had to + be captured again (the dashboard's zone editor, which can now place it on the + candidate, did not yet exist). The candidate was published with placeholder names because packing happens at publish time. - The bags themselves — the actual source — live in the robot's `~/.mote/bags/` under a pruner that trims older bags on every recording run. @@ -188,13 +189,14 @@ Requirements, in priority order: 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.** Rename a zone, accept/reject a proposed match, - edit aliases/kind, adjust or delete a polygon, and **click-to-teach a pose** - (place `office` by clicking the office on the map — replaces drive-to-teach - for goto targets; taught-by-driving remains for poses that need real - approach headings). Edits write back to the candidate's `zones.yaml` on the - server (bumping `vocabulary_revision`); the candidate stays inert - throughout. +- **Edit before promoting.** *Built* (task 339's review 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 + approach heading. A save derives a new candidate from the one under review + rather than writing into it (bumping `vocabulary_revision`); the source's + bytes never change and the result stays inert until promoted. Still to do: + accepting or rejecting a proposed carry-forward match. - **See the build report.** The scoring diff from stage 2, on the same page as the promote button. - **Promote.** Unchanged M4 semantics: audited operator action, symlink flip, @@ -285,9 +287,10 @@ Sized so each is one dispatchable task; existing tasks noted. report. 7. **Candidate preview** — task 339 (see the map you promote), extended with the zones overlay. -8. **Candidate zone editing** — rename/alias/kind/polygon/click-to-teach on - the candidate from the dashboard; server-side writes to candidate - `zones.yaml`, audited, inert until promotion. +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 diff --git a/docs/fleet/README.md b/docs/fleet/README.md index 161325c..84b7f67 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -694,7 +694,7 @@ M4, robots publish into rather than an operator rsyncing (§11). A robot with no basemap on the server still appears in the roster with its health and its mission; the map pane says so rather than drawing an empty grid. -**Taught zones are drawn on the basemap**: a circle for a `radius` footprint, an +**A floor's bound zones are drawn on the basemap**: a circle for a `radius` footprint, an outline for a `polygon`, a cross for a bare waypoint, each labelled — so the `goto ` targets you can type are the ones you can see. They come from the canonical revision, in that revision's map frame. @@ -730,7 +730,7 @@ and the detail pane. Two things follow from losing the side-by-side view: **The dispatch form is generated from the robot's own capability set.** It arrives retained on the broker, so the page knows the keys and the input shapes without asking; a select lists what this robot offers, and one field appears per -input property. A field becomes a **zone picker** — the taught zones of the +input property. A field becomes a **zone picker** — the bound zones of the floor on screen — exactly when its schema `$ref`s zone/v0's zone reference, which is what that `$ref` is for. So the page contains no list of capabilities and no list of which inputs are places, a robot that grows a capability grows @@ -942,7 +942,7 @@ which appears whenever the floor on screen has something waiting. It shows: can be continued in this frame), its bytes and digest. - **The zones in it**, and — the part that is easy to miss — an `inherited` mark beside the heading when they are not the revision's own. A revision that - carries no zones is drawn with the floor's, taught in a previous session's + carries no zones is drawn with the floor's, bound in a previous session's frame: they draw perfectly over the new map and are out by however far the two origins differ, which the canvas cannot show. Zones that belong to the map they are drawn on are marked nothing at all — that is what "zones" means. @@ -986,9 +986,18 @@ The controls are the map and the list together: claims precision the map does not have. A whole zone moves by whole pixels, so a traced room keeps its shape. **Hold shift to move freely**, for the rare case that wants a coordinate between two pixels. Only what you drag is - snapped: a pose taught by driving a robot there is a measurement, and it is - left exactly where the robot said, while an outline this editor invents starts - on the grid. + snapped, whoever put it there — a pose a robot measured by driving to it and + one an earlier edit placed are alike in being already recorded — while an + outline this editor invents starts on the grid. Drag a pose and it does snap: + a drag is a fresh coordinate, with the map's precision and no more. +- **What you move stops claiming a robot drove there.** Each zone's coordinate + records how it came to be — `taught` when a robot was driven there and + `save-zone` captured the pose, `derived` when `segment-map` read it off the + map — and that is what tells whoever looks next whether a re-map invalidates + it. Anything you place or drag here is neither, so it is saved as `external`, + stamped with your operator name and the server's clock. A zone you did not + touch keeps what it had, so a segmented room stays `derived` until somebody + reshapes it. - **It is the same list either way.** The zones of a revision are listed under the map whether or not you are editing them — the name, then whether it is a **point** or an **area** — and `edit zones` puts controls into those rows @@ -1045,7 +1054,7 @@ it looks right. Two consequences worth knowing: revision plus the five newest candidates, so the intermediates fall off on their own. - **A revision that inherited the floor's zones stops inheriting.** The saved - candidate carries them, which is what you want: inherited zones were taught in + candidate carries them, which is what you want: inherited zones were bound in another session's frame, and dragging them onto this map is the correction. `cancel` discards the edit. There is no autosave and nothing is written until @@ -1068,7 +1077,7 @@ digest, stages it in a temporary directory, renames it into `maps//` and flips its local `map` symlink. A half-transferred revision is never visible. **Zones travel with the map.** A revision from a different mapping session is a -different map frame, so the zones taught in the old one are wrong the moment the +different map frame, so the zones bound in the old one are wrong the moment the new map is published — the bundle's `binding.yaml` therefore replaces the floor's, and the one it replaces is kept beside it as `zones..yaml`. @@ -1083,7 +1092,7 @@ how the dashboard shows a robot that has not picked the new map up yet. Nothing is merged, and nothing is lost. Both are candidates, an operator promotes one, and the other is retained. This is not a limitation to fix: a map frame's origin is an accident of where SLAM started, so silently merging two -frames would break every taught zone coordinate. If both robots proposed the +frames would break every bound zone coordinate. If both robots proposed the same revision id (they are per-second timestamps), the second is stored as `-2` and `fleetctl sites ` shows which robot uploaded which. @@ -1152,9 +1161,9 @@ pixi run save-zone sluice --radius 0.8 --no-navigable ``` `--no-navigable` marks a place a robot must not be sent to — `goto sluice` is -then refused by the robot rather than driven to. Re-teaching a pose (`save-zone -"the kitchen"` again) keeps the note and the flag: a better coordinate is not a -rename. +then refused by the robot rather than driven to. Capturing a pose again +(`save-zone "the kitchen"`) keeps the note and the flag: a better coordinate is +not a rename. Other names a place answers to belong in the note. There is no alias list: the mission layer's resolver reads free text and already knows what a store room is, @@ -1170,7 +1179,7 @@ zones: note: the good kettle is in the store room} ``` -A floor taught before place-names still loads without being re-taught: its +A floor written before place-names still loads unchanged: its `kind`, `display_name`, `aliases`, `parent` and `tags` are accepted and dropped, its `description` is read as the note it was, and `kind: keepout` still means `navigable: false`. What it loses is alias matching — `goto galley` no longer diff --git a/docs/fleet/fleet-api.md b/docs/fleet/fleet-api.md index 142f05b..1dc51fa 100644 --- a/docs/fleet/fleet-api.md +++ b/docs/fleet/fleet-api.md @@ -251,7 +251,7 @@ overwriting the directory. ### `GET /v1/maps///zones.json` -The floor's taught places **with their coordinates**, in the same map frame as +The floor's bound places **with their coordinates**, in the same map frame as the basemap, so the dashboard can draw them and an operator can see the `goto` targets they are about to type. This is the zone **binding**: it is served beside the basemap, to a client that already has the basemap, and it is not @@ -266,9 +266,9 @@ below. ``` Read from the **canonical revision's** `zones.yaml`, falling back to the -floor-level file for a bundle seeded by rsync. `404` for a floor with no taught -zones — an empty list would claim the floor has none, which is a different -statement — and `404` for a floor with no published map, because a coordinate +floor-level file for a bundle seeded by rsync. `404` for a floor whose +revision binds nothing — an empty list would claim the floor names no places, +which is a different statement — and `404` for a floor with no published map, because a coordinate with no map frame to be in is not an answer. --- @@ -323,7 +323,7 @@ where the stationery lives. | `name` | What the place is called, which is also what a dispatcher types. Printable text with no leading or trailing space; unique within a **floor**, not within a site — two floors may each have a `reception`. Matched exactly, then case-insensitively and whitespace-normalised. | | `note` | Free text for where reality diverges from what the name implies. The other names a place answers to belong here: a resolver reads the sentence, and there is no alias list to keep in step by hand. | | `navigable` | Whether it is a legal destination. Not vocabulary — it is the planner's contract — but it travels with the names because it is not a coordinate. | -| `revision` | Bumped every time a zone is taught, so a binding can record which vocabulary it was built against. | +| `revision` | Bumped every time a zone's vocabulary is written, so a binding can record which vocabulary it was built against. | | `problems` | Empty when the vocabulary is well-formed; see below. | `kind`, `display_name`, `aliases`, `parent` and `tags` were part of this @@ -390,7 +390,7 @@ The shape of it is one rule: That is also the conflict answer. Two robots that map the same floor produce two candidates, both kept, neither merged — a map frame's origin is an accident of -where SLAM started, so merging two frames would break every taught zone +where SLAM started, so merging two frames would break every bound zone coordinate (fleet.md Q4). The loser is retained for audit. **A revision is an immutable directory, and distribution is a copy plus one @@ -529,7 +529,7 @@ which looks entirely convincing and is the exact failure this route removes. `source` is `revision` when the revision carries its own `zones.yaml` and `floor` when it inherits the floor's. The difference matters and the coordinates -cannot express it: inherited zones were taught in a *previous* SLAM session's +cannot express it: inherited zones were bound in a *previous* SLAM session's frame, so they draw perfectly over this map and are wrong by however far the two origins differ. @@ -591,6 +591,16 @@ frame they were drawn in: the operator is looking at that revision's own map. Omitted, the canonical revision is edited, which is the same thing for a floor whose published map is what is on screen. +**An entry's `anchor` is carried, not re-invented.** zone/v0's +`anchor.method` says how a coordinate came to be — `taught` for a pose a robot +was driven to, `derived` for one an algorithm read off a map — and a submitted +entry keeps whatever it names, so a zone this edit did not touch keeps its +provenance. The dashboard's editor sends `{"method": "external", "by": +"zone-editor"}` on geometry it placed or moved, because neither of the other +two is true of a click; the server fills in `at` from its own clock and +rewrites `by` to name the operator holding the token, which is the half a +browser cannot be trusted for. A method outside zone/v0's four is a `422`. + **The bar is the source's, not the upload's.** A revision with no posegraph is one mapping cannot be continued from — an error for a robot's upload, where the session can be re-run, and a *warning* on a stored revision, which navigates diff --git a/docs/fleet/m4-verification.md b/docs/fleet/m4-verification.md index 27ea2f3..d9e9b89 100644 --- a/docs/fleet/m4-verification.md +++ b/docs/fleet/m4-verification.md @@ -142,11 +142,11 @@ scratch registry seeded from the committed sim bundles, and one robot's retained presence/health/pose published straight to the wire — driven with a headless Chromium. -![The fleet map with taught zones and the revision picker](../images/fleet-map-registry.webp) +![The fleet map with the floor's zones and the revision picker](../images/fleet-map-registry.webp) What the picture verifies, none of which a unit test can: -- **Taught zones land on the map.** The four dashed rectangles are +- **The floor's zones land on the map.** The four dashed rectangles are `hospital_world`'s ward polygons drawn through the Q5 transform, and they sit exactly on the room walls in the basemap. `pickup`, `home` and `dropoff` are bare waypoints and draw as labelled crosses. @@ -226,7 +226,7 @@ a reason: 5. **Zones travel inside the revision**, and replace the floor's on install (the old file is kept as `zones..yaml`). The design says map and zones must travel together; the consequence nobody writes down is that installing a - *different* session's map makes the previously taught zones wrong, so leaving + *different* session's map makes the previously bound zones wrong, so leaving them in place would be the silent failure the rule exists to prevent. Also worth recording: `save-map` now runs the same validation locally, so a map diff --git a/docs/getting-started.md b/docs/getting-started.md index d040688..2ec9393 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -103,7 +103,7 @@ autonomously — run it *on the Pi*, so a WiFi drop cannot end the mission. `save-map` writes an immutable map revision into the active site's floor, runs a cleaning pass over it, and validates it. [Sites, maps & zones](robot/sites.md) explains what a site bundle is, why maps are revisions, -and how to teach named places. +and the three ways a named place gets a coordinate. ## 6. Drive the map diff --git a/docs/robot/sites.md b/docs/robot/sites.md index c7dea6c..bc63a78 100644 --- a/docs/robot/sites.md +++ b/docs/robot/sites.md @@ -70,7 +70,7 @@ pixi run save-map # into the active site's floor `save-map` stores the posegraph alongside the map, which is what lets a later session *continue* in the same frame. Extend a map; do not remap it — remapping -starts a new frame and every zone taught in the old one becomes wrong. +starts a new frame and every zone bound in the old one becomes wrong. Mapping runs record a rosbag by default (`mapping_launch.py record:=true`; the sim passes false) and `save-map` stamps that session's bag into the revision's @@ -93,15 +93,20 @@ image. Details: [map cleaning](map-cleanup.md). ## Zones -A **zone** is the one named-place concept: a taught pose in the floor's map -frame that the robot can navigate to. `goto ` drives to any of them, and +A **zone** is the one named-place concept: a pose in the floor's map frame +that the robot can navigate to. `goto ` drives to any of them, and `fetch` uses them as its pickup and drop waypoints. A zone may *optionally* carry an area **footprint** — a circle or a polygon — which turns a bare waypoint into something that also answers "am I inside it?". That is optional metadata on one concept, not a second kind of thing: one YAML section, one -loader, one teach command. +loader. -Teach a zone by driving there, not by editing YAML: +Geometry reaches a floor three ways: `save-zone` on the robot, `segment-map` +over a saved map, and the dashboard's zone editor. Only the first needs a robot, +and it is the only one that measures an approach heading — so a zone a mission +has to arrive at facing something is still worth driving to. + +Drive there and capture the pose: ```bash pixi run save-zone "the kitchen" --radius 1.5 @@ -110,7 +115,7 @@ pixi run save-zone "the kitchen" --radius 1.5 Re-teaching a zone's pose keeps whatever footprint it already had; passing `--radius` is the deliberate way to replace one. -Rooms do not have to be taught one at a time, either: +Rooms do not have to be bound one at a time, either: ```bash pixi run segment-map # propose one polygon zone per room @@ -118,10 +123,12 @@ pixi run segment-map --write # merge the proposal into zones.yaml to rename ``` `segment-map` carves a saved map's free space into rooms on one physical -assumption — a doorway is narrow. It is additive over hand-taught zones (a +assumption — a doorway is narrow. It is additive over zones already bound (a candidate covering an already-footprinted zone is dropped), so re-running is a no-op, and it writes beside `zones.yaml`, never into the immutable map -revision. Two consequences worth knowing: a corridor network is not proposed at +revision. A proposed room is anchored `derived`, not `taught`: an algorithm read +it off a map, which is what tells an operator later that a re-map invalidates +it. Two consequences worth knowing: a corridor network is not proposed at all, and the geometry is Manhattan after rotation. See [map cleaning & room segmentation](map-cleanup.md) and the [validation run](../tuning/2026-07-27-room-segmentation.md). @@ -134,8 +141,9 @@ are covered in [Missions](missions.md#zones-and-go-to-the-kitchen). **A zone is a place-name**: a human name bound to geometry. Beside its coordinates it carries a **vocabulary** — the `name` it is called, a free-text `note` for what the name cannot say ("stationery lives here, not in the -office"), and `navigable`. Both of the first two come from `save-zone`; a zone -that says nothing but its name is a place a robot may drive to. +office"), and `navigable`. `save-zone` and the dashboard's zone editor both +write the first two; a zone that says nothing but its name is a place a robot +may drive to. That split is the whole reason the vocabulary exists separately. `(2.0, 3.5)` is a different physical point for the robot standing beside this one, and no @@ -165,8 +173,8 @@ whole mapping session. That inertness is also the conflict answer: two robots that map one floor leave two candidates, never a merge. A pulled revision replaces the floor's `zones.yaml` (keeping the old one as -`zones..yaml`), because a different session's map makes previously -taught zones wrong, and takes effect on the **next bringup**, since +`zones..yaml`), because a different session's map makes the floor's +existing bindings wrong, and takes effect on the **next bringup**, since `map_server` reads its map at startup. The operator flow is in the diff --git a/mote_arm/README.md b/mote_arm/README.md index 217b474..02ba2aa 100644 --- a/mote_arm/README.md +++ b/mote_arm/README.md @@ -351,7 +351,7 @@ command is for. ### Named poses, and narrowing the envelope -The base layer teaches map positions by driving there and running +The base layer captures a map position by driving there and running `pixi run save-zone`; the arm's analogue is `pixi run arm-pose`. Pose the limp arm by hand, capture it, and later command it back: diff --git a/mote_arm/mote_arm/poses.py b/mote_arm/mote_arm/poses.py index 3e8adf6..041464a 100644 --- a/mote_arm/mote_arm/poses.py +++ b/mote_arm/mote_arm/poses.py @@ -1,6 +1,6 @@ """Named arm poses — teach a safe pose, then return to it. -The base layer teaches map positions by driving there and running +The base layer captures a map position by driving there and running ``pixi run save-zone`` (see ``mote_tasks``/Sites); this is the arm's analogue: pose the limp arm by hand, capture it, and later command that exact pose back. diff --git a/mote_bringup/mote_bringup/bundle.py b/mote_bringup/mote_bringup/bundle.py index e94038c..b0ec767 100644 --- a/mote_bringup/mote_bringup/bundle.py +++ b/mote_bringup/mote_bringup/bundle.py @@ -46,7 +46,7 @@ A floor's zones are two documents (zone/v0, ``mote_bringup.spec.zone``), and only one of them belongs in a revision. ``binding.yaml`` — poses, footprints, anchors — travels *inside* a published revision, because those coordinates are -only meaningful in the map frame they were taught in and the two must move +only meaningful in the map frame they were bound in and the two must move together or the fleet ends up drawing a kitchen through a wall. ``vocabulary.yaml`` — the names — stays at floor level, because the rooms did not change what they are called when the robot re-mapped them. A combined @@ -113,6 +113,7 @@ normalise_name = zone.normalise_name check_vocabulary = zone.check_vocabulary ambiguities = zone.ambiguities +read_anchor = zone.read_anchor #: Present and non-empty in every revision, whoever wrote it. REQUIRED = (MAP_YAML, META_YAML) @@ -349,6 +350,15 @@ def parse_zones(raw: dict, where: str = "zones") -> dict: polygon = entry.get("polygon") if polygon is not None: zone["polygon"] = _polygon(where, name, polygon) + if entry.get("anchor") is not None: + # Provenance, carried rather than re-invented downstream. The zone + # editor submits this shape, and a coordinate an operator placed on + # a map must not come back out of the reader as one a robot was + # driven to — which is what dropping the field here meant. + try: + zone["anchor"] = read_anchor(where, entry["anchor"]) + except SpecError as exc: + raise BundleError(f"{where}: zone {name!r} {exc}") from exc if "x" not in zone or "y" not in zone: # A polygon-only zone is legal — the loader derives a pose inside # the outline (mote_tasks.zones) — but a zone with neither is not @@ -399,8 +409,8 @@ def vocabulary(zones: dict, site: str, floor: str) -> dict: know what places can be *named* — because names are portable. The binding beside it is not. - Local extensions are left out: a zone this robot was taught but nobody has - named for the site is real and usable here, and advertising it as a shared + Local extensions are left out: a zone this robot holds a binding for that + nobody has named for the site is real and usable here, and advertising it as a shared zone would be this robot inventing vocabulary for its neighbours. """ terms = [ @@ -414,7 +424,14 @@ def vocabulary(zones: dict, site: str, floor: str) -> dict: def binding(zones: dict, site: str, floor: str, platform_id: str) -> dict: - """The private half, as a zone/v0 document. Never leaves this robot.""" + """The private half, as a zone/v0 document. + + Private to a *map frame*, not to a machine. It travels inside the revision + it was measured in — that is how M4 hands a floor's geometry to a robot + that has never driven it — and never on its own, because a coordinate + without the frame beside it is a number that means something somewhere + else. + """ bindings = [] for item in zones["zones"].values(): footprint = None @@ -507,8 +524,9 @@ def read_floor(directory, site: str = "", floor: str = "") -> dict: # Either half alone is a legitimate directory. A **map revision** # carries only the binding, because coordinates travel with the frame # they mean something in and the names of the rooms do not; a floor - # nobody has driven yet carries only the vocabulary, which is the - # portability the split buys. What comes back is the join either way. + # that has been named but has no geometry yet carries only the + # vocabulary, which is the portability the split buys. What comes back + # is the join either way. vocabulary_doc = ( load_yaml_file(vocabulary_path) if vocabulary_path.is_file() @@ -746,7 +764,11 @@ def validate(revision_dir, *, require_posegraph: bool = True) -> Report: for problem in check_vocabulary(report.zones["zones"].values()) ) else: - report.warnings.append(f"no {BINDING_YAML} — this floor has no taught places") + report.warnings.append( + f"no {BINDING_YAML} — no geometry for any of this floor's places, so " + "every name resolves unbound. Place them in the dashboard's zone " + "editor on this revision, or drive there and run save-zone" + ) unexpected = sorted(set(report.files) - ALLOWED) if unexpected: @@ -778,7 +800,7 @@ def _validate_image(revision_dir: Path, report: Report): # The raw and the cleaned map are the same frame with different pixels # (sites._promote_cleaned), so a size that differs means one of them is - # not what it claims and every zone taught on this floor is suspect. + # not what it claims and every zone bound on this floor is suspect. raw = revision_dir / "map_raw.png" if raw.is_file(): raw_size = png_size(raw) diff --git a/mote_bringup/mote_bringup/map_cleanup/README.md b/mote_bringup/mote_bringup/map_cleanup/README.md index 0d65dfe..d8475fb 100644 --- a/mote_bringup/mote_bringup/map_cleanup/README.md +++ b/mote_bringup/mote_bringup/map_cleanup/README.md @@ -65,7 +65,7 @@ res.directions_deg # detected wall orientations `room_segmentation.py` is the second stage: it takes the same occupancy grid and carves its free space into **rooms**, each proposed to the task layer as a zone with a polygon footprint, so a freshly mapped floor arrives with its rooms -already outlined instead of every one taught by driving to it. +already outlined instead of every one captured by driving to it. ```bash pixi run segment-map [MAP.yaml] [--write] [--out DIR] @@ -76,8 +76,8 @@ With no argument it segments the active site floor's current map and writes beside that floor's `zones.yaml`; `--write` merges the proposal in, where the generated `room_NN` names are meant to be renamed to what the rooms are called. Merging never overwrites — a candidate covering a zone that already has a -footprint is dropped as already-named — so it is additive after hand-teaching -and a no-op run twice. +footprint is dropped as already-named — so it is additive over zones that are +already bound, and a no-op run twice. It follows ROSE²'s idea (extend the walls into lines, let the lines partition the map into faces, merge the faces back into rooms) with the FFT orientation scan diff --git a/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py b/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py index 2d79430..923d56e 100644 --- a/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py +++ b/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py @@ -11,7 +11,7 @@ Merging never overwrites: a candidate covering the pose of a zone that already has a footprint is dropped as already-named, and an existing zone is never -touched. That makes re-running after teaching a few rooms by hand additive, and +touched. That makes re-running after binding a few rooms by hand additive, and running twice in a row a no-op. """ diff --git a/mote_bringup/mote_bringup/sites.py b/mote_bringup/mote_bringup/sites.py index 1a015d8..77eb53f 100644 --- a/mote_bringup/mote_bringup/sites.py +++ b/mote_bringup/mote_bringup/sites.py @@ -154,7 +154,7 @@ def has_zones(fdir: Path) -> bool: def zones_for_write() -> Path: - """The floor newly taught zones should be written into.""" + """The floor a newly bound zone should be written into.""" act = active() if not act: sys.exit("no active site (run: site create )") @@ -253,14 +253,25 @@ def cmd_info(): notes = [f"{len(zones)} zones"] if with_fp: notes.append(f"{with_fp} with a footprint") - # A name this robot has never been taught is worth saying: it is - # the difference between a floor it can work on and one it has only - # been told about. + # A name the binding carries no geometry for is worth saying: it is + # the difference between a floor this robot can work on and one it + # has only been told the names of. if unbound: - notes.append(f"{unbound} not taught here") + notes.append(f"{unbound} unbound here") legacy = " (combined zones.yaml — migrates on next write)" split = (fdir / bundle.VOCABULARY_YAML).exists() print(f" zones ok ({', '.join(notes)}){'' if split else legacy}") + if unbound: + # On its own line because the remedy is three sentences and the + # count is one word, and because the count alone reads as a + # tally rather than as something to act on. + print( + " an unbound name has no geometry in this " + "floor's binding: place it in\n" + " the dashboard's zone editor and promote, " + "pull the revision that binds\n" + " it, or drive there and run save-zone" + ) else: print(" zones missing") current = current_revision(fdir) @@ -493,11 +504,11 @@ def install_revision(site: str, floor: str, revision: str, blob: bytes) -> str: ``current`` (nothing to do), ``flipped`` (already had it) or ``installed``. **Coordinates travel with the map; names do not.** A revision from a - different mapping session is a different map frame, so the poses taught in + different mapping session is a different map frame, so the poses bound in the old one are wrong the instant the new map is published — the bundle's ``binding.yaml`` therefore replaces the floor's, and the one it replaces is kept beside it as ``binding..yaml``, because losing a map is - recoverable and losing every taught place silently is not. The + recoverable and losing every bound place silently is not. The ``vocabulary.yaml`` is left alone: the rooms did not change their names when the robot re-mapped them, which is the practical dividend of the zone/v0 split. diff --git a/mote_bringup/mote_bringup/spec/zone.py b/mote_bringup/mote_bringup/spec/zone.py index 17c3e10..2c7eb41 100644 --- a/mote_bringup/mote_bringup/spec/zone.py +++ b/mote_bringup/mote_bringup/spec/zone.py @@ -51,7 +51,7 @@ #: Kinds a **retired** ``kind`` field used to give a zone to say that a robot #: may not or should not go there. The taxonomy is gone; this pair is kept -#: because it is the only record an already-taught floor has that a zone is a +#: because it is the only record an already-written floor has that a zone is a #: keepout, and dropping it on read would turn a barrier into a destination — #: silently, on the first load after an upgrade. :func:`term` reads it to seed #: ``navigable`` and writes ``navigable`` back, which is the migration. @@ -88,19 +88,41 @@ LEGACY_KEYS = ("kind", "display_name", "aliases", "parent", "tags", "description") #: How a binding's coordinate came to be, which is what tells a consumer whether -#: to trust it after the map changes. ``taught`` is the honest default and what -#: Mote does; ``fiducial`` is the only one that survives re-mapping without -#: re-teaching, and the only one under which two platforms can independently -#: arrive at the same physical point without sharing a map. +#: to trust it after the map changes. ``fiducial`` is the only one that survives +#: re-mapping without re-teaching, and the only one under which two platforms +#: can independently arrive at the same physical point without sharing a map. +#: Mote writes three of the four: +#: +#: * ``taught`` — a robot was driven there and ``save-zone`` captured its pose. +#: A measurement, taken by that platform in that map frame. +#: * ``derived`` — read off a saved map by an algorithm, which ``by`` names: +#: ``segment-map``'s room outlines, and the pose a polygon-only zone gets. +#: * ``external`` — resolved off the platform. What the fleet dashboard's zone +#: editor writes for geometry an operator placed or moved on the map: no +#: robot measured it and no algorithm read it off the map, so neither of the +#: other two would be true. ``by`` names what did it. +#: +#: ``external`` is the closest of zone/v0's four to "a person pointed at the +#: map" rather than an exact fit — the spec glosses it as an off-platform +#: localisation system. The enum is closed, so the alternatives were stamping a +#: click as a measurement or as an algorithm's output, which are the two larger +#: lies. A successor revision should carry a method for it (mote #616). ANCHOR_METHODS = ("taught", "derived", "fiducial", "external") TAUGHT = "taught" DERIVED = "derived" +FIDUCIAL = "fiducial" +EXTERNAL = "external" + +#: What the zone editor puts in an ``external`` anchor's ``by``. Shared with +#: ``server/ui/zone_editor.mjs``, which stamps it, and with the server, which +#: recognises it in order to record *which* operator was at the keyboard. +EDITOR = "zone-editor" # -- why a name did not resolve ------------------------------------------- UNKNOWN_NAME = "unknown_name" # not in the vocabulary -UNBOUND = "unbound" # in it; this platform has never been taught it +UNBOUND = "unbound" # in it; this platform's binding has no geometry for it WRONG_FLOOR = "wrong_floor" # bound, but not on the active floor STALE_REVISION = "stale_revision" # bound against a revision with no continuity NOT_NAVIGABLE = "not_navigable" # a constraint zone used as a destination @@ -110,8 +132,11 @@ #: ``unknown_name`` and ``unbound`` are deliberately distinct, and this is the #: pair the split exists to make representable. The first is a mistake in the -#: request; the second is a gap in *this* robot's training, on a floor where its -#: neighbours may know the place perfectly well. An operator does different +#: request: no floor names that place. The second is a name the floor does +#: carry with no geometry beside it in the binding this platform holds — which +#: may be because the promoted revision binds it for nobody, because this +#: platform is running an older revision than the one that binds it, or because +#: it has simply never been given a coordinate here. An operator does different #: things about them, and collapsing both to "not found" hides the fleet's most #: common real fault. DISTINCT_REASONS = (UNKNOWN_NAME, UNBOUND) @@ -289,6 +314,28 @@ def anchor( return record +def read_anchor(where: str, value) -> dict: + """One anchor as it arrives from a document or a browser, through + :func:`anchor` so an unknown method is refused at the edge. + + A submitted anchor is the only part of a zone's provenance the platform + does not author, so it is the one part that could claim anything: a caller + that copied it straight through would let a browser record a click as a + measurement. Refusing here is what stops the claim reaching a stored + revision, where nothing afterwards can tell it from a real one. + """ + if not isinstance(value, dict): + raise SpecError(f"{where} anchor is not a mapping") + return anchor( + value.get("method") or TAUGHT, + at=value.get("at"), + by=str(value.get("by") or ""), + fiducial_id=value.get("fiducial_id"), + offset=value.get("offset"), + confidence=value.get("confidence"), + ) + + def bound( name: str, x: float, @@ -355,10 +402,10 @@ def binding( map_revision: str = "", vocabulary_revision: int = 0, ) -> dict: - """The private document: where *this* platform believes those places are. + """The private document: where those places are in this map frame. ``map_revision`` is not bookkeeping. A binding is valid only against a - revision that declares frame continuity with the one it was taught on, and + revision that declares frame continuity with the one it was bound in, and a platform whose active revision is not continuous with its binding must resolve every affected zone as ``stale_revision`` rather than return the old coordinate. Zones, map and pose-graph travel together or not at all. @@ -542,14 +589,19 @@ def split( same dict: each document is built from its own key list, so a geometry key added later cannot leak into the vocabulary by being forgotten. - Every migrated binding is anchored ``taught`` with no timestamp. That is the - honest record — the file it came from did not say when, or by whom, and - inventing either would put a fact into a provenance field that nothing - measured. + An entry that carries its own ``anchor`` keeps it. That matters for the one + combined file nobody hand-wrote: the zone editor packs its result as a + ``zones.yaml`` and it comes back through here, so dropping the anchor would + re-stamp every coordinate an operator placed as one a robot drove to. + Where an entry says nothing, the binding is anchored ``taught`` with no + timestamp — the honest record for a file written before there was a field + to say otherwise, which did not say when or by whom either. """ terms, bindings = [], [] for name, entry in zones["zones"].items(): terms.append(term("zones.yaml", name, entry)) + carried = entry.get("anchor") + carried = read_anchor(f"zone {name!r}", carried) if carried else None footprint = None if entry.get("polygon"): footprint = {"type": "polygon", "vertices": entry["polygon"]} @@ -563,7 +615,7 @@ def split( entry["y"], entry.get("yaw", 0.0), footprint=footprint, - anchored=anchor(TAUGHT), + anchored=carried or anchor(TAUGHT), ) ) continue @@ -581,7 +633,7 @@ def split( px, py, footprint=footprint, - anchored=anchor(DERIVED, by="polygon"), + anchored=carried or anchor(DERIVED, by="polygon"), ) ) revision = zones.get("revision", 0) @@ -605,8 +657,8 @@ def merge(vocabulary_doc: dict, binding_doc: dict | None) -> dict: What a *reader* wants — the task layer resolving a name, the dashboard drawing a floor — is both halves at once, and rebuilding that join in three places would be three chances to get the unbound case wrong. The join is - outer on the vocabulary: a name with no binding is present with no - geometry, which is what makes ``unbound`` answerable rather than + outer on the vocabulary: a name the binding has no geometry for is present + with none, which is what makes ``unbound`` answerable rather than indistinguishable from ``unknown_name``. """ bindings = { @@ -617,8 +669,8 @@ def merge(vocabulary_doc: dict, binding_doc: dict | None) -> dict: zones[item["name"]] = dict(item, **_geometry(bindings.pop(item["name"], None))) for name, item in bindings.items(): # A binding for a name the vocabulary does not have is a *local - # extension*: this robot was taught a place nobody has named for the - # site. It stays usable here and is never advertised as a shared zone. + # extension*: this platform holds a binding for a place nobody has + # named for the site. It stays usable here and is never advertised as a shared zone. zones[name] = dict(term("binding", name, {}), **_geometry(item), local=True) return { "site": vocabulary_doc.get("site") or "", diff --git a/mote_bringup/test/test_bundle.py b/mote_bringup/test/test_bundle.py index 8ef5b59..f68416c 100644 --- a/mote_bringup/test/test_bundle.py +++ b/mote_bringup/test/test_bundle.py @@ -62,6 +62,41 @@ def test_segment_map_output_is_a_bundle_this_can_read(tmp_path): assert bundle.load_yaml(text) == yaml.safe_load(text) +def test_a_combined_file_keeps_the_anchor_it_carries(tmp_path): + """Provenance survives the reader and the migration, or the migration + invents it. + + A combined ``zones.yaml`` is not only a legacy file: the fleet dashboard's + zone editor packs its result as one, so this is the path an + operator-placed coordinate takes. Dropping the field on the way through + would have ``split`` stamp ``taught`` over it, recording a click as a pose + a robot was driven to — which is precisely the claim ``anchor.method`` + exists to make checkable. An entry that says nothing still gets ``taught``, + because a file written before there was a field to say otherwise did not + say when or by whom either. + """ + (tmp_path / bundle.ZONES_YAML).write_text( + "zones:\n" + " the kitchen: {x: 1.0, y: 2.0, anchor: {method: external, by: zone-editor}}\n" + " office: {x: 3.0, y: 4.0}\n" + ) + zones = bundle.read_floor(tmp_path, "home", "ground")["zones"] + assert zones["the kitchen"]["anchor"]["method"] == "external" + assert zones["the kitchen"]["anchor"]["by"] == "zone-editor" + assert zones["office"]["anchor"]["method"] == "taught" + + +def test_an_anchor_method_outside_the_spec_is_refused(tmp_path): + """The anchor is the one field a client authors, so it is the one field a + client could use to claim anything. Refused at the parse, where a bad + method is still a bad method rather than a stored fact.""" + (tmp_path / bundle.ZONES_YAML).write_text( + "zones:\n office: {x: 3.0, y: 4.0, anchor: {method: surveyed}}\n" + ) + with pytest.raises(bundle.BundleError, match="surveyed"): + bundle.read_floor(tmp_path, "home", "ground") + + @pytest.mark.parametrize( "name", [ @@ -441,7 +476,7 @@ def test_the_image_must_be_the_one_map_yaml_names(tmp_path): def test_a_raw_map_of_a_different_size_is_refused(tmp_path): """map.png and map_raw.png are the same frame with different pixels, so a - size that differs means every zone taught on this floor is suspect.""" + size that differs means every zone bound on this floor is suspect.""" directory = revision(tmp_path / "rev") png(directory / "map_raw.png", 10, 10, lambda x, y: 254) assert any("share a frame" in error for error in bundle.validate(directory).errors) diff --git a/mote_bringup/test/test_sites.py b/mote_bringup/test/test_sites.py index 6a57d4f..02542fe 100644 --- a/mote_bringup/test/test_sites.py +++ b/mote_bringup/test/test_sites.py @@ -103,7 +103,7 @@ def test_resolve_zones_and_write_target(mote_home): sites.zones_for_write() sites.create("home") expected = sites.floor_dir("home", "ground") - # Nothing to resolve until something is taught — but that is where it goes. + # Nothing to resolve until a zone is bound — but that is where it goes. assert sites.resolve_zones() == "" assert sites.zones_for_write() == expected diff --git a/mote_bringup/tools/icp_excursions.py b/mote_bringup/tools/icp_excursions.py index a8b467d..a1fa710 100755 --- a/mote_bringup/tools/icp_excursions.py +++ b/mote_bringup/tools/icp_excursions.py @@ -9,7 +9,7 @@ The two look identical in a per-interval speed histogram and are worlds apart downstream: a spike briefly disturbs the costmap, a step corrupts the map frame -for the rest of the session and every zone taught in it. +for the rest of the session and every zone bound in it. Wheel odometry is the local reference. It is not truth over a session, but over the couple of seconds either side of one scan it drifts far less than the metres diff --git a/mote_fleet/mote_fleet/mapsync.py b/mote_fleet/mote_fleet/mapsync.py index 339ef13..9ae29a5 100644 --- a/mote_fleet/mote_fleet/mapsync.py +++ b/mote_fleet/mote_fleet/mapsync.py @@ -129,7 +129,7 @@ def publish( """Upload one local revision as a candidate. Returns the server's answer. The floor's **binding** is packed *into* the revision: zone coordinates - only mean anything in the map frame they were taught in, so they travel + only mean anything in the map frame they were bound in, so they travel with it or they travel wrong. The vocabulary is not — it is a fact about the building rather than about this map, and it is served on its own route to anything that only needs to know what the places are called. diff --git a/mote_fleet/server/bundle_store.py b/mote_fleet/server/bundle_store.py index 1e0c81d..4c66ac4 100644 --- a/mote_fleet/server/bundle_store.py +++ b/mote_fleet/server/bundle_store.py @@ -16,7 +16,7 @@ makes "two robots mapped the same floor" a non-event — both candidates are kept, neither is merged, and a human picks (fleet.md Q4: a map frame's origin is an accident of where SLAM started, so silently merging two frames breaks every -taught zone coordinate). +bound zone coordinate). **The filesystem is the truth about what is canonical.** The symlink *is* the answer, not a row that describes one, and it is flipped by an atomic @@ -55,6 +55,7 @@ sys.path.insert(0, str(_candidate)) from mote_bringup import bundle # noqa: E402 +from mote_bringup.spec import zone as zone_spec # noqa: E402 #: Candidates kept per floor, on top of whatever is canonical. Enough to see a #: mapping session's history and to roll back through it; not so many that a @@ -280,10 +281,10 @@ def read_map(self, site: str, floor: str, revision: str | None = None) -> dict: return meta def read_zones(self, site: str, floor: str) -> dict: - """The floor's taught zones, in the map frame the basemap is drawn in. + """The floor's bound zones, in the map frame the basemap is drawn in. This is the **binding**: coordinates, and therefore meaningful only - against the map they were taught on. It is served beside the basemap, + against the map they were bound in. It is served beside the basemap, to the one client that also has the basemap, and it is not what a dispatcher gets — see :meth:`read_vocabulary`. @@ -331,7 +332,7 @@ def read_revision_zones(self, site: str, floor: str, revision: str) -> dict: # Which of ``_zones_file``'s two candidates answered. The reviewer # cannot tell from the coordinates, and the difference is the trap # M4 named: a revision carrying no zones inherits the floor's, which - # were taught in a *previous* SLAM session's frame and are therefore + # were bound in a *previous* SLAM session's frame and are therefore # wrong for this map by exactly however far the two origins differ. "source": "revision" if path == directory else "floor", "frame_id": zones["frame_id"], @@ -413,6 +414,8 @@ def derive_zones( for name, entry in zones.items(): entry = dict(entry) entry.pop("name", None) + if entry.get("anchor"): + entry["anchor"] = self._stamp_anchor(entry["anchor"], by) cleaned[name] = entry payload = { "frame_id": previous.get("frame_id") or "map", @@ -433,6 +436,35 @@ def derive_zones( ) return stored, report, source + @staticmethod + def _stamp_anchor(anchor, operator: str) -> dict: + """The provenance the browser could not supply, on an anchor it marked. + + The editor stamps ``external`` on geometry it placed or moved, and + names itself in ``by`` because that is the whole of what it knows. When + it happened and who did it are the server's to say: a browser's clock + is the operator's laptop and a browser's word for who is at it is + whatever the page was told. An anchor arriving with either already + filled in is left alone — it came from the revision being edited, on a + zone this edit did not touch. + """ + if not isinstance(anchor, dict): + return anchor + if anchor.get("method") != zone_spec.EXTERNAL: + return anchor + if anchor.get("by") != zone_spec.EDITOR: + return anchor + stamped = dict(anchor) + stamped["at"] = ( + datetime.now(timezone.utc) + .isoformat(timespec="seconds") + .replace("+00:00", "Z") + ) + stamped["by"] = ( + f"{operator} ({zone_spec.EDITOR})" if operator else zone_spec.EDITOR + ) + return stamped + def vocabularies(self) -> list: """Every floor's vocabulary, for a dispatcher bootstrapping a whole fleet in one call. A floor with no zones yet is skipped, not an error. diff --git a/mote_fleet/server/fleet_server.py b/mote_fleet/server/fleet_server.py index cce2e50..9d5e548 100644 --- a/mote_fleet/server/fleet_server.py +++ b/mote_fleet/server/fleet_server.py @@ -68,7 +68,7 @@ ``registry/site//floor//current`` every agent pulls from. Two robots mapping one floor therefore end with two candidates and no merge — a map frame's origin is an accident of where SLAM started, so merging frames would break every -taught zone (fleet.md Q4). The bytes live in :mod:`bundle_store`, and both ends +bound zone (fleet.md Q4). The bytes live in :mod:`bundle_store`, and both ends validate with the *same* ROS-free module the robot writes with (``mote_bringup.bundle``). @@ -629,7 +629,7 @@ def _vocabulary(self, rest: str): The split zone/v0 asks for is expressed by the route, which is why this is not another leaf under ``/v1/maps``. Everything under that prefix is - bound to a basemap and is only true for the robot that taught it; this + bound to a basemap and is only true against that basemap's frame; this is bound to nothing, and is true for every robot at the site. A caller that must never be handed a map — an MCP front door turning "take it to the kitchen" into ``goto kitchen`` — can be given this and only this. diff --git a/mote_fleet/server/ui/app.mjs b/mote_fleet/server/ui/app.mjs index c08d80e..d344df0 100644 --- a/mote_fleet/server/ui/app.mjs +++ b/mote_fleet/server/ui/app.mjs @@ -49,7 +49,7 @@ const state = { operator: null, mapKey: null, floor: null, // the registry's view of the floor on screen: revisions, candidates - zones: [], // the floor's taught places, for the map and the dispatch picker + zones: [], // the floor's bound places, for the map and the dispatch picker }; const dom = {}; @@ -169,14 +169,14 @@ async function ensureMap(record) { dom.mapLabel.textContent = `${key} — no basemap on the fleet server (${error.message})`; return; } - // Taught places, in the same frame as the basemap. A floor may have none, + // Bound places, in the same frame as the basemap. A floor may have none, // which is a 404 and not an error worth showing. api(`/v1/maps/${site}/${floor}/zones.json`) .then((body) => state.mapKey === key && setZones(body.zones)) .catch(() => state.mapKey === key && setZones([])); } -// Taught places go two ways: onto the basemap, and into the dispatch picker. +// Bound places go two ways: onto the basemap, and into the dispatch picker. // Both are the floor's, so they arrive and are cleared together. function setZones(zones) { state.zones = zones || []; @@ -274,7 +274,7 @@ function renderDispatch(record) { el('span', { class: 'mission-field-name', text: key + (required.includes(key) ? ' *' : '') }), sub.$ref === ZONE_REF && names.length ? el('select', { 'data-input': key }, [ - el('option', { value: '', text: 'a taught zone…' }), + el('option', { value: '', text: 'a zone…' }), ...names.map((name) => el('option', { value: name, text: name })), ]) : el('input', { diff --git a/mote_fleet/server/ui/index.html b/mote_fleet/server/ui/index.html index 4760dd0..c462555 100644 --- a/mote_fleet/server/ui/index.html +++ b/mote_fleet/server/ui/index.html @@ -112,9 +112,10 @@

provenance

zones

-