diff --git a/CLAUDE.md b/CLAUDE.md index 8880ce8..438c543 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -154,11 +154,11 @@ holds where the design does put it: `protocol.py`. refused while the mapping session is still up. On the robot, `mote_fleet/ mapsync.py` + a worker thread in the agent stage a pulled revision in a temp directory, verify the announced sha256, rename it into `maps//` and flip the -local symlink; **the binding travels inside the revision** and replaces the -floor's (the old one is kept as `binding..yaml`), because a different -session's map makes previously taught coordinates wrong. The **vocabulary does -not** — the names of the rooms did not change when the robot re-mapped the -floor, which is the practical dividend of the zone/v0 split. Three deliberate consequences: +local symlink; **a copy of the floor's zones travels inside the revision** and +replaces the floor's (the old one is kept as `zones..yaml`), which is +how the places an operator named in the dashboard reach a robot that has never +driven there. The floor owns them either way — a zone is a coordinate in the +floor's frame and a revision is an estimate registered into it. Three deliberate consequences: the flip and the announcement are reported separately (a broker that is down must not half-promote a floor; the server re-announces every floor at startup, which repairs it), an **upload carries no operator credential** — it names an enrolled @@ -179,13 +179,12 @@ revision-aware**, because a transform from the revision and pixels from and wrong, which is the exact failure being removed. The zones read is **revision-scoped and deliberately not gated on a published map** where `read_zones` is (the review that matters most is the first candidate on a floor -with nothing published), which does not loosen the vocabulary/binding split: -naming a revision is naming a map frame, and these stay under `/v1/maps`-shaped -paths and never under `/v1/zones`. And it reports **`source: revision|floor`**, -because `_zones_file` falls back to the floor's `zones.yaml` and inherited zones -were taught in a *previous* session's frame — they draw perfectly over the new -map and are wrong by however far the two origins differ, which no coordinate can -say. One UI ordering bug fell out and is fixed in both panes: the floor's +with nothing published); it stays under a `/v1/maps`-shaped path and never under +`/v1/zones`, because it is served beside a basemap and that is what the two +prefixes divide. And it reports **`source: revision|floor`**, because +`_zones_file` falls back to the floor's `zones.yaml` and an operator reviewing a +candidate is entitled to know that what is drawn came from beside it rather than +from inside it. One UI ordering bug fell out and is fixed in both panes: the floor's revisions were fetched only *after* its basemap loaded, behind an early return, so a floor whose only revisions were candidates listed none of them and **the first promotion on any floor could never be made from a browser**. @@ -309,23 +308,17 @@ 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. +**An edit says what placed it.** `source` records what made a zone — +`save-zone` for a pose a robot was driven to, `segment-map` for a room read off +a map, `editor` for a click — and the editor stamps `EDITOR_SOURCE` (through +`sourced` in `zone_editor.mjs`) on every zone whose geometry it moves, pose, +vertex or body, leaving an untouched one carrying what it arrived with. It is a +note and nothing decides anything from it: a zone is a coordinate in the floor's +frame however it got there. What it buys is an operator being able to see which +zones somebody drew. It travels the ordinary way: into the derived candidate's +`zones.yaml`, through `bundle.parse_zones`, and out to the wire as the +`anchor.method` zone/v0 requires (`zone.bound`, one mapping in +`_ANCHOR_METHOD`). **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 @@ -408,106 +401,90 @@ present (`$AUGEREAI_SPEC`, or a sibling `augereai-spec/`) and skips where it is not. **A v1 robot and a v2 server do not interoperate**, deliberately: a translating shim would be a third definition of the wire. -## Fleet: the zone vocabulary/binding split (zone/v0) - -`zones.yaml` held names and coordinates in one file. zone/v0 pulls them apart, -and the split is what the whole spec is for: **names are shared, coordinates -are not, maps are never shared.** A floor is now two documents — -`floors//vocabulary.yaml` (site, floor, and what the places are -*called*: `name`, `note`, `navigable` — see "Fleet: zones are place-names") and -`floors//binding.yaml` (this robot's poses, footprints and `anchor`, -stamped with `platform_id`, `frame_id` and `map_revision`). Both are built by -**`mote_bringup/spec/zone.py`**, which also holds the containment geometry, so -the robot and the fleet server give the same answer on a boundary case; the -vocabulary rules moved there out of `bundle.py`, which re-exports them. - -**The split is structural, not a rule to remember.** The vocabulary is *built* -from the fields a vocabulary may carry, never stripped of the ones it may not — -stripping holds only until someone adds a geometry key and forgets, and the -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 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 -one robot must not invent shared vocabulary for its neighbours. - -**Migration is a side effect of writing, never a step.** `bundle.read_floor` -accepts a floor directory *or* a legacy combined file, and reads the latter -through `zone.split` so both paths produce the same structure by construction; -the first `save-zone` or `segment-map --write` on an old floor writes the pair -and keeps the original as `zones.yaml.premigration`. The sim worlds' committed -`.zones.yaml` files stay combined on purpose — one file is the right -shape for a fixture with one robot in it — and are migrated on read. - -**Which half travels where.** A map revision carries the **binding**, because a -coordinate means nothing without the frame beside it; installing a pulled -revision replaces the floor's binding and leaves the vocabulary alone, so -re-mapping a floor no longer costs an operator the names they typed. The -**vocabulary** is floor-level and is what `/v1/zones` serves — and, from the -dashboard's zone editor, a candidate carries *both* halves and promotion is -what lifts its vocabulary to floor level: uploading is not publishing, applied -to names as well as to coordinates. A polygon-only zone gets its binding pose -**derived once, on write** (`zone.representative_point`), because zone/v0 -requires a binding to carry a pose and it is right to — a footprint alone -cannot say where a mission navigates to. - -Still unanswerable here: `wrong_floor` (the robot holds one floor at a time) -and `stale_revision` (the bundle declares no frame continuity — which zone/v0 -says is out of its own scope too). Broadcasting the vocabulary over the retained -registry subtree, so a second robot at a site learns the names before it has -driven a metre, is M6's and is the thing this split was the prerequisite for. - -## Fleet: the zone vocabulary - -The API served the roster, the basemaps and dispatch, but not the one thing a -dispatcher most needs — *what places can I name?* — so an MCP front door had to +## Fleet: a zone is the floor's coordinate + +**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* of the same layout, +registered into that frame; a new map is a better or worse estimate and changes +nothing about where the kitchen is. Where a map and the zones disagree it is +the **map** that gets aligned — pose-graph continuation today, rigid alignment +as the fallback. So a floor's zones are one document, `floors// +zones.yaml`, belonging to the floor: `name`, `note`, `navigable`, `x`/`y`/`yaw`, +an optional footprint (`radius` or `polygon`), and `source`. Not to a map +revision, and not to one robot. + +zone/v0 says otherwise — "names are shared, coordinates are not, maps are +never shared" — because it describes a heterogeneous fleet in which every +platform carries its own SLAM frame. Mote's is neither: M4 distributes one +canonical revision to every robot on a floor, and the frame is the floor's. So +**the spec's two documents are *views*** built by `mote_bringup/spec/zone.py` +from the single record, at the wire and never stored: `vocabulary()` is what +`GET /v1/zones` serves, `binding()` is the geometry with the +`platform_id`/`frame_id`/`map_revision` of whoever is serialising filled in +there. That module also holds the containment geometry, so the robot and the +fleet server give the same answer on a boundary case. The argument the spec is +owed is #629. + +**The names-only view is *built*, never stripped.** Stripping holds only until +someone adds a geometry key and forgets, and the leak would be a +plausible-looking coordinate rather than a crash. Tests walk the whole payload +for geometry-shaped keys rather than the ones somebody thought of. + +**A name with no coordinate is not a zone on this floor** — it is a name nobody +has placed, which is what `unknown_name` says. `mission/v0`'s +`unresolved_zone` therefore carries `unknown_name` and `ambiguous` only, and +zone/v0's `unbound` is a reason Mote cannot produce. `wrong_floor` and +`stale_revision` are unanswerable for a different reason: the robot holds one +floor at a time, and the bundle declares no frame continuity (which zone/v0 says +is out of its own scope too). + +**A zone carries `source`, not an `anchor`.** zone/v0's `anchor` answers "does +this coordinate survive a re-map?", and here every coordinate does, because the +floor's frame does not move. `source` — `save-zone`, `segment-map` or `editor` — +says what *made* the zone and nothing more; reading it to list or filter zones +is fine, and nothing may treat it as a claim about the coordinate. zone/v0 still +requires an `anchor.method` on a binding entry, so `zone.bound` fills one in +from `source` at the wire and `_ANCHOR_METHOD` is the one place that mapping +lives. + +**How a zone reaches another robot.** A revision carries a copy of +the floor's zones at pack time, promotion is what publishes an edit, and +`mapsync.install` copies the revision's copy to the floor's `zones.yaml`, +keeping the one it replaces as `zones..yaml`. A polygon-only zone gets +its pose derived at the wire (`zone.representative_point`) rather than stored, +so nothing has to keep it in step with the outline. Broadcasting the names over +the retained registry subtree, so a second robot at a site learns them before it +has driven a metre, is still M6's. + +**The two routes are two views of one file.** `GET /v1/zones` and +`/v1/zones//` answer the question a dispatcher actually asks — +*what places can I name?* — which the API did not, so an MCP front door had to work around it out of band or by scraping the list a robot prints when it -refuses an unknown zone, which is an accident of an error message rather than a -contract. `GET /v1/zones` and `/v1/zones//` answer it from the site -bundles the server already reads for maps, in the shape of **zone/v0** -(`docs/fleet/fleet-api.md`, operator flow `README.md` §12; the spec's own -`spec/zone/v0/README.md`). **The whole design is one rule: names travel, -coordinates do not.** A zone's pose is a coordinate in one robot's map frame, -whose origin is an accident of where its SLAM session started, so `(2.0, 3.5)` -is a different physical point on the robot beside it and no fleet-level -transform fixes that; the *name* is true for both. So the **vocabulary** — -the `name` of each place, a free-text `note`, and `navigable` — is published and -the **binding** is not. The split is expressed by the route rather -than by a rule someone has to remember: everything under `/v1/maps` is bound to -a basemap and served to the client that already has one, everything under -`/v1/zones` is bound to nothing. Four things are load-bearing. The payload is -**built** from the fields a vocabulary may carry, never *stripped* of the ones -it may not — stripping holds only until someone adds a geometry key to -`zones.yaml` and never reads this code, and the leak would be a -plausible-looking coordinate rather than a crash (`test_zone_vocabulary.py` -walks the whole payload for geometry-shaped keys rather than checking the ones -it thought of). The vocabulary is deliberately **not gated on a published map** -where the binding rightly is: names are a fact about the building, so a floor -someone has named but no robot has mapped still answers, which is the -portability the split buys. `problems` is **reported, not enforced** — two places -called the same thing, or a name with a stray space at one end, leave the map -perfectly good, and refusing to serve a floor's basemap over it would be the -wrong price; the name is served verbatim, because inventing one is a rename -nobody asked for. But the *robot* refuses an ambiguous vocabulary at +refuses an unknown zone (`docs/fleet/fleet-api.md`, operator flow `README.md` +§12). It serves the names and nothing else. Not because a coordinate would be +*wrong* — every robot on the floor holds the same one — but because a caller +with no basemap has nothing to draw one on, and being handed a number it cannot +place is worse than not being handed it. That is what the two prefixes say: +everything under `/v1/maps` is served beside a basemap and gated on there being +one, everything under `/v1/zones` is gated on nothing, so a floor someone has +named but no robot has mapped still answers. `problems` is **reported, not +enforced** — two places called the same thing, or a name with a stray space at +one end, leave the map perfectly good, and refusing to serve a floor's basemap +over it would be the wrong price; the name is served verbatim, because inventing +one is a rename nobody asked for. But the *robot* refuses an ambiguous set at `load_zones`, since it could not honour `goto` unambiguously. Contradictions with no reading at all — a legacy `keepout` marked `navigable: true`, a `navigable` that is neither true nor false — are refused at the parse by the shared `mote_bringup/bundle.py`, so `save-map` catches them locally and the -server catches them on upload. `save-zone` teaches the name and `--note`; -`segment-map` emits geometry and nothing else, because an enclosure with walls -round it is all it found. Related: mote #249 (the rest of spec v0 — capability -set and typed failures). +server catches them on upload. + +**Two things Mote and zone/v0 now disagree about**, both outstanding against the +spec's own repository, which is not in this checkout. Its premise — one SLAM +frame per platform — is #629. Its vocabulary schema still requires `kind`, +retired here by #609, which is #616 and is why +`test_spec_conformance.py::test_a_vocabulary_conforms` carries a strict `xfail`: +Mote's `/v1/zones` payload does not validate against it, and the marker is what +makes that fail loudly the moment a successor revision lands. ## Fleet: zones are place-names @@ -560,7 +537,9 @@ the legacy `navigable` seed), `mote_tasks/zones.py` (`Zone`, `resolve`, `append_zone`), `mote_fleet/server/ui/` (the review pane, per the "Zone Gazetteer" design), `docs/fleet/fleet-api.md` §the zone vocabulary. **The specification's own `spec/zone/v0/README.md` is not in this repo** and still -describes the seven-field vocabulary; a successor revision there is outstanding. +describes the seven-field vocabulary; a successor revision there is outstanding, +and `test_spec_conformance.py` carries a strict `xfail` that will fail loudly +when it lands. ## Fleet: the server pipelines (Ms) @@ -568,7 +547,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 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`. +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 frame — the building's, which the zones are coordinates in — and each map revision is one SLAM session registered into it; 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 — continuation is what keeps a new map registered into the floor's frame, and there is no rigid-alignment fallback yet). 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. A floor's zones are **one file**, `floors//zones.yaml`, and the floor owns it: a zone is a coordinate in the floor's frame — a fact about the building — and a map revision is an estimate registered into that frame, so a revision carries a *copy* (which is how a floor's places reach a robot that has never driven there) and never owns them. See "Fleet: a zone is the floor's coordinate" 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 the zones already there (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 records `source: segment-map` — what made it, and nothing about what the coordinate is worth. 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) @@ -659,7 +638,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 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. +- `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 `note`, `navigable` and `source` (see "Fleet: zones are place-names"), all 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 floor's names mean. And `append_zone` carries the name, note and `navigable` through a re-teach and bumps the floor's `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`. @@ -672,7 +651,7 @@ The fleet control plane — one package for both ends of one wire, the same spli - `enroll.py` (`pixi run enroll`) + `facts.py` — the robot side of enrollment and the hardware fingerprint it is idempotent on. `fleet_config.py` owns `$MOTE_HOME/fleet.yaml`. - `server/` — ROS-free scripts for the fleet box: `fleet_server.py` (stdlib `http.server`: enrollment, roster, dispatch, audit, basemaps, and the UI), `registry.py` (SQLite rows — robots, enrollment tokens, operators, the audit log; state under `$MOTE_FLEET_HOME`, default `~/.mote-fleet`), `fleetctl.py` (`pixi run fleetctl`: token/operator/robots/dispatch/audit/watch), `ui/` (the dashboard: static ES modules, a subscribe-only MQTT client, the Q5 map transform), `mosquitto.conf` + `broker.sh` (conda or container, the latter for WebSockets). Every write to `mission/command` — CLI or browser — goes through the API, so dispatch is authorized and audited in one place. - `mapsync.py` + `publish.py` (`pixi run publish-map`) — the map registry's robot side (M4): pull the canonical revision announced on the retained topic, or offer a saved one as a candidate. ROS-free, so the whole distribution flow is testable as function calls. -- `server/bundle_store.py` — the registry's byte store: candidate revisions, validation on the way in (via `mote_bringup.bundle`), and the atomic symlink flip that publishes one. The filesystem is the truth about what is canonical; the database records who promoted it. It is also where the vocabulary/binding split is enforced in reads: `read_zones` (the binding) is gated on a published map, `read_vocabulary`/`vocabularies` are not, and only the latter go out over `/v1/zones`. +- `server/bundle_store.py` — the registry's byte store: candidate revisions, validation on the way in (via `mote_bringup.bundle`), and the atomic symlink flip that publishes one. The filesystem is the truth about what is canonical; the database records who promoted it. One floor read backs three views: `read_zones` (poses, gated on a published map because it is served beside one), `read_revision_zones` (a candidate's, ungated) and `read_vocabulary`/`vocabularies` (names only, ungated) — and only the last go out over `/v1/zones`. - `test/` — four tiers: contract (payloads, schema files, and every HTTP route over a real socket, including the registry's — `api_harness.py` is the live server they share), bridge (fake MQTT client; plus `test_mapsync.py`, the robot's map staging against a real server with no ROS), browser (`ui_test.mjs` under node — the MQTT codec, the map transform and zone placement, skipped without node), and the end-to-end pair `test_e2e_fleet.py` / `test_e2e_map_registry.py`, which run a real mosquitto and the real fleet server — the first with the actual `mote_tasks` tree against a mock Nav2 including a dispatch through the API, the second publishing and promoting a map and starting a *second* robot's agent afterwards, so only a retained message can have told it. Those skip without a broker, so `pixi run test` covers the rest and `pixi run -e dev test-fleet` covers all four. ### `mote_arm` (Python/ament) diff --git a/docs/design/mapping-pipeline.md b/docs/design/mapping-pipeline.md index 99464ef..430d5c7 100644 --- a/docs/design/mapping-pipeline.md +++ b/docs/design/mapping-pipeline.md @@ -12,7 +12,7 @@ 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 +> zone vocabulary (`GET /v1/zones` serves the names — 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/`). > None of those move. What this design changes is **what happens between them, @@ -155,13 +155,11 @@ 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 names** — the previous revision's zone names, notes + and poses re-bind onto the new geometry: same-frame rebuilds by containment + (a named zone whose pose lands inside a proposed room claims it); a new + frame gets proposed matches for the operator to confirm in review. + Placeholder names are minted only for genuinely new rooms. 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 @@ -194,7 +192,7 @@ Requirements, in priority order: 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 + rather than writing into it (bumping the floor's `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 diff --git a/docs/fleet/README.md b/docs/fleet/README.md index 84b7f67..d435150 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -1076,10 +1076,12 @@ holds, ignores the rest of the fleet's, downloads the revision, checks its 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 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`. +**A revision carries a copy of the floor's zones.** The floor owns them — a zone +is a coordinate in the floor's frame and a revision is an estimate registered +into it — but the revision is how a floor's places reach a robot that has never +driven there, and the copy inside the revision an operator promoted is the +fleet's current answer. Installing one therefore replaces the floor's +`zones.yaml`, keeping the one it replaces beside it as `zones..yaml`. **The running navigation stack keeps the map it loaded.** Nav2's `map_server` reads the map at startup, so the flip takes effect on the next `pixi run robot` @@ -1143,10 +1145,10 @@ curl -s http://fleet-box:8080/v1/zones/home/ground | python -m json.tool ``` This is what to point a dispatcher at — anything turning "take it to the -kitchen" into `goto kitchen`. It is safe to hand out precisely because it -carries no coordinates: a vocabulary is portable, a binding is not. The route -that *does* carry coordinates is `/v1/maps///zones.json`, and it -is for the thing drawing zones on the basemap, which already has the basemap. +kitchen" into `goto kitchen`. It carries no coordinates because a caller of this +route has nothing to draw one on, not because a coordinate would be wrong. The +route that *does* carry them is `/v1/maps///zones.json`, and it is +for the thing drawing zones on the basemap, which already has the basemap. ### Teaching the vocabulary @@ -1171,7 +1173,7 @@ where a hand-maintained list of spellings was one more thing to keep in step. `pixi run segment-map` gives every candidate it proposes a footprint and nothing else; the names it invents (`zone_01`…) are placeholders for you to replace, in -the dashboard's zone editor or by hand in `vocabulary.yaml`: +the dashboard's zone editor or by hand in the floor's `zones.yaml`: ```yaml zones: diff --git a/docs/fleet/control-plane.md b/docs/fleet/control-plane.md index 05a8ab7..46291dc 100644 --- a/docs/fleet/control-plane.md +++ b/docs/fleet/control-plane.md @@ -317,7 +317,7 @@ broker that lost its retained state with its volume, repairs itself. |---|---|---| | `schema` | int | `1` | | `site` | string | | -| `floor` | string | one floor is one SLAM session, i.e. one map frame | +| `floor` | string | one floor is one frame; each map revision is one SLAM session registered into it | | `revision` | string | the immutable revision id; also its directory name at both ends | | `url` | string | path on the fleet server to fetch the packed revision from | | `sha256` | string | `sha256:` of the packed bundle | diff --git a/docs/fleet/fleet-api.md b/docs/fleet/fleet-api.md index 1dc51fa..cd80bd8 100644 --- a/docs/fleet/fleet-api.md +++ b/docs/fleet/fleet-api.md @@ -275,29 +275,28 @@ with no map frame to be in is not an answer. ## The zone vocabulary -**Names are shared; coordinates are not.** This is the half of a zone that is -portable between robots, served so that the question a dispatcher most needs to -ask — *what places can I name?* — has an answer in the API rather than out of -band. The shape is [zone/v0](https://spec.augereai.com/zone/v0/). - -A zone's pose is a coordinate in one robot's map frame, and that frame's origin -is an accident of where its SLAM session happened to start. `(2.0, 3.5)` on -`mote-01` is a different physical point from `(2.0, 3.5)` on `mote-02`, and -there is no fleet-level transform that fixes it — the two are independent -estimates of the same building, drifting apart. The name, by contrast, is true -for both. So the vocabulary travels and the binding does not, and the split is -in the route: everything under `/v1/maps` is bound to a basemap, everything -under `/v1/zones` is bound to nothing. +**The names, and nothing else**, served so that the question a dispatcher most +needs to ask — *what places can I name?* — has an answer in the API rather than +out of band. The shape is [zone/v0](https://spec.augereai.com/zone/v0/). + +Not because a coordinate would be wrong. A zone is a coordinate in the floor's +frame — a fact about the building — and every robot on the floor holds the same +one. It is that a caller of this route has no basemap to draw a coordinate on, +and being handed a number it cannot place is worse than not being handed it. So +the division is in the prefix: everything under `/v1/maps` is served beside a +basemap and gated on there being one, everything under `/v1/zones` is gated on +nothing. A caller that must never be handed a map can be given `/v1/zones` and only `/v1/zones`. -**The split is now also in the files.** A floor is two documents — -`vocabulary.yaml` and `binding.yaml` — rather than one `zones.yaml` filtered two -ways, so this route serves a document rather than a projection of one, and the -kind of leak a filter permits (a geometry key added later that nobody remembers -to strip) is not representable. A map revision carries the *binding*, because -coordinates travel with the frame they mean something in; the vocabulary sits at +**The payload is built, never stripped.** A floor's zones are one file, and this +route is a *view* over it assembled from the fields a vocabulary may carry — +never that file with the geometry keys filtered out. The difference is the leak +a filter permits: a geometry key added later that nobody remembers to strip, +arriving as a plausible-looking coordinate rather than as a crash. A map +revision carries a copy of the whole file, because that is how a floor's places +reach a robot that has never driven there; the names sit at floor level, which is why this route answers for a floor with no published map at all. A candidate produced by the zone editor carries **both** halves, and promotion is what lifts its vocabulary to the floor: uploading is not @@ -323,7 +322,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's vocabulary is written, so a binding can record which vocabulary it was built against. | +| `revision` | Bumped every time a floor's zones are written, so a reader can tell which of two copies is the later one. | | `problems` | Empty when the vocabulary is well-formed; see below. | `kind`, `display_name`, `aliases`, `parent` and `tags` were part of this @@ -336,11 +335,11 @@ served. There are **no coordinates, no `frame_id` and no map reference**, by construction: the payload is built from the fields a vocabulary may carry rather than filtered of the ones it may not, so a geometry key added to -`zones.yaml` later cannot leak into it. `test_zone_vocabulary.py` asserts this -by walking the whole payload for geometry-shaped keys rather than checking the -ones it happens to know about. +`zones.yaml` later cannot leak into it. The tests assert this by walking the +whole payload for geometry-shaped keys rather than checking the ones they +happen to know about. -Unlike the binding, this is **not** gated on a published map. A floor someone +Unlike the routes under `/v1/maps`, this is **not** gated on a published map. A floor someone has named but no robot has mapped still answers here — names are a fact about the building and do not wait on a SLAM session. `404` only when the floor has no `zones.yaml` at all. @@ -528,17 +527,15 @@ 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 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. +`floor` when it inherits the floor's. An operator reviewing a candidate is +entitled to know that what is drawn came from beside it rather than from inside +it, and the coordinates cannot say. Unlike `read_zones` on the canonical route, this is **not gated on there being a published map** — the review that matters most is the first candidate on a floor -with nothing published at all. That does not loosen the vocabulary/binding -split: these are still coordinates, still served under a path bound to a -basemap, and still never over `/v1/zones`. Naming a revision is naming a map -frame. +with nothing published at all. It stays under a `/v1/maps`-shaped path and never +over `/v1/zones`, because it is served beside a basemap and that is what the two +prefixes divide. All three are reads, so like every other read route they take no operator token; M7 changes that for all of them at once. @@ -591,15 +588,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`. +**An entry's `source` is carried, not re-invented.** It says what made the +zone — `save-zone` for a pose a robot was driven to, `segment-map` for a room an +algorithm read off a map, `editor` for a click — and a submitted entry keeps +whatever it names, so a zone this edit did not touch keeps what it arrived with. +The dashboard's editor sends `editor` on geometry it placed or moved. Nothing +decides anything from the field: a zone is a coordinate in the floor's frame +however it got there, and what the field buys is an operator being able to see +which zones somebody drew. A value outside the three is dropped rather than +refused, for the same reason — it costs nothing to ignore and a `422` would cost +the whole save. **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 diff --git a/docs/robot/sites.md b/docs/robot/sites.md index bc63a78..d130552 100644 --- a/docs/robot/sites.md +++ b/docs/robot/sites.md @@ -2,11 +2,13 @@ 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**. A zone's pose is a coordinate in a map frame whose origin is -an accident of where SLAM happened to start, so those three artefacts must live -and travel as one unit or they quietly stop describing the same building. +**site bundle**. A zone is a coordinate in the floor's frame — a fact about the +building — and a map revision is one SLAM session registered into that frame, so +those three artefacts live and travel as one unit or they quietly stop +describing the same building. -- A **floor** is one SLAM session, i.e. one map frame. +- A **floor** is one frame — the building's, which its zones are coordinates + in. Each map revision is one SLAM session registered into it. - A **site** groups floors that share a location. - A **revision** is one immutable set of map artefacts for a floor. @@ -123,39 +125,37 @@ 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 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. 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 +assumption — a doorway is narrow. It is additive over the zones already there +(a candidate covering an already-footprinted zone is dropped), so re-running is +a no-op, and it writes into the floor's `zones.yaml`, never into the immutable +map revision. A proposed room records `source: segment-map` — what made it, and +nothing about what the coordinate is worth. 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). The shape of the file, circles versus polygons, and how membership is answered are covered in [Missions](missions.md#zones-and-go-to-the-kitchen). -### Names travel, coordinates do not +### What a zone says besides where it is **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`. `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 -fleet-level transform fixes that; the *name* is true for both. So the fleet -publishes the vocabulary and not the binding, over -[`GET /v1/zones`](../fleet/fleet-api.md) — expressed by the route rather than -by a rule someone has to remember: everything under `/v1/maps` is bound to a -basemap, everything under `/v1/zones` is bound to nothing. - -Locally, `load_zones` **refuses** a vocabulary in which two zones answer one -query, because loading it would resolve `goto` by dictionary order — silently, -once per boot, and differently after an edit. +coordinates it carries the `name` it is called, a free-text `note` for what the +name cannot say ("stationery lives here, not in the 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. + +The names are what [`GET /v1/zones`](../fleet/fleet-api.md) publishes, and it +publishes them and nothing else — not because a coordinate would be wrong (a +zone is a coordinate in the floor's frame, and every robot on the floor holds +the same one) but because a caller of that route has no basemap to draw one on. +That is what the two prefixes say: everything under `/v1/maps` is served beside +a basemap, everything under `/v1/zones` needs nothing. + +Locally, `load_zones` **refuses** a floor in which two zones answer one query, +because loading it would resolve `goto` by dictionary order — silently, once per +boot, and differently after an edit. ## Publishing a map to the fleet diff --git a/mote_bringup/mote_bringup/bundle.py b/mote_bringup/mote_bringup/bundle.py index b0ec767..2bd6ddb 100644 --- a/mote_bringup/mote_bringup/bundle.py +++ b/mote_bringup/mote_bringup/bundle.py @@ -43,15 +43,12 @@ dead end, which is an error for a published revision and a warning for one being read. -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 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 -``zones.yaml`` is still read (:func:`read_floor` migrates it) and is replaced -by the pair the first time anything writes. +A floor's zones are one document, ``zones.yaml``, and it belongs to the +**floor**. A zone is a coordinate in the floor's frame — a fact about the +building — and a map revision is an *estimate* of that layout registered into +the frame, so re-mapping moves the map and not the kitchen. A revision carries +a copy of the floor's zones so that they reach a robot which has never driven +there, and installing one replaces the floor's copy; it never owns them. """ import gzip @@ -78,25 +75,14 @@ MAP_YAML = "map.yaml" META_YAML = "meta.yaml" -#: The combined names-and-coordinates file every floor used to have. Still -#: read, never written: :func:`read_floor` migrates one it finds, because a -#: robot that has been mapping a building for a year should not have to be -#: re-taught to gain the split. +#: A floor's zones: what the places are called, and where they are. ZONES_YAML = "zones.yaml" -#: The two halves zone/v0 splits it into. ``binding.yaml`` is coordinates in -#: one robot's map frame and travels *inside* a map revision; ``vocabulary.yaml`` -#: is names, sits at floor level, and is the one of the two that may be -#: broadcast to every robot at the site. -BINDING_YAML = "binding.yaml" -VOCABULARY_YAML = "vocabulary.yaml" - #: What a floor outside a site bundle calls itself. zone/v0 requires a #: vocabulary to name its site and floor, and it is right to — a document with -#: no place is a document nobody can file. Mote still has floors with no site: -#: the legacy ``~/.mote/zones.yaml`` a robot used before site bundles existed, -#: and a bench directory. Naming them ``local/default`` says so, and is a -#: better answer than an empty string that would only be discovered on upload. +#: no place is a document nobody can file. A floor read from a bare path has +#: neither, so it is served as ``local/default`` rather than as an empty string +#: that would only be discovered on upload. LOCAL_SITE = "local" LOCAL_FLOOR = "default" POSEGRAPH = "map.posegraph" @@ -113,7 +99,7 @@ normalise_name = zone.normalise_name check_vocabulary = zone.check_vocabulary ambiguities = zone.ambiguities -read_anchor = zone.read_anchor +read_source = zone.read_source #: Present and non-empty in every revision, whoever wrote it. REQUIRED = (MAP_YAML, META_YAML) @@ -129,8 +115,6 @@ MAP_YAML, META_YAML, ZONES_YAML, - BINDING_YAML, - VOCABULARY_YAML, POSEGRAPH, POSEGRAPH_DATA, "map.png", @@ -313,13 +297,11 @@ def read_map(path) -> dict: def read_zones(path) -> dict: - """A **combined** ``zones.yaml`` — the shape a floor had before the split. + """One ``zones.yaml``, parsed and checked. - Kept because that shape is still written by the sim world files and by any - robot mapping a building since before zone/v0. It is not what a floor reads - through: :func:`read_floor` is, and it puts a combined file through - ``zone.split`` so the legacy path and the split path cannot produce - different structures. + :func:`read_floor` is what a *floor* reads through; this is the file, and it + is also what a sim world's committed ``.zones.yaml`` and the packaged + fallback are. """ return parse_zones(load_yaml_file(path), Path(path).name) @@ -327,9 +309,9 @@ def read_zones(path) -> dict: def parse_zones(raw: dict, where: str = "zones") -> dict: """:func:`read_zones` over a document already in memory. - The zone editor submits one rather than writing a file first, and it must - go through the same reader: a second parser for "the shape a combined - zones file has" is the thing whose two implementations disagreed last time. + The zone editor submits one rather than writing a file first, and goes + through this reader so that there is one answer to what a zones document + means whether it arrives as bytes or as a browser's JSON. """ zones = raw.get("zones") or {} if not isinstance(zones, dict): @@ -338,37 +320,34 @@ def parse_zones(raw: dict, where: str = "zones") -> dict: for name, entry in zones.items(): if not isinstance(entry, dict): raise BundleError(f"{where}: zone {name!r} is not a mapping") - zone = {"name": str(name), **zone_term(where, name, entry)} + record = {"name": str(name), **zone_term(where, name, entry)} for key in ("x", "y", "yaw", "radius"): if entry.get(key) is not None: try: - zone[key] = float(entry[key]) + record[key] = float(entry[key]) except (TypeError, ValueError) as exc: raise BundleError( f"{where}: zone {name!r} has a bad {key}" ) from exc 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: + record["polygon"] = _polygon(where, name, polygon) + # What made the zone, carried rather than re-invented downstream. It is + # a note and nothing decides anything from it, so an unrecognised value + # is dropped rather than costing the floor its map. + source = zone.read_source(entry.get("source")) + if source: + record["source"] = source + if "x" not in record or "y" not in record: # A polygon-only zone is legal — the loader derives a pose inside # the outline (mote_tasks.zones) — but a zone with neither is not # a place at all. - if "polygon" not in zone: + if "polygon" not in record: raise BundleError(f"{where}: zone {name!r} has no position") - parsed[str(name)] = zone + parsed[str(name)] = record return { "frame_id": raw.get("frame_id") or "map", - "revision": _revision(where, raw.get("vocabulary_revision")), + "revision": _revision(where, raw.get("revision")), "zones": parsed, } @@ -379,9 +358,9 @@ def _revision(where: str, raw) -> int: try: revision = int(raw) except (TypeError, ValueError) as exc: - raise BundleError(f"{where}: vocabulary_revision must be an integer") from exc + raise BundleError(f"{where}: revision must be an integer") from exc if revision < 0: - raise BundleError(f"{where}: vocabulary_revision must not be negative") + raise BundleError(f"{where}: revision must not be negative") return revision @@ -402,35 +381,38 @@ def _polygon(where: str, name, polygon) -> list: def vocabulary(zones: dict, site: str, floor: str) -> dict: - """The shared half of :func:`read_floor`'s output, as a zone/v0 document. - - This is the whole point of the split. A vocabulary travels — to a - dispatcher, to a second robot at the same site, to anything that needs to - 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 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. + """The names-only view of :func:`read_floor`'s output, as a zone/v0 document. + + This is what ``GET /v1/zones`` serves: which places exist here and what they + are called, for a dispatcher with no basemap to draw a coordinate on. It is + **built** from :data:`VOCABULARY_KEYS` rather than stripped of the geometry + keys, and that is the one safety property it has — stripping holds only + until someone adds a key and never reads this function, and the leak would + be a plausible-looking coordinate rather than a crash. """ terms = [ {key: item[key] for key in ("name",) + VOCABULARY_KEYS} for item in zones["zones"].values() - if not item.get("local") ] document = zone.vocabulary(site, floor, terms, revision=zones.get("revision", 0)) document["problems"] = check_vocabulary(terms) return document -def binding(zones: dict, site: str, floor: str, platform_id: str) -> dict: - """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. +def binding( + zones: dict, + site: str, + floor: str, + platform_id: str, + *, + map_revision: str = "", +) -> dict: + """The geometry view of the same output, as a zone/v0 document. + + ``platform_id`` and ``map_revision`` say who answered and what they were + running when they did; neither is stored, because neither is a property of + the floor. A zone is a coordinate in the floor's frame, and a map revision + is an estimate registered into that frame rather than a frame of its own. """ bindings = [] for item in zones["zones"].values(): @@ -439,21 +421,28 @@ def binding(zones: dict, site: str, floor: str, platform_id: str) -> dict: footprint = {"type": "polygon", "vertices": item["polygon"]} elif item.get("radius") is not None: footprint = {"type": "circle", "radius": item["radius"]} - anchored = item.get("anchor") or zone.anchor() if "x" in item and "y" in item: x, y, yaw = item["x"], item["y"], item.get("yaw", 0.0) elif footprint is not None and footprint["type"] == "polygon": - # zone/v0 requires a binding to carry a pose: it is where a mission - # navigates to, and an outline alone cannot say. Derived once, here. + # zone/v0 requires a binding entry to carry a pose: it is where a + # mission navigates to, and an outline alone cannot say. Derived + # here rather than stored, so nothing has to keep it in step with + # the outline it comes from. x, y = zone.representative_point(footprint["vertices"]) yaw = 0.0 - anchored = zone.anchor(zone.DERIVED, by="polygon") else: raise BundleError( f"zone {item['name']!r} has neither a pose nor an outline" ) bindings.append( - zone.bound(item["name"], x, y, yaw, footprint=footprint, anchored=anchored) + zone.bound( + item["name"], + x, + y, + yaw, + footprint=footprint, + source=item.get("source", ""), + ) ) return zone.binding( platform_id, @@ -461,39 +450,39 @@ def binding(zones: dict, site: str, floor: str, platform_id: str) -> dict: floor, bindings, frame_id=zones.get("frame_id") or "map", - map_revision=zones.get("map_revision") or "", + map_revision=map_revision, vocabulary_revision=zones.get("revision", 0), ) -def write_floor( - directory, - merged: dict, - *, - site: str = "", - floor: str = "", - platform_id: str | None = None, -): - """Write a floor's zones as the split pair, replacing a combined file. - - Both documents are written before either is moved into place, and the - combined file is removed only afterwards: a floor caught halfway through - this by a power cut must come back as *one* readable layout, not as a - vocabulary with no coordinates under it. - """ +def write_floor(directory, merged: dict): + """Write a floor's zones — one file, replaced atomically.""" directory = Path(directory).expanduser() directory.mkdir(parents=True, exist_ok=True) - site = site or merged.get("site") or LOCAL_SITE - floor = floor or merged.get("floor") or LOCAL_FLOOR - if platform_id is None: - platform_id = merged.get("platform_id") or "" - _atomic(directory / VOCABULARY_YAML, vocabulary(merged, site, floor)) - _atomic(directory / BINDING_YAML, binding(merged, site, floor, platform_id)) - legacy = directory / ZONES_YAML - if legacy.exists(): - # Kept, not deleted: it is the only record of what the floor looked - # like before the split, and it costs a few kilobytes. - legacy.rename(directory / f"{ZONES_YAML}.premigration") + document = { + "frame_id": merged.get("frame_id") or "map", + "revision": _revision(ZONES_YAML, merged.get("revision")), + "zones": {name: _entry(item) for name, item in sorted(merged["zones"].items())}, + } + _atomic(directory / ZONES_YAML, document) + + +def _entry(item: dict) -> dict: + """One zone as it is written: the key carries the name, so the value does + not, and a field at its default is left out rather than restated.""" + entry = {} + for key in ("x", "y", "yaw", "radius"): + if item.get(key) is not None: + entry[key] = item[key] + if item.get("polygon"): + entry["polygon"] = item["polygon"] + if item.get("note"): + entry["note"] = item["note"] + if not item.get("navigable", True): + entry["navigable"] = False + if item.get("source"): + entry["source"] = item["source"] + return entry def _atomic(path: Path, document: dict): @@ -504,89 +493,27 @@ def _atomic(path: Path, document: dict): def read_floor(directory, site: str = "", floor: str = "") -> dict: - """A floor's zones, from whichever of the two layouts is on disk. - - The split pair wins where it exists. A lone ``zones.yaml`` is **migrated on - read** rather than refused: a robot that has been mapping a building for a - year should not have to be re-taught to gain the split, and the sim worlds - and the committed default ship as combined files on purpose — one file is - the right shape for a fixture that has exactly one robot in it. - - ``site``/``floor`` are only needed to stamp a *migration*; reading a split - pair takes them from the documents, which is where they belong. + """A floor's zones. + + ``directory`` is the floor directory, or a ``zones.yaml`` named outright — + which is what a sim world's committed file and the packaged fallback are. + ``site``/``floor`` are what the caller knows about where it is reading from + and are stamped onto the result; the file records neither, because the + floor's place in the layout is the directory it is in and a copy in the file + would be free to disagree with the path after a rename. """ directory = Path(directory).expanduser() - if directory.is_file(): - return _migrate(directory, site, floor) - vocabulary_path = directory / VOCABULARY_YAML - binding_path = directory / BINDING_YAML - if vocabulary_path.is_file() or binding_path.is_file(): - # 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 - # 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() - else {"site": site, "floor": floor, "revision": 0, "zones": []} - ) - binding_doc = load_yaml_file(binding_path) if binding_path.is_file() else None - try: - merged = zone.merge(vocabulary_doc, binding_doc) - except SpecError as exc: - raise BundleError(f"{vocabulary_path.name}: {exc}") from exc - return _typed(vocabulary_path.name, merged) - legacy = directory / ZONES_YAML - if legacy.is_file(): - # site/floor are stamped onto the migrated documents and then thrown - # away by the merge, so a caller that has them passes them and one that - # does not — a revision directory, whose path names a revision and not - # a floor — gets placeholders rather than a wrong answer dressed up as - # a right one. - return _migrate(legacy, site, floor) - raise BundleError(f"{directory}: no {VOCABULARY_YAML} and no {ZONES_YAML}") - - -def _migrate(path, site: str, floor: str) -> dict: - """A combined ``zones.yaml``, read through the split and back. - - Round-tripping through :func:`~mote_bringup.spec.zone.split` rather than - parsing the legacy shape directly is deliberate: it means the legacy path - and the split path produce the *same* structure by construction, so a bug - in one is a bug in both rather than a difference nobody notices until a - floor is migrated. - """ + path = directory if directory.is_file() else directory / ZONES_YAML + if not path.is_file(): + raise BundleError(f"{directory}: no {ZONES_YAML}") zones = read_zones(path) - try: - vocabulary_doc, binding_doc = zone.split( - zones, - site=site or LOCAL_SITE, - floor=floor or LOCAL_FLOOR, - platform_id="", - ) - return _typed(Path(path).name, zone.merge(vocabulary_doc, binding_doc)) - except SpecError as exc: - raise BundleError(f"{Path(path).name}: {exc}") from exc - - -def _typed(where: str, merged: dict) -> dict: - """Numbers as numbers, and a polygon that is a list of pairs.""" - for name, item in merged["zones"].items(): - for key in ("x", "y", "yaw", "radius"): - if item.get(key) is not None: - try: - item[key] = float(item[key]) - except (TypeError, ValueError) as exc: - raise BundleError( - f"{where}: zone {name!r} has a bad {key}" - ) from exc - if item.get("polygon") is not None: - item["polygon"] = _polygon(where, name, item["polygon"]) - if "x" not in item and "polygon" not in item and item.get("bound"): - raise BundleError(f"{where}: zone {name!r} has no position") - return merged + return { + "site": site or LOCAL_SITE, + "floor": floor or LOCAL_FLOOR, + "frame_id": zones["frame_id"], + "revision": zones["revision"], + "zones": zones["zones"], + } def png_size(path) -> tuple[int, int] | None: @@ -746,10 +673,10 @@ def validate(revision_dir, *, require_posegraph: bool = True) -> Report: if not report.meta.get("saved"): report.warnings.append("meta.yaml records no save time") - # A revision carries the *binding*: coordinates are only meaningful in the - # map frame beside them, so they travel with the map or not at all. The - # vocabulary is a floor-level fact about the building and does not have to. - if BINDING_YAML in report.files or ZONES_YAML in report.files: + # A revision carries a copy of the floor's zones. It does not own them — + # the floor does — but it is how a floor's places reach a robot that has + # never driven there, so a revision without them is worth saying. + if ZONES_YAML in report.files: try: report.zones = read_floor(revision_dir) except BundleError as exc: @@ -758,16 +685,18 @@ def validate(revision_dir, *, require_posegraph: bool = True) -> Report: # An ambiguous vocabulary is a warning here, not an error: the map # is good and every coordinate in it is good. What it costs is # dispatch by name, so it must be *said* — but refusing to publish - # a floor's map over a duplicated alias would be the wrong price. + # a floor's map over two rooms called the same thing would be the + # wrong price. report.warnings.extend( - f"vocabulary: {problem}" + f"zones: {problem}" for problem in check_vocabulary(report.zones["zones"].values()) ) else: 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" + f"no {ZONES_YAML} — this revision names no places, so a robot " + "installing it has nowhere to be sent by name. Place them in the " + "dashboard's zone editor on this revision, or drive there and run " + "save-zone" ) unexpected = sorted(set(report.files) - ALLOWED) diff --git a/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py b/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py index 923d56e..9d51f70 100644 --- a/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py +++ b/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py @@ -90,7 +90,7 @@ def merge_into_zones(path: Path, rooms: list[Room]) -> tuple[list[str], list[str they name a spot, not the room around it. """ if path.suffix == ".yaml": - # A caller that named the old combined file means the floor it is in. + # A caller that named the file means the floor it is in. path = path.parent try: floor = bundle.read_floor(path) @@ -116,11 +116,7 @@ def merge_into_zones(path: Path, rooms: list[Room]) -> tuple[list[str], list[str entry = zone_entry(room) zones[name] = dict( bundle.zone_term("segment-map", name, entry), - bound=True, - # Read off the map by an algorithm rather than driven to, which is - # what ``derived`` is for — and what tells an operator, later, that - # a re-map invalidates it without a human having done anything. - anchor=zone_spec.anchor(zone_spec.DERIVED, by="segment-map"), + source=zone_spec.SEGMENT_MAP, **{k: entry[k] for k in ("x", "y", "yaw", "polygon") if k in entry}, ) added.append(name) diff --git a/mote_bringup/mote_bringup/sites.py b/mote_bringup/mote_bringup/sites.py index 77eb53f..fd4a57a 100644 --- a/mote_bringup/mote_bringup/sites.py +++ b/mote_bringup/mote_bringup/sites.py @@ -5,15 +5,11 @@ ~/.mote/sites// site.yaml -> {schema: 1, name, default_floor} floors// - vocabulary.yaml what the places here are CALLED (zone/v0). - No coordinates: safe to share with every - robot at the site. - binding.yaml where THIS robot believes they are, in this - floor's map frame. Never shared; travels - inside a map revision, since a coordinate - means nothing without the map beside it. - zones.yaml the combined file both used to be. Still - read; migrated to the pair on first write. + zones.yaml the floor's places: what they are called + and where they are, in the floor's frame. A + map revision carries a copy so that they + reach a robot which has never driven here; + the floor owns them. map -> maps// symlink to the current map revision maps// immutable once published: map.yaml + map.png nav2 map_server pair — the cleaned map, @@ -36,10 +32,10 @@ rolling back is flipping to an older revision (``site use-map``). The newest revisions are kept, older ones pruned. -Zone poses are coordinates in a map frame whose origin is an accident of -where SLAM started, so zones/map/posegraph must live and travel together. -A floor is one SLAM session (one frame); a site groups floors that share a -location. The whole bundle is plain files + YAML so it can be zipped, +A zone is a coordinate in the floor's frame — a fact about the building — +and a map revision is one SLAM session registered into that frame, so +zones/map/posegraph live and travel together. A site groups floors that share +a location. The whole bundle is plain files + YAML so it can be zipped, synced, or served by a web API without translation. Site bundles are per-robot state, so they live under ``MOTE_HOME`` (``~/.mote`` @@ -132,12 +128,10 @@ def resolve_map() -> str: def resolve_zones() -> str: - """The active floor's zones, or ''. + """The active floor's zones directory, or ''. - A *directory* now, not a file: the floor's zones are two documents, and - which of them a reader wants is the reader's business. What comes back is - what ``bundle.read_floor`` takes, so a legacy combined file still works — - it is inside the same directory. + The directory rather than the file, because that is what + ``bundle.read_floor`` takes. """ act = active() if act: @@ -148,9 +142,7 @@ def resolve_zones() -> str: def has_zones(fdir: Path) -> bool: - return (fdir / bundle.VOCABULARY_YAML).exists() or ( - fdir / bundle.ZONES_YAML - ).exists() + return (fdir / bundle.ZONES_YAML).exists() def zones_for_write() -> Path: @@ -249,29 +241,10 @@ def cmd_info(): print(f" zones UNREADABLE ({exc})") else: with_fp = sum(1 for z in zones.values() if "radius" in z or "polygon" in z) - unbound = sum(1 for z in zones.values() if not z.get("bound")) notes = [f"{len(zones)} zones"] if with_fp: notes.append(f"{with_fp} with a footprint") - # 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} 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" - ) + print(f" zones ok ({', '.join(notes)})") else: print(" zones missing") current = current_revision(fdir) @@ -503,15 +476,12 @@ def install_revision(site: str, floor: str, revision: str, blob: bytes) -> str: and nothing has to be undone if the transfer dies. Returns what it did: ``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 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 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. + **The revision's zones replace the floor's.** A revision carries a copy of + the floor's places so that they reach a robot which has never driven here, + and the copy in the revision an operator promoted is the fleet's current + answer to what this floor's places are. The copy it replaces is kept beside + it as ``zones..yaml``, because losing a map is recoverable and + losing every named place silently is not. """ fdir = floor_dir(site, floor) if not fdir.is_dir(): @@ -553,26 +523,22 @@ def install_revision(site: str, floor: str, revision: str, blob: bytes) -> str: def _adopt_zones(fdir: Path, rev_dir: Path, previous: str | None): - """Install a revision's **binding** as the floor's. - - Only the binding: it is the half that is bound to this revision's map - frame, and it is the half a different SLAM session makes wrong. The - vocabulary stays where it is, because the names of the rooms did not change - when the robot re-mapped the floor — which is the practical dividend of the - split, and the reason re-mapping no longer costs an operator the names - they typed. + """Install a revision's copy of the floor's zones as the floor's. + + The floor owns its zones and a revision carries a copy, so promoting a + revision is also how an edit made in the dashboard reaches a robot. The copy + being replaced is kept as ``zones..yaml``, because losing a map is + recoverable and losing every named place silently is not. """ - for name in (bundle.BINDING_YAML, bundle.ZONES_YAML): - source = rev_dir / name - if not source.is_file(): - continue - target = fdir / name - if target.is_file(): - if target.read_bytes() == source.read_bytes(): - return - target.rename(fdir / f"{Path(name).stem}.{previous or 'previous'}.yaml") - shutil.copyfile(source, target) + source = rev_dir / bundle.ZONES_YAML + target = fdir / bundle.ZONES_YAML + if not source.is_file(): return + if target.is_file(): + if target.read_bytes() == source.read_bytes(): + return + target.rename(fdir / f"zones.{previous or 'previous'}.yaml") + shutil.copyfile(source, target) def use_map(rev: str): diff --git a/mote_bringup/mote_bringup/spec/zone.py b/mote_bringup/mote_bringup/spec/zone.py index 2c7eb41..0af33c8 100644 --- a/mote_bringup/mote_bringup/spec/zone.py +++ b/mote_bringup/mote_bringup/spec/zone.py @@ -1,44 +1,41 @@ -"""zone/v0 — places are named once for a fleet and located once per robot. +"""zone/v0 shapes: the documents a floor's zones are serialised into. -**Names are shared. Coordinates are not. Maps are never shared.** +A zone is a coordinate in the floor's frame — a fact about the building. The +kitchen does not move. A map revision is an *estimate* of that layout, +registered into the frame, so where a map and the zones disagree it is the map +that gets aligned: pose-graph continuation today, rigid alignment as the +fallback. A floor's zones are therefore one document, ``zones.yaml``, held by +the floor rather than by a map revision or by one robot. -A robot's map frame has its origin wherever that robot's SLAM session happened -to start, so ``(2.0, 3.5)`` on one robot is a different physical point on the -one beside it — and no fleet-level transform fixes it, because the discrepancy -is not a constant offset but two independent estimates of a building drifting -against each other. Mote has stated that invariant since the site bundles -landed. What zone/v0 adds is the **split**, and this module is it: +zone/v0 specifies two documents, and both are **views over that single record** +— built here at the wire, never stored: * a :func:`vocabulary` — site, floor, and what the places are *called*. No - coordinates, no frame, no map reference. Safe to broadcast to every robot at - the site, and to a dispatcher that has never seen one. -* a :func:`binding` — one platform's poses and footprints for those names, - stamped with the platform id, the frame and the map revision they are only - valid against. **It must not be copied to another platform.** - -The split is structural rather than a rule someone has to remember. The -vocabulary document is **built** from the fields a vocabulary may carry, never -*stripped* of the ones it may not: stripping holds only until someone adds a -geometry key and forgets this function exists, and the leak would be a -plausible-looking coordinate rather than a crash. - -**A zone is a place-name**: a human name bound to geometry, and the record -carries only what a prior cannot guess. The semantics come from the mission -layer's resolver, which already knows what a store room is; what it cannot know -is that *this* building's store room is where the stationery lives. So the -vocabulary is the :data:`name` and a free-text :data:`note`, and nothing else. -``kind``, ``display_name``, ``aliases``, ``parent`` and ``tags`` were a -taxonomy for a reader that did not need one — five fields to fill in, four ways -to spell one place, and a machine name beside a human one for a resolver that -reads either. They are **tolerated on read** so that no floor taught before -this has to be re-taught, and they are neither written nor served. + coordinates, no frame, no map reference. This is what ``GET /v1/zones`` + serves to a dispatcher that has no basemap to draw a coordinate on. +* a :func:`binding` — the poses and footprints, stamped with the + ``platform_id``, ``frame_id`` and ``map_revision`` of whoever is serialising + them. None of the three is a property of the floor, so each is supplied by + the caller at serialisation. + +The vocabulary view is **built** from the fields a vocabulary may carry, never +*stripped* of the ones it may not, and that is the whole safety property: +stripping holds only until someone adds a geometry key and forgets this +function exists, and the leak would be a plausible-looking coordinate rather +than a crash. + +**A zone is a place-name**: a human name bound to geometry, carrying only what +a prior cannot guess. The mission layer's resolver already knows what a store +room is; what it cannot know is that *this* building's store room is where the +stationery lives. So the vocabulary is the :data:`name` and a free-text +:data:`note`, and nothing else. This module is stdlib-only, like the rest of :mod:`mote_bringup.spec`. Reading and writing these documents as YAML is :mod:`mote_bringup.bundle`'s, which already owns the site bundle's files and already imports PyYAML; what lives -here is the shapes and the rules — which are needed identically by the robot -that resolves a name, the ``save-map`` that validates a floor, and the fleet -server that serves a vocabulary to a dispatcher. +here is the shapes and the rules — needed identically by the robot that +resolves a name, the ``save-map`` that validates a floor, and the fleet server +that serves a vocabulary to a dispatcher. """ import math @@ -49,19 +46,15 @@ SCHEMA = 1 VERSION = "v0" -#: 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-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. +#: ``kind`` values that say a robot may not or should not be sent to a zone. +#: A floor may carry one; :func:`term` reads it to seed ``navigable`` and writes +#: ``navigable`` back, so a barrier reads as a barrier rather than as somewhere +#: to drive to. CONSTRAINT_KINDS = frozenset(("keepout", "slow")) #: A place-name: what an operator calls the room, and what a dispatcher types. -#: One field, so there is one answer to "what is this place called" — the -#: machine name beside a display name was two fields for one fact, and the -#: resolver reads the human one anyway. Any printable text, with no leading or -#: trailing space to make two names look identical and resolve differently. +#: Any printable text, with no leading or trailing space — two names differing +#: only by one look identical on screen and resolve differently. ZONE_NAME_RE = re.compile(r"^(?!\s)[^\x00-\x1f\x7f]+(? str: @@ -149,16 +116,14 @@ def _place(where: str, value) -> str: return text -# -- the vocabulary -------------------------------------------------------- +# -- what a zone is called -------------------------------------------------- def term(where: str, name, entry: dict) -> dict: - """One zone's naming half: what the place is called, and a note about it. + """What one zone is called, and the note beside the name. - Both fields are optional in the file, and a floor taught before place-names - reads perfectly: its ``kind``/``display_name``/``aliases``/``parent``/ - ``tags`` are accepted and dropped, and its ``description`` is read as the - ``note`` it was. + Both fields are optional in the file. :data:`LEGACY_KEYS` are accepted and + dropped, ``description`` being read into ``note``. """ return { "name": str(name), @@ -168,14 +133,12 @@ def term(where: str, name, entry: dict) -> dict: def _navigable(where: str, name, entry: dict) -> bool: - """Whether a robot may be dispatched here — stated, or read off a legacy kind. - - A zone says nothing about this and is a destination. The exception is a - floor written before place-names, where ``kind: keepout`` is the only place - the fact was recorded: reading it here is what carries a barrier across the - change rather than turning it into somewhere to drive to. The contradiction - (a keepout that says it is navigable) is still refused, because the flag - would otherwise mean whichever of the two the file mentioned last. + """Whether a robot may be dispatched here — stated, or read off a ``kind``. + + A zone that says nothing is a destination. A zone whose only record of the + fact is a :data:`CONSTRAINT_KINDS` ``kind`` is not. A zone that says both — + a keepout claiming ``navigable: true`` — is refused, because the flag would + otherwise mean whichever of the two the file mentioned last. """ constraint = str(entry.get("kind") or "") in CONSTRAINT_KINDS navigable = entry.get("navigable") @@ -192,11 +155,12 @@ def _navigable(where: str, name, entry: dict) -> bool: def vocabulary(site: str, floor: str, terms, *, revision: int = 0) -> dict: - """The shared document: which places exist here and what they are called. + """The names-only view: which places exist here and what they are called. - Carries no coordinates, no frame and no map reference, because none of those - are portable between robots — which is exactly what makes it safe to - broadcast to every platform at the site. + Carries no coordinates, no frame and no map reference — not because a + coordinate would be wrong, but because the caller this is for has no + basemap to draw one on, and a number it cannot place is worse than no + number. Built from :data:`VOCABULARY_KEYS`, never stripped of the rest. """ return { "schema": SCHEMA, @@ -262,8 +226,8 @@ def ambiguities(terms) -> list: name is hard to type is still a zone it can be told to drive to, but two zones answering to one query means ``goto`` has no single answer, and guessing between them is the one thing zone/v0 says a resolver must not do. - Now that a zone has one name and no aliases, the only way to make one is to - call two places the same thing — which is worth saying plainly. + A zone has one name, so the only way to make one is to call two places the + same thing — which the message says plainly. """ problems = [] claimed = {} @@ -281,59 +245,19 @@ def ambiguities(terms) -> list: return problems -# -- the binding ----------------------------------------------------------- +# -- where a zone is ------------------------------------------------------- -def anchor( - method: str = TAUGHT, - *, - at: str | None = None, - by: str = "", - fiducial_id: str | None = None, - offset: dict | None = None, - confidence: float | None = None, -) -> dict: - """How this coordinate came to be. +def read_source(value) -> str: + """The ``source`` a document or a browser submitted, or ``""``. - ``confidence`` is the platform's own estimate; null is a legitimate answer - and means "not estimated", never "certain". + A value outside :data:`SOURCES` is dropped rather than refused. It is a note + about what made the zone, nothing reads it to decide anything, and costing an + operator a whole floor over a field with no consequences would be the wrong + price. """ - if method not in ANCHOR_METHODS: - raise SpecError(f"unknown anchor method {method!r}") - if method == "fiducial" and not fiducial_id: - # The one anchor that survives re-mapping does so by naming a marker; - # without the marker it is a taught pose wearing a better label. - raise SpecError("a fiducial anchor must name its fiducial_id") - if confidence is not None and not 0.0 <= float(confidence) <= 1.0: - raise SpecError("anchor confidence must be between 0 and 1") - record = {"method": method, "at": at, "by": by, "confidence": confidence} - if fiducial_id: - record["fiducial_id"] = fiducial_id - if offset is not None: - record["offset"] = offset - 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"), - ) + text = str(value or "") + return text if text in SOURCES else "" def bound( @@ -343,17 +267,21 @@ def bound( yaw: float = 0.0, *, footprint: dict | None = None, - anchored: dict | None = None, + source: str = "", ) -> dict: - """One zone's coordinate half, for a :func:`binding`. + """One zone's geometry, as an entry in a :func:`binding` view. - The name is **not** refused for its spelling. A name is a fact about a - floor an operator already has, the map it is bound to is perfectly good, - and refusing to read the floor over a spelling would be the wrong price. + The name is **not** refused for its spelling. A name is a fact about a floor + an operator already has, the map it is drawn on is perfectly good, and + refusing to read the floor over a spelling would be the wrong price. :func:`check_vocabulary` reports it instead, which is where an operator can - act on it, and a binding never leaves this robot anyway. What is refused is - a *vocabulary* that cannot be resolved at all — two places called the same - thing — because that one has no correct behaviour to fall back on. + act on it. What is refused is a *vocabulary* that cannot be resolved at all + — two places called the same thing — because that one has no correct + behaviour to fall back on. + + ``anchor`` is required by zone/v0 and is filled from ``source`` here, which + is the one place the mapping lives. It is a fact about what made the zone, + not about what the coordinate is worth. """ if footprint is not None: check_footprint(name, footprint) @@ -365,7 +293,7 @@ def bound( "yaw": round(float(yaw), 4), }, "footprint": footprint, - "anchor": anchored if anchored is not None else anchor(), + "anchor": {"method": _ANCHOR_METHOD.get(source, "taught"), "by": source}, } @@ -402,13 +330,14 @@ def binding( map_revision: str = "", vocabulary_revision: int = 0, ) -> dict: - """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 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. + """The geometry view: where the floor's places are, in a named frame. + + ``platform_id``, ``frame_id`` and ``map_revision`` are supplied by whoever + is serialising and are not properties of the floor — a zone is a coordinate + in the floor's frame, and the map revision is an estimate registered into + it rather than a frame of its own. They are here because zone/v0 requires + them, and they say which platform answered and what it was running at the + time. """ return { "schema": SCHEMA, @@ -484,10 +413,10 @@ def resolution( # # The geometry is zone/v0's normative containment semantics and it lives here # rather than in the task layer for two reasons. The *server* needs it too — a -# binding this module writes must carry a pose, and for a zone drawn as an -# outline that pose has to be derived from the outline — and one -# implementation of "is this point inside" is the only way two ends of a fleet -# can agree about a boundary case. +# zone drawn as an outline still has to say where a mission navigates to, and +# that point is derived from the outline — and one implementation of "is this +# point inside" is the only way two ends of a fleet can agree about a boundary +# case. def edges(vertices): @@ -551,11 +480,11 @@ def representative_point(vertices) -> tuple: interior span of the horizontal line through it — so a U- or L-shaped room gets a pose in the room rather than in the notch outside it. - This is what a polygon-only zone's **binding pose** is, computed once when - the binding is written rather than by every reader. zone/v0 requires a - binding to carry a pose, and it is right to: a binding is where a mission - navigates to, and a zone that could not say where that is would be a - footprint pretending to be a destination. + This is the pose a polygon-only zone gets — what ``segment-map`` writes, + since it read a room off a map rather than driving to it. Derived here, by + the one implementation of "inside", so a reader deriving its own would be a + second one. A zone that could not say where a mission navigates to would be + a footprint pretending to be a destination. """ cx, cy = centroid(vertices) if polygon_contains(vertices, cx, cy): @@ -572,128 +501,11 @@ def representative_point(vertices) -> tuple: return (lo + hi) / 2.0, cy -# -- the migration --------------------------------------------------------- +# -- the keys that are geometry ------------------------------------------- -#: Geometry keys a legacy combined ``zones.yaml`` entry may carry. Named here -#: because :func:`split` has to know which half of an entry is which, and -#: because a key that is in neither list is a key nobody has decided about. +#: Geometry keys a zone entry may carry. Named here because the tests walk a +#: serialised :func:`vocabulary` for them — the leak this module exists to +#: prevent is a coordinate reaching a document that promises none — and because +#: a key in neither this list nor :data:`VOCABULARY_KEYS` is a key nobody has +#: decided about. GEOMETRY_KEYS = ("x", "y", "yaw", "radius", "polygon") - - -def split( - zones: dict, *, site: str, floor: str, platform_id: str, map_revision: str = "" -): - """``(vocabulary, binding)`` from a combined ``zones.yaml``'s parsed form. - - This is the migration, and it is a *split* rather than two filters over the - 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. - - 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"]} - elif entry.get("radius") is not None: - footprint = {"type": "circle", "radius": entry["radius"]} - if "x" in entry and "y" in entry: - bindings.append( - bound( - name, - entry["x"], - entry["y"], - entry.get("yaw", 0.0), - footprint=footprint, - anchored=carried or anchor(TAUGHT), - ) - ) - continue - if footprint is None or footprint["type"] != "polygon": - raise SpecError(f"zone {name!r} has neither a pose nor an outline") - # A polygon-only zone — what ``segment-map`` emits, since it is reading - # rooms off a map rather than driving to them. The pose is derived from - # the outline *here*, once, because zone/v0 requires a binding to carry - # one and because a reader deriving its own would be a second - # implementation of "inside". - px, py = representative_point(footprint["vertices"]) - bindings.append( - bound( - name, - px, - py, - footprint=footprint, - anchored=carried or anchor(DERIVED, by="polygon"), - ) - ) - revision = zones.get("revision", 0) - return ( - vocabulary(site, floor, terms, revision=revision), - binding( - platform_id, - site, - floor, - bindings, - frame_id=zones.get("frame_id") or "map", - map_revision=map_revision, - vocabulary_revision=revision, - ), - ) - - -def merge(vocabulary_doc: dict, binding_doc: dict | None) -> dict: - """One combined view, in the shape ``zones.yaml`` parses to. - - 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 the binding has no geometry for is present - with none, which is what makes ``unbound`` answerable rather than - indistinguishable from ``unknown_name``. - """ - bindings = { - item["name"]: item for item in ((binding_doc or {}).get("bindings") or ()) - } - zones = {} - for item in vocabulary_doc.get("zones") or (): - 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 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 "", - "floor": vocabulary_doc.get("floor") or "", - "platform_id": (binding_doc or {}).get("platform_id") or "", - "frame_id": (binding_doc or {}).get("frame_id") or "map", - "revision": vocabulary_doc.get("revision", 0), - "map_revision": (binding_doc or {}).get("map_revision") or "", - "zones": zones, - } - - -def _geometry(item: dict | None) -> dict: - if item is None: - return {"bound": False} - geometry = {"bound": True, "anchor": item.get("anchor") or anchor()} - pose = item.get("pose") - if pose: - geometry.update(x=pose["x"], y=pose["y"], yaw=pose.get("yaw", 0.0)) - footprint = item.get("footprint") - if footprint: - if footprint["type"] == "circle": - geometry["radius"] = footprint["radius"] - else: - geometry["polygon"] = footprint["vertices"] - return geometry diff --git a/mote_bringup/test/test_bundle.py b/mote_bringup/test/test_bundle.py index f68416c..97e73f6 100644 --- a/mote_bringup/test/test_bundle.py +++ b/mote_bringup/test/test_bundle.py @@ -54,47 +54,30 @@ def test_segment_map_output_is_a_bundle_this_can_read(tmp_path): zones = bundle.read_floor(tmp_path)["zones"] assert zones["room_1"]["polygon"][2] == [4.0, 3.0] - # Read off the map by an algorithm, never driven to — which is what tells - # an operator later that a re-map invalidates it. - assert zones["room_1"]["anchor"]["method"] == "derived" - for name in ("vocabulary.yaml", "binding.yaml"): - text = (tmp_path / name).read_text() - 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. + assert zones["room_1"]["source"] == "segment-map" + text = (tmp_path / bundle.ZONES_YAML).read_text() + assert bundle.load_yaml(text) == yaml.safe_load(text) + + +def test_a_file_keeps_the_source_it_carries(tmp_path): + """What made a zone survives the reader, and an unrecognised value does not. + + ``source`` says what put the coordinate there — ``save-zone``, + ``segment-map`` or the dashboard's ``editor``. Nothing decides anything from + it, which is why a value outside the three is dropped rather than costing + the floor its map; and why a zone that says nothing carries nothing rather + than a default that would read as a claim. """ (tmp_path / bundle.ZONES_YAML).write_text( "zones:\n" - " the kitchen: {x: 1.0, y: 2.0, anchor: {method: external, by: zone-editor}}\n" + " the kitchen: {x: 1.0, y: 2.0, source: editor}\n" " office: {x: 3.0, y: 4.0}\n" + " yard: {x: 5.0, y: 6.0, source: surveyed}\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") + assert zones["the kitchen"]["source"] == "editor" + assert "source" not in zones["office"] + assert "source" not in zones["yard"] @pytest.mark.parametrize( @@ -389,26 +372,16 @@ def revision(directory, **kwargs) -> Path: (directory / "map.posegraph").write_bytes(b"graph") (directory / "map.data").write_bytes(b"data") if kwargs.get("zones", True): - # A revision carries the *binding*, since coordinates are only - # meaningful in the map frame beside them. + # A revision carries a copy of the floor's zones, so that a floor's + # places reach a robot which has never driven there. bundle.write_floor( directory, - {"frame_id": "map", "revision": 1, "zones": {}}, - site="home", - floor="ground", - platform_id="mote-01", - ) - (directory / "vocabulary.yaml").unlink() - binding = yaml.safe_load((directory / "binding.yaml").read_text()) - binding["bindings"] = [ { - "name": "a", - "pose": {"x": 0.0, "y": 0.0, "yaw": 0.0}, - "footprint": None, - "anchor": {"method": "taught"}, - } - ] - (directory / "binding.yaml").write_text(yaml.safe_dump(binding)) + "frame_id": "map", + "revision": 1, + "zones": {"a": {"name": "a", "x": 0.0, "y": 0.0, "yaw": 0.0}}, + }, + ) return directory @@ -485,7 +458,7 @@ def test_a_raw_map_of_a_different_size_is_refused(tmp_path): def test_a_floor_with_no_zones_validates_but_says_so(tmp_path): report = bundle.validate(revision(tmp_path / "rev", zones=False)) assert report.ok - assert any("no binding.yaml" in warning for warning in report.warnings) + assert any("no zones.yaml" in warning for warning in report.warnings) # ---- the wire form ------------------------------------------------------ @@ -501,7 +474,7 @@ def test_pack_round_trips_through_unpack(tmp_path): "meta.yaml", "map.posegraph", "map.data", - "binding.yaml", + "zones.yaml", } assert bundle.validate(tmp_path / "landed").ok for name in written: @@ -516,10 +489,10 @@ def test_packing_is_deterministic(tmp_path): assert bundle.digest(bundle.pack(source)).startswith("sha256:") -def test_extra_files_travel_with_the_frame(tmp_path): +def test_extra_files_travel_with_the_revision(tmp_path): source = revision(tmp_path / "rev", zones=False) - blob = bundle.pack(source, {"binding.yaml": b"schema: 1\nbindings: []\n"}) - assert "binding.yaml" in bundle.unpack(blob, tmp_path / "landed") + blob = bundle.pack(source, {"zones.yaml": b"frame_id: map\nzones: {}\n"}) + assert "zones.yaml" in bundle.unpack(blob, tmp_path / "landed") def test_a_file_that_is_not_part_of_a_bundle_cannot_be_packed(tmp_path): diff --git a/mote_bringup/test/test_spec_conformance.py b/mote_bringup/test/test_spec_conformance.py index 81d835e..0f3d558 100644 --- a/mote_bringup/test/test_spec_conformance.py +++ b/mote_bringup/test/test_spec_conformance.py @@ -162,46 +162,77 @@ def test_a_local_mission_status_conforms(validator_for): # -- zone/v0 --------------------------------------------------------------- -def a_floor(): - """One floor's zones, split — the migration a real ``zones.yaml`` takes.""" - return zone.split( - { - "frame_id": "map", - "revision": 4, - "zones": { - "kitchen": { - "x": 2.0, - "y": 3.5, - "yaw": 1.57, - "radius": 1.5, - "kind": "room", - "display_name": "Kitchen", - "aliases": ["the kitchen", "galley"], - }, - "ward_a": {"polygon": [[4, 0], [9, 0], [9, 3], [4, 3]], "kind": "room"}, - "server_room": {"x": 1.0, "y": 1.0, "kind": "keepout"}, - }, - }, - site="acme_hq", - floor="ground", - platform_id="mote-01", - map_revision="2026-07-24T09-12-03", +def a_floor(directory: Path): + """One floor's zones, as the two zone/v0 views the wire is written in. + + Read from a real ``zones.yaml`` through the reader every consumer uses, so + what is validated here is what a floor actually produces — including the + fields nothing writes but everything accepts — rather than a document + written for the test. + """ + from mote_bringup import bundle + + (directory / bundle.ZONES_YAML).write_text( + "frame_id: map\n" + "revision: 4\n" + "zones:\n" + " kitchen:\n" + " {x: 2.0, y: 3.5, yaw: 1.57, radius: 1.5, kind: room,\n" + " display_name: Kitchen, aliases: [the kitchen, galley]}\n" + " ward_a: {polygon: [[4, 0], [9, 0], [9, 3], [4, 3]], kind: room}\n" + " server_room: {x: 1.0, y: 1.0, kind: keepout}\n" + ) + floor = bundle.read_floor(directory, "acme_hq", "ground") + return ( + bundle.vocabulary(floor, "acme_hq", "ground"), + bundle.binding( + floor, + "acme_hq", + "ground", + "mote-01", + map_revision="2026-07-24T09-12-03", + ), ) -def test_a_vocabulary_conforms_and_carries_no_coordinates(validator_for): - document, _ = a_floor() - validator_for("zone/v0/zone-vocabulary.schema.json").validate(document) - # The invariant, checked over the whole payload rather than over the keys - # someone thought of: a geometry key here is the leak the split exists to - # prevent, and it would look like a plausible coordinate rather than a crash. - text = json.dumps(document) +def a_vocabulary(directory: Path) -> dict: + """The names-only view as ``GET /v1/zones`` serves it. + + ``problems`` is dropped: it is Mote's own, for an operator reading a floor, + and it is not part of the document the spec describes. + """ + document, _ = a_floor(directory) + return {key: value for key, value in document.items() if key != "problems"} + + +@pytest.mark.xfail( + strict=True, + reason="zone/v0's vocabulary schema requires `kind`, and Mote's vocabulary " + "is a name, a note and `navigable` — so this payload does not validate. A " + "successor revision of the spec is outstanding (mote #616); when it lands " + "this passes and the marker comes off.", +) +def test_a_vocabulary_conforms(validator_for, tmp_path): + validator_for("zone/v0/zone-vocabulary.schema.json").validate( + a_vocabulary(tmp_path) + ) + + +def test_a_vocabulary_carries_no_coordinates(validator_for, tmp_path): + """Separate from the schema check, and not expected to fail with it. + + This is the invariant the names-only view exists for, and it is Mote's to + hold whatever the spec's schema currently requires — so it is checked over + the whole payload rather than over the keys someone thought of, and a + failure here is a plausible-looking coordinate rather than a crash. + """ + text = json.dumps(a_vocabulary(tmp_path)) for key in zone.GEOMETRY_KEYS + ("frame_id", "map_revision", "pose", "footprint"): - assert f'"{key}"' not in text, f"{key} leaked into the vocabulary" + assert f'"{key}"' not in text, f"{key} leaked into the names-only view" -def test_a_binding_conforms(validator_for): - _, document = a_floor() +def test_a_binding_conforms(validator_for, tmp_path): + _, document = a_floor(tmp_path) validator_for("zone/v0/zone-binding.schema.json").validate(document) assert document["platform_id"] == "mote-01" assert document["map_revision"] == "2026-07-24T09-12-03" @@ -225,7 +256,6 @@ def test_every_resolution_reason_conforms(validator_for): frame_id="map", map_revision="2026-07-24T09-12-03", pose={"x": 2.0, "y": 3.5, "yaw": 1.57}, - kind="room", navigable=True, anchor_method="taught", ) diff --git a/mote_fleet/mote_fleet/mapsync.py b/mote_fleet/mote_fleet/mapsync.py index 9ae29a5..4757816 100644 --- a/mote_fleet/mote_fleet/mapsync.py +++ b/mote_fleet/mote_fleet/mapsync.py @@ -128,11 +128,11 @@ def publish( ) -> dict: """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 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. + The floor's zones are packed *into* the revision. The floor owns them — a + zone is a coordinate in the floor's frame and a map revision is an estimate + registered into it — but a revision is the vehicle the fleet already has for + getting a floor's places to a robot that has never driven there, so a + revision carries a copy. """ fdir = sites.floor_dir(site, floor) rev_dir = fdir / "maps" / revision @@ -142,18 +142,9 @@ def publish( if not report.ok: raise SyncError(f"refusing to publish {revision}: {report.summary()}") extra = {} - for name in (bundle.BINDING_YAML, bundle.ZONES_YAML): - source = fdir / name - if source.is_file() and not (rev_dir / name).is_file(): - extra[name] = source.read_bytes() - break - # The vocabulary rides along too, unpacked into the floor rather than kept - # in the revision: a fleet server holding it can answer "what places are - # here" for a floor no robot has published a map of, which is the whole - # point of splitting it out. - vocabulary = fdir / bundle.VOCABULARY_YAML - if vocabulary.is_file(): - extra[bundle.VOCABULARY_YAML] = vocabulary.read_bytes() + floor_zones = fdir / bundle.ZONES_YAML + if floor_zones.is_file() and not (rev_dir / bundle.ZONES_YAML).is_file(): + extra[bundle.ZONES_YAML] = floor_zones.read_bytes() blob = bundle.pack(rev_dir, extra) path = ( diff --git a/mote_fleet/server/bundle_store.py b/mote_fleet/server/bundle_store.py index 4c66ac4..61ada81 100644 --- a/mote_fleet/server/bundle_store.py +++ b/mote_fleet/server/bundle_store.py @@ -55,7 +55,6 @@ 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 @@ -281,21 +280,18 @@ 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 bound zones, in the map frame the basemap is drawn in. + """The floor's zones with their coordinates, for a client drawing them. - This is the **binding**: coordinates, and therefore meaningful only - 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`. + Served beside the basemap, to the one client that also has the basemap, + and gated on there being a published map for the same reason: what this + route is for is drawing zones *on* one, and a client with nothing to + draw them on has asked the wrong question. ``/v1/zones`` is the one that + answers without a map — see :meth:`read_vocabulary`. - Zones travel inside a published revision, because a zone is a - coordinate in one SLAM session's frame. A floor seeded by rsync keeps - them at floor level, as ``sites.py`` writes them, so that is the - fallback rather than an error. + A revision carries a copy of the floor's zones and a floor seeded by + rsync keeps them at floor level, as ``sites.py`` writes them, so that is + the fallback rather than an error. """ - # A binding without the map it is bound to is the coordinate-shaped - # thing this whole split exists to stop, so it stays gated on there - # being a published revision. path = self._zones_file(site, floor, revision=self._live(site, floor)) if path is None: raise StoreError(f"no zones for {site}/{floor}", 404) @@ -308,15 +304,13 @@ def read_zones(self, site: str, floor: str) -> dict: } def read_revision_zones(self, site: str, floor: str, revision: str) -> dict: - """One **revision's** zones, for the operator reviewing it. - - Still a binding, and still served the way a binding must be: under the - revision's own path, beside the revision's own basemap, never over - ``/v1/zones``. What it drops is :meth:`read_zones`' gate on there being - a published map — which is the one thing that would make it useless - here, because the review that matters most is the *first* candidate on - a floor with nothing published at all. Naming a revision is naming a - map frame, so the coordinates still arrive with something to mean. + """The zones **one revision carries**, for the operator reviewing it. + + Served the same way :meth:`read_zones` is — under the revision's own + path, beside the revision's own basemap, never over ``/v1/zones``. What + it drops is that method's gate on there being a published map, which is + the one thing that would make it useless here: the review that matters + most is the *first* candidate on a floor with nothing published at all. """ # An unknown revision is a 404 about the revision rather than about the # zones, and the floor-level fallback below must not answer for one. @@ -329,38 +323,34 @@ def read_revision_zones(self, site: str, floor: str, revision: str) -> dict: "site": site, "floor": floor, "revision": revision, - # 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 bound in a *previous* SLAM session's frame and are therefore - # wrong for this map by exactly however far the two origins differ. + # Which of ``_zones_file``'s two candidates answered. A revision + # carrying no zones of its own falls back to the floor's, and an + # operator reviewing a candidate is entitled to know that what is + # drawn came from beside it rather than from inside it. "source": "revision" if path == directory else "floor", "frame_id": zones["frame_id"], "zones": list(zones["zones"].values()), } def read_vocabulary(self, site: str, floor: str) -> dict: - """The floor's zone **vocabulary** — the name of each place and a note - about it, no coordinates and no frame. - - Deliberately *not* gated on a published map, where - :meth:`read_zones` is. A vocabulary is a fact about the building, so a - floor that has been named but never mapped still has one, and a robot - arriving at a site can be told what the places are called before it has - driven a metre. That is the portability the split buys, and gating this - on a promoted revision would have quietly given it back. + """The names-only view of the same floor: what each place is called and + a note about it, no coordinates and no frame. + + The same read as :meth:`read_zones` and a different *view* of it — + ``bundle.vocabulary`` builds the payload from the fields a vocabulary + may carry rather than stripping the ones it may not, which is what keeps + a coordinate out of it when someone adds a key. + + Deliberately *not* gated on a published map, where :meth:`read_zones` + is. What places a building has is a fact about the building, so a floor + that has been named but never mapped still answers, and a robot arriving + at a site can be told what the places are called before it has driven a + metre. """ - # The floor's own directory first, because the vocabulary lives there - # and *only* there: a revision carries the binding. A floor with no - # published map still answers, which is the portability the split buys. - for directory in ( - self.floor_dir(site, floor), - self._zones_file(site, floor, revision=self.canonical(site, floor)), - ): - if directory is not None and (directory / bundle.VOCABULARY_YAML).is_file(): - return bundle.vocabulary( - self._read(directory, site, floor), site, floor - ) + # The canonical revision's copy where there is one, exactly as + # `read_zones` reads it: promotion is what publishes an edit, so the + # copy inside the revision an operator promoted is the fleet's current + # answer and the floor's own file is the fallback. path = self._zones_file(site, floor, revision=self.canonical(site, floor)) if path is None: raise StoreError(f"no zones for {site}/{floor}", 404) @@ -383,10 +373,10 @@ def derive_zones( first build of a floor arrives with `zone_01`..`zone_07` from `segment-map`, and defaulting to the canonical would have meant promoting placeholder names in order to be allowed to fix them — - publishing a map *because* it was wrong. It also puts the edit in the - frame it was drawn in: the operator renaming rooms is looking at the - candidate's own map in the review pane, and rebinding those names onto - the published map's frame is exactly the trap the pane exists to name. + publishing a map *because* it was wrong. It also derives from what the + operator was looking at: the review pane draws the candidate's own map, + and deriving from a different revision's zones would save an edit + nobody made. Returns ``(stored_revision, report, derived_from)``. """ source = source or self.canonical(site, floor) @@ -399,9 +389,9 @@ def derive_zones( # The zones the operator was shown, which is what the edit is a delta # of: `_zones_file` falls back to the floor's file for a revision # carrying none, and so does the review pane that fed the editor. The - # `frame_id` and `vocabulary_revision` come from there for the same - # reason — an edit that silently reset the vocabulary revision would - # make a later carry-forward unable to tell which naming is newer. + # `frame_id` and `revision` come from there for the same reason — an + # edit that silently reset the revision would make a later + # carry-forward unable to tell which of two copies is newer. zones_file = self._zones_file(site, floor, revision=source) if zones_file is not None: try: @@ -414,12 +404,10 @@ 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", - "vocabulary_revision": int(previous.get("revision") or 0) + 1, + "revision": int(previous.get("revision") or 0) + 1, "zones": cleaned, } blob = bundle.pack( @@ -436,38 +424,9 @@ 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. + """Every floor's names, for a dispatcher bootstrapping a whole fleet in + one call. A floor with no zones yet is skipped, not an error. Walks the floors itself rather than reusing :meth:`sites`, which lists floors that have a *map*. The two sets are not the same one, and the @@ -496,21 +455,14 @@ def _floors(self): def _zones_file(self, site: str, floor: str, revision: str = ""): """The directory holding the zones of the given revision, else the - floor's own. - - A directory rather than a file since the split: a revision carries the - binding and the floor carries the vocabulary, and which of the two a - caller wants is what ``read_zones`` and ``read_vocabulary`` differ by. - """ + floor's own. A directory rather than a file, because that is what + ``bundle.read_floor`` takes.""" candidates = [] if revision: candidates.append(self.revision_dir(site, floor, revision)) candidates.append(self.floor_dir(site, floor)) for directory in candidates: - if any( - (directory / name).is_file() - for name in (bundle.BINDING_YAML, bundle.ZONES_YAML) - ): + if (directory / bundle.ZONES_YAML).is_file(): return directory return None diff --git a/mote_fleet/server/fleet_server.py b/mote_fleet/server/fleet_server.py index 9d5e548..c924028 100644 --- a/mote_fleet/server/fleet_server.py +++ b/mote_fleet/server/fleet_server.py @@ -27,8 +27,8 @@ GET /v1/maps basemaps this server can serve GET /v1/maps///map.json resolution + origin (the Q5 transform) GET /v1/maps///map.png the basemap image - GET /v1/maps///zones.json the floor's zone *binding* - GET /v1/zones every floor's zone *vocabulary* + GET /v1/maps///zones.json the floor's zones, with poses + GET /v1/zones every floor's zone *names* GET /v1/zones// one floor's, as a zone/v0 document GET /v1/sites the registry: floors + canonical rev GET /v1/sites//floors/ revisions, validated, with provenance @@ -36,7 +36,7 @@ GET .../revisions//bundle.tar.gz pull one (robot) GET .../revisions//map.json that revision's own Q5 transform GET .../revisions//map.png that revision's own image - GET .../revisions//zones.json that revision's own zone binding + GET .../revisions//zones.json the zones that revision carries POST .../revisions//promote make it canonical (operator) POST /v1/sites//floors//zones edited zones of a revision -> a new candidate (operator) @@ -67,21 +67,20 @@ flips the floor's ``map`` symlink and publishes the retained ``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 -bound zone (fleet.md Q4). The bytes live in :mod:`bundle_store`, and both ends +origin is an accident of where SLAM started, so merging frames would put the +floor's zones somewhere nobody put them (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``). -**Names are served; coordinates are not (zone/v0).** The same site bundles hold -the answer to the question a dispatcher actually asks — *what places can I -name?* — and until now the only ways to get it were an out-of-band document or -scraping the list a robot prints when it refuses an unknown zone. ``/v1/zones`` -answers it directly, and answers it with a **vocabulary**: the name of each -place and a note about it, no coordinates, no frame. That restraint is what -makes publishing it safe. A zone's pose is a coordinate in one robot's map frame, whose origin is an -accident of where its SLAM session started, so it is true for that robot and -false for the one beside it; the name is true for both. The binding stays where -it was, under ``/v1/maps``, served to the client that also has the basemap. +**Two routes, two views of one file.** The site bundles hold the answer to the +question a dispatcher asks — *what places can I name?* — and ``/v1/zones`` +answers it with the **names** and nothing else: what each place is called and a +note about it, no coordinates. Not because a coordinate would be wrong — a zone +is a coordinate in the floor's frame and every robot on the floor holds the same +one — but because a caller with no basemap has nothing to draw one on, and being +handed a number it cannot place is worse than not being handed it. The poses +stay under ``/v1/maps``, gated on there being a published map, served to the +client that also has the basemap to put them on. **Security posture for M3:** the read routes are still unauthenticated, exactly as M1 left them, and the broker is still anonymous. What M3 adds is a credential @@ -627,12 +626,12 @@ def _send_map(self, leaf: str, load): def _vocabulary(self, rest: str): """``/v1/zones//`` — what places can be named here. - 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 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. + Its own prefix rather than another leaf under ``/v1/maps``, because + everything under that one is served beside a basemap and gated on there + being one. This is gated on nothing and needs nothing to make sense of, + so 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. """ parts = rest.split("/") if len(parts) != 2: diff --git a/mote_fleet/server/ui/zone_editor.mjs b/mote_fleet/server/ui/zone_editor.mjs index bedca78..5202067 100644 --- a/mote_fleet/server/ui/zone_editor.mjs +++ b/mote_fleet/server/ui/zone_editor.mjs @@ -22,7 +22,7 @@ // and an editor that could only edit the published map would have required // promoting those placeholder names in order to be allowed to fix them. // -// Geometry and the vocabulary rules live in pure functions over zone objects in +// Geometry and the naming rules live in pure functions over zone objects in // *world* metres, so every edit operation is testable under node with no canvas // and no DOM. @@ -111,42 +111,40 @@ export function cursorFor(target, placing = false) { return target.kind === 'vertex' ? 'crosshair' : 'move'; } -// zone/v0's `anchor` records how a coordinate came to be, which is what tells a -// reader whether to trust it after the map changes. Geometry placed or moved -// here is neither `taught` — no robot drove there and captured a pose — nor -// `derived` — no algorithm read it off the map. `external` is the one of -// zone/v0's four that is true of it: resolved off the platform, `by` naming -// what did it. The server fills in `at` and which operator was at the keyboard, -// which a browser's clock and a browser's word for it cannot be trusted for. -export const EDITOR_ANCHOR = { method: 'external', by: 'zone-editor' }; - -// A binding carries one anchor for the whole of its geometry, so any edit to -// that geometry re-anchors it: reshaping a `segment-map` room is no longer -// something an algorithm alone produced. A zone this edit does not touch keeps -// whatever provenance it arrived with, which is why every geometry helper goes -// through here and nothing stamps the whole set on save. -export function reanchored(zone) { - return { ...zone, anchor: { ...EDITOR_ANCHOR } }; +// `source` records what made the zone: `save-zone` for a pose a robot was +// driven to and captured, `segment-map` for a room read off a saved map, and +// `editor` for geometry placed or dragged here. It is a note and nothing reads +// it to decide anything — a zone is a coordinate in the floor's frame however +// it got there — so what it buys is an operator being able to see which zones +// somebody drew. +export const EDITOR_SOURCE = 'editor'; + +// Any edit to a zone's geometry makes this editor what most recently placed it, +// so every geometry helper goes through here. A zone this edit does not touch +// keeps the source it arrived with, which is why nothing stamps the whole set +// on save. +export function sourced(zone) { + return { ...zone, source: EDITOR_SOURCE }; } export function withVertex(zone, index, x, y) { const polygon = zone.polygon.map((point, i) => i === index ? [round(x), round(y)] : point, ); - return reanchored({ ...zone, polygon }); + return sourced({ ...zone, polygon }); } export function withInsertedVertex(zone, afterIndex, x, y) { const polygon = zone.polygon.slice(); polygon.splice(afterIndex + 1, 0, [round(x), round(y)]); - return reanchored({ ...zone, polygon }); + return sourced({ ...zone, polygon }); } // A polygon needs three vertices to enclose anything; refuse rather than // letting a delete quietly produce a line. export function withoutVertex(zone, index) { if (!zone.polygon || zone.polygon.length <= 3) return null; - return reanchored({ ...zone, polygon: zone.polygon.filter((_, i) => i !== index) }); + return sourced({ ...zone, polygon: zone.polygon.filter((_, i) => i !== index) }); } // Moving a zone moves its footprint and its pose together: they name the same @@ -158,11 +156,11 @@ export function translated(zone, dx, dy) { } if (typeof zone.x === 'number') moved.x = round(zone.x + dx); if (typeof zone.y === 'number') moved.y = round(zone.y + dy); - return reanchored(moved); + return sourced(moved); } export function withPose(zone, x, y) { - return reanchored({ ...zone, x: round(x), y: round(y) }); + return sourced({ ...zone, x: round(x), y: round(y) }); } // A new zone arrives as a rectangle at the view centre with the first free @@ -171,7 +169,7 @@ export function freshZone(existing, cx, cy, half = 1.0) { const names = new Set(existing.map((zone) => zone.name)); let n = 1; while (names.has(`zone_${String(n).padStart(2, '0')}`)) n += 1; - return reanchored({ + return sourced({ name: `zone_${String(n).padStart(2, '0')}`, x: round(cx), y: round(cy), @@ -206,9 +204,8 @@ export function snapDelta(map, delta) { return round(Math.round(delta / map.resolution) * map.resolution); } -// One zone's geometry in a phrase: the binding half, which is the half that is -// only true against the map beside it. Shown in the list whether or not the -// list is being edited — the shape is a fact about the zone, not a control. +// One zone's geometry in a phrase. Shown in the list whether or not the list is +// being edited — the shape is a fact about the zone, not a control. // // A zone is a *point* or an *area*, and that is geometry rather than a type it // was declared to be: a zone has a pose, and it may also have an extent. The @@ -606,7 +603,7 @@ export class ZoneEditor { // Every rename is checked against the same rules the robot's loader enforces // — the name shape, and two zones answering one query — because the loader - // *refuses* an ambiguous vocabulary rather than resolving it by dict order. + // *refuses* two zones answering one query rather than picking by dict order. // A set this editor is willing to save is a set a robot will load. problems() { const seen = new Set(); diff --git a/mote_fleet/test/api_harness.py b/mote_fleet/test/api_harness.py index a362fea..83e8e10 100644 --- a/mote_fleet/test/api_harness.py +++ b/mote_fleet/test/api_harness.py @@ -111,7 +111,7 @@ def chunk(kind, data): ) ZONES_YAML = ( - "frame_id: map\nvocabulary_revision: 4\nzones:\n" + "frame_id: map\nrevision: 4\nzones:\n" " kitchen: {x: 1.0, y: 2.0, yaw: 0.0, radius: 1.5, kind: room,\n" " display_name: The Kitchen, aliases: [galley]}\n" " ward: {x: 4.0, y: 1.0, yaw: 1.57, kind: room,\n" diff --git a/mote_fleet/test/test_mapsync.py b/mote_fleet/test/test_mapsync.py index 8d351a7..ce25bf2 100644 --- a/mote_fleet/test/test_mapsync.py +++ b/mote_fleet/test/test_mapsync.py @@ -116,9 +116,10 @@ def test_a_revision_already_on_disk_is_flipped_rather_than_downloaded( def test_zones_arrive_with_the_map_and_the_old_ones_are_kept( server, operator, robot_home, tmp_path ): - """A revision from another mapping session is another map frame, so the - zones bound in the old one are wrong the moment it is published — but - losing every bound place silently is not acceptable either.""" + """A revision carries a copy of the floor's zones, and promoting it is how + an edit made in the dashboard reaches a robot — so installing one replaces + what the floor held. Losing every named place silently is not acceptable + either, so the copy it replaces is kept.""" floor_dir = sites.floor_dir(SITE, FLOOR) floor_dir.mkdir(parents=True) (floor_dir / "zones.yaml").write_text( @@ -217,25 +218,22 @@ def test_publishing_packs_the_floors_zones_into_the_revision( assert uploaded["zones"] == ["bay"] -def test_publishing_a_split_floor_sends_the_binding_in_the_revision( - server, robot_home, tmp_path -): - """The zone/v0 layout, end to end. - - The coordinates go *inside* the revision, because they are only meaningful - in that revision's map frame. The names ride along too — but an upload is - inert, and that now covers names as well as coordinates: until an operator - promotes, ``/v1/zones`` still answers with the floor's published - vocabulary. Otherwise a robot could rename every room on a floor its - neighbours are driving, by uploading a map nobody accepted. +def test_publishing_sends_the_floor_zones_in_the_revision(server, robot_home, tmp_path): + """A revision carries a copy of the floor's zones, end to end. + + The floor owns them, and the revision is the vehicle the fleet already has + for getting them to a robot that has never driven here. An upload is inert, + and that covers names as well as coordinates: until an operator promotes, + ``/v1/zones`` still answers with what is published. Otherwise a robot could + rename every room on a floor its neighbours are driving, by uploading a map + nobody accepted. """ enroll(server, "serial:ddd", name="Scout") floor_dir = sites.floor_dir(SITE, FLOOR) write_revision(floor_dir / "maps" / REVISION, zones=False) # Written with `bundle`, not with `mote_tasks.zones`: these tests run in the # ROS-free `fleet` environment, and reaching for the task layer's writer - # here would be the seam the split exists to keep — the robot's half needs - # ROS, the fleet's half must never. + # here would cross the seam that lets the fleet box install no ROS. bundle.write_floor( floor_dir, { @@ -246,18 +244,14 @@ def test_publishing_a_split_floor_sends_the_binding_in_the_revision( "name": "bay", "note": "", "navigable": True, - "bound": True, "x": 1.5, "y": -2.0, "yaw": 0.0, } }, }, - site=SITE, - floor=FLOOR, - platform_id="mote-01", ) - assert (floor_dir / "binding.yaml").is_file() + assert (floor_dir / "zones.yaml").is_file() sites._publish_revision(floor_dir, REVISION) mapsync.publish(server.url, SITE, FLOOR, REVISION, "mote-01") diff --git a/mote_fleet/test/test_zone_edit.py b/mote_fleet/test/test_zone_edit.py index be2a5f9..d530f71 100644 --- a/mote_fleet/test/test_zone_edit.py +++ b/mote_fleet/test/test_zone_edit.py @@ -151,7 +151,7 @@ def test_editing_a_candidate_on_a_floor_with_nothing_published(server, tmp_path, assert floor["canonical"] is None # still nothing published -def test_the_vocabulary_revision_advances_from_the_edited_revision( +def test_the_revision_counter_advances_from_the_edited_revision( server, tmp_path, robot ): """A carry-forward has to be able to tell which naming is newer, so the @@ -160,56 +160,36 @@ def test_the_vocabulary_revision_advances_from_the_edited_revision( token = server.registry.new_operator(name="editor") _, body = edit(server, ZONES, token) first = body["revision"] - assert stored_zones(server, first)[1]["vocabulary_revision"] == 5 + assert stored_zones(server, first)[1]["revision"] == 5 # Editing the result again continues from *it*, not from the canonical. _, body = edit(server, ZONES, token, revision=first) - assert stored_zones(server, body["revision"])[1]["vocabulary_revision"] == 6 + assert stored_zones(server, body["revision"])[1]["revision"] == 6 -def test_an_edited_pose_stops_saying_a_robot_was_driven_there(server): - """``anchor.method`` is how a later reader decides whether to trust a - coordinate, 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 an algorithm, and what this editor writes is neither. The editor - stamps ``external`` on what it moved; the server, which has a clock worth - trusting and knows who is holding the token, fills in when and who. +def test_what_made_a_zone_travels_with_it(server): + """``source`` says what put the coordinate there. + + It is a note and nothing decides anything from it — a zone is a coordinate + in the floor's frame however it got there — so what it buys is an operator + being able to see which zones somebody drew. A zone the edit did not touch + keeps what it arrived with; a value outside the three is dropped rather than + costing the whole save. """ token = server.registry.new_operator(name="editor") edited = { - # As the editor submits it: placed here, and saying only that. - "the kitchen": dict( - ZONES["the kitchen"], anchor={"method": "external", "by": "zone-editor"} - ), - # Untouched, so it keeps the provenance it arrived with. - "office": dict(ZONES["office"], anchor={"method": "taught", "by": "michael"}), + "the kitchen": dict(ZONES["the kitchen"], source="editor"), + "office": dict(ZONES["office"], source="save-zone"), + "yard": dict(ZONES["the kitchen"], source="surveyed"), } status, body = edit(server, edited, token) assert status == 201, body stored = server.store.read_revision_zones(SITE, FLOOR, body["revision"]) - anchors = {zone["name"]: zone["anchor"] for zone in stored["zones"]} - assert anchors["the kitchen"]["method"] == "external" - assert anchors["the kitchen"]["by"] == "editor (zone-editor)" - assert anchors["the kitchen"]["at"], "the server stamps when, the browser cannot" - # A zone the edit did not touch is not re-anchored on the way past. - assert anchors["office"] == { - "method": "taught", - "at": None, - "by": "michael", - "confidence": None, - } - - -def test_an_anchor_claiming_a_method_that_does_not_exist_is_refused(server): - """The anchor is the one part of a zone the platform does not author, so it - is the one part a client could use to claim anything. A method outside - zone/v0's four is refused at the parse rather than stored, where nothing - afterwards could tell it from a real one.""" - token = server.registry.new_operator(name="editor") - bad = {"office": dict(ZONES["office"], anchor={"method": "surveyed"})} - reason = expect_error(lambda: edit(server, bad, token), 422) - assert "surveyed" in str(reason) + sources = {zone["name"]: zone.get("source") for zone in stored["zones"]} + assert sources["the kitchen"] == "editor" + assert sources["office"] == "save-zone" + assert sources["yard"] is None def test_a_revision_with_no_posegraph_can_still_have_its_zones_edited( diff --git a/mote_fleet/test/ui_test.mjs b/mote_fleet/test/ui_test.mjs index 8938da3..2f13898 100644 --- a/mote_fleet/test/ui_test.mjs +++ b/mote_fleet/test/ui_test.mjs @@ -714,7 +714,7 @@ test('both map canvases take their own touch gestures', () => { // pointer events on top of exactly these. const { - EDITOR_ANCHOR, + EDITOR_SOURCE, NAME_RE, ambiguities, cursorFor, @@ -862,21 +862,20 @@ test('moving a zone carries footprint and pose together', () => { ...square, x: 5, y: 5, - anchor: EDITOR_ANCHOR, + source: EDITOR_SOURCE, }); }); -test('geometry this editor places or moves stops claiming a robot drove there', () => { - // `anchor.method` is what tells a later reader whether to trust a coordinate - // after the map changes, and the answer differs: a taught pose was measured - // by driving a robot to it, a `segment-map` room was read off the map by an - // algorithm, and one of these is a click. Stamping a click `taught` puts a - // measurement nobody took into the one field that exists to say so. - const driven = { ...square, anchor: { method: 'taught', by: 'michael' } }; +test('geometry this editor places or moves says the editor made it', () => { + // `source` is what put the coordinate there — a robot driven to it, an + // algorithm reading a map, or a click. Nothing decides anything from it, so + // what it buys is an operator being able to see which zones somebody drew; + // and that only works if a click stops answering `save-zone`. + const driven = { ...square, source: 'save-zone' }; const derived = { name: 'ward_a', polygon: [[0, 0], [2, 0], [2, 2], [0, 2]], - anchor: { method: 'derived', by: 'segment-map' }, + source: 'segment-map', }; for (const edited of [ @@ -887,20 +886,15 @@ test('geometry this editor places or moves stops claiming a robot drove there', withoutVertex({ ...derived, polygon: [...derived.polygon, [-1, 1]] }, 0), freshZone([], 0, 0), ]) { - assert.deepEqual(edited.anchor, EDITOR_ANCHOR); + assert.equal(edited.source, EDITOR_SOURCE); } - // And a zone nobody touched keeps whatever provenance it arrived with — - // re-anchoring the whole set on save would lose the one fact that separates - // a room an algorithm proposed from one an operator drew. + // And a zone nobody touched keeps what it arrived with — stamping the whole + // set on save would lose the one fact that separates a room an algorithm + // proposed from one an operator drew. const untouched = zonesPayload([driven, derived]); - assert.deepEqual(untouched.kitchen.anchor, { method: 'taught', by: 'michael' }); - assert.deepEqual(untouched.ward_a.anchor, { method: 'derived', by: 'segment-map' }); - - // The editor names itself and says nothing about when or who: a browser's - // clock is the operator's laptop, and the server fills both in. - assert.equal(EDITOR_ANCHOR.method, 'external'); - assert.ok(!('at' in EDITOR_ANCHOR)); + assert.equal(untouched.kitchen.source, 'save-zone'); + assert.equal(untouched.ward_a.source, 'segment-map'); }); test('a fresh zone gets the first free generated name and a real footprint', () => { diff --git a/mote_tasks/README.md b/mote_tasks/README.md index 55509f9..6a2251f 100644 --- a/mote_tasks/README.md +++ b/mote_tasks/README.md @@ -65,11 +65,9 @@ answers "am I inside it?". That footprint is just optional metadata on the single zone concept — not a second kind of thing — so there's one loader whatever wrote the file. -A floor stores this as the zone/v0 pair: `vocabulary.yaml` for what the places -are called, `binding.yaml` for where geometry says they are. The combined -`zones.yaml` below is the pre-split shape, still read (and migrated the first -time anything writes) because the sim worlds and the committed default ship -that way — one file is the right shape for a fixture with one robot in it. +A floor stores this in one `zones.yaml`, and the floor owns it: a zone is a +coordinate in the floor's frame — a fact about the building — where a map +revision is an estimate registered into that frame. ```yaml frame_id: map diff --git a/mote_tasks/mote_tasks/save_zone.py b/mote_tasks/mote_tasks/save_zone.py index 07c7a0d..75556d3 100644 --- a/mote_tasks/mote_tasks/save_zone.py +++ b/mote_tasks/mote_tasks/save_zone.py @@ -36,8 +36,6 @@ from rclpy.node import Node from rclpy.time import Time -from mote_bringup import identity - from mote_tasks.zones import append_zone, yaw_from_quaternion LOOKUP_TIMEOUT = 10.0 @@ -115,7 +113,6 @@ def main(): navigable, site=site, floor=floor, - platform_id=identity.robot_id() or "", ) verb = "replaced" if replaced else "added" extra = f" radius={radius:.3f}" if radius is not None else "" diff --git a/mote_tasks/mote_tasks/task_server.py b/mote_tasks/mote_tasks/task_server.py index 5c62bf3..06e3f26 100644 --- a/mote_tasks/mote_tasks/task_server.py +++ b/mote_tasks/mote_tasks/task_server.py @@ -92,7 +92,6 @@ #: ``recoverable`` says has not happened. ZONE_REASON_RECOVERABLE = { "unknown_name": False, - "unbound": False, "wrong_floor": False, "stale_revision": True, "not_navigable": False, @@ -125,22 +124,8 @@ def __init__(self, **node_kwargs): "config", "zones.default.yaml", ) - # load_floor, not load_zones: a name in the floor's vocabulary that the - # binding carries no geometry for must reach the resolver, so a mission - # for it can be refused as `unbound` — "I know that place, nothing has - # said where it is" — rather than as an unknown name, which sends an - # operator hunting for a typo that is not there. - self.zones = zones.load_floor(zones_file) - bound = sorted(name for name, z in self.zones.items() if z.bound) - unbound = sorted(name for name, z in self.zones.items() if not z.bound) - self.get_logger().info(f"Zones {bound} from {zones_file}") - if unbound: - self.get_logger().warning( - f"named here, no geometry in the revision this robot is running:" - f" {', '.join(unbound)} (place them in the dashboard's zone " - "editor and promote, pull the revision that binds them, or " - "drive there and run save-zone)" - ) + self.zones = zones.load_zones(zones_file) + self.get_logger().info(f"Zones {sorted(self.zones)} from {zones_file}") self.platform_id = platform_id or identity.robot_id() or UNENROLLED self.capabilities = capabilities.capability_set( diff --git a/mote_tasks/mote_tasks/zones.py b/mote_tasks/mote_tasks/zones.py index c66e6e1..3c838e2 100644 --- a/mote_tasks/mote_tasks/zones.py +++ b/mote_tasks/mote_tasks/zones.py @@ -1,65 +1,52 @@ -"""Named places (zones), loaded from a floor's zone/v0 documents. - -A **zone** is a named pose in a map frame the robot can navigate to — as a -fetch waypoint (`pickup`/`dropoff`) or a `goto ` target. A zone may also -carry an **area footprint**, so it can answer "is (x, y) inside this zone?". -The footprint is optional metadata on the one zone concept, not a separate kind -of thing: a bare zone is just a pose; a room-like zone adds a footprint. A -footprint is either a circle (`radius`, the simple taught default) or a -`polygon` of explicit vertices, which follows the actual room outline — an -L-shaped ward or a corridor stretch that no circle can describe. - -**A zone's two halves live in two files** (zone/v0, `mote_bringup.spec.zone`): - - floors//vocabulary.yaml what the places are CALLED. No - coordinates, so it is safe to share with - every robot at the site. - floors//binding.yaml where geometry says they are, in this - floor's map frame. Travels only inside - the revision that frame belongs to. - -They are not the same kind of fact. `(2.0, 3.5)` in this map frame is a -different physical point in the next robot's, so the fleet publishes the -vocabulary and never the binding on its own. A legacy combined `zones.yaml` is -still read (`bundle.read_floor` migrates it) and is replaced by the pair the -first time anything writes. +"""Named places (zones), loaded from a floor's ``zones.yaml``. + +A **zone** is a named pose the robot can navigate to — as a fetch waypoint +(`pickup`/`dropoff`) or a `goto ` target. A zone may also carry an **area +footprint**, so it can answer "is (x, y) inside this zone?". The footprint is +optional metadata on the one zone concept, not a separate kind of thing: a bare +zone is just a pose; a room-like zone adds a footprint. A footprint is either a +circle (`radius`, the simple taught default) or a `polygon` of explicit +vertices, which follows the actual room outline — an L-shaped ward or a corridor +stretch that no circle can describe. + +**A zone is a coordinate in the floor's frame — a fact about the building.** +The kitchen does not move. A map revision is an *estimate* of the same layout +registered into that frame, so re-mapping the floor changes how well the robot +knows where it is and changes nothing about where the kitchen is; where the two +disagree it is the map that gets aligned. So the floor holds its zones, in one +`floors//zones.yaml`, and neither a map revision nor one robot owns them. +A promoted revision carries a copy, which is how a floor's places reach a robot +that has never driven there. Geometry reaches a floor three ways, and only the first involves a robot: driving there and running `save-zone`, which is the one that also measures an approach heading; `segment-map` reading room outlines off a saved map; and the fleet dashboard's zone editor, where an operator places and drags zones on a -candidate revision. A promoted revision then carries the result to every robot -at the site, so a robot can hold geometry for a floor it has never driven. - -What the split buys a reader is :data:`unbound`. :func:`load_floor` returns -every name the floor carries, bound or not, so a name with no geometry in the -binding this robot holds is answerable as "I know that place, nothing has said -where it is" rather than as an unknown name — which sent an operator hunting -for a typo that was not there. :func:`load_zones` is the same minus the unbound -ones, for every caller that only ever wanted a pose. - -**A zone is a place-name**, so the vocabulary is one human name and a free-text -`note` — nothing else. The mission layer's resolver already knows what a store -room is; what it cannot know is that this building's store room is where the -stationery lives, which is what the note is for. A floor written before this -carries `kind`, `display_name`, `aliases`, `parent` and `tags`; they still load -(`kind: keepout` still means non-navigable, and `description` is read as the -note it was) and they are never written again. - -Example (`binding.yaml` alongside): - - schema: 1 - site: acme_hq - floor: ground +candidate revision. + +**A zone is also a place-name**, so the naming half is one human name and a +free-text `note` — nothing else. The mission layer's resolver already knows what +a store room is; what it cannot know is that this building's store room is where +the stationery lives, which is what the note is for. A floor may carry `kind`, +`display_name`, `aliases`, `parent` and `tags` as well; nothing here writes +them, they load and are dropped, and the two that carry meaning are read for it +(`description` into the note, `kind: keepout` into `navigable: false`). + +Example: + + frame_id: map revision: 4 zones: - - {name: the kitchen, note: 'the good kettle is in the store room'} - - {name: plant, navigable: false} + the kitchen: + x: 2.0 + y: 3.5 + yaw: 0.0 + note: the good kettle is in the store room + plant: {x: 1.0, y: 0.5, radius: 0.4, navigable: false} """ import math from dataclasses import dataclass -from datetime import datetime, timezone from pathlib import Path from geometry_msgs.msg import PoseStamped @@ -98,7 +85,7 @@ def contains(self, px: float, py: float) -> bool: class Polygon: """A footprint following an outline of explicit vertices. - The outline is a simple (non-self-intersecting) polygon in the binding's + The outline is a simple (non-self-intersecting) polygon in the floor's frame, closed implicitly, in either winding order. Concave outlines are supported — membership is a ray cast, not a convex-hull test — so an L-shaped room or a corridor stretch is representable. @@ -131,9 +118,8 @@ def representative_point(self) -> tuple[float, float]: class Zone: """A named place: a pose to navigate to, plus an optional area footprint. - ``pose`` and ``footprint`` are the binding — only meaningful in this - robot's ``frame_id``. Everything else is the vocabulary, and is the same on - every robot at the site. + ``pose`` and ``footprint`` are coordinates in the floor's frame. Every robot + on the floor holds the same ones, because the floor holds them. """ name: str @@ -148,23 +134,18 @@ class Zone: #: cannot supply, and the reason there is no alias list — another name #: this place answers to belongs in the sentence a resolver reads. note: str = "" - #: Whether the binding this robot holds carries geometry for it. A name in - #: the vocabulary with no binding is a real place with no pose here — which - #: is the whole reason zone/v0 splits the two, and the difference between - #: "you typed it wrong" and "nothing has said where that is". - bound: bool = True - #: A zone this robot holds a binding for that the site's vocabulary does - #: not name. Usable here; never advertised as a shared zone. - local: bool = False + #: What made the zone: ``save-zone``, ``segment-map`` or ``editor``. A note + #: about provenance and nothing more — a zone read off a map is as much a + #: coordinate in the floor's frame as one a robot was driven to. + source: str = "" @property def label(self) -> str: """What to call it when talking to a human — which is its name. - Kept as a property because a zone is *labelled* in half a dozen places - and the split between a machine name and a human one was exactly the - thing place-names removed; a caller that asks for a label should not - have to know that the answer is now the same field. + A property rather than the bare attribute because a zone is *labelled* + in half a dozen places, and none of them should have to know which + field the answer comes from. """ return self.name @@ -192,33 +173,27 @@ def append_zone( *, site: str = "", floor: str = "", - platform_id: str | None = None, ) -> bool: - """Teach one zone into a floor, writing back the zone/v0 pair. + """Teach one zone into a floor, writing the floor's ``zones.yaml`` back. ``radius`` (optional) gives the zone a circular footprint, replacing any footprint it had. Re-teaching without one keeps the existing footprint, so capturing a better pose for a room does not discard its outline. Returns True if an existing zone was replaced. - Re-teaching is a new *coordinate*, never a new name, so the vocabulary a - zone already carries is carried through untouched unless ``note`` or - ``navigable`` says otherwise — driving somewhere to capture a better pose - must not silently drop what an operator typed in by hand. Under the split - that is no longer a rule this function has to remember: the two documents - are written separately, and a coordinate cannot reach the one that holds - the names. - - ``path`` is the floor directory. A legacy combined file is migrated on the - way through, so the first ``save-zone`` on an old floor is also what splits - it — nobody has to run a migration, and nobody can forget to. + Re-teaching is a new *coordinate*, never a new name: the name, note and + ``navigable`` a zone already carries come through untouched unless ``note`` + or ``navigable`` says otherwise, because driving somewhere to capture a + better pose must not silently drop what an operator typed in by hand. + + ``path`` is the floor directory, or the ``zones.yaml`` in it. """ path = Path(path).expanduser() if path.suffix == ".yaml": - # A caller that named the old combined file means the floor it is in. - # Matched on the name rather than on ``is_file``, because the file may - # not exist yet and creating a *directory* called zones.yaml is the one - # outcome nothing recovers from. + # A caller that named the file means the floor it is in. Matched on the + # name rather than on ``is_file``, because the file may not exist yet + # and creating a *directory* called zones.yaml is the one outcome + # nothing recovers from. path = path.parent try: floor_zones = bundle.read_floor(path, site, floor) @@ -226,10 +201,8 @@ def append_zone( floor_zones = { "site": site, "floor": floor, - "platform_id": platform_id or "", "frame_id": "map", "revision": 0, - "map_revision": "", "zones": {}, } previous = floor_zones["zones"].get(name) or {} @@ -238,8 +211,7 @@ def append_zone( x=round(x, 3), y=round(y, 3), yaw=round(yaw, 3), - bound=True, - anchor=zone_spec.anchor(zone_spec.TAUGHT, at=_stamp()), + source=zone_spec.SAVE_ZONE, ) if radius is not None: entry["radius"] = round(radius, 3) @@ -253,38 +225,19 @@ def append_zone( entry["navigable"] = bool(navigable) replaced = name in floor_zones["zones"] floor_zones["zones"][name] = entry - # The vocabulary revision is what a binding records itself as built - # against, so it has to move whenever a name could have. + # What a reader compares to tell which of two copies of a floor's zones is + # the later one, so it has to move whenever anything here could have. floor_zones["revision"] = int(floor_zones.get("revision") or 0) + 1 - bundle.write_floor( - path, - floor_zones, - site=site, - floor=floor, - platform_id=platform_id, - ) + bundle.write_floor(path, floor_zones) return replaced -def _stamp() -> str: - stamp = datetime.now(timezone.utc).isoformat(timespec="seconds") - return stamp.replace("+00:00", "Z") - - -def load_floor(path) -> dict[str, Zone]: - """Every zone this floor names, bound or not. - - ``path`` is either a floor directory holding the zone/v0 pair - (``vocabulary.yaml`` + ``binding.yaml``) or a legacy combined - ``zones.yaml``, which :func:`mote_bringup.bundle.read_floor` migrates on - read. Both produce the same structure, so nothing downstream knows which - layout is on disk. +def load_zones(path) -> dict[str, Zone]: + """Every zone this floor names. - A zone with no binding comes back with ``pose=None`` and ``bound=False`` - rather than being dropped. That is the point of the split: the robot can - then say ``unbound`` — "I know that place, nothing has told me where it - is" — where before it could only say the name was unknown, which sent an - operator hunting for a typo that was not there. + ``path`` is either a floor directory holding ``zones.yaml`` or that file + named outright, which is what a sim world's committed zones and the packaged + fallback are. """ try: floor = bundle.read_floor(path) @@ -298,21 +251,26 @@ def load_floor(path) -> dict[str, Zone]: if "polygon" in spec else None ) - pose = None footprint = polygon - if spec.get("bound"): + if "x" in spec: x, y = spec["x"], spec["y"] pose = pose_from_xy_yaw(frame_id, x, y, float(spec.get("yaw", 0.0))) if footprint is None and "radius" in spec: footprint = Circle(x, y, float(spec["radius"])) + else: + # A polygon with no pose of its own — what ``segment-map`` writes, + # since it read a room off a map rather than driving to it. The + # point is guaranteed to lie inside the outline, which a centroid + # is not for a concave room. + x, y = polygon.representative_point() + pose = pose_from_xy_yaw(frame_id, x, y, 0.0) zones[name] = Zone( name, pose, footprint, navigable=spec["navigable"], note=spec["note"], - bound=bool(spec.get("bound")), - local=bool(spec.get("local")), + source=spec.get("source", ""), ) # zone/v0: a conforming platform rejects a vocabulary with a collision. # Loading one anyway would mean `goto kitchen` picking a winner by dict @@ -324,26 +282,13 @@ def load_floor(path) -> dict[str, Zone]: return zones -def load_zones(path) -> dict[str, Zone]: - """The zones this robot can actually drive to. - - :func:`load_floor` minus the unbound ones, for every caller that only ever - wanted a pose — ``containing``, the dashboard's basemap, ``save-zone``. - The task layer uses ``load_floor``, because refusing a mission is where the - difference between "unknown" and "unbound" is worth saying. - """ - return {name: zone for name, zone in load_floor(path).items() if zone.bound} - - class ZoneUnresolved(ValueError): """A name this robot cannot act on, with zone/v0's reason for it. - The reason is the point. "Not found" collapses two different faults an - operator does different things about: a name that is not in the vocabulary - at all is a mistake in the request, while one that is there and unbound - here is a gap in the geometry this robot holds, on a floor where its - neighbours may know the place perfectly well. mission/v0 carries it out as - ``failure.class: "unresolved_zone"`` with the reason in ``detail``. + The reason is the point: a name no zone on the floor answers to is a mistake + in the request, while a place marked ``navigable: false`` exists and was + drawn on purpose. mission/v0 carries it out as ``failure.class: + "unresolved_zone"`` with the reason in ``detail``. """ def __init__(self, reason: str, message: str): @@ -359,8 +304,7 @@ def resolve(zones: dict[str, Zone], query: str) -> Zone | None: the matching: a zone has one name, and the other spellings a place answers to are a job for the mission layer's resolver reading the ``note``, not for a list of aliases an operator has to keep in step by hand. Ambiguity cannot - arise here because :func:`load_floor` refuses a vocabulary that contains - any. + arise here because :func:`load_zones` refuses a floor that contains any. """ if query in zones: return zones[query] @@ -374,17 +318,9 @@ def resolve(zones: dict[str, Zone], query: str) -> Zone | None: def resolve_reason(zones: dict[str, Zone], query) -> tuple[Zone | None, str | None]: """``(zone, reason)`` — the zone, and why it cannot be driven to. - Three of zone/v0's six reasons are answerable here, and the split is what - made the second one answerable at all: - - * ``unknown_name`` — not in this floor's vocabulary. A mistake in the - request. - * ``unbound`` — in the vocabulary, and the binding this robot holds carries - no geometry for it. Not a typo, so the remedies are the three that put a - coordinate there: place it in the dashboard's zone editor on a candidate - revision and promote that; pull the revision that already binds it (the - robot may be running an older one); or drive there and ``save-zone``, - which is the one that measures an approach heading. + Two of zone/v0's reasons are answerable here: + + * ``unknown_name`` — no zone on this floor answers to the query. * ``not_navigable`` — a constraint zone used as a destination. It exists and an operator drew it on purpose, so saying "unknown" would send them hunting for a spelling mistake that is not there. @@ -392,16 +328,16 @@ def resolve_reason(zones: dict[str, Zone], query) -> tuple[Zone | None, str | No ``wrong_floor`` and ``stale_revision`` are the two this robot cannot yet answer: it holds one floor at a time, and the map bundle does not declare frame continuity — which zone/v0 says is out of its own scope too. - ``ambiguous`` cannot arise, because :func:`load_floor` refuses a vocabulary - that contains one. + ``ambiguous`` cannot arise, because :func:`load_zones` refuses a floor that + contains one. ``unbound`` cannot arise either, and Mote never reports it: a + zone is a coordinate in the floor's frame, so a name with no coordinate is + not a zone on this floor — it is a name nobody has placed. """ zone = resolve(zones, query) if isinstance(query, str) else None if zone is None: return None, zone_spec.UNKNOWN_NAME if not zone.navigable: return zone, zone_spec.NOT_NAVIGABLE - if not zone.bound: - return zone, zone_spec.UNBOUND return zone, None @@ -414,16 +350,8 @@ def destination(zones: dict[str, Zone], query, where: str = "zone") -> Zone: f"{where} {zone.name!r} is not a destination — it is marked " "navigable: false", ) - if reason == zone_spec.UNBOUND: - raise ZoneUnresolved( - reason, - f"{where} {zone.name!r} is a place on this floor, but the map " - "revision this robot is running has no geometry for it — place it " - "in the dashboard's zone editor and promote, pull the revision " - "that binds it, or drive there and run save-zone", - ) if reason is not None: - known = sorted(name for name, z in zones.items() if z.navigable and z.bound) + known = sorted(name for name, z in zones.items() if z.navigable) raise ZoneUnresolved( reason, f"{where} {query!r} is not a place here; " @@ -439,9 +367,7 @@ def containing(zones: dict[str, Zone], x: float, y: float) -> list[str]: """ hits = [] for zone in zones.values(): - # An unbound zone has a footprint only if a binding gave it one, so - # `pose is None` here would mean a footprint in no frame. - if zone.pose is None or zone.footprint is None: + if zone.footprint is None: continue if zone.footprint.contains(x, y): p = zone.pose.pose.position diff --git a/mote_tasks/test/test_goto_tree.py b/mote_tasks/test/test_goto_tree.py index ddb1c26..3d918d1 100644 --- a/mote_tasks/test/test_goto_tree.py +++ b/mote_tasks/test/test_goto_tree.py @@ -219,3 +219,18 @@ def test_idle_rate_is_floored_at_the_mission_rate(ros, tmp_path): assert server.idle_tick_period == pytest.approx(0.5) assert server.tick_timer.timer_period_ns / 1e9 == pytest.approx(0.5) server.destroy_node() + + +def test_every_reason_a_zone_can_fail_for_has_a_recoverable_answer(): + """``ZONE_REASON_RECOVERABLE`` is indexed, not looked up with a default. + + A reason missing from it is a ``KeyError`` inside the command callback — + the mission gets no status at all, which is worse than a wrong one. So the + table has to cover zone/v0's reasons exactly, and adding or retiring one + has to fail here rather than in the field. + """ + from mote_bringup.spec import zone as zone_spec + + from mote_tasks.task_server import ZONE_REASON_RECOVERABLE + + assert set(ZONE_REASON_RECOVERABLE) == set(zone_spec.REASONS) diff --git a/mote_tasks/test/test_segmented_zones.py b/mote_tasks/test/test_segmented_zones.py index b18cad4..76a76b3 100644 --- a/mote_tasks/test/test_segmented_zones.py +++ b/mote_tasks/test/test_segmented_zones.py @@ -89,16 +89,15 @@ def test_a_hand_bound_room_is_not_renamed_by_a_later_run(tmp_path, rooms): def test_running_it_twice_adds_nothing(tmp_path, rooms): path = tmp_path merge_into_zones(path, rooms) - documents = ("vocabulary.yaml", "binding.yaml") - before = {name: (path / name).read_text() for name in documents} + before = (path / "zones.yaml").read_text() added, skipped = merge_into_zones(path, rooms) assert added == [] assert len(skipped) == 2 - # Byte-identical, both halves: a re-run that bumped the vocabulary revision - # would make every binding on the fleet look a version behind. - assert {name: (path / name).read_text() for name in documents} == before + # Byte-identical: a re-run that bumped the revision would make every copy + # of this floor's zones on the fleet look a version behind. + assert (path / "zones.yaml").read_text() == before def test_a_proposed_room_says_nothing_it_cannot_know(tmp_path, rooms): diff --git a/mote_tasks/test/test_zone_split.py b/mote_tasks/test/test_zone_split.py deleted file mode 100644 index c623cd8..0000000 --- a/mote_tasks/test/test_zone_split.py +++ /dev/null @@ -1,158 +0,0 @@ -"""The vocabulary/binding split, from the robot's side. - -The split's dividend is a distinction the robot could not previously draw: -between a name that means nothing here and a name that means something on this -floor which the binding this robot holds carries no geometry for. An operator -does different things about them — fix a typo, or put a coordinate there — -and before the split both arrived as "unknown zone", which sent them looking -for the typo. - -The other half of the dividend is what does *not* travel. Everything published -here is names; everything with a coordinate in it stays on the robot that -measured it. That is checked over the whole document rather than over the keys -someone thought of, because the failure mode is a plausible-looking coordinate -rather than a crash. -""" - -import yaml - -import pytest - -from mote_bringup import bundle -from mote_bringup.spec import zone as zone_spec - -from mote_tasks import zones as mote_zones - - -def a_floor(tmp_path, *, vocabulary, bindings): - (tmp_path / "vocabulary.yaml").write_text( - yaml.safe_dump( - zone_spec.vocabulary( - "acme_hq", - "ground", - [zone_spec.term("test", name, entry) for name, entry in vocabulary], - revision=3, - ), - sort_keys=False, - ) - ) - (tmp_path / "binding.yaml").write_text( - yaml.safe_dump( - zone_spec.binding( - "mote-01", - "acme_hq", - "ground", - bindings, - map_revision="20260727T101500", - ), - sort_keys=False, - ) - ) - return tmp_path - - -def test_a_named_but_unbound_zone_resolves_unbound(tmp_path): - """The distinction the split exists to make representable.""" - floor = a_floor( - tmp_path, - vocabulary=[("kitchen", {}), ("ward_a", {})], - bindings=[zone_spec.bound("kitchen", 2.0, 3.5)], - ) - zones = mote_zones.load_floor(floor) - assert sorted(zones) == ["kitchen", "ward_a"] - assert zones["ward_a"].bound is False and zones["ward_a"].pose is None - - _, reason = mote_zones.resolve_reason(zones, "ward_a") - assert reason == zone_spec.UNBOUND - _, reason = mote_zones.resolve_reason(zones, "nowhere") - assert reason == zone_spec.UNKNOWN_NAME - - # And the refusal says what to do about it, which is not "check the - # spelling" — and is not only "drive there" either, since the dashboard's - # zone editor and a newer revision both put geometry on a floor without a - # robot going anywhere. - with pytest.raises(mote_zones.ZoneUnresolved) as excinfo: - mote_zones.destination(zones, "ward_a") - assert excinfo.value.reason == zone_spec.UNBOUND - message = str(excinfo.value) - for remedy in ("zone editor", "pull the revision", "save-zone"): - assert remedy in message - - -def test_only_bound_zones_are_drivable_and_containable(tmp_path): - floor = a_floor( - tmp_path, - vocabulary=[("kitchen", {}), ("ward_a", {})], - bindings=[ - zone_spec.bound( - "kitchen", 2.0, 3.5, footprint={"type": "circle", "radius": 1.5} - ) - ], - ) - zones = mote_zones.load_floor(floor) - assert sorted(mote_zones.load_zones(floor)) == ["kitchen"] - # An unbound zone is in no place, so it can contain nothing. - assert mote_zones.containing(zones, 2.0, 3.5) == ["kitchen"] - - -def test_a_binding_the_vocabulary_does_not_name_is_a_local_extension(tmp_path): - """This robot holds a binding for a place nobody has named for the site. - - It stays usable here — refusing it would lose a real coordinate over a - naming gap — and it is left out of the vocabulary, because advertising it - would be one robot inventing shared vocabulary for its neighbours. - """ - floor = a_floor( - tmp_path, - vocabulary=[("kitchen", {})], - bindings=[ - zone_spec.bound("kitchen", 2.0, 3.5), - zone_spec.bound("my_bench", 1.0, 1.0), - ], - ) - zones = mote_zones.load_floor(floor) - assert zones["my_bench"].local is True and zones["my_bench"].bound is True - assert mote_zones.destination(zones, "my_bench").name == "my_bench" - - published = bundle.vocabulary(bundle.read_floor(floor), "acme_hq", "ground") - assert [item["name"] for item in published["zones"]] == ["kitchen"] - - -def test_the_shared_document_carries_no_coordinates(tmp_path): - floor = a_floor( - tmp_path, - vocabulary=[("kitchen", {"note": "the good kettle"})], - bindings=[ - zone_spec.bound( - "kitchen", 2.0, 3.5, 1.57, footprint={"type": "circle", "radius": 1.5} - ) - ], - ) - shared = (floor / "vocabulary.yaml").read_text() - assert "the good kettle" in shared - for leak in zone_spec.GEOMETRY_KEYS + ("frame_id", "map_revision", "pose"): - assert f"{leak}:" not in shared, f"{leak} leaked into the vocabulary" - - # ...and the private one carries the three things that say what its numbers - # are only true against. - private = yaml.safe_load((floor / "binding.yaml").read_text()) - assert private["platform_id"] == "mote-01" - assert private["frame_id"] == "map" - assert private["map_revision"] == "20260727T101500" - - -def test_a_legacy_combined_file_still_loads(tmp_path): - """A robot that has been mapping a building for a year is not re-bound. - - Its retired fields load and are dropped: the zone keeps its name, its - coordinate and its footprint, and `galley` no longer reaches anything. - """ - (tmp_path / "zones.yaml").write_text( - "frame_id: map\nzones:\n" - " kitchen: {x: 2.0, y: 3.5, radius: 1.5, kind: room, aliases: [galley]}\n" - ) - zones = mote_zones.load_floor(tmp_path) - assert zones["kitchen"].bound is True - assert zones["kitchen"].footprint is not None - assert mote_zones.resolve(zones, "kitchen").name == "kitchen" - assert mote_zones.resolve(zones, "galley") is None diff --git a/mote_tasks/test/test_zones.py b/mote_tasks/test/test_zones.py index fee9f23..777152e 100644 --- a/mote_tasks/test/test_zones.py +++ b/mote_tasks/test/test_zones.py @@ -29,19 +29,14 @@ def test_yaw_from_quaternion_round_trip(): def test_append_zone_creates_replaces_and_loads(tmp_path): - """Teaching writes the zone/v0 pair, and reading joins it back.""" + """Teaching writes the floor's one document, and reading gives it back.""" assert append_zone(tmp_path, "bin", 1.0, -2.0, 0.5) is False assert append_zone(tmp_path, "sofa", 0.25, 0.0, -1.0) is False assert append_zone(tmp_path, "bin", 1.5, -2.5, 0.5) is True - # The names went one way and the coordinates the other. This is the whole - # split, checked over the text rather than over the keys someone thought - # of: a coordinate in the shared document is the leak it exists to stop. - shared = (tmp_path / "vocabulary.yaml").read_text() - assert "bin" in shared and "sofa" in shared - for leak in ("x:", "y:", "yaw:", "radius:", "polygon:", "frame_id:"): - assert leak not in shared, f"{leak} leaked into the vocabulary" - assert "x: 1.5" in (tmp_path / "binding.yaml").read_text() + written = (tmp_path / "zones.yaml").read_text() + assert "bin" in written and "sofa" in written + assert "x: 1.5" in written zones = load_zones(tmp_path) assert sorted(zones) == ["bin", "sofa"] @@ -178,12 +173,9 @@ def test_append_zone_keeps_a_polygon_but_radius_replaces_it(tmp_path): path.write_text( "zones:\n ward: {x: 1.0, y: 1.0, polygon: [[0, 0], [2, 0], [2, 2], [0, 2]]}\n" ) - # Re-capturing the pose must not silently un-room the zone. It is also the - # first write to a combined file, so it is the migration: nobody runs one, - # and nobody can forget to. + # Re-capturing the pose must not silently un-room the zone. append_zone(path, "ward", 1.5, 0.5, 0.0) - assert (tmp_path / "vocabulary.yaml").exists() - assert not (tmp_path / "zones.yaml").exists() + assert (tmp_path / "zones.yaml").exists() ward = load_zones(tmp_path)["ward"] assert isinstance(ward.footprint, Polygon) assert ward.pose.pose.position.x == pytest.approx(1.5) @@ -204,15 +196,10 @@ def test_save_zone_output_is_readable_by_the_bundle_validator(tmp_path): append_zone(tmp_path, "kitchen", 1.0, 2.0, 0.5, radius=1.5) append_zone(tmp_path, "ward_east", 4.0, 1.0, 0.0) - binding = yaml.safe_load((tmp_path / "binding.yaml").read_text()) - for item in binding["bindings"]: - if item["name"] == "ward_east": - item["footprint"] = { - "type": "polygon", - "vertices": [[3.0, 0.0], [5.0, 0.0], [5.0, 2.0]], - } - (tmp_path / "binding.yaml").write_text( - yaml.safe_dump(binding, sort_keys=False, default_flow_style=None) + document = yaml.safe_load((tmp_path / "zones.yaml").read_text()) + document["zones"]["ward_east"]["polygon"] = [[3.0, 0.0], [5.0, 0.0], [5.0, 2.0]] + (tmp_path / "zones.yaml").write_text( + yaml.safe_dump(document, sort_keys=False, default_flow_style=None) ) append_zone(tmp_path, "ward_east", 4.1, 1.1, 0.0) # recapture: keeps the outline @@ -337,37 +324,49 @@ def test_re_capturing_a_pose_keeps_the_vocabulary(tmp_path): assert zone.pose.pose.position.x == 1.2 -def test_binding_a_zone_bumps_the_vocabulary_revision(tmp_path): - """A binding elsewhere records which vocabulary it was built against, so - the counter has to move whenever a name could have.""" +def test_teaching_a_zone_bumps_the_revision(tmp_path): + """What a reader compares to tell which of two copies of a floor's zones is + the later one, so it has to move whenever anything in them could have.""" def revision(): - return yaml.safe_load((tmp_path / "vocabulary.yaml").read_text())["revision"] + return yaml.safe_load((tmp_path / "zones.yaml").read_text())["revision"] append_zone(tmp_path, "kitchen", 1.0, 2.0, 0.0) first = revision() append_zone(tmp_path, "ward", 3.0, 4.0, 0.0) assert revision() > first - # The binding says which vocabulary it was built against, which is what - # makes a stale one detectable rather than merely wrong. - binding = yaml.safe_load((tmp_path / "binding.yaml").read_text()) - assert binding["vocabulary_revision"] == revision() def test_marking_a_place_a_robot_may_not_go_writes_the_flag(tmp_path): """`--no-navigable` is what `--kind keepout` was: the fact, rather than a taxonomy the fact had to be inferred from.""" append_zone(tmp_path, "sluice", 1.0, 2.0, 0.0, navigable=False) - document = yaml.safe_load((tmp_path / "vocabulary.yaml").read_text()) - assert document["zones"][0]["navigable"] is False + document = yaml.safe_load((tmp_path / "zones.yaml").read_text()) + assert document["zones"]["sluice"]["navigable"] is False assert load_zones(tmp_path)["sluice"].navigable is False -def test_a_note_travels_in_the_vocabulary_and_not_the_binding(tmp_path): - """The note is a fact about the building, so it is shared; the pose is a - coordinate in this robot's frame, so it is not.""" +def test_a_note_is_written_and_read_back(tmp_path): + """The one field a prior cannot supply: what this building's store room is + for, as against what a store room is.""" append_zone(tmp_path, "store room", 1.0, 2.0, 0.0, note="stationery lives here") - vocabulary = yaml.safe_load((tmp_path / "vocabulary.yaml").read_text()) - assert vocabulary["zones"][0]["note"] == "stationery lives here" - binding = yaml.safe_load((tmp_path / "binding.yaml").read_text()) - assert "note" not in binding["bindings"][0] + document = yaml.safe_load((tmp_path / "zones.yaml").read_text()) + assert document["zones"]["store room"]["note"] == "stationery lives here" + assert load_zones(tmp_path)["store room"].note == "stationery lives here" + + +def test_the_names_only_view_carries_no_coordinates(tmp_path): + """``GET /v1/zones`` serves this: what places exist and what they are + called, for a dispatcher with no basemap to draw a coordinate on. Checked + over the whole payload rather than over the keys someone thought of, + because a leak here is a plausible-looking coordinate rather than a crash. + """ + from mote_bringup import bundle + from mote_bringup.spec import zone as zone_spec + + append_zone(tmp_path, "store room", 1.0, 2.0, 0.7, radius=1.5, note="stationery") + view = bundle.vocabulary(bundle.read_floor(tmp_path), "acme_hq", "ground") + text = yaml.safe_dump(view) + assert "stationery" in text + for leak in zone_spec.GEOMETRY_KEYS + ("frame_id", "map_revision", "pose"): + assert f"{leak}:" not in text, f"{leak} leaked into the names-only view"