From b5e8fcb5e0c21e1224667a0e4aca027b30573266 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Fri, 31 Jul 2026 15:28:29 +0100 Subject: [PATCH] Fleet API: serve the zone vocabulary from the site bundle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API served the roster, the basemaps and dispatch, but not the one thing a dispatcher most needs -- what places can I name? Zone names lived only in the site bundle on the robot, so a caller had to configure them out of band or scrape 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 same site bundles the server already reads for maps, shaped as zone/v0. 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 it 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 (name, kind, display_name, aliases, navigable, parent, tags) is published and the binding is not, and 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 a client that already has one, everything under /v1/zones is bound to nothing. zones.json keeps its shape and its coordinates; the dashboard is unaffected. 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. - The vocabulary is 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 -- that is the portability the split buys, and gating it would have quietly given it back. - problems is reported, not enforced. An ambiguous alias or a name a dispatcher cannot type (Café) leaves the map perfectly good, and refusing to serve a floor's basemap over one would be the wrong price. The name is served verbatim rather than slugified, because inventing one is a rename nobody asked for. The robot does refuse an ambiguous vocabulary at load_zones, since it could not honour goto unambiguously. - Contradictions with no reading at all -- an unknown kind, a keepout marked navigable: true -- are refused at the parse by the shared ROS-free bundle.py, so save-map catches them locally and the server on upload. Every vocabulary field is optional, so no zones.yaml needed rewriting: an untouched zone is a navigable area. save-zone --kind teaches the one field it can know, segment-map emits kind: room because what it segments are rooms, and re-teaching a pose carries the vocabulary through and bumps vocabulary_revision -- a better coordinate is not a rename. goto and fetch now resolve aliases and display names, and 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 "keepout". Verified: 304 tests pass (the 6 skips are the pre-existing broker-dependent e2e tests; no MQTT path is touched), pre-commit clean, and the new routes driven over a real socket -- the vocabulary walked for geometry-shaped keys at any depth, the binding asserted to still carry them, and a named-but- unmapped floor serving a vocabulary while its binding 404s. Related: mote #249 (the rest of spec v0), AugereAI #192/#193. --- CLAUDE.md | 47 +++- docs/fleet/README.md | 83 ++++++ docs/fleet/fleet-api.md | 102 +++++++- mote_bringup/mote_bringup/bundle.py | 237 +++++++++++++++++- .../mote_bringup/map_cleanup/rooms_cli.py | 10 +- mote_fleet/server/bundle_store.py | 89 +++++-- mote_fleet/server/fleet_server.py | 40 ++- mote_fleet/server/ui/map.mjs | 5 +- mote_fleet/test/api_harness.py | 8 +- mote_fleet/test/test_map_registry.py | 2 +- mote_fleet/test/test_mapsync.py | 2 +- mote_fleet/test/test_zone_vocabulary.py | 222 ++++++++++++++++ mote_simulation/worlds/gen_hospital.py | 38 ++- .../worlds/hospital_world.zones.yaml | 17 +- mote_simulation/worlds/mote_world.zones.yaml | 10 +- .../worlds/office_world.zones.yaml | 14 +- mote_tasks/config/zones.default.yaml | 10 +- mote_tasks/mote_tasks/save_zone.py | 28 ++- mote_tasks/mote_tasks/trees/fetch.py | 22 +- mote_tasks/mote_tasks/trees/goto.py | 24 +- mote_tasks/mote_tasks/zones.py | 119 ++++++++- mote_tasks/test/test_goto_command.py | 38 ++- mote_tasks/test/test_parse_command.py | 27 +- mote_tasks/test/test_segmented_zones.py | 14 ++ mote_tasks/test/test_zones.py | 114 +++++++++ 25 files changed, 1219 insertions(+), 103 deletions(-) create mode 100644 mote_fleet/test/test_zone_vocabulary.py diff --git a/CLAUDE.md b/CLAUDE.md index fb09e20..e8d4890 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -163,13 +163,54 @@ running. M3's `/v1/maps` routes kept their shape and changed source; the dashboard additionally draws the floor's taught zones (circle, polygon or waypoint cross) from `/v1/maps///zones.json`. +## 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 +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** — +`name`, `kind`, `display_name`, `aliases`, `navigable`, `parent`, `tags` — 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** — an +ambiguous alias or a name a dispatcher cannot type (`Café`) leaves the map +perfectly good, and refusing to serve a floor's basemap over it would be the +wrong price; the name is served verbatim rather than slugified to `cafe`, +because inventing one is a rename nobody asked for. But the *robot* refuses an +ambiguous vocabulary at `load_zones`, since it could not honour `goto` +unambiguously. Contradictions with no reading at all — an unknown `kind`, a +`keepout` marked `navigable: true`, `aliases` that are not a list — 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 --kind` teaches the +one field it can know; `segment-map` emits `kind: room` because what it segments +*are* rooms. Related: mote #249 (the rest of spec v0 — capability set and typed +failures). + ## Fleet: the server pipelines (Ms) Milestone Ms of `docs/design/fleet.md`: how the two **non-robot** machines are built and updated, runbook in `docs/fleet/server-pipelines.md`, measurements in `docs/fleet/ms-verification.md`. Both are container deploys **driven by their operator, not by the fleet server** — a robot is fleet-managed, a server is infrastructure — and the pipelines differ in exactly one thing, state. **The fleet server** (`mote_fleet/deploy/`: `Dockerfile` + `docker-compose.yml` + `fleet-deploy.sh`, image `ghcr.io/clachdev/mote-fleet` built by `.github/workflows/fleet-image.yml`) is two containers — `eclipse-mosquitto:2` mounting **`server/mosquitto.conf`, the same file `fleet-broker` uses**, so deployed and workstation brokers cannot drift, and a python image carrying the API, the registry and the M3 dashboard — plus two named volumes holding the only state that matters: `/var/lib/mote-fleet` (registry.db + the `sites/` bundles the dashboard's basemaps come from) and the broker's retained messages. `.env` is the declared state; `BROKER_HOST` is the one value that must be right (handed to robots verbatim at enrollment, so the compose file refuses to start without it) and `BROKER_WS_PORT` is published *and* passed as `--broker-ws-port`, because it is the port the browser is told to reach the broker on. It holds state, so its update is a **gated recreate**, not blue/green: `fleet-deploy.sh update` tags the running image `:previous` before pulling, recreates, health-gates on `/healthz` over the *published* port, and puts the old image back automatically if that fails; `backup`/`restore` snapshot both volumes (registry via sqlite3's online backup API, not `cp`). **The inference server** (`mote_perception/deploy/inference-deploy.sh`, one file curled onto the host) is stateless, so it *is* blue/green: the candidate runs on shadow ports 5611/5612 while the current one keeps serving, and must pass `mote_perception/tools/probe.py` — health **and a real synthetic frame**, because a health sentinel is answered before the model has ever loaded and cannot see a broken weight download or a CUDA mismatch. The **flip is a stop-then-start**, deliberately: pushing a new port out to robots (as the design sketch had it) means editing `perception.yaml` on every robot, a worse outage than the seconds this costs, which the robot's warn-and-skip fallback makes a non-event. Every check runs *inside the image being deployed*, so the GPU box still installs nothing. `mote_perception/deploy/test/drill.sh` (`pixi run deploy-test`) exercises that whole pipeline with stub images on any machine with docker — no GPU — and `mote_fleet/test/test_fleet_outage.py` is the other half of the milestone's acceptance: kill the broker under a live agent and the robot still finishes its task, then the agent reconnects by itself. ## Sites (maps & zones) -Everything that is only meaningful relative to one mapped place — the Nav2 map pair, the slam_toolbox posegraph, and named zones — lives together as a **site bundle** under `~/.mote/sites//floors//`, managed by `mote_bringup/sites.py` (CLI: `pixi run site`, docs in the module docstring). A floor is one SLAM session (one map frame); a site groups floors sharing a location. `~/.mote/active.yaml` selects the active site/floor per robot; launch files resolve the map (`nav2_launch.py`, `robot_launch.py`) and zones (`tasks_launch.py`) from it at launch time (zones fall back to the committed default). `MOTE_HOME` overrides `~/.mote` for tests/experiments. What a revision must *contain* — and how it validates, packs and travels — is `mote_bringup/bundle.py` (ROS-free, shared with the fleet server; see the map registry section above). Map artifacts are immutable **revisions** under `floors//maps//`, published by atomically flipping the `floors//map` symlink once the revision is complete — a half-written save or interrupted transfer is never visible, and `site use-map ` rolls back. `save-map` stores the posegraph alongside the map so mapping can be *continued* in the same frame later (extend, don't remap — remapping breaks zone coordinates). Mapping runs also record the `mapping` rosbag stream by default (`mapping_launch.py record:=true`; the sim passes false), and `save-map` stamps the session's bag into the revision's `meta.yaml` for provenance (`site info` shows it). Zones are taught by driving there and running `pixi run save-zone `, not by editing YAML; a zone is a named pose (a fetch waypoint or a `goto ` target) that may optionally carry an area **footprint** — a taught `--radius` circle, or a `polygon` outline that follows the actual room walls — so it reads as a room and answers "am I in it"; one concept, one `zones.yaml`, `site info` shows the zone/footprint counts. 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.yaml` for the operator to rename — additive over hand-taught zones (a candidate covering an already-footprinted zone is dropped as named, so re-running is a no-op) and written beside `zones.yaml`, never into the immutable map revision. The method is one physical assumption — a doorway is narrow — applied to a grid the wall lines cut into faces: faces merge wherever their shared boundary has a clear span wider than a door, so it is indifferent to room size where a distance-transform threshold is not. Two consequences: a **corridor network is not proposed at all** (a footprint is a single outline, so a region encircling a block of rooms would claim them; those are dropped, taking with them any room wrongly absorbed into the corridor), and the geometry is **Manhattan after rotation** — an arbitrarily rotated map frame is fine, a building with wings at 30° to each other is not. Scored against ground-truth room rectangles on the sim ladder by `pixi run segment-eval` (30/33 mapped hospital rooms, 10/10 office, 1/1 mote, **zero merges**, unchanged with the map turned 17° or -31°); results in `docs/tuning/2026-07-27-room-segmentation.md`. +Everything that is only meaningful relative to one mapped place — the Nav2 map pair, the slam_toolbox posegraph, and named zones — lives together as a **site bundle** under `~/.mote/sites//floors//`, managed by `mote_bringup/sites.py` (CLI: `pixi run site`, docs in the module docstring). A floor is one SLAM session (one map frame); a site groups floors sharing a location. `~/.mote/active.yaml` selects the active site/floor per robot; launch files resolve the map (`nav2_launch.py`, `robot_launch.py`) and zones (`tasks_launch.py`) from it at launch time (zones fall back to the committed default). `MOTE_HOME` overrides `~/.mote` for tests/experiments. What a revision must *contain* — and how it validates, packs and travels — is `mote_bringup/bundle.py` (ROS-free, shared with the fleet server; see the map registry section above). Map artifacts are immutable **revisions** under `floors//maps//`, published by atomically flipping the `floors//map` symlink once the revision is complete — a half-written save or interrupted transfer is never visible, and `site use-map ` rolls back. `save-map` stores the posegraph alongside the map so mapping can be *continued* in the same frame later (extend, don't remap — remapping breaks zone coordinates). Mapping runs also record the `mapping` rosbag stream by default (`mapping_launch.py record:=true`; the sim passes false), and `save-map` stamps the session's bag into the revision's `meta.yaml` for provenance (`site info` shows it). Zones are taught by driving there and running `pixi run save-zone `, not by editing YAML; a zone is a named pose (a fetch waypoint or a `goto ` target) that may optionally carry an area **footprint** — a taught `--radius` circle, or a `polygon` outline that follows the actual room walls — so it reads as a room and answers "am I in it"; one concept, one `zones.yaml`, `site info` shows the zone/footprint counts. That same file holds each zone's **vocabulary** beside its coordinates (`kind` via `save-zone --kind`, plus hand-edited `display_name`/`aliases`) — taught together, but only the names are portable off this robot, which is what the fleet serves and the map registry does not (see Fleet: the zone vocabulary). 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.yaml` for the operator to rename — additive over hand-taught zones (a candidate covering an already-footprinted zone is dropped as named, so re-running is a no-op) and written beside `zones.yaml`, never into the immutable map revision. 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) @@ -245,7 +286,7 @@ Home for camera-derived perception. Runs on the robot (feeds Nav2), so unlike `m ### `mote_tasks` (Python/ament) 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: - `task_server.py` — node hosting the mission trees: subscribes `task/command` (String), publishes `task/status`, ticks the active tree on a timer, and dispatches on the command's first word (`fetch`/`goto`; unknown words rejected). Commands: `fetch ` — a target matching a zone name drives there, anything else is an open-vocabulary label for the L2 detector (underscores→spaces); `goto ` — drive to any named zone. 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. -- `zones.py` — the one named-place concept: `load_zones` returns `{name: Zone(name, pose, footprint)}` from the `zones:` section. A **zone** is a pose the robot navigates to (both fetch waypoints *and* goto targets resolve against this single table); it *optionally* carries an area **footprint** — a `radius` circle or a `polygon` of explicit vertices — that's just optional metadata, not a second type. `zones.containing(zones, x, y)` is the "which zone am I in" membership query (nearest-pose first) over zones that have a footprint; `goto` itself only needs the pose. A polygon may be concave (ray-cast membership, so an L-shaped ward or a corridor stretch works where a circle can only under- or over-cover: the hospital wards are 4.7x5.6 m, of which a `radius: 1.5` circle claimed 7.1 m² of 26.5 m²), wins over a `radius` if a zone carries both, and — since polygons come from post-processing a map rather than from driving — may omit `x`/`y`, in which case the loader derives a pose guaranteed to lie inside the outline. `save-zone` therefore preserves an existing footprint when it re-teaches a pose; `--radius` is the explicit way to replace one. Polygon zones no longer have to be hand-written either: `pixi run segment-map` proposes one per room of a saved map (see Sites). +- `zones.py` — the one named-place concept: `load_zones` returns `{name: Zone(name, pose, footprint)}` from the `zones:` section. A **zone** is a pose the robot navigates to (both fetch waypoints *and* goto targets resolve against this single table); it *optionally* carries an area **footprint** — a `radius` circle or a `polygon` of explicit vertices — that's just optional metadata, not a second type. `zones.containing(zones, x, y)` is the "which zone am I in" membership query (nearest-pose first) over zones that have a footprint; `goto` itself only needs the pose. A polygon may be concave (ray-cast membership, so an L-shaped ward or a corridor stretch works where a circle can only under- or over-cover: the hospital wards are 4.7x5.6 m, of which a `radius: 1.5` circle claimed 7.1 m² of 26.5 m²), wins over a `radius` if a zone carries both, and — since polygons come from post-processing a map rather than from driving — may omit `x`/`y`, in which case the loader derives a pose guaranteed to lie inside the outline. `save-zone` therefore preserves an existing footprint when it re-teaches a pose; `--radius` is the explicit way to replace one. Polygon zones no longer have to be hand-written either: `pixi run segment-map` proposes one per room of a saved map (see Sites). A `Zone` also carries the **vocabulary** half — `kind`, `display_name`, `aliases`, `navigable`, `parent`, `tags` (zone/v0; see Fleet: the zone vocabulary) — every field optional, so no existing `zones.yaml` needed rewriting and an untouched zone is a navigable `area`. Three consequences here. `resolve(zones, query)` is what `goto`/`fetch` match on, so an alias or display name reaches the zone (case-insensitive, whitespace-normalised); 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 "keepout". `load_zones` **refuses a vocabulary with a collision** (two zones answering one query), because loading it would resolve `goto` by dict order — silently, once per boot, differently after an edit; the rules live once in `mote_bringup.bundle` (`zone_term`, `ambiguities`, `check_vocabulary`) so the robot, `save-map` and the fleet server cannot disagree about what a vocabulary means. And `append_zone` carries the vocabulary through a re-teach and bumps `vocabulary_revision`: a better coordinate is not a rename. - `behaviours/` — `DriveTo` (Nav2 NavigateToPose action client as a behaviour; cancels in-flight goals on preemption), `AcquireObject` (label missions: publishes the label to `detect/labels`, waits for a matching `detected_objects` detection, writes a standoff goal — 0.4 m short of the object, facing it — to `object_pose`; zone missions pass through), and `TimedStub` (placeholder pick/place until the SO-101 arm is actuated). - `trees/` — `common.py` (shared `WaitForTask` + the `task` blackboard key), `fetch.py` (wait → acquire object → drive to object → pick stub → drive to drop → place stub; blackboard keys `task`/`object_pose`/`object_label`/`drop_pose` are the seam between the command grammar and perception), and `goto.py` (wait → drive to the zone's pose; success == Nav2 success). - `test/` — mock-`navigate_to_pose` tree ticks (`test_fetch_tree.py`, `test_fetch_object.py` against a mock detector, `test_goto_tree.py`) plus pure parser/loader tests (`test_parse_command.py`, `test_goto_command.py`, `test_zones.py` — which covers zone footprints and `containing`), no Gazebo/Nav2 needed, run by `pixi run test`. @@ -258,7 +299,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 `task/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. +- `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`. - `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/fleet/README.md b/docs/fleet/README.md index 45c4662..dfb017a 100644 --- a/docs/fleet/README.md +++ b/docs/fleet/README.md @@ -859,3 +859,86 @@ rsync -aL --delete michael@mote-01:~/.mote/sites/home ~/.mote-fleet/sites/ seeded this way serves basemaps normally, but cannot be promoted onto until its `map/` is a symlink into `maps//` — the API answers `409` rather than overwriting a directory it did not create. + +--- + +## 12. Zone names: what a dispatcher may say + +Everything in §11 is about the *map*. This is about the **names**, which are a +different kind of fact and travel differently. + +A zone in `zones.yaml` holds two things at once. Its pose is a coordinate in +one robot's map frame, and that frame's origin is wherever that robot's SLAM +session happened to start — so `(2.0, 3.5)` on `mote-01` is a different +physical point from `(2.0, 3.5)` on `mote-02`, and no fleet-level transform +fixes it. Its *name*, though, is true for every robot at the site. So the fleet +serves the names and never the poses: + +```bash +curl -s http://fleet-box:8080/v1/zones | python -m json.tool # every floor +curl -s http://fleet-box:8080/v1/zones/home/ground | python -m json.tool +``` + +```json +{"schema":1,"site":"home","floor":"ground","revision":4,"zones":[ + {"name":"kitchen","display_name":"The Kitchen","aliases":["galley"], + "kind":"room","navigable":true,"parent":null,"tags":[],"description":""}], + "problems":[]} +``` + +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. + +### Teaching the vocabulary + +`kind` is the one field worth setting as you teach, and `save-zone` takes it: + +```bash +pixi run save-zone kitchen --radius 1.5 --kind room +pixi run save-zone bay_3 --kind dock +pixi run save-zone sluice --radius 0.8 --kind keepout +``` + +The kinds are `area room corridor doorway threshold elevator stair dock charger +pickup dropoff staging home keepout slow`; `area` is the default and claims +nothing, so a zone taught without `--kind` is still perfectly valid. `keepout` +and `slow` are **constraints, not destinations** — they come out `navigable: +false`, and `goto sluice` is refused by the robot rather than driven to. + +`display_name` and `aliases` are edited into `zones.yaml` by hand, since only +you know what people call the place: + +```yaml +zones: + kitchen: {x: 2.0, y: 3.5, yaw: 1.57, radius: 1.5, kind: room, + display_name: The Kitchen, aliases: [galley, the kitchen]} +``` + +Aliases are matched case-insensitively and whitespace-normalised, so `goto "the +Kitchen"` reaches `kitchen`. Re-teaching a pose (`save-zone kitchen` again) +keeps the kind and the aliases — a better coordinate is not a rename. + +`pixi run segment-map` fills in `kind: room` on every candidate it proposes, +because what it segments *are* rooms; the names it invents (`room_01`…) are +placeholders for you to replace. + +### When `problems` is not empty + +The server reports a broken vocabulary rather than refusing to serve it — the +map is unaffected, and a floor's basemap must not stop being served over a +duplicated alias. Two things show up there: + +- **two zones answering to one query.** Nothing may pick between them, so the + name is unusable until you fix it. The robot's own loader *refuses* such a + file outright, so this one will also stop `task_server` starting: fix it + before it reaches a robot. +- **a name a dispatcher cannot type**, e.g. a zone taught as `Café`. It is + served verbatim rather than silently renamed to `cafe`. The fix is to rename + the zone and put the label in `display_name`. + +A file with no coherent reading at all — an unknown `kind`, a `keepout` marked +`navigable: true` — is refused at the parse, by `save-map` locally and by the +server on upload, so it never becomes a candidate. diff --git a/docs/fleet/fleet-api.md b/docs/fleet/fleet-api.md index 978a380..4a24fef 100644 --- a/docs/fleet/fleet-api.md +++ b/docs/fleet/fleet-api.md @@ -90,7 +90,9 @@ GET /v1/audit[?limit=&robot_id=] what was dispatched, by whom GET /v1/maps basemaps this server can serve GET /v1/maps///map.json resolution + origin + size GET /v1/maps///map.png the basemap image -GET /v1/maps///zones.json the floor's taught zones +GET /v1/maps///zones.json the floor's zone *binding* (has coordinates) +GET /v1/zones every floor's zone *vocabulary* (no coordinates) +GET /v1/zones// one floor's, as a zone/v0 document GET /v1/sites the registry: every floor + its canonical revision GET /v1/sites//floors/ every revision, validated, with provenance POST …/revisions/ upload a candidate revision (a robot) @@ -224,20 +226,108 @@ overwriting the directory. ### `GET /v1/maps///zones.json` -The floor's taught places, in the same map frame as the basemap, so the -dashboard can draw them and an operator can see the `goto` targets they are -about to type. +The floor's taught places **with their coordinates**, in the same map frame as +the basemap, so the dashboard can draw them and an operator can see the `goto` +targets they are about to type. This is the zone **binding**: it is served +beside the basemap, to a client that already has the basemap, and it is not +what a dispatcher should be given — see [the vocabulary](#the-zone-vocabulary) +below. ```json {"schema":1,"site":"home","floor":"ground","frame_id":"map","zones":[ - {"name":"kitchen","x":1.0,"y":2.0,"yaw":0.0,"radius":1.5}, + {"name":"kitchen","x":1.0,"y":2.0,"yaw":0.0,"radius":1.5,"kind":"room", + "display_name":"The Kitchen","aliases":["galley"],"navigable":true}, {"name":"ward","x":4.0,"y":1.0,"polygon":[[3,0],[5,0],[5,2],[3,2]]}]} ``` Read from the **canonical revision's** `zones.yaml`, falling back to the floor-level file for a bundle seeded by rsync. `404` for a floor with no taught zones — an empty list would claim the floor has none, which is a different -statement. +statement — and `404` for a floor with no published map, because a coordinate +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. + +A caller that must never be handed a map can be given `/v1/zones` and only +`/v1/zones`. + +### `GET /v1/zones//` + +```json +{"schema":1,"site":"home","floor":"ground","revision":4,"zones":[ + {"name":"kitchen","display_name":"The Kitchen","aliases":["galley"], + "kind":"room","navigable":true,"parent":null,"tags":[],"description":""}, + {"name":"sluice","display_name":"","aliases":[], + "kind":"keepout","navigable":false,"parent":null,"tags":[],"description":""}], + "problems":[]} +``` + +| field | | +|---|---| +| `name` | The shared token. `^[a-z][a-z0-9_]*$`, unique within a **floor**, not within a site — two floors may each have a `reception`. | +| `display_name` | What an operator sees. Free text. Empty means "use the name". | +| `aliases` | The other things people call it, for natural-language dispatch. Matched case-insensitively and whitespace-normalised. | +| `kind` | One of `area room corridor doorway threshold elevator stair dock charger pickup dropoff staging home keepout slow`. `area` is the default and claims nothing. | +| `navigable` | Whether it is a legal destination. Always `false` for `keepout` and `slow`. | +| `parent` | An enclosing zone on the same floor, or `null`. | +| `revision` | Bumped every time a zone is taught, so a binding can record which vocabulary it was built against. | +| `problems` | Empty when the vocabulary is well-formed; see below. | + +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. + +Unlike the binding, 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. + +### `GET /v1/zones` + +Every floor's vocabulary in one call, for a dispatcher bootstrapping a fleet: +`{"schema":1,"vocabularies":[…]}`, each element the document above. Floors with +no zones yet are omitted rather than listed empty. + +### `problems` + +Reported, not enforced. Two things can be wrong with a vocabulary while the map +around it is perfectly good, so the server says so and still serves it — a +floor's basemap must not stop being served over a duplicated alias: + +- **an ambiguous query** — two zones answering to one name or alias. A resolver + must not pick between them, so the name is simply unusable until an operator + fixes it. The robot's own loader *does* refuse such a file, because it would + otherwise resolve `goto` by dictionary order. +- **a name a dispatcher cannot type** — e.g. a zone taught as `Café`. It is + served verbatim rather than silently slugified to `cafe`: inventing a name is + a rename nobody asked for. The fix is an operator's, and is to move the label + into `display_name`. + +A file that has no coherent reading at all — an unknown `kind`, a `keepout` +marked `navigable: true`, `aliases` that are not a list — is refused at the +parse instead, by the same shared validator (`mote_bringup/bundle.py`) that +`save-map` runs locally. Those are not ambiguities to report; they are +contradictions, and honouring the last one written would make the flag mean +whatever was typed most recently. --- diff --git a/mote_bringup/mote_bringup/bundle.py b/mote_bringup/mote_bringup/bundle.py index 81e2b48..3fc7d7f 100644 --- a/mote_bringup/mote_bringup/bundle.py +++ b/mote_bringup/mote_bringup/bundle.py @@ -52,6 +52,7 @@ import gzip import hashlib import io +import re import tarfile from dataclasses import dataclass, field from pathlib import Path @@ -72,6 +73,53 @@ POSEGRAPH = "map.posegraph" POSEGRAPH_DATA = "map.data" +#: The semantic role of a place, so a planner can reason about one it has never +#: seen (zone/v0 "Kinds"). ``area`` is the unopinionated default: a named place +#: with no further claim. The order is the spec's — structure, then level +#: transitions, then where a platform services itself, then where work happens, +#: then constraints. +ZONE_KINDS = ( + "area", + "room", + "corridor", + "doorway", + "threshold", + "elevator", + "stair", + "dock", + "charger", + "pickup", + "dropoff", + "staging", + "home", + "keepout", + "slow", +) + +#: Kinds that say where a robot may *not* or *should not* go. They are in the +#: same vocabulary as destinations because they are the same thing to an +#: operator drawing on a floor plan; the distinction is ``navigable``, and it is +#: machine-checkable rather than a convention. Dispatching to one is bad input, +#: not a route, so ``navigable: true`` on one of these is refused rather than +#: honoured — otherwise the flag would mean whatever the file last said. +CONSTRAINT_KINDS = frozenset(("keepout", "slow")) + +#: A dispatchable zone name. The shared token a dispatcher types, so it is a +#: machine name rather than a label: lowercase, no spaces, no punctuation to +#: guess at. Anything an operator wants to *see* belongs in ``display_name``. +ZONE_NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") + +#: Vocabulary keys a zone entry may carry, beside the geometry that binds it. +VOCABULARY_KEYS = ( + "display_name", + "aliases", + "kind", + "navigable", + "parent", + "tags", + "description", +) + #: Present and non-empty in every revision, whoever wrote it. REQUIRED = (MAP_YAML, META_YAML) @@ -227,12 +275,17 @@ def read_map(path) -> dict: def read_zones(path) -> dict: - """A floor's ``zones.yaml`` — ``{frame_id, zones: {name: {...}}}``. + """A floor's ``zones.yaml`` — ``{frame_id, revision, zones: {name: {...}}}``. The authority on what a zone *means* is ``mote_tasks.zones``; this is the off-robot reader for the same file, so the fleet can draw taught places and the operator can see the ``goto`` targets they are about to type. It keeps the shape and checks the numbers rather than reimplementing membership. + + Each parsed zone carries **both halves** of zone/v0: the geometry that binds + it to this floor's map frame, and the vocabulary that names it. The file + holds them together because they are taught together; :func:`vocabulary` + is what separates them for anything off-robot. """ raw = load_yaml_file(path) zones = raw.get("zones") or {} @@ -242,7 +295,7 @@ def read_zones(path) -> dict: for name, entry in zones.items(): if not isinstance(entry, dict): raise BundleError(f"{Path(path).name}: zone {name!r} is not a mapping") - zone = {"name": str(name)} + zone = {"name": str(name), **zone_term(Path(path).name, name, entry)} for key in ("x", "y", "yaw", "radius"): if entry.get(key) is not None: try: @@ -261,7 +314,23 @@ def read_zones(path) -> dict: if "polygon" not in zone: raise BundleError(f"{Path(path).name}: zone {name!r} has no position") parsed[str(name)] = zone - return {"frame_id": raw.get("frame_id") or "map", "zones": parsed} + return { + "frame_id": raw.get("frame_id") or "map", + "revision": _revision(Path(path).name, raw.get("vocabulary_revision")), + "zones": parsed, + } + + +def _revision(where: str, raw) -> int: + if raw is None: + return 0 + try: + revision = int(raw) + except (TypeError, ValueError) as exc: + raise BundleError(f"{where}: vocabulary_revision must be an integer") from exc + if revision < 0: + raise BundleError(f"{where}: vocabulary_revision must not be negative") + return revision def _polygon(where: str, name, polygon) -> list: @@ -280,6 +349,159 @@ def _polygon(where: str, name, polygon) -> list: return vertices +# -- the vocabulary half (zone/v0) ---------------------------------------- + + +def zone_term(where: str, name, entry: dict) -> dict: + """The naming half of one zone entry, with zone/v0's defaults filled in. + + Every field is optional in the file: a zone taught by ``save-zone`` before + any of this existed is a perfectly good ``area``, and the defaults here are + what make that true without rewriting a single ``zones.yaml``. + """ + kind = entry.get("kind") or "area" + if kind not in ZONE_KINDS: + raise BundleError( + f"{where}: zone {name!r} has unknown kind {kind!r} " + f"(one of {', '.join(ZONE_KINDS)})" + ) + constraint = kind in CONSTRAINT_KINDS + navigable = entry.get("navigable") + if navigable is None: + navigable = not constraint + elif not isinstance(navigable, bool): + raise BundleError(f"{where}: zone {name!r} navigable must be true or false") + elif navigable and constraint: + raise BundleError( + f"{where}: zone {name!r} is a {kind} zone, which is not a destination; " + "navigable cannot be true" + ) + parent = entry.get("parent") + if parent is not None and not isinstance(parent, str): + raise BundleError(f"{where}: zone {name!r} parent must be a zone name") + return { + "display_name": str(entry.get("display_name") or ""), + "aliases": _strings(where, name, "aliases", entry.get("aliases")), + "kind": kind, + "navigable": navigable, + "parent": parent or None, + "tags": _strings(where, name, "tags", entry.get("tags")), + "description": str(entry.get("description") or ""), + } + + +def _strings(where: str, name, key: str, raw) -> list: + if raw is None: + return [] + if isinstance(raw, str) or not isinstance(raw, (list, tuple)): + raise BundleError(f"{where}: zone {name!r} {key} must be a list of strings") + return [str(item) for item in raw] + + +def normalise_alias(text: str) -> str: + """The form two spellings of one place have to share to count as a clash. + + zone/v0 matches aliases case-insensitively and whitespace-normalised, so + "The Kitchen" and "the kitchen" are the same query. Collision detection has + to use the *same* comparison the resolver will, or a vocabulary passes + authoring and is ambiguous in the field. + """ + return " ".join(str(text).split()).casefold() + + +def check_vocabulary(terms) -> list: + """Everything wrong with a floor's vocabulary, as operator-readable lines. + + Returns problems rather than raising because the two callers want opposite + things from the same rules: the robot refuses to load an ambiguous + vocabulary (it could not honour ``goto`` unambiguously), while the fleet + server reports one and still serves the map, which is unaffected. + """ + terms = list(terms) + problems = [ + f"zone {term['name']!r} is not a dispatchable name (want lowercase " + "a-z0-9_ starting with a letter); put the label in display_name" + for term in terms + if not ZONE_NAME_RE.match(term["name"]) + ] + return problems + ambiguities(terms) + _check_parents(terms) + + +def ambiguities(terms) -> list: + """The subset of :func:`check_vocabulary` that makes a name unanswerable. + + Split out because it is the only half a robot must *refuse*: a zone it + cannot spell is still a zone it can be told to go to by its exact key, 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. + """ + problems = [] + claimed = {} + for term in terms: + name = term["name"] + for spelling in [name, *term["aliases"]]: + key = normalise_alias(spelling) + owner = claimed.get(key) + if owner is not None and owner != name: + problems.append( + f"zones {owner!r} and {name!r} both answer to {key!r}; " + "a query matching both can only be ambiguous" + ) + else: + claimed[key] = name + return problems + + +def _check_parents(terms) -> list: + """``parent`` must name a zone on this floor and must not form a cycle — + a cycle is not merely wrong, it is a containment walk that never ends.""" + parents = {term["name"]: term["parent"] for term in terms} + problems = [] + for name, parent in parents.items(): + if parent is None: + continue + if parent not in parents: + problems.append(f"zone {name!r} names parent {parent!r}, which is not here") + continue + seen, walk = {name}, parent + while walk is not None: + if walk in seen: + problems.append(f"zone {name!r} is inside itself via {parent!r}") + break + seen.add(walk) + walk = parents.get(walk) + return problems + + +def vocabulary(zones: dict, site: str, floor: str) -> dict: + """The shared half of :func:`read_zones`' 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: ``(2.0, 3.5)`` in one robot's map frame is a different + physical point in another's, and no fleet-level transform fixes that. + + So the document is **built** from the fields a vocabulary may carry, never + *stripped* of the ones it may not. Stripping is the version of this that + leaks: it holds only until someone adds a geometry key to ``zones.yaml`` + and forgets this function exists, and the leak is a plausible-looking + coordinate rather than a crash. + """ + terms = [ + {key: zone[key] for key in ("name",) + VOCABULARY_KEYS} + for zone in zones["zones"].values() + ] + return { + "schema": SCHEMA, + "site": site, + "floor": floor, + "revision": zones.get("revision", 0), + "zones": terms, + "problems": check_vocabulary(terms), + } + + def png_size(path) -> tuple[int, int] | None: """Pixel dimensions of a map image, or None if it cannot be read. @@ -438,6 +660,15 @@ def validate(revision_dir, *, require_posegraph: bool = True) -> Report: report.zones = read_zones(revision_dir / ZONES_YAML) except BundleError as exc: report.errors.append(str(exc)) + else: + # 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. + report.warnings.extend( + f"vocabulary: {problem}" + for problem in check_vocabulary(report.zones["zones"].values()) + ) else: report.warnings.append("no zones.yaml — this floor has no taught places") diff --git a/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py b/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py index ca413d7..d3c8ad3 100644 --- a/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py +++ b/mote_bringup/mote_bringup/map_cleanup/rooms_cli.py @@ -63,11 +63,19 @@ def load_map(map_yaml: Path) -> tuple[np.ndarray, MapGeometry]: def zone_entry(room: Room) -> dict: - """A room as a ``mote_tasks.zones`` entry: a pose plus a polygon footprint.""" + """A room as a ``mote_tasks.zones`` entry: a pose plus a polygon footprint. + + ``kind: room`` is stated rather than left to default to ``area`` because + this tool knows it: what it segments *are* rooms, carved out of free space + by where the doorways are. That is the one piece of a candidate's + vocabulary it can honestly fill in — the name is a placeholder for the + operator to replace, and only they know the aliases. + """ return { "x": round(room.pose[0], 3), "y": round(room.pose[1], 3), "yaw": 0.0, + "kind": "room", "polygon": [[round(x, 3), round(y, 3)] for x, y in room.polygon], } diff --git a/mote_fleet/server/bundle_store.py b/mote_fleet/server/bundle_store.py index 1294206..9c0ff4b 100644 --- a/mote_fleet/server/bundle_store.py +++ b/mote_fleet/server/bundle_store.py @@ -265,25 +265,86 @@ def read_map(self, site: str, floor: str, revision: str | None = None) -> dict: def read_zones(self, site: str, floor: str) -> dict: """The floor's taught zones, in the map frame the basemap is drawn in. + This is the **binding**: coordinates, and therefore meaningful only + against the map they were taught on. It is served beside the basemap, + to the one client that also has the basemap, and it is not what a + dispatcher gets — 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. """ - directory = self.revision_dir(site, floor, self._live(site, floor)) - for candidate in ( - directory / bundle.ZONES_YAML, - self.floor_dir(site, floor) / bundle.ZONES_YAML, - ): - if candidate.is_file(): - zones = bundle.read_zones(candidate) - return { - "site": site, - "floor": floor, - "frame_id": zones["frame_id"], - "zones": list(zones["zones"].values()), - } - raise StoreError(f"no zones for {site}/{floor}", 404) + # 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) + zones = bundle.read_zones(path) + return { + "site": site, + "floor": 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** — names, kinds and aliases, 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. + """ + path = self._zones_file(site, floor, revision=self.canonical(site, floor)) + if path is None: + raise StoreError(f"no zones for {site}/{floor}", 404) + return bundle.vocabulary(bundle.read_zones(path), site, floor) + + 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. + + 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 + difference is the interesting case: a floor someone has named but not + yet mapped belongs here. + """ + found = [] + for site, floor in self._floors(): + try: + found.append(self.read_vocabulary(site, floor)) + except (StoreError, bundle.BundleError): + continue + return found + + def _floors(self): + """Every ``(site, floor)`` in the layout, which is the record.""" + if not self.root or not self.root.is_dir(): + return + for site_dir in sorted(self.root.iterdir()): + floors_dir = site_dir / "floors" + if not floors_dir.is_dir(): + continue + for floor_dir in sorted(floors_dir.iterdir()): + if floor_dir.is_dir(): + yield site_dir.name, floor_dir.name + + def _zones_file(self, site: str, floor: str, revision: str = ""): + """``zones.yaml`` from the given revision, else the floor-level one.""" + candidates = [] + if revision: + candidates.append(self.revision_dir(site, floor, revision)) + candidates.append(self.floor_dir(site, floor)) + for directory in candidates: + path = directory / bundle.ZONES_YAML + if path.is_file(): + return path + return None def _live(self, site: str, floor: str) -> str: canonical = self.canonical(site, floor) diff --git a/mote_fleet/server/fleet_server.py b/mote_fleet/server/fleet_server.py index d657888..70147c7 100644 --- a/mote_fleet/server/fleet_server.py +++ b/mote_fleet/server/fleet_server.py @@ -26,7 +26,9 @@ 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 taught zones + GET /v1/maps///zones.json the floor's zone *binding* + GET /v1/zones every floor's zone *vocabulary* + 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 POST .../revisions/ upload a candidate revision (robot) @@ -54,6 +56,17 @@ 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**: names, kinds and +aliases, 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. + **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 on the *write* path and a record of who used it. That stays proportionate only @@ -287,6 +300,10 @@ def do_GET(self): ) elif path.startswith("/v1/maps/"): self._map(path[len("/v1/maps/") :]) + elif path == "/v1/zones": + self._store(lambda store: {"vocabularies": store.vocabularies()}) + elif path.startswith("/v1/zones/"): + self._vocabulary(path[len("/v1/zones/") :]) elif path == "/v1/sites": self._store(lambda store: {"sites": store.sites()}) elif path.startswith("/v1/sites/"): @@ -527,6 +544,27 @@ def _map(self, rest: str): Cache_Control="no-cache", ) + # -- the zone vocabulary ---------------------------------------------- + + 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 for the robot that taught it; 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. + """ + parts = rest.split("/") + if len(parts) != 2: + self._error(404, "expected /v1/zones//") + return + site, floor = parts + if not self._names(site, floor): + return + self._store(lambda store: store.read_vocabulary(site, floor)) + # -- the map registry ------------------------------------------------- def _names(self, *names) -> bool: diff --git a/mote_fleet/server/ui/map.mjs b/mote_fleet/server/ui/map.mjs index 9093932..492fe71 100644 --- a/mote_fleet/server/ui/map.mjs +++ b/mote_fleet/server/ui/map.mjs @@ -304,7 +304,10 @@ export class MapView { ctx.font = '11px ui-monospace, monospace'; ctx.textAlign = 'center'; ctx.fillStyle = ZONE_STROKE; - ctx.fillText(zone.name, point.x, point.y + 14); + // display_name is what an operator calls the place; the machine name is + // what they would type. Prefer the former on the map, where this is a + // label rather than a thing to copy. + ctx.fillText(zone.display_name || zone.name, point.x, point.y + 14); } _screenOf(pixel) { diff --git a/mote_fleet/test/api_harness.py b/mote_fleet/test/api_harness.py index 2f7469f..a362fea 100644 --- a/mote_fleet/test/api_harness.py +++ b/mote_fleet/test/api_harness.py @@ -111,10 +111,12 @@ def chunk(kind, data): ) ZONES_YAML = ( - "frame_id: map\nzones:\n" - " kitchen: {x: 1.0, y: 2.0, yaw: 0.0, radius: 1.5}\n" - " ward: {x: 4.0, y: 1.0, yaw: 1.57,\n" + "frame_id: map\nvocabulary_revision: 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" " polygon: [[3.0, 0.0], [5.0, 0.0], [5.0, 2.0], [3.0, 2.0]]}\n" + " sluice: {x: 6.0, y: 6.0, radius: 0.8, kind: keepout}\n" ) diff --git a/mote_fleet/test/test_map_registry.py b/mote_fleet/test/test_map_registry.py index ef5a0df..272a030 100644 --- a/mote_fleet/test/test_map_registry.py +++ b/mote_fleet/test/test_map_registry.py @@ -72,7 +72,7 @@ def test_a_floor_reports_each_revision_with_why_it_could_be_promoted(server): revision = body["revisions"][0] assert revision["canonical"] is True assert revision["ok"] is True - assert revision["zones"] == ["kitchen", "ward"] + assert revision["zones"] == ["kitchen", "sluice", "ward"] assert revision["map"]["resolution"] == 0.05 diff --git a/mote_fleet/test/test_mapsync.py b/mote_fleet/test/test_mapsync.py index 5f990ce..3fd2cdb 100644 --- a/mote_fleet/test/test_mapsync.py +++ b/mote_fleet/test/test_mapsync.py @@ -128,7 +128,7 @@ def test_zones_arrive_with_the_map_and_the_old_ones_are_kept( mapsync.pull(server.url, announcement(promoted)) zones = bundle.read_zones(floor_dir / "zones.yaml")["zones"] - assert set(zones) == {"kitchen", "ward"} + assert set(zones) == {"kitchen", "sluice", "ward"} kept = list(floor_dir.glob("zones.*.yaml")) assert len(kept) == 1 assert "old" in bundle.read_zones(kept[0])["zones"] diff --git a/mote_fleet/test/test_zone_vocabulary.py b/mote_fleet/test/test_zone_vocabulary.py new file mode 100644 index 0000000..19dd2b8 --- /dev/null +++ b/mote_fleet/test/test_zone_vocabulary.py @@ -0,0 +1,222 @@ +"""The zone vocabulary over a real socket: what places can be named here. + +The question a dispatcher actually asks is *what may I say?*, and until this +route existed the fleet API could not answer it — the roster, the basemaps and +the dispatch route were all served, and the names were not. The workarounds +were an out-of-band document or scraping the list a robot prints when it +refuses an unknown zone, which is an accident of an error message rather than +a contract. + +What is asserted here is the property that makes serving it safe at all: the +vocabulary carries **no coordinates**. Not "the ones we remembered to remove" — +none, checked by walking the payload rather than by naming the keys we thought +of, because the leak this guards against is a *future* geometry key added to +``zones.yaml`` by someone who never reads this file. A vocabulary is portable +between robots; a binding is not, and a coordinate that escapes into a portable +document looks entirely plausible right up until a second robot acts on it. + +The binding's own route is unchanged and still served — see +``test_map_registry.py`` — because the client that draws zones on a basemap +already has the basemap. +""" + +import pytest +from api_harness import expect_error, get, write_bundle + +from mote_bringup import bundle + +SITE, FLOOR = "home", "ground" + +#: Any of these appearing anywhere in a vocabulary payload is the bug this +#: whole split exists to prevent. +GEOMETRY_KEYS = frozenset( + ("x", "y", "yaw", "radius", "polygon", "frame_id", "pose", "footprint") +) + + +def coordinates_in(payload) -> list: + """Every geometry-shaped key anywhere in the payload, however nested. + + Deliberately a walk rather than a check of the keys this test happens to + know about: the failure mode is a key nobody here anticipated. + """ + found = [] + if isinstance(payload, dict): + found += [key for key in payload if key in GEOMETRY_KEYS] + for value in payload.values(): + found += coordinates_in(value) + elif isinstance(payload, list): + for item in payload: + found += coordinates_in(item) + return found + + +# ---- one floor's vocabulary --------------------------------------------- + + +def test_a_floor_serves_its_names_kinds_and_aliases(server): + status, body = get(server, f"/v1/zones/{SITE}/{FLOOR}") + assert status == 200 + assert (body["site"], body["floor"]) == (SITE, FLOOR) + assert body["revision"] == 4 + by_name = {term["name"]: term for term in body["zones"]} + assert sorted(by_name) == ["kitchen", "sluice", "ward"] + assert by_name["kitchen"]["kind"] == "room" + assert by_name["kitchen"]["display_name"] == "The Kitchen" + assert by_name["kitchen"]["aliases"] == ["galley"] + assert by_name["kitchen"]["navigable"] is True + + +def test_the_vocabulary_carries_no_coordinates(server): + _, body = get(server, f"/v1/zones/{SITE}/{FLOOR}") + assert coordinates_in(body) == [] + + +def test_the_binding_still_carries_them(server): + """The control: the same floor, the route that is allowed to say where.""" + _, body = get(server, f"/v1/maps/{SITE}/{FLOOR}/zones.json") + assert coordinates_in(body) != [] + + +def test_a_constraint_zone_is_named_but_not_navigable(server): + """A keepout belongs in the vocabulary — an operator draws it on the same + floor plan — and the flag is what stops it being dispatched to.""" + _, body = get(server, f"/v1/zones/{SITE}/{FLOOR}") + sluice = next(t for t in body["zones"] if t["name"] == "sluice") + assert sluice["kind"] == "keepout" + assert sluice["navigable"] is False + + +def test_a_zone_taught_before_any_of_this_is_a_plain_area(server, tmp_path): + """Every vocabulary field is optional, so no zones.yaml needed rewriting.""" + floor = tmp_path / "sites" / SITE / "floors" / FLOOR + (floor / "map" / "zones.yaml").write_text( + "frame_id: map\nzones:\n bench: {x: 1.0, y: 1.0}\n" + ) + _, body = get(server, f"/v1/zones/{SITE}/{FLOOR}") + bench = next(t for t in body["zones"] if t["name"] == "bench") + assert bench["kind"] == "area" + assert bench["navigable"] is True + assert bench["aliases"] == [] + assert body["revision"] == 0 + + +def test_an_unknown_floor_is_404(server): + expect_error(lambda: get(server, f"/v1/zones/{SITE}/attic"), 404) + + +def test_a_malformed_path_is_404(server): + expect_error(lambda: get(server, f"/v1/zones/{SITE}"), 404) + expect_error(lambda: get(server, f"/v1/zones/{SITE}/{FLOOR}/extra"), 404) + + +@pytest.mark.parametrize( + "path", + [ + "/v1/zones/..%2F..%2Fetc/ground", + "/v1/zones/home/..%2F..%2Fregistry.db", + ], +) +def test_a_name_cannot_escape_the_bundle_root(server, path): + """Percent-encoded, so the traversal survives the client and is refused + here rather than normalised away before it ever arrives.""" + expect_error(lambda: get(server, path), 400) + + +# ---- the whole fleet's vocabulary --------------------------------------- + + +def test_every_floor_is_listed_in_one_call(server, tmp_path): + write_bundle(tmp_path / "sites", "depot", "first") + status, body = get(server, "/v1/zones") + assert status == 200 + assert {(v["site"], v["floor"]) for v in body["vocabularies"]} == { + ("home", "ground"), + ("depot", "first"), + } + assert coordinates_in(body) == [] + + +def test_a_named_but_unmapped_floor_still_has_a_vocabulary(server, tmp_path): + """The point of the split, stated as a test. + + Names are a fact about the building and do not wait on a SLAM session, so a + floor an operator has named but no robot has mapped answers here. Gating + this on a published revision — as the *binding* is rightly gated — would + have handed back the portability the split exists to buy. + """ + floor = tmp_path / "sites" / SITE / "floors" / "attic" + floor.mkdir(parents=True) + (floor / "zones.yaml").write_text( + "frame_id: map\nzones:\n loft: {x: 0.0, y: 0.0, kind: room}\n" + ) + _, body = get(server, f"/v1/zones/{SITE}/attic") + assert [t["name"] for t in body["zones"]] == ["loft"] + + # ...while the binding for that floor is still refused, because there is no + # map frame for those coordinates to be in. + expect_error(lambda: get(server, f"/v1/maps/{SITE}/attic/zones.json"), 404) + + _, listing = get(server, "/v1/zones") + assert (SITE, "attic") in {(v["site"], v["floor"]) for v in listing["vocabularies"]} + + +def test_a_floor_with_no_zones_is_skipped_not_an_error(server, tmp_path): + (tmp_path / "sites" / SITE / "floors" / "empty").mkdir(parents=True) + status, body = get(server, "/v1/zones") + assert status == 200 + assert (SITE, "empty") not in { + (v["site"], v["floor"]) for v in body["vocabularies"] + } + + +# ---- problems are reported, not hidden ---------------------------------- + + +def test_an_ambiguous_vocabulary_is_reported_and_still_served(server, tmp_path): + """The server reports; it does not refuse. The map is unaffected by a + duplicated alias, and a floor's basemap must not stop being served over + one — but a dispatcher has to be able to see that a name is unanswerable.""" + floor = tmp_path / "sites" / SITE / "floors" / FLOOR + (floor / "map" / "zones.yaml").write_text( + "frame_id: map\nzones:\n" + " kitchen: {x: 1.0, y: 1.0}\n" + " galley: {x: 2.0, y: 2.0, aliases: [Kitchen]}\n" + ) + status, body = get(server, f"/v1/zones/{SITE}/{FLOOR}") + assert status == 200 + assert len(body["zones"]) == 2 + assert any("ambiguous" in problem for problem in body["problems"]) + + +def test_a_name_a_dispatcher_cannot_type_is_reported(server, tmp_path): + floor = tmp_path / "sites" / SITE / "floors" / FLOOR + (floor / "map" / "zones.yaml").write_text( + 'frame_id: map\nzones:\n "Café": {x: 1.0, y: 1.0}\n' + ) + _, body = get(server, f"/v1/zones/{SITE}/{FLOOR}") + # Served verbatim rather than silently slugified: inventing 'cafe' would be + # a rename nobody asked for, and the operator is the one who must fix it. + assert [t["name"] for t in body["zones"]] == ["Café"] + assert any("dispatchable" in problem for problem in body["problems"]) + + +@pytest.mark.parametrize( + "entry, message", + [ + ("{x: 1.0, y: 1.0, kind: lounge}", "unknown kind"), + ("{x: 1.0, y: 1.0, kind: keepout, navigable: true}", "not a destination"), + ("{x: 1.0, y: 1.0, aliases: galley}", "list of strings"), + ], +) +def test_a_vocabulary_the_file_cannot_mean_is_refused(tmp_path, entry, message): + """These are not ambiguities to report — they are files with no reading. + + A ``navigable: true`` keepout is the interesting one: honouring it would + make the flag mean whatever was typed last, so the contradiction is refused + at the parse rather than resolved by precedence. + """ + path = tmp_path / "zones.yaml" + path.write_text(f"frame_id: map\nzones:\n a: {entry}\n") + with pytest.raises(bundle.BundleError, match=message): + bundle.read_zones(path) diff --git a/mote_simulation/worlds/gen_hospital.py b/mote_simulation/worlds/gen_hospital.py index 9216932..eb64fcb 100755 --- a/mote_simulation/worlds/gen_hospital.py +++ b/mote_simulation/worlds/gen_hospital.py @@ -66,7 +66,7 @@ _walls = [] # list of (cx, cy, sx, sy) _furniture = [] # list of (cx, cy, sx, sy) -_zones = {} # name -> (x, y, yaw, polygon|None) +_zones = {} # name -> (x, y, yaw, polygon|None, kind) _rooms = [] # walkable rectangles (x0, y0, x1, y1) -- room-segmentation truth @@ -92,18 +92,24 @@ def _room_rect(x0, x1, y0, y1): _rooms.append((xa + inset, ya + inset, xb - inset, yb - inset)) -def zone(name, x, y, yaw=0.0, polygon=None): +def zone(name, x, y, yaw=0.0, polygon=None, kind=None): """A named place for the task layer (mote_tasks): a pose to navigate to (fetch waypoint or ``goto ``), plus an optional ``polygon`` giving it - an area footprint (rooms carry one; bare waypoints don't).""" - _zones[name] = (x, y, yaw, polygon) + an area footprint (rooms carry one; bare waypoints don't). + + ``kind`` is the zone/v0 semantic role, and defaults to what the footprint + already says: a zone outlined by walls is a ``room``, one without is an + unopinionated ``area``. Worth emitting because the fleet serves kinds to a + dispatcher (``/v1/zones``), and this ladder is where that gets exercised. + """ + _zones[name] = (x, y, yaw, polygon, kind or ("room" if polygon else "area")) def _check_zone_clearance(): """Every zone pose must clear every box and lie inside its own footprint, so a layout edit cannot silently strand a zone inside geometry or outside the room it names.""" - for name, (zx, zy, _yaw, polygon) in _zones.items(): + for name, (zx, zy, _yaw, polygon, _kind) in _zones.items(): for cx, cy, sx, sy in _walls + _furniture: dx = max(abs(zx - cx) - sx / 2, 0.0) dy = max(abs(zy - cy) - sy / 2, 0.0) @@ -272,9 +278,17 @@ def build_world(): # pickup: in the waiting hall, facing the reception desk. dropoff: the far # north-east shallow ward, so a fetch crosses the spine, a cross corridor, # and an access corridor. home: the spawn point in the spine corridor. - zone("pickup", (b0 + b1) / 2, (spine_n + acc_n_in) / 2 - 1.5, math.pi / 2) - zone("dropoff", c0 + (c1 - c0) * 7 / 8, acc_n_out + 1.55, math.pi / 2) - zone("home", 0.0, 0.0) + zone( + "pickup", + (b0 + b1) / 2, + (spine_n + acc_n_in) / 2 - 1.5, + math.pi / 2, + kind="pickup", + ) + zone( + "dropoff", c0 + (c1 - c0) * 7 / 8, acc_n_out + 1.55, math.pi / 2, kind="dropoff" + ) + zone("home", 0.0, 0.0, kind="home") # --- Room zones (goto ; see mote_tasks.zones) ---------------------- # Named rooms in the deep shallow-ward rows (5.75 m deep, so plenty of @@ -431,12 +445,14 @@ def render_zones(): "# the layout changes. Zones with a polygon are room footprints (goto\n" "# targets, outlined by the room walls); the rest are bare fetch\n" "# waypoints. The pose is the doorway approach, not the room centre --\n" - "# a bed sits in the middle of each ward.\n" + "# a bed sits in the middle of each ward. `kind` is the zone/v0\n" + "# semantic role the fleet serves to a dispatcher at /v1/zones.\n" "frame_id: map\n" "zones:\n" ] - for name, (x, y, yaw, polygon) in _zones.items(): + for name, (x, y, yaw, polygon, kind) in _zones.items(): pose = f"x: {round(x, 3):g}, y: {round(y, 3):g}, yaw: {round(yaw, 3):g}" + pose += f", kind: {kind}" if polygon is None: lines.append(f" {name}: {{{pose}}}\n") continue @@ -478,7 +494,7 @@ def main(): print( f"wrote {out}: {len(_walls)} wall segments, {len(_furniture)} furniture boxes" ) - rooms = sum(1 for v in _zones.values() if v[-1] is not None) + rooms = sum(1 for v in _zones.values() if v[3] is not None) print(f"wrote {zones_out}: {len(_zones)} zones ({rooms} with a footprint)") print(f"wrote {rooms_out}: {len(_rooms)} rooms") diff --git a/mote_simulation/worlds/hospital_world.zones.yaml b/mote_simulation/worlds/hospital_world.zones.yaml index 6045072..7dc50f2 100644 --- a/mote_simulation/worlds/hospital_world.zones.yaml +++ b/mote_simulation/worlds/hospital_world.zones.yaml @@ -4,17 +4,18 @@ # the layout changes. Zones with a polygon are room footprints (goto # targets, outlined by the room walls); the rest are bare fetch # waypoints. The pose is the doorway approach, not the room centre -- -# a bed sits in the middle of each ward. +# a bed sits in the middle of each ward. `kind` is the zone/v0 +# semantic role the fleet serves to a dispatcher at /v1/zones. frame_id: map zones: - pickup: {x: -11, y: 4.5, yaw: 1.571} - dropoff: {x: 18.312, y: 14.8, yaw: 1.571} - home: {x: 0, y: 0, yaw: 0} - kitchen: {x: -18.312, y: 14.5, yaw: 1.571, + pickup: {x: -11, y: 4.5, yaw: 1.571, kind: pickup} + dropoff: {x: 18.312, y: 14.8, yaw: 1.571, kind: dropoff} + home: {x: 0, y: 0, yaw: 0, kind: home} + kitchen: {x: -18.312, y: 14.5, yaw: 1.571, kind: room, polygon: [[-20.675, 13.325], [-15.95, 13.325], [-15.95, 18.925], [-20.675, 18.925]]} - pharmacy: {x: -8.562, y: 14.5, yaw: 1.571, + pharmacy: {x: -8.562, y: 14.5, yaw: 1.571, kind: room, polygon: [[-10.925, 13.325], [-6.2, 13.325], [-6.2, 18.925], [-10.925, 18.925]]} - laboratory: {x: -13.438, y: -14.5, yaw: -1.571, + laboratory: {x: -13.438, y: -14.5, yaw: -1.571, kind: room, polygon: [[-15.8, -18.925], [-11.075, -18.925], [-11.075, -13.325], [-15.8, -13.325]]} - ward_east: {x: 18.312, y: -14.5, yaw: -1.571, + ward_east: {x: 18.312, y: -14.5, yaw: -1.571, kind: room, polygon: [[15.95, -18.925], [20.675, -18.925], [20.675, -13.325], [15.95, -13.325]]} diff --git a/mote_simulation/worlds/mote_world.zones.yaml b/mote_simulation/worlds/mote_world.zones.yaml index 2589850..25f7a30 100644 --- a/mote_simulation/worlds/mote_world.zones.yaml +++ b/mote_simulation/worlds/mote_world.zones.yaml @@ -5,11 +5,11 @@ # mission mode is up, so `fetch pickup dropoff` runs anywhere on the ladder. frame_id: map zones: - pickup: {x: 1.8, y: -1.5, yaw: 0.0} - dropoff: {x: -1.8, y: 1.5, yaw: 3.14} - home: {x: 0.0, y: 0.0, yaw: 0.0} + pickup: {x: 1.8, y: -1.5, yaw: 0.0, kind: pickup} + dropoff: {x: -1.8, y: 1.5, yaw: 3.14, kind: dropoff} + home: {x: 0.0, y: 0.0, yaw: 0.0, kind: home} # Room-like zones reachable by `goto `; the radius gives them an area # footprint. This single 6x6 room has no real rooms, so these are token areas # clear of the two obstacles. - kitchen: {x: 2.0, y: 2.0, yaw: 0.0, radius: 1.0} - lounge: {x: -2.0, y: 2.0, yaw: 3.14, radius: 1.0} + kitchen: {x: 2.0, y: 2.0, yaw: 0.0, radius: 1.0, kind: room} + lounge: {x: -2.0, y: 2.0, yaw: 3.14, radius: 1.0, kind: room} diff --git a/mote_simulation/worlds/office_world.zones.yaml b/mote_simulation/worlds/office_world.zones.yaml index 6374294..6605efc 100644 --- a/mote_simulation/worlds/office_world.zones.yaml +++ b/mote_simulation/worlds/office_world.zones.yaml @@ -4,14 +4,14 @@ # ends and sides, so a fetch traverses most of the corridor and two doorways. frame_id: map zones: - pickup: {x: -8.8, y: 3.5, yaw: 1.571} - dropoff: {x: 8.8, y: -3.5, yaw: -1.571} - home: {x: 0.0, y: 0.0, yaw: 0.0} + pickup: {x: -8.8, y: 3.5, yaw: 1.571, kind: pickup} + dropoff: {x: 8.8, y: -3.5, yaw: -1.571, kind: dropoff} + home: {x: 0.0, y: 0.0, yaw: 0.0, kind: home} # Room zones reachable by `goto `; the radius gives each an area # footprint. One per ward room, centred well inside the 4.4 m-wide rooms # (side walls at x = +/-2.2, +/-6.6) and short of the back walls, facing into # the room from its doorway. - pharmacy: {x: -4.4, y: 3.0, yaw: 1.571, radius: 1.3} - kitchen: {x: 4.4, y: 3.0, yaw: 1.571, radius: 1.3} - lab: {x: -4.4, y: -3.0, yaw: -1.571, radius: 1.3} - office: {x: 4.4, y: -3.0, yaw: -1.571, radius: 1.3} + pharmacy: {x: -4.4, y: 3.0, yaw: 1.571, radius: 1.3, kind: room} + kitchen: {x: 4.4, y: 3.0, yaw: 1.571, radius: 1.3, kind: room} + lab: {x: -4.4, y: -3.0, yaw: -1.571, radius: 1.3, kind: room} + office: {x: 4.4, y: -3.0, yaw: -1.571, radius: 1.3, kind: room} diff --git a/mote_tasks/config/zones.default.yaml b/mote_tasks/config/zones.default.yaml index aed527b..7974ccd 100644 --- a/mote_tasks/config/zones.default.yaml +++ b/mote_tasks/config/zones.default.yaml @@ -8,11 +8,11 @@ # own taught zones.yaml. frame_id: map zones: - pickup: {x: 1.8, y: -1.5, yaw: 0.0} - dropoff: {x: -1.8, y: 1.5, yaw: 3.14} - home: {x: 0.0, y: 0.0, yaw: 0.0} + pickup: {x: 1.8, y: -1.5, yaw: 0.0, kind: pickup} + dropoff: {x: -1.8, y: 1.5, yaw: 3.14, kind: dropoff} + home: {x: 0.0, y: 0.0, yaw: 0.0, kind: home} # Room zones (here a radius footprint; a `polygon:` of [x, y] vertices is the # other form) reachable by `goto `, mirroring mote_world.zones.yaml so # `goto kitchen` works out of the box. - kitchen: {x: 2.0, y: 2.0, yaw: 0.0, radius: 1.0} - lounge: {x: -2.0, y: 2.0, yaw: 3.14, radius: 1.0} + kitchen: {x: 2.0, y: 2.0, yaw: 0.0, radius: 1.0, kind: room} + lounge: {x: -2.0, y: 2.0, yaw: 3.14, radius: 1.0, kind: room} diff --git a/mote_tasks/mote_tasks/save_zone.py b/mote_tasks/mote_tasks/save_zone.py index 250bc45..eaf1987 100644 --- a/mote_tasks/mote_tasks/save_zone.py +++ b/mote_tasks/mote_tasks/save_zone.py @@ -1,20 +1,26 @@ """Teach a zone by driving to it: capture the robot's current map-frame pose into the active site's zones.yaml (legacy ~/.mote/zones.yaml if no site). - ros2 run mote_tasks save_zone [--radius R] [base_frame] + ros2 run mote_tasks save_zone [--radius R] [--kind K] [base_frame] Poses taught this way are reachable by construction. ``--radius`` (metres) gives the zone a circular area footprint, so it answers "am I in it" and reads as a room rather than a bare waypoint; omit it for a plain navigation target. Re-teaching an existing name replaces its pose but keeps its footprint, which may be a polygon outline this command cannot capture (see mote_tasks.zones). + +``--kind`` says what sort of place it is (``bundle.ZONE_KINDS``), which is the +half of a zone that travels: the fleet serves names and kinds to a dispatcher +at ``/v1/zones`` and never the pose, because the pose is only true in this +robot's map frame. Re-teaching keeps the kind and any aliases already on the +zone — a better coordinate is not a rename. """ import sys import rclpy import tf2_ros -from mote_bringup import sites +from mote_bringup import bundle, sites from rclpy.node import Node from rclpy.time import Time @@ -25,6 +31,7 @@ def main(): radius = None + kind = None positional = [] argv = sys.argv[1:] i = 0 @@ -37,13 +44,25 @@ def main(): radius = float(argv[i]) elif arg.startswith("--radius="): radius = float(arg.split("=", 1)[1]) + elif arg == "--kind": + i += 1 + if i >= len(argv): + sys.exit("--kind needs a value") + kind = argv[i] + elif arg.startswith("--kind="): + kind = arg.split("=", 1)[1] elif not arg.startswith("-"): positional.append(arg) i += 1 if not positional: - sys.exit("usage: save_zone [--radius R] [base_frame]") + sys.exit("usage: save_zone [--radius R] [--kind K] [base_frame]") name = positional[0] base_frame = positional[1] if len(positional) > 1 else "base_link" + # Checked before ROS starts: the operator is standing at the robot having + # just driven it somewhere, and finding out about a typo after the ten + # second transform wait means driving there again. + if kind is not None and kind not in bundle.ZONE_KINDS: + sys.exit(f"unknown kind '{kind}' (one of {', '.join(bundle.ZONE_KINDS)})") rclpy.init() node = Node("save_zone") @@ -62,9 +81,10 @@ def main(): yaw = yaw_from_quaternion(q.x, q.y, q.z, q.w) path = sites.zones_for_write() - replaced = append_zone(path, name, t.x, t.y, yaw, radius) + replaced = append_zone(path, name, t.x, t.y, yaw, radius, kind) verb = "replaced" if replaced else "added" extra = f" radius={radius:.3f}" if radius is not None else "" + extra += f" kind={kind}" if kind is not None else "" print( f"{verb} zone '{name}': x={t.x:.3f} y={t.y:.3f} yaw={yaw:.3f}{extra} in {path}" ) diff --git a/mote_tasks/mote_tasks/trees/fetch.py b/mote_tasks/mote_tasks/trees/fetch.py index ed09860..858f86e 100644 --- a/mote_tasks/mote_tasks/trees/fetch.py +++ b/mote_tasks/mote_tasks/trees/fetch.py @@ -12,6 +12,7 @@ import py_trees +from mote_tasks import zones as mote_zones from mote_tasks.behaviours.manipulation import TimedStub from mote_tasks.behaviours.nav import DriveTo from mote_tasks.behaviours.perception import AcquireObject @@ -36,11 +37,22 @@ def parse_command(zones: dict, words: list): if len(words) != 3 or words[0] != COMMAND: raise ValueError(f"expected: {COMMAND} ") target, drop = words[1], words[2] - if drop not in zones: - raise ValueError(f"unknown drop zone '{drop}', have {sorted(zones)}") - if target in zones: - return zones[target].pose, None, zones[drop].pose - return None, target.replace("_", " "), zones[drop].pose + drop_zone = mote_zones.resolve(zones, drop) + if drop_zone is None or not drop_zone.navigable: + known = sorted(name for name, z in zones.items() if z.navigable) + raise ValueError(f"unknown drop zone '{drop}', have {known}") + # A target naming a zone is a place to drive to; anything else is a label + # for the detector. A *non-navigable* zone name is neither — falling + # through would send the detector hunting for an object called "keepout". + target_zone = mote_zones.resolve(zones, target) + if target_zone is not None: + if not target_zone.navigable: + raise ValueError( + f"zone '{target_zone.name}' is a {target_zone.kind} zone, " + "not a destination" + ) + return target_zone.pose, None, drop_zone.pose + return None, target.replace("_", " "), drop_zone.pose def create_fetch_tree( diff --git a/mote_tasks/mote_tasks/trees/goto.py b/mote_tasks/mote_tasks/trees/goto.py index a14c2ac..489bc86 100644 --- a/mote_tasks/mote_tasks/trees/goto.py +++ b/mote_tasks/mote_tasks/trees/goto.py @@ -11,6 +11,7 @@ import py_trees +from mote_tasks import zones as mote_zones from mote_tasks.behaviours.nav import DriveTo from mote_tasks.trees.common import WaitForTask @@ -23,14 +24,27 @@ def parse_command(zones: dict, words: list): """Parse a ``goto `` command against known zones. Returns the zone's PoseStamped; raises ValueError with a user-facing - message when the command is malformed or the zone is unknown. + message when the command is malformed, the zone is unknown, or the zone is + not somewhere the robot may drive. + + The target may be a zone's name or any of its aliases. The refusal still + lists the names it *would* have taken, because a dispatcher with no other + source has been reading that list — the vocabulary the fleet now serves at + ``/v1/zones`` is the supported way to ask, but breaking the accident while + something depends on it would be gratuitous. """ if len(words) != 2 or words[0] != COMMAND: raise ValueError(f"expected: {COMMAND} ") - name = words[1] - if name not in zones: - raise ValueError(f"unknown zone '{name}', have {sorted(zones)}") - return zones[name].pose + target = words[1] + zone = mote_zones.resolve(zones, target) + if zone is None: + known = sorted(name for name, z in zones.items() if z.navigable) + raise ValueError(f"unknown zone '{target}', have {known}") + if not zone.navigable: + # A keepout is in the vocabulary because it is a place an operator + # draws on a floor plan, not because it is a destination. + raise ValueError(f"zone '{zone.name}' is a {zone.kind} zone, not a destination") + return zone.pose def create_goto_tree() -> py_trees.trees.BehaviourTree: diff --git a/mote_tasks/mote_tasks/zones.py b/mote_tasks/mote_tasks/zones.py index a125031..e871929 100644 --- a/mote_tasks/mote_tasks/zones.py +++ b/mote_tasks/mote_tasks/zones.py @@ -9,16 +9,34 @@ `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 also carries a **vocabulary**: what it is called and what kind of place +it is (zone/v0). This file holds both halves because they are taught together +— you drive somewhere, name it, and say what it is — but they are not the same +kind of fact. The names travel; the coordinates do not. `(2.0, 3.5)` in this +robot's map frame is a different physical point in the next robot's, so the +fleet publishes the vocabulary and never the binding (`mote_bringup.bundle`'s +`vocabulary`, served at `/v1/zones`). + Schema: frame_id: + vocabulary_revision: # bumped by save-zone zones: : + # the binding — this robot's frame, never shared x: y: yaw: radius: # circular footprint centred on (x, y) polygon: [[x, y], ...] # footprint outline, >= 3 vertices + # the vocabulary — shared by every robot at this site + kind: # bundle.ZONE_KINDS + display_name: # what an operator sees + aliases: [, ...] # what people call it + navigable: # false for keepout/slow + parent: # enclosing zone on this floor + tags: [, ...] + description: x, y and the polygon vertices are metres in frame_id; yaw is radians about +z. A polygon takes precedence over a radius when a zone carries both. x and y are @@ -30,10 +48,12 @@ frame_id: map zones: - pickup: {x: 1.8, y: -1.5, yaw: 0.0} - dropoff: {x: -1.8, y: 1.5} - kitchen: {x: 2.0, y: 2.0, radius: 1.5} + pickup: {x: 1.8, y: -1.5, yaw: 0.0, kind: pickup} + dropoff: {x: -1.8, y: 1.5, kind: dropoff} + kitchen: {x: 2.0, y: 2.0, radius: 1.5, kind: room, + display_name: "The Kitchen", aliases: [galley]} ward: {x: 6.0, y: 1.0, polygon: [[4, 0], [9, 0], [9, 3], [4, 3]]} + plant: {x: 0.0, y: 4.0, radius: 1.0, kind: keepout} """ import math @@ -43,6 +63,7 @@ import yaml from geometry_msgs.msg import PoseStamped +from mote_bringup import bundle def yaw_from_quaternion(x: float, y: float, z: float, w: float) -> float: @@ -155,11 +176,28 @@ def _on_segment(px: float, py: float, a, b, tol: float = 1e-9) -> bool: @dataclass(frozen=True) class Zone: - """A named place: a pose to navigate to, plus an optional area footprint.""" + """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. + """ name: str pose: PoseStamped footprint: Footprint | None = None + kind: str = "area" + navigable: bool = True + display_name: str = "" + aliases: tuple = () + parent: str | None = None + tags: tuple = () + description: str = "" + + @property + def label(self) -> str: + """What to call it when talking to a human.""" + return self.display_name or self.name def _polygon(name: str, raw) -> Polygon: @@ -174,7 +212,13 @@ def _polygon(name: str, raw) -> Polygon: def append_zone( - path, name: str, x: float, y: float, yaw: float, radius: float | None = None + path, + name: str, + x: float, + y: float, + yaw: float, + radius: float | None = None, + kind: str | None = None, ) -> bool: """Write/replace one zone in the file, creating the file if needed. @@ -182,6 +226,11 @@ def append_zone( 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 ``kind`` says + otherwise — driving somewhere to capture a better pose must not silently + drop the aliases an operator typed into the file by hand. """ path = Path(path).expanduser() data = ( @@ -195,8 +244,24 @@ def append_zone( entry["radius"] = round(radius, 3) else: entry.update({k: previous[k] for k in ("radius", "polygon") if k in previous}) + entry.update({k: previous[k] for k in bundle.VOCABULARY_KEYS if k in previous}) + if kind is not None: + if kind not in bundle.ZONE_KINDS: + raise ValueError( + f"unknown kind '{kind}' (one of {', '.join(bundle.ZONE_KINDS)})" + ) + entry["kind"] = kind + # A constraint zone is not a destination, and the flag saying so has to + # be in the file: a reader that inferred it from the kind would be a + # second copy of the rule, free to disagree with this one. + entry.pop("navigable", None) + if kind in bundle.CONSTRAINT_KINDS: + entry["navigable"] = False replaced = name in data["zones"] data["zones"][name] = entry + # The vocabulary revision is what a binding elsewhere records itself as + # built against, so it has to move whenever a name could have. + data["vocabulary_revision"] = int(data.get("vocabulary_revision") or 0) + 1 path.parent.mkdir(parents=True, exist_ok=True) tmp = path.with_name(f".{path.name}.{os.getpid()}") tmp.write_text(yaml.safe_dump(data, sort_keys=False, default_flow_style=None)) @@ -221,10 +286,52 @@ def load_zones(path: str) -> dict[str, Zone]: footprint = polygon if footprint is None and "radius" in spec: footprint = Circle(x, y, float(spec["radius"])) - zones[name] = Zone(name, pose, footprint) + try: + term = bundle.zone_term(Path(path).name, name, spec) + except bundle.BundleError as exc: + raise ValueError(str(exc)) from exc + zones[name] = Zone( + name, + pose, + footprint, + kind=term["kind"], + navigable=term["navigable"], + display_name=term["display_name"], + aliases=tuple(term["aliases"]), + parent=term["parent"], + tags=tuple(term["tags"]), + description=term["description"], + ) + # zone/v0: a conforming platform rejects a vocabulary with a collision. + # Loading one anyway would mean `goto kitchen` picking a winner by dict + # order — which is the guess the spec exists to forbid, and it would be + # made silently, once per boot, differently after an edit. + clashes = bundle.ambiguities( + [{"name": z.name, "aliases": list(z.aliases)} for z in zones.values()] + ) + if clashes: + raise ValueError(f"{Path(path).name}: {clashes[0]}") return zones +def resolve(zones: dict[str, Zone], query: str) -> Zone | None: + """The zone a human's words name, or None. + + Exact name first, then aliases and display names case-insensitively and + whitespace-normalised, so "the Kitchen" reaches ``kitchen``. Ambiguity + cannot arise here because :func:`load_zones` refuses a vocabulary that + contains any. + """ + if query in zones: + return zones[query] + wanted = bundle.normalise_alias(query) + for zone in zones.values(): + spellings = (zone.name, zone.display_name, *zone.aliases) + if any(s and bundle.normalise_alias(s) == wanted for s in spellings): + return zone + return None + + def containing(zones: dict[str, Zone], x: float, y: float) -> list[str]: """Names of the zones whose footprint contains ``(x, y)``, nearest-pose first — so ``containing(...)[0]`` is the best single answer to "which zone diff --git a/mote_tasks/test/test_goto_command.py b/mote_tasks/test/test_goto_command.py index e98e9fc..60ad2bf 100644 --- a/mote_tasks/test/test_goto_command.py +++ b/mote_tasks/test/test_goto_command.py @@ -1,13 +1,21 @@ -from types import SimpleNamespace - import pytest from mote_tasks.trees.goto import parse_command +from mote_tasks.zones import Zone -# parse_command reads the matched zone's `.pose`. +# A real Zone, because parse_command reads the vocabulary as well as the pose. +# Zone does not care what `pose` is, so a sentinel still saves building a +# PoseStamped, and the vocabulary defaults come out right by construction. ZONES = { - "kitchen": SimpleNamespace(pose="KITCHEN_POSE"), - "pickup": SimpleNamespace(pose="PICKUP_POSE"), + "kitchen": Zone( + "kitchen", + "KITCHEN_POSE", + kind="room", + display_name="The Kitchen", + aliases=("galley",), + ), + "pickup": Zone("pickup", "PICKUP_POSE", kind="pickup"), + "server_room": Zone("server_room", "SERVER_POSE", kind="keepout", navigable=False), } @@ -20,9 +28,25 @@ def test_goto_works_for_any_zone_not_just_rooms(): assert parse_command(ZONES, ["goto", "pickup"]) == "PICKUP_POSE" -def test_unknown_zone_raises(): - with pytest.raises(ValueError): +def test_alias_and_display_name_reach_the_zone(): + # What a dispatcher gets from /v1/zones is what it may say here. + assert parse_command(ZONES, ["goto", "galley"]) == "KITCHEN_POSE" + assert parse_command(ZONES, ["goto", "The Kitchen"]) == "KITCHEN_POSE" + + +def test_constraint_zone_is_refused_as_a_destination(): + # A keepout is in the vocabulary but is not a place to drive to, and the + # refusal says which kind it is rather than pretending the name is unknown. + with pytest.raises(ValueError, match="keepout"): + parse_command(ZONES, ["goto", "server_room"]) + + +def test_unknown_zone_lists_only_navigable_names(): + # The listed names are what a dispatcher may actually send. + with pytest.raises(ValueError) as excinfo: parse_command(ZONES, ["goto", "nowhere"]) + assert "kitchen" in str(excinfo.value) + assert "server_room" not in str(excinfo.value) def test_malformed_command_raises(): diff --git a/mote_tasks/test/test_parse_command.py b/mote_tasks/test/test_parse_command.py index e570d98..2849824 100644 --- a/mote_tasks/test/test_parse_command.py +++ b/mote_tasks/test/test_parse_command.py @@ -1,14 +1,15 @@ -from types import SimpleNamespace - import pytest from mote_tasks.trees.fetch import parse_command +from mote_tasks.zones import Zone -# parse_command reads each match's `.pose`; a SimpleNamespace stands in for a -# Zone without needing a real PoseStamped. +# A real Zone, because parse_command reads the vocabulary as well as the pose. +# Zone does not care what `pose` is, so a sentinel still saves building a +# PoseStamped. ZONES = { - "pickup": SimpleNamespace(pose="PICKUP_POSE"), - "dropoff": SimpleNamespace(pose="DROPOFF_POSE"), + "pickup": Zone("pickup", "PICKUP_POSE", kind="pickup"), + "dropoff": Zone("dropoff", "DROPOFF_POSE", kind="dropoff", aliases=("the bin",)), + "server_room": Zone("server_room", "SERVER_POSE", kind="keepout", navigable=False), } @@ -40,3 +41,17 @@ def test_malformed_command_raises(): def test_unknown_drop_zone_raises(): with pytest.raises(ValueError): parse_command(ZONES, ["fetch", "pickup", "nowhere"]) + + +def test_drop_zone_alias_resolves(): + _, _, drop_pose = parse_command(ZONES, ["fetch", "red_box", "the bin"]) + assert drop_pose == "DROPOFF_POSE" + + +def test_constraint_zone_is_refused_rather_than_read_as_a_label(): + # The dangerous shape: falling through to the label branch would send the + # detector hunting for an object called "server room" and look like success. + with pytest.raises(ValueError, match="keepout"): + parse_command(ZONES, ["fetch", "server_room", "dropoff"]) + with pytest.raises(ValueError): + parse_command(ZONES, ["fetch", "red_box", "server_room"]) diff --git a/mote_tasks/test/test_segmented_zones.py b/mote_tasks/test/test_segmented_zones.py index a0fdf88..59e6f01 100644 --- a/mote_tasks/test/test_segmented_zones.py +++ b/mote_tasks/test/test_segmented_zones.py @@ -94,3 +94,17 @@ def test_running_it_twice_adds_nothing(tmp_path, rooms): assert added == [] assert len(skipped) == 2 assert path.read_text() == before + + +def test_a_proposed_room_declares_itself_a_room(tmp_path, rooms): + """The one piece of vocabulary segmentation can honestly fill in. + + What it carves out of free space *are* rooms, so `kind: room` is known + rather than guessed — and it is what the fleet serves to a dispatcher. The + name it invents is a placeholder; the kind is not. + """ + path = tmp_path / "zones.yaml" + merge_into_zones(path, rooms) + loaded = zones_lib.load_zones(str(path)) + assert {zone.kind for zone in loaded.values()} == {"room"} + assert all(zone.navigable for zone in loaded.values()) diff --git a/mote_tasks/test/test_zones.py b/mote_tasks/test/test_zones.py index 9a7bbc9..dc71384 100644 --- a/mote_tasks/test/test_zones.py +++ b/mote_tasks/test/test_zones.py @@ -9,6 +9,7 @@ append_zone, containing, load_zones, + resolve, yaw_from_quaternion, ) @@ -201,3 +202,116 @@ def test_save_zone_output_is_readable_by_the_bundle_validator(tmp_path): assert zones["kitchen"]["radius"] == 1.5 assert zones["ward_east"]["polygon"][2] == [5.0, 2.0] assert load_zones(str(path))["ward_east"].footprint is not None + + +# ---- the vocabulary half (zone/v0) -------------------------------------- + + +def write_zones(tmp_path, body: str): + path = tmp_path / "zones.yaml" + path.write_text(f"frame_id: map\nzones:\n{body}") + return path + + +def test_a_zone_with_no_vocabulary_is_a_navigable_area(tmp_path): + """Every field is optional, so no existing zones.yaml needed rewriting.""" + zone = load_zones(str(write_zones(tmp_path, " bench: {x: 1.0, y: 2.0}\n")))[ + "bench" + ] + assert (zone.kind, zone.navigable) == ("area", True) + assert zone.aliases == () and zone.parent is None + assert zone.label == "bench" # falls back to the name + + +def test_display_name_and_aliases_reach_the_zone(tmp_path): + path = write_zones( + tmp_path, + " kitchen: {x: 1.0, y: 2.0, kind: room, display_name: The Kitchen,\n" + " aliases: [galley, the kitchen]}\n", + ) + zone = load_zones(str(path))["kitchen"] + assert zone.kind == "room" + assert zone.label == "The Kitchen" + assert zone.aliases == ("galley", "the kitchen") + + +def test_a_constraint_kind_is_not_navigable_without_being_told(tmp_path): + """`navigable` follows from the kind, so a keepout cannot be taught as a + destination by forgetting a field.""" + path = write_zones(tmp_path, " sluice: {x: 1.0, y: 2.0, kind: keepout}\n") + assert load_zones(str(path))["sluice"].navigable is False + + +def test_a_navigable_keepout_is_refused_rather_than_honoured(tmp_path): + path = write_zones( + tmp_path, " sluice: {x: 1.0, y: 2.0, kind: keepout, navigable: true}\n" + ) + with pytest.raises(ValueError, match="not a destination"): + load_zones(str(path)) + + +def test_an_unknown_kind_is_refused(tmp_path): + path = write_zones(tmp_path, " lounge: {x: 1.0, y: 2.0, kind: snug}\n") + with pytest.raises(ValueError, match="unknown kind"): + load_zones(str(path)) + + +def test_an_ambiguous_vocabulary_is_refused_at_load(tmp_path): + """zone/v0: a conforming platform rejects a collision rather than picking a + winner. Loading it anyway would make `goto kitchen` depend on dict order. + """ + path = write_zones( + tmp_path, + " kitchen: {x: 1.0, y: 2.0}\n galley: {x: 3.0, y: 4.0, aliases: [Kitchen]}\n", + ) + with pytest.raises(ValueError, match="ambiguous"): + load_zones(str(path)) + + +def test_resolve_matches_name_then_alias_then_display_name(tmp_path): + path = write_zones( + tmp_path, + " kitchen: {x: 1.0, y: 2.0, display_name: The Kitchen, aliases: [galley]}\n", + ) + zones = load_zones(str(path)) + for query in ("kitchen", "galley", "The Kitchen", "the kitchen", "GALLEY"): + assert resolve(zones, query).name == "kitchen", query + assert resolve(zones, "pantry") is None + + +def test_re_teaching_a_pose_keeps_the_vocabulary(tmp_path): + """Driving somewhere to capture a better pose is a new coordinate, never a + rename — dropping the aliases an operator typed would be silent data loss. + """ + path = tmp_path / "zones.yaml" + append_zone(path, "kitchen", 1.0, 2.0, 0.0, radius=1.5, kind="room") + data = yaml.safe_load(path.read_text()) + data["zones"]["kitchen"]["aliases"] = ["galley"] + path.write_text(yaml.safe_dump(data, sort_keys=False, default_flow_style=None)) + + append_zone(path, "kitchen", 1.2, 2.2, 0.1) + zone = load_zones(str(path))["kitchen"] + assert zone.aliases == ("galley",) and zone.kind == "room" + assert zone.pose.pose.position.x == 1.2 + + +def test_teaching_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.""" + path = tmp_path / "zones.yaml" + append_zone(path, "kitchen", 1.0, 2.0, 0.0) + first = yaml.safe_load(path.read_text())["vocabulary_revision"] + append_zone(path, "ward", 3.0, 4.0, 0.0) + assert yaml.safe_load(path.read_text())["vocabulary_revision"] > first + + +def test_teaching_a_constraint_kind_writes_the_flag(tmp_path): + path = tmp_path / "zones.yaml" + append_zone(path, "sluice", 1.0, 2.0, 0.0, kind="keepout") + assert yaml.safe_load(path.read_text())["zones"]["sluice"]["navigable"] is False + assert load_zones(str(path))["sluice"].navigable is False + + +def test_teaching_an_unknown_kind_is_refused(tmp_path): + with pytest.raises(ValueError, match="unknown kind"): + append_zone(tmp_path / "zones.yaml", "snug", 1.0, 2.0, 0.0, kind="snug")