diff --git a/CLAUDE.md b/CLAUDE.md index db2dbf3..aad3168 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,7 +21,7 @@ Reasoning belongs in `docs/`. | `../layout-orchestration/docs/mqtt-contract.md` | **Binding.** Topics, payloads, QoS, retention, the 30 s re-assert. This repo does not own it and must never amend it. | | `docs/pin-allocation.md` | Which sensor is on which expander pin, and why allocation is not installation | | `docs/block-detector-wiring.md` | The LM-iD output stage: its two modes, why Input A is unpowered, the 3.3 V hazard if it is not | -| `docs/point-position-feedback.md` | Board 3 (`0x22`): why two inputs per point, the `0x22` allocation, and why command and feedback being different devices is the hazard. **Not built** | +| `docs/point-position-feedback.md` | Board 3 (`0x22`): why two inputs per point, the `0x22` allocation, and why command and feedback being different devices is the hazard. **Firmware built; no `S2` wiring landed** | | `docs/startup-and-status-led.md` | The power-on race with the modem, the retry-then-reset policy, and **what the LED flash codes mean** | | `docs/DEVELOPMENT_NOTES.md` | Module conventions, the MCP23017 pin map, hardware wiring | | `docs/PN7150_STATE_MACHINE_PLAN.md` | The NCI state machine the RFID reader implements | @@ -111,19 +111,26 @@ bench check that covers it instead. Do not leave it silently untested. The IO node is **deployed and working end to end** (#1, #2, #3). Both Goods Shed sensors publish contract readings that the orchestrator trusts, and the block follows them. +Point position feedback on board 3 (`0x22`) is **built but publishes nothing yet** (#15): the +expander answers on the bus, no `S2` contacts are landed, so `POINTS_INSTALLED` is empty. +Bring points up **one at a time** — every point fault kind Safe-Stops the whole layout, +including a fault on a point no route holds. + ``` src/lib/ -> device /lib mqtt_at.py ESP-AT transport. Length-aware frames, reconnect, failures returned. Waits for the modem to answer before commanding it. - layout_mqtt.py The contract: topics, payloads, QoS, retention, the 25 s re-assert. + layout_mqtt.py The contract: sensor and point topics, payloads, QoS, retention, + the 25 s re-assert, and point query bookkeeping. sensor_wiring.py Allocation vs installed, and the occupancy polarity. No `machine`. + point_wiring.py Point allocation, and the S2 contact pair -> position. No `machine`. node_startup.py Retry-then-reset startup policy, and the LED code table. No `machine`. status_led.py The onboard LED as a countable flash code. No `machine`. mcp23017_io.py I2C expander, in and out. Poll-only. input_pin_monitor.py Native Pico GPIO, IRQ + timer debounce. Currently unused. pcf8591_adc.py I2C ADC. Wired to nothing; no contract topic for analog yet. pn7150.py PN7150 + TCA9548A mux, callback-based. Awaiting #4. -src/apps/io-node/ -> device / main.py + config.py. Deployed. +src/apps/io-node/ -> device / main.py + config.py. Deployed. Sensors + points. tools/ host-side, never deployed uart_bridge.py Talk to the ESP-AT modem by hand. mqtt_example.py Minimal publish/subscribe against a real modem. @@ -165,7 +172,14 @@ directory on the path for the same reason. `S2` contacts on board 3. So `points.dcc_address` (orchestrator) and `pointId` → pins (here) are independent mappings that nothing cross-checks — get one wrong and the system commands one motor while reading another, with both ends looking healthy - (`docs/point-position-feedback.md`). + (`docs/point-position-feedback.md`). The commissioning check is to throw each point + **individually** and confirm the expected pair, and only that pair, moves. +- **Which `S2` throw is `normal` is unverifiable authored data.** Nothing in the wiring reveals + which way round a point is fitted, or which terminal the orchestrator calls `normal`. It has + to be established per point by throwing it and looking. Assume nothing from terminal labels. +- **A point's `unknown` is real and must not be debounced away.** A break-before-make + changeover passes through both-open on every throw, so the honest sequence is + `normal` → `unknown` → `reverse`. The backend expects it; its confirmation timeout is 8 s. - **A broken sensor wire reads as `clear`, not `occupied`** (#9). Both sensor types switch to ground, so an open circuit floats up to the pull-up — the permissive state. The re-assert cannot catch it: the node is alive and republishing. Accepted knowingly — but #9 rejected diff --git a/docs/pin-allocation.md b/docs/pin-allocation.md index 70b0ec8..9b42a3d 100644 --- a/docs/pin-allocation.md +++ b/docs/pin-allocation.md @@ -47,6 +47,44 @@ three-line config change. It costs the alignment. | 8 | GPB0 | 1 | Goods Shed | `ir---goods-shed` | | 9–15 | GPB1–7 | 2–8 | *spare* | | +## Board 3 — MCP23017 `0x22` — point position feedback + +**Two pins per point, not one.** A single input would infer `reverse` from the absence of +`normal`, and an absence is equally a broken wire, a lost supply, or a point sitting +mid-throw. The design and the hazards are in `docs/point-position-feedback.md`. + +The pair for a point always stays on this one chip, so a chip that drops off the bus fails +both halves together and the point reads `unknown` — the correct answer. Split across two +chips, a half failure leaves exactly one closed reading: a confident, corroborated-looking +position that nothing is corroborating. + +| Pin | Port | Chip pin | Point | Throw | `pointId` | +|---|---|---|---|---|---| +| 0 | GPA0 | 21 | P1 Fiddle Yard | normal | `88d5527d-c7db-40ab-b27b-532503487c32` | +| 1 | GPA1 | 22 | P1 Fiddle Yard | reverse | " | +| 2 | GPA2 | 23 | P2 Layout Entry | normal | `acc2150b-ffa1-4733-b2ff-1e4d2147dfe8` | +| 3 | GPA3 | 24 | P2 Layout Entry | reverse | " | +| 4 | GPA4 | 25 | P3 Siding 1 | normal | `8ccb1cf8-b9ce-448e-bb97-c56a886f46c3` | +| 5 | GPA5 | 26 | P3 Siding 1 | reverse | " | +| 6 | GPA6 | 27 | P4 Siding 2 | normal | `0e615557-a952-41ed-a5f2-c43d9eefa78f` | +| 7 | GPA7 | 28 | P4 Siding 2 | reverse | " | +| 8 | GPB0 | 1 | P5 Goods Shed | normal | `720bde49-5e2e-47f9-b691-1391e0195240` | +| 9 | GPB1 | 2 | P5 Goods Shed | reverse | " | +| 10 | GPB2 | 3 | P6 Engine Shed | normal | `210cf5c3-150a-4ef5-99b0-4a494eb48a4f` | +| 11 | GPB3 | 4 | P6 Engine Shed | reverse | " | +| 12–15 | GPB4–7 | 5–8 | *spare* | | | + +Wiring is `S2-C` to 0 V and each throw to its own pin, so a closed contact reads 0 against +the expander's internal pull-up — the same polarity as the sensor boards, by coincidence +rather than by rule, which is why it has its own `POINTS_ACTIVE_LOW` flag. + +**Which throw is `normal` cannot be determined from the wiring.** Nothing reveals which way +round a point is fitted, or which `S2` terminal the orchestrator calls `normal`. It has to be +established per point by throwing it and looking. The `normal`/`reverse` columns above are the +allocation's *intent*; commissioning confirms or corrects them. + +`S1` is not available — it is spoken for by frog polarity. + ## Pin numbering is the thing that goes wrong The numbers in these tables are **logical pins, 0-based**: 0–7 are GPA0–7, 8–15 are GPB0–7. @@ -114,6 +152,12 @@ Two, both on the **Goods Shed** block: | 1 (`0x20`) | 8 | GPB0 | 1 | `cs---goods-shed` | | 2 (`0x21`) | 8 | GPB0 | 1 | `ir---goods-shed` | +**No points.** Board 3 is fitted and answers on the bus, but no `S2` contacts are landed, so +`POINTS_INSTALLED` is empty and nothing is published on any `point/*/reading`. Bring points up +**one at a time**: every point fault kind Safe-Stops the whole layout, including a fault on a +point no route holds, so flipping all six to `positionFeedback: "required"` at once produces a +layout that halts on the first flaky contact with five other unproven points to rule out. + Two things follow from this being the pair brought up first, both of which paid off: - **Goods Shed is the only block with both a CS and an IR sensor installed**, so the bring-up diff --git a/docs/point-position-feedback.md b/docs/point-position-feedback.md index abd7706..7de4e45 100644 --- a/docs/point-position-feedback.md +++ b/docs/point-position-feedback.md @@ -3,11 +3,10 @@ Design note for the third MCP23017 (`0x22`), which reads the Cobalt iP Digital point motors' auxiliary switch contacts and publishes `point/{pointId}/reading`. -**Nothing here is built yet.** This is the design agreed before code, so that the wiring, the -allocation and the contract questions are settled while they are still cheap. The node work is -#15; the one genuinely blocking question is -[`bazauto/layout-orchestration#167`](https://github.com/bazauto/layout-orchestration/issues/167) -and is explained at the end. +**Status (2026-08-27): the firmware is built; the wiring is not landed.** The expander is +fitted and answers on the bus, `POINTS_INSTALLED` in `config.py` is empty, and nothing is +published until an `S2` pair is wired and its point added. The blocking question below settled +before the code was written — see *The blocking question, which was not ours*. The orchestrator's decision record for the feature is `../layout-orchestration/docs/point-feedback.md` (D1–D10), and @@ -179,38 +178,56 @@ first flaky contact with five other unproven points to rule out. Flip one, run i while. The escape hatch is per point and immediate: setting a point back to `'none'` clears its latched fault. -## The blocking question, which is not ours - -**Nothing checks that a point controller is still alive.** The pieces compound: - -- `point/*/reading` is not retained and has **no periodic re-assert**, unlike `sensor/*/reading` - with its 30 s rule. It is published on change and on query only. -- The confirmation deadline arms on a **`command`**; a **`query` deliberately does not** arm it. -- The backend queries on startup, on broker reconnect and on operator request — **not - periodically**. - -So once points are `'required'`, a node that dies quietly leaves the last `confirmedPosition` -standing indefinitely, and the layout keeps setting roads over points whose position nothing -has observed since it stopped. A dead *sensor* node degrades its blocks to `unknown` inside -the freshness window. A dead *point* node degrades nothing. - -#25 left this open deliberately rather than by omission: - -> Whether `sensor/*/reading` additionally needs its own liveness/staleness check remains #28's -> call, not this document's. - -That is the sensor side. There is no equivalent on the point side at all. - -**A periodic point re-assert is a `docs/mqtt-contract.md` amendment, and the contract changes -before the code.** It is not this repo's to make — raised as -[`bazauto/layout-orchestration#167`](https://github.com/bazauto/layout-orchestration/issues/167). - -This node should not be built against the current shape until #167 settles, because the two -candidate answers are *different firmware*: "republish every N seconds" is a timer in the -publish loop, "answer a periodic query" is subscription handling and nothing else. Wiring and -bench work are not blocked; only the publish behaviour is. - -The topology sharpens the problem rather than softening it. Because the commanded device is -not the reporting device, silence after a command carries **no signal at all** — a DCC -accessory command is fire-and-forget and will keep succeeding long after the feedback node has -stopped. On a self-reporting controller, silence after a command would at least be suspicious. +## The blocking question, which was not ours — settled + +**Nothing checked that a point controller was still alive.** `point/*/reading` was not +retained and had no periodic re-assert, the confirmation deadline arms on a `command` and +never on a `query`, and the backend queried only on startup, on reconnect and on operator +request. So a node that died quietly left the last `confirmedPosition` standing indefinitely +while the layout kept setting roads over points nothing had observed since. + +The topology sharpened that rather than softening it: because the commanded device is not the +reporting device, silence after a command carried **no signal at all** — a DCC accessory +command is fire-and-forget and keeps succeeding long after the feedback node has stopped. + +This was a `docs/mqtt-contract.md` amendment and not this repo's to make. Raised as +[`bazauto/layout-orchestration#167`](https://github.com/bazauto/layout-orchestration/issues/167), +**closed 2026-08-24**. It settled as the first of the two candidate answers — republish on a +timer, which is the same shape this repo already runs for sensors, rather than a periodic +query. The contract now reads: + +> Published by the point controller whenever its observed position changes, in response to a +> `point/{pointId}/query`, **and** re-published unchanged at least every **30 seconds** as a +> liveness assertion. NOT retained. + +with `POINT_FRESHNESS_TIMEOUT_MS` at 90 s — three missed re-asserts. A `required` point that +goes quiet degrades to `confirmation: "stale"` with `confirmedPosition: "unknown"` and its +edges become untraversable. That is a **degradation, not a fault**: it latches no `PointFault` +and does not Safe-Stop. A point left at `positionFeedback: "none"` has nothing reporting on it +and never goes stale. + +So this node re-asserts every point on the same 25 s timer as its sensors, through the same +`LayoutMQTT.tick()`. One rule, one loop — the re-assert is the single easiest thing here to +forget and the most expensive to get wrong, and it should not be possible to implement it for +one topic and miss it for the other. + +## Where this lives in the code + +| Piece | File | +|---|---| +| Contact pair → position, and the allocation checks | `src/lib/point_wiring.py` | +| Topics, payload, QoS/retention, the re-assert, query bookkeeping | `src/lib/layout_mqtt.py` | +| The `0x22` allocation and the installed allow-list | `src/apps/io-node/config.py` | +| Pin setup, query subscription, the read loop | `src/apps/io-node/main.py` | + +Two things worth knowing about the implementation: + +- **A query is noted, never answered from the callback.** `poll()` dispatches the handler, so + publishing from inside it would re-enter the modem in the middle of whatever command + `poll()` was called from. `note_query()` sets a flag and `tick()` answers on the next pass, + a few tens of milliseconds later. Safe to be late, because a query does not arm the + confirmation deadline. +- **A failed subscribe refuses to start.** Unlike a failed publish, it is not survivable: the + node would come up healthy, re-assert happily, and never answer a query — leaving every + queried point at `unknown` with nothing reporting a fault. It raises, the supervisor retries, + and the LED flashes code 5. diff --git a/src/apps/io-node/config.py b/src/apps/io-node/config.py index b75c417..5aae3df 100644 --- a/src/apps/io-node/config.py +++ b/src/apps/io-node/config.py @@ -35,9 +35,10 @@ I2C_SDA = 4 I2C_SCL = 5 -# Board 1 is current sensing, board 2 is IR. +# Board 1 is current sensing, board 2 is IR, board 3 is point position feedback. EXPANDER_CS = 0x20 EXPANDER_IR = 0x21 +EXPANDER_POINTS = 0x22 # The sensors are switches to ground: closed (conducting) when the block is occupied, # open otherwise, with the MCP23017's internal pull-up holding the line high when open. @@ -90,6 +91,73 @@ # topics. The expander's output support is kept in lib/ for when that changes. OUTPUTS = () +# --- board 3 (0x22): point position feedback ------------------------------ +# +# The Cobalt iP Digital motors are commanded over **DCC** and have no feedback path of +# their own — they cannot be asked anything. Position is read only from each motor's +# free `S2` changeover, wired `S2-C` to 0 V with each throw to its own input here. +# +# Two inputs per point, never one. A single input would infer `reverse` from the absence +# of `normal`, and an absence is equally a broken wire, a lost supply, or a point sitting +# mid-throw. See docs/point-position-feedback.md. +# +# `S2-C` to 0 V against the expander's pull-up means a closed throw reads 0. Its own flag +# rather than `ACTIVE_LOW` above: this is a third device that happens to agree with the +# other two, and treating that coincidence as a rule is a mistake this repo has already +# made once. +POINTS_ACTIVE_LOW = True + +# Contact bounce on the changeover. Safe at this value because a Cobalt iP takes seconds +# to travel, so the genuine both-open window during a throw is far wider than the +# debounce and is still reported. It is a separate knob from DEBOUNCE_MS so that tuning +# the sensors cannot silently swallow the `normal` -> `unknown` -> `reverse` sequence the +# backend expects — suppressing that transient is the mistake to avoid here. +POINT_DEBOUNCE_MS = 200 + +# The full allocation, from docs/point-position-feedback.md. Pairs are adjacent and stay +# on one expander, so a chip that drops off the bus fails both halves together and the +# point reads `unknown` — the correct answer. Split across two chips, a half failure +# would leave exactly one closed reading: a confident, corroborated-looking position that +# nothing is corroborating. +# +# The `dcc_address` comment on each row is **not used by this node**. It is here only so +# the two halves of the system can be checked against each other by eye: which motor gets +# commanded lives in the orchestrator's `points.dcc_address`, which motor gets read lives +# here, and nothing cross-checks them. +# +# **Which throw is `normal` is unverifiable authored data.** Nothing in the wiring reveals +# which way round a point is fitted or which `S2` terminal the orchestrator calls +# `normal`. It must be established per point by throwing it and looking — assume nothing +# from the terminal labels. +POINTS = ( + # point_id (orchestrator points.id) dcc normal reverse + {"point_id": "88d5527d-c7db-40ab-b27b-532503487c32", # P1 Fiddle Yard, dcc 11 + "expander": EXPANDER_POINTS, "normal_pin": 0, "reverse_pin": 1}, + {"point_id": "acc2150b-ffa1-4733-b2ff-1e4d2147dfe8", # P2 Layout Entry, dcc 12 + "expander": EXPANDER_POINTS, "normal_pin": 2, "reverse_pin": 3}, + {"point_id": "8ccb1cf8-b9ce-448e-bb97-c56a886f46c3", # P3 Siding 1, dcc 13 + "expander": EXPANDER_POINTS, "normal_pin": 4, "reverse_pin": 5}, + {"point_id": "0e615557-a952-41ed-a5f2-c43d9eefa78f", # P4 Siding 2, dcc 14 + "expander": EXPANDER_POINTS, "normal_pin": 6, "reverse_pin": 7}, + {"point_id": "720bde49-5e2e-47f9-b691-1391e0195240", # P5 Goods Shed, dcc 15 + "expander": EXPANDER_POINTS, "normal_pin": 8, "reverse_pin": 9}, + {"point_id": "210cf5c3-150a-4ef5-99b0-4a494eb48a4f", # P6 Engine Shed, dcc 16 + "expander": EXPANDER_POINTS, "normal_pin": 10, "reverse_pin": 11}, +) + +# Physically wired, and therefore the only points published. **Empty: the expander is +# fitted and answers on the bus, but no `S2` contacts are landed yet.** +# +# Bring one up at a time. Every point fault kind Safe-Stops the whole layout, including a +# fault on a point no route holds, so flipping all six to `positionFeedback: 'required'` +# in the orchestrator at once produces a layout that halts on the first flaky contact with +# five other unproven points to rule out. +# +# The commissioning check that catches the mapping hazard: throw each point individually +# and confirm the expected pair — and only that pair — moves. Throwing them together +# proves nothing, and neither does testing one while the others sit in matching positions. +POINTS_INSTALLED = () + # How long to wait between main-loop passes. Sensor edges are caught by polling the # expanders, which are poll-only devices. LOOP_SLEEP_MS = 50 diff --git a/src/apps/io-node/main.py b/src/apps/io-node/main.py index 8e424f5..aa65147 100644 --- a/src/apps/io-node/main.py +++ b/src/apps/io-node/main.py @@ -1,4 +1,4 @@ -"""IO node: MCP23017 block detectors and IR beams to the layout MQTT contract. +"""IO node: MCP23017 block detectors, IR beams and point position feedback to MQTT. Deployed as the board's `/main.py`, with `src/lib/*` in `/lib`. All wiring and identity lives in `config.py`; this file only assembles it and runs the loop. @@ -13,8 +13,11 @@ from layout_mqtt import LayoutMQTT from mcp23017_io import MCP23017Expander -from mqtt_at import MQTTATClient +from mqtt_at import BrokerUnreachable, MQTTATClient from node_startup import CODE_RUNTIME, SensorWiringError, start_supervised +from point_wiring import ( + assert_no_pin_overlap, decode_position, is_closed, select_installed_points, +) from sensor_wiring import is_occupied, select_installed from status_led import StatusLED @@ -26,29 +29,35 @@ # any longer than the diagnosis needs. RUNTIME_CRASH_FLASH_MS = 5000 +# The contract's `point/*/query` is QoS 1. +QUERY_QOS = 1 -def build_sensors(i2c, wiring): - """Configure one input pin per installed sensor, sharing an expander per address. - All pins on a device must come from the same `MCP23017Expander`, because it caches - the register state it writes; two expanders for one chip would each overwrite the - other's IODIR and GPPU. +def get_expander(expanders, i2c, available, address, what): + """Return the one `MCP23017Expander` for `address`, creating it on first use. + + All pins on a device must come from the same instance, because it caches the register + state it writes; two expanders for one chip would each overwrite the other's IODIR + and GPPU. """ - expanders = {} - available = i2c.scan() - sensors = [] + if address not in available: + raise SensorWiringError( + "expander 0x%02X not on the bus (found %s) — %s cannot be published" + % (address, [hex(a) for a in available], what)) - for entry in wiring: - address = entry["expander"] - if address not in available: - raise SensorWiringError( - "expander 0x%02X not on the bus (found %s) — %s cannot be published" - % (address, [hex(a) for a in available], entry["sensor_id"])) + if address not in expanders: + expanders[address] = MCP23017Expander(i2c, address=address) + return expanders[address] - if address not in expanders: - expanders[address] = MCP23017Expander(i2c, address=address) - pin = expanders[address].create_pin( +def build_sensors(i2c, available, expanders, wiring): + """Configure one input pin per installed sensor.""" + sensors = [] + + for entry in wiring: + expander = get_expander( + expanders, i2c, available, entry["expander"], entry["sensor_id"]) + pin = expander.create_pin( entry["pin"], mode="input", debounce_ms=config.DEBOUNCE_MS) pin.setup() sensors.append((entry["sensor_id"], pin)) @@ -56,6 +65,73 @@ def build_sensors(i2c, wiring): return sensors +def build_points(i2c, available, expanders, wiring): + """Configure the two input pins per installed point. + + Both halves of a pair come from the same expander instance, which is what gives them + a shared failure domain: a chip that drops off the bus fails both together and the + point reads `unknown`. One live input and one dead one would read as a confident, + corroborated-looking position that nothing is corroborating. + """ + points = [] + + for entry in wiring: + expander = get_expander( + expanders, i2c, available, entry["expander"], entry["point_id"]) + pins = [] + for which in ("normal_pin", "reverse_pin"): + pin = expander.create_pin( + entry[which], mode="input", debounce_ms=config.POINT_DEBOUNCE_MS) + pin.setup() + pins.append(pin) + points.append((entry["point_id"], pins[0], pins[1])) + + return points + + +def read_point(normal_pin, reverse_pin): + """Decode a point's contact pair into a contract position.""" + return decode_position( + is_closed(normal_pin.state, config.POINTS_ACTIVE_LOW), + is_closed(reverse_pin.state, config.POINTS_ACTIVE_LOW), + ) + + +def report_cross_wiring(point_id): + print("io-node: point %s reads both contacts closed — impossible on an SPDT " + "changeover, so the wiring is not what config.py says. Reporting unknown." + % point_id) + + +def make_query_handler(layout, point_id): + """Handle `point/{pointId}/query` by noting it, never by publishing. + + `poll()` dispatches this, and a publish from here would re-enter the modem in the + middle of whatever command `poll()` was called from. `tick()` answers on the next + pass, which is safe to be late: a `query` deliberately does not arm the backend's + confirmation deadline the way a `command` does. + """ + def handle(_topic, _payload): + layout.note_query(point_id) + + return handle + + +def subscribe_to_queries(client, layout, points): + """Subscribe to each installed point's query topic, and refuse to start without it. + + A failed subscribe is not survivable in the way a failed publish is. The node would + come up looking healthy, publish its re-asserts, and never answer a query — leaving + every queried point at `unknown` and every edge through it untraversable, with + nothing anywhere reporting a fault. + """ + for point_id, _normal, _reverse in points: + topic = layout.point_query_topic(point_id) + if not client.subscribe(topic, qos=QUERY_QOS): + raise BrokerUnreachable("could not subscribe to %s" % topic) + client.register_topic_handler(topic, make_query_handler(layout, point_id)) + + def bring_up(): """One complete startup attempt, from a clean slate. @@ -65,11 +141,17 @@ def bring_up(): # Resolve and validate the wiring before touching the modem, so a config mistake # fails immediately and obviously rather than as silence on the broker. wiring = select_installed(config.SENSORS, config.INSTALLED) - print("io-node: publishing %d of %d allocated sensors" - % (len(wiring), len(config.SENSORS))) + point_wiring = select_installed_points(config.POINTS, config.POINTS_INSTALLED) + assert_no_pin_overlap(wiring, point_wiring) + print("io-node: publishing %d of %d allocated sensors, %d of %d allocated points" + % (len(wiring), len(config.SENSORS), + len(point_wiring), len(config.POINTS))) i2c = I2C(config.I2C_ID, sda=Pin(config.I2C_SDA), scl=Pin(config.I2C_SCL)) - sensors = build_sensors(i2c, wiring) + available = i2c.scan() + expanders = {} + sensors = build_sensors(i2c, available, expanders, wiring) + points = build_points(i2c, available, expanders, point_wiring) client = MQTTATClient( host=config.MQTT_HOST, port=config.MQTT_PORT, @@ -86,22 +168,35 @@ def bring_up(): layout = LayoutMQTT( client, config.LAYOUT_ID, - on_publish_failure=lambda sensor_id, exc: print( - "io-node: publish failed for %s (%s)" % (sensor_id, exc)), + on_publish_failure=lambda topic_id, exc: print( + "io-node: publish failed for %s (%s)" % (topic_id, exc)), ) for sensor_id, _pin in sensors: layout.register_sensor(sensor_id) + for point_id, _normal, _reverse in points: + layout.register_point(point_id) + + try: + subscribe_to_queries(client, layout, points) + except Exception: + client.close() + raise # Publish what is on the track right now. Without this the first reading waits for # an edge, and a block occupied since before boot would never be reported at all. now = ticks_ms() for sensor_id, pin in sensors: layout.publish_sensor(sensor_id, is_occupied(pin.state, config.ACTIVE_LOW), now) + for point_id, normal_pin, reverse_pin in points: + position, cross_wired = read_point(normal_pin, reverse_pin) + if cross_wired: + report_cross_wiring(point_id) + layout.publish_point(point_id, position, now) - return sensors, layout, client + return sensors, points, layout, client -def run(sensors, layout, client): +def run(sensors, points, layout, client): """The main loop. Returns only on KeyboardInterrupt.""" while True: now = ticks_ms() @@ -111,9 +206,21 @@ def run(sensors, layout, client): layout.publish_sensor( sensor_id, is_occupied(pin.state, config.ACTIVE_LOW), now) - # Re-assert anything that has gone quiet, and retry anything that failed. - # Without this every block these sensors feed degrades to `unknown` within - # the backend's freshness window, and `unknown` is treated as occupied. + for point_id, normal_pin, reverse_pin in points: + # Both, every pass, and never short-circuited: a throw moves one contact + # before the other, and skipping the second read would decode half a change. + normal_changed = normal_pin.update_state() + reverse_changed = reverse_pin.update_state() + if normal_changed or reverse_changed: + position, cross_wired = read_point(normal_pin, reverse_pin) + if cross_wired: + report_cross_wiring(point_id) + layout.publish_point(point_id, position, now) + + # Re-assert anything that has gone quiet, answer any query, and retry anything + # that failed. Without this every block these sensors feed degrades to `unknown` + # within the backend's freshness window, and every point configured as requiring + # feedback degrades to `stale` within its own. layout.tick(now) client.poll() @@ -129,13 +236,13 @@ def report(exc, code, attempt, delay_ms): print("io-node: flashing code %d for %d ms, then retrying" % (code, delay_ms)) led.flash(code, delay_ms) - sensors, layout, client = start_supervised(bring_up, report, reset) + sensors, points, layout, client = start_supervised(bring_up, report, reset) led.on() print("io-node: running") try: - run(sensors, layout, client) + run(sensors, points, layout, client) except KeyboardInterrupt: print("io-node: interrupted") led.off() diff --git a/src/lib/layout_mqtt.py b/src/lib/layout_mqtt.py index 80fb506..7e90b12 100644 --- a/src/lib/layout_mqtt.py +++ b/src/lib/layout_mqtt.py @@ -41,6 +41,24 @@ SENSOR_QOS = 1 SENSOR_RETAIN = 1 +# Point position vocabulary. Unlike a command, a reading MAY be `unknown`. +POSITION_NORMAL = "normal" +POSITION_REVERSE = "reverse" +POSITION_UNKNOWN = "unknown" + +# This node reads changeover contacts, so everything it reports is independently sensed. +# It commands nothing and must never claim `driver`, which is a delivery acknowledgement +# rather than a position confirmation and never confirms a point requiring feedback. +SOURCE_SENSOR = "sensor" + +# `point/*/reading` is QoS 1 and explicitly **NOT** retained. A sensor's occupancy +# self-corrects on the next movement; a point's position can change while its controller +# is offline — hand-thrown during a shutdown, power lost mid-travel, a linkage dropped — +# and nothing on the layout corrects it afterwards. A retained point reading is a +# confident assertion with no correction path. Restart recovery is `point/*/query`. +POINT_QOS = 1 +POINT_RETAIN = 0 + # Comfortably inside the contract's 30 s, which is itself comfortably inside the # backend's 90 s freshness window. Two whole re-asserts can be lost before a sensor # stops being trusted. @@ -77,6 +95,13 @@ def __init__(self, client, layout_id, *, reassert_interval_ms=REASSERT_INTERVAL_ # very start of the clock leaves the timestamp at its initial value, which reads # as "just published", and the reading is then held for a full interval. self._sensors = {} + # point_id -> [last_position_or_None, last_published_ms, pending, query_pending] + # + # `query_pending` is set by `note_query()` from a message callback and answered + # by `tick()`. Publishing from inside the callback would re-enter the modem: + # `poll()` dispatches the handler, and a publish from there calls `poll()` again + # underneath it. + self._points = {} # -- topics ---------------------------------------------------------- @@ -87,6 +112,12 @@ def topic_base(self): def sensor_topic(self, sensor_id): return "%s/sensor/%s/reading" % (self.topic_base, sensor_id) + def point_topic(self, point_id): + return "%s/point/%s/reading" % (self.topic_base, point_id) + + def point_query_topic(self, point_id): + return "%s/point/%s/query" % (self.topic_base, point_id) + # -- registration ---------------------------------------------------- def register_sensor(self, sensor_id): @@ -150,13 +181,108 @@ def publish_sensor(self, sensor_id, occupied, now_ms): entry[2] = True return ok + # -- points ---------------------------------------------------------- + + def register_point(self, point_id): + """Declare a point this node will report the position of. + + As with sensors, registration is not publication. A registered point that has + never been read is not re-asserted, because an unread pair would assert + `unknown` for a point nothing is watching. + """ + validate_sensor_id(point_id, what="point id") + if point_id not in self._points: + self._points[point_id] = [None, 0, False, False] + return point_id + + @property + def point_ids(self): + return sorted(self._points) + + def last_position(self, point_id): + entry = self._points.get(point_id) + return None if entry is None else entry[0] + + def point_reading_payload(self, point_id, position): + """The contract's point reading. + + The backend's schema is `.strict()` — an unexpected field is a malformed + payload, which is a Fail-Safe Trigger — so this carries exactly `pointId`, + `position` and `source` and nothing else. `updatedAt` is optional there and is + omitted for the same reason it is omitted from a sensor reading: the board has + no RTC and no NTP, and a boot-relative value would look authoritative and be + wrong. + + `pointId` must equal the topic segment. A mismatch is a Fail-Safe Trigger, which + is why both come from the same argument rather than from two lookups. + """ + return json.dumps({ + "pointId": point_id, + "position": position, + "source": SOURCE_SENSOR, + }) + + def publish_point(self, point_id, position, now_ms): + """Publish a position now, and reset that point's re-assert timer. + + Returns whether the modem accepted it. As with `publish_sensor()`, a False + deliberately does not update the last-published time, so `tick()` retries on the + next pass rather than holding a lost reading for a full interval. + """ + if point_id not in self._points: + raise ValueError("point %r was never registered" % point_id) + if position not in (POSITION_NORMAL, POSITION_REVERSE, POSITION_UNKNOWN): + raise ValueError( + "position %r is not in the contract's vocabulary" % (position,)) + + ok = self._publish_point(point_id, position, now_ms) + entry = self._points[point_id] + entry[0] = position + entry[3] = False # any publish answers an outstanding query + if ok: + entry[1] = now_ms + entry[2] = False + else: + entry[2] = True + return ok + + def note_query(self, point_id): + """Record that the backend asked for this point's position. + + Called from a `point/{pointId}/query` message handler, which must not publish: + `poll()` dispatches the handler, so publishing from there would re-enter the + modem mid-command. `tick()` answers on the next pass instead — a few tens of + milliseconds later, and safe to be late, because a `query` deliberately does not + arm the backend's confirmation deadline the way a `command` does. + + A query for a point this node does not report is ignored rather than answered: + replying about a point we cannot see is exactly the fabricated reading the + installed allow-list exists to prevent. + """ + entry = self._points.get(point_id) + if entry is None: + return False + if entry[0] is None: + return False # never read; nothing honest to say yet + entry[3] = True + return True + + # -- the re-assert --------------------------------------------------- + def tick(self, now_ms): - """Re-assert anything that has gone quiet, and retry anything that failed. + """Re-assert anything that has gone quiet, answer queries, retry failures. - Call every loop. Returns the number of sensors published, which is useful for a + Call every loop. Returns the number of messages published, which is useful for a heartbeat log and for the bring-up check that counts re-asserts. + + Both `sensor/*/reading` and `point/*/reading` carry the same 30 s obligation. + They reach it for different reasons — a sensor's retained copy is a bootstrap for + a value about to be reconfirmed, while a point is never retained and the + re-assert is the *only* thing that says its controller is alive — but the rule + here is one rule, so it is one loop. """ published = 0 + for sensor_id in sorted(self._sensors): state, last_ms, pending = self._sensors[sensor_id] if state is None: @@ -169,6 +295,22 @@ def tick(self, now_ms): published += 1 else: self._sensors[sensor_id][2] = True + + for point_id in sorted(self._points): + position, last_ms, pending, queried = self._points[point_id] + if position is None: + continue # never read; nothing to assert + if (not pending and not queried + and _elapsed(now_ms, last_ms) < self._reassert_interval_ms): + continue + if self._publish_point(point_id, position, now_ms): + self._points[point_id][1] = now_ms + self._points[point_id][2] = False + self._points[point_id][3] = False + published += 1 + else: + self._points[point_id][2] = True + return published def _publish(self, sensor_id, occupied, now_ms): @@ -184,6 +326,19 @@ def _publish(self, sensor_id, occupied, now_ms): self._report_failure(sensor_id, None) return bool(ok) + def _publish_point(self, point_id, position, now_ms): + topic = self.point_topic(point_id) + payload = self.point_reading_payload(point_id, position) + try: + ok = self._client.publish(topic, payload, qos=POINT_QOS, retain=POINT_RETAIN) + except Exception as exc: + ok = False + self._report_failure(point_id, exc) + else: + if not ok: + self._report_failure(point_id, None) + return bool(ok) + def _report_failure(self, sensor_id, exc): if self._on_publish_failure is None: return diff --git a/src/lib/point_wiring.py b/src/lib/point_wiring.py new file mode 100644 index 0000000..bb0aa46 --- /dev/null +++ b/src/lib/point_wiring.py @@ -0,0 +1,160 @@ +"""Resolve a node's point allocation table, and decode a pair of S2 contacts. + +Pure — no `machine` import — so every check below runs in the host suite. They all fail +at startup rather than at runtime, because a point mis-wired or mis-allocated produces a +node that looks perfectly healthy while reporting another motor's position. + +The design and the hazards are in `docs/point-position-feedback.md`. The two facts that +shape this module: + +- **A point is commanded and read by two devices that know nothing about each other.** + The Cobalt iP takes DCC accessory commands and can report nothing; position comes only + from its `S2` changeover wired into an MCP23017 here. So `points.dcc_address` in the + orchestrator and `point_id` -> pins here are independent mappings that nothing + cross-checks. +- **Two inputs per point, not one.** A single input infers `reverse` from the absence of + `normal`, and an absence is equally a broken wire, a lost supply, or a point sitting + mid-throw. Two inputs make `unknown` observable instead of assumed. +""" + +from layout_mqtt import ( + POSITION_NORMAL, POSITION_REVERSE, POSITION_UNKNOWN, validate_sensor_id, +) + + +def select_installed_points(points, installed): + """Return the allocation entries for `installed`, in that order. + + `points` is the whole allocation table; `installed` names what is physically wired. + Only the returned entries should ever be published — an unwired pair reads both-open + and would publish a confident `unknown` for a point nothing is watching, which is a + reading the orchestrator will act on. + + Raises `ValueError` on anything that would produce a wrong or silent node. + """ + by_id = {} + by_position = {} + + for entry in points: + try: + point_id = entry["point_id"] + expander = entry["expander"] + normal_pin = entry["normal_pin"] + reverse_pin = entry["reverse_pin"] + except (KeyError, TypeError): + raise ValueError( + "every POINTS entry needs point_id, expander, normal_pin and " + "reverse_pin — got %r" % (entry,)) + + validate_sensor_id(point_id, what="point id") + + if point_id in by_id: + raise ValueError("point id %r appears twice in POINTS" % point_id) + by_id[point_id] = entry + + for name, pin in (("normal_pin", normal_pin), ("reverse_pin", reverse_pin)): + if not 0 <= pin <= 15: + raise ValueError( + "%s: %s %r is out of range — an MCP23017 has pins 0-15" + % (point_id, name, pin)) + + # One input read twice is the one-input design this replaces, wearing the + # two-input design's clothes: it would corroborate itself and always agree. + if normal_pin == reverse_pin: + raise ValueError( + "%s: normal_pin and reverse_pin are both %d — a point needs two " + "separate contacts to tell `unknown` from `reverse`" + % (point_id, normal_pin)) + + for name, pin in (("normal", normal_pin), ("reverse", reverse_pin)): + position = (expander, pin) + if position in by_position: + raise ValueError( + "%s %s and %s are both on expander 0x%02X pin %d" + % (point_id, name, by_position[position], expander, pin)) + by_position[position] = "%s %s" % (point_id, name) + + selected = [] + seen = set() + for point_id in installed: + if point_id in seen: + raise ValueError("%r is listed twice in POINTS_INSTALLED" % point_id) + seen.add(point_id) + if point_id not in by_id: + raise ValueError( + "POINTS_INSTALLED names %r, which is not in POINTS — check it against " + "the orchestrator's points table" % point_id) + selected.append(by_id[point_id]) + + return selected + + +def assert_no_pin_overlap(sensor_entries, point_entries): + """Refuse a pin claimed by both a sensor and a point. + + The two allocation tables are validated separately, so nothing else would notice. + Both would configure the same expander pin, one of them would be reading the other's + hardware, and both would look healthy on the broker. + """ + claimed = {} + + for entry in sensor_entries: + claimed[(entry["expander"], entry["pin"])] = "sensor %s" % entry["sensor_id"] + + for entry in point_entries: + for name in ("normal_pin", "reverse_pin"): + position = (entry["expander"], entry[name]) + owner = claimed.get(position) + if owner is not None: + raise ValueError( + "point %s %s and %s are both on expander 0x%02X pin %d" + % (entry["point_id"], name, owner, position[0], position[1])) + claimed[position] = "point %s %s" % (entry["point_id"], name) + + +def is_closed(level, active_low): + """Whether one throw of an `S2` changeover is closed. + + `S2-C` is wired to 0 V and each throw to its own expander input, so a closed contact + pulls its input down and reads 0 against the internal pull-up. Its own flag rather + than the sensors' `ACTIVE_LOW`: this is a third device that happens to agree with the + other two, and the repo has already been bitten once by treating a coincidence as a + rule. + """ + return (level == 0) if active_low else (level == 1) + + +def decode_position(normal_closed, reverse_closed): + """Turn a pair of contact states into a contract position. + + Returns `(position, cross_wired)`. `cross_wired` is True only for the electrically + impossible both-closed case. + + | `normal` | `reverse` | Reading | Meaning | + |---|---|---|---| + | closed | open | `normal` | corroborated | + | open | closed | `reverse` | corroborated | + | open | open | `unknown` | mid-travel, broken wire, or lost supply | + | closed | closed | `unknown` | impossible on an SPDT — cross-wiring | + + The both-open row is not a fault and must not be suppressed. A break-before-make + changeover genuinely passes through it on every throw, so the honest sequence is + `normal` -> `unknown` -> `reverse`. The backend expects it and its confirmation + timeout is 8000 ms, so a normal throw lands well inside. + + The both-closed row cannot happen on an SPDT changeover, so seeing it means the + wiring is not what this node thinks it is. It reads `unknown` — the safe answer — + and is flagged separately so it can be reported rather than silently averaged away. + """ + if normal_closed and reverse_closed: + return POSITION_UNKNOWN, True + if normal_closed: + return POSITION_NORMAL, False + if reverse_closed: + return POSITION_REVERSE, False + return POSITION_UNKNOWN, False + + +__all__ = [ + "select_installed_points", "assert_no_pin_overlap", "is_closed", "decode_position", +] diff --git a/tests/test_io_node_config.py b/tests/test_io_node_config.py index 950c416..35926ab 100644 --- a/tests/test_io_node_config.py +++ b/tests/test_io_node_config.py @@ -7,6 +7,7 @@ import config from layout_mqtt import LayoutMQTT, validate_sensor_id +from point_wiring import assert_no_pin_overlap, select_installed_points from sensor_wiring import select_installed @@ -64,10 +65,94 @@ def test_sensors_are_active_low(): def test_no_outputs_are_configured(): - """Points are DCC-controlled and the contract has no output topic class.""" + """The contract has no output topic class, and this node drives nothing. + + Point *readings* are a separate matter — this node reports position, it does not + command it. The motors take DCC accessory commands and cannot be driven from here. + """ assert tuple(config.OUTPUTS) == () +# --- board 3: point position feedback --------------------------------------- + +def test_the_real_point_config_resolves(): + selected = select_installed_points(config.POINTS, config.POINTS_INSTALLED) + + assert [e["point_id"] for e in selected] == list(config.POINTS_INSTALLED) + + +def test_no_points_are_installed_yet(): + """The expander is fitted and answers on the bus, but no S2 contacts are landed. + + An unwired pair reads both-open and would publish a confident `unknown` for a point + nothing is watching — the same *allocation is not installation* rule as the sensors. + """ + assert tuple(config.POINTS_INSTALLED) == () + + +def test_every_allocated_point_id_is_a_valid_contract_id(): + for entry in config.POINTS: + validate_sensor_id(entry["point_id"], what="point id") + + +def test_the_allocated_points_are_the_orchestrators_six(): + """Keyed on `points.id`, not a display name — they are independent fields upstream + and a rename does not update the topic.""" + assert [e["point_id"] for e in config.POINTS] == [ + "88d5527d-c7db-40ab-b27b-532503487c32", # P1 Fiddle Yard + "acc2150b-ffa1-4733-b2ff-1e4d2147dfe8", # P2 Layout Entry + "8ccb1cf8-b9ce-448e-bb97-c56a886f46c3", # P3 Siding 1 + "0e615557-a952-41ed-a5f2-c43d9eefa78f", # P4 Siding 2 + "720bde49-5e2e-47f9-b691-1391e0195240", # P5 Goods Shed + "210cf5c3-150a-4ef5-99b0-4a494eb48a4f", # P6 Engine Shed + ] + + +def test_every_pair_shares_one_expander_and_is_adjacent(): + """A pair on one chip shares a failure domain: the chip drops off the bus, both + halves read open, and the point reads `unknown` — the correct answer. Split across + two chips, a half failure leaves exactly one closed reading, which looks corroborated + and is not.""" + for entry in config.POINTS: + assert entry["expander"] == config.EXPANDER_POINTS == 0x22 + assert entry["reverse_pin"] == entry["normal_pin"] + 1 + + +def test_the_documented_allocation_is_what_deploys(): + """docs/point-position-feedback.md: six pairs, pins 0-11, 12-15 spare.""" + pins = [] + for entry in config.POINTS: + pins.extend((entry["normal_pin"], entry["reverse_pin"])) + + assert pins == list(range(12)) + + +def test_no_sensor_and_point_share_a_pin(): + assert_no_pin_overlap(list(config.SENSORS), list(config.POINTS)) is None + + +def test_point_contacts_are_active_low(): + """S2-C wired to 0 V against the expander pull-up, so a closed throw reads 0. + + Its own flag, not `ACTIVE_LOW`: a third device agreeing with the other two is a + coincidence, and this repo has been bitten once by treating that as a rule. + """ + assert config.POINTS_ACTIVE_LOW is True + + +def test_the_point_topics_the_node_will_use(): + """Spelled out in full, so a change to topic construction is visible in a diff.""" + layout = LayoutMQTT(_NullClient(), config.LAYOUT_ID) + p1 = "88d5527d-c7db-40ab-b27b-532503487c32" + + assert layout.point_topic(p1) == ( + "layout/c4e587aa-478d-46ab-a1df-9dd8359fc040/point/" + "88d5527d-c7db-40ab-b27b-532503487c32/reading") + assert layout.point_query_topic(p1) == ( + "layout/c4e587aa-478d-46ab-a1df-9dd8359fc040/point/" + "88d5527d-c7db-40ab-b27b-532503487c32/query") + + def test_the_topics_the_node_will_publish(): """Spelled out in full, so a change to topic construction is visible in a diff.""" layout = LayoutMQTT(_NullClient(), config.LAYOUT_ID) diff --git a/tests/test_layout_points.py b/tests/test_layout_points.py new file mode 100644 index 0000000..fb425b8 --- /dev/null +++ b/tests/test_layout_points.py @@ -0,0 +1,270 @@ +"""`point/{pointId}/reading` against the binding contract. + +The rules that differ from a sensor reading, and why: + +- **Never retained.** A sensor's occupancy self-corrects on the next movement. A point's + position can change while its controller is offline — hand-thrown during a shutdown, + power lost mid-travel — and nothing on the layout corrects it afterwards. +- **Answers a query.** New behaviour for this repo; the node only published before. +- **Same 30 s re-assert** (orchestration#167), but for a different reason: it is the only + thing that says the controller is alive, since nothing is retained and a fire-and-forget + DCC command keeps succeeding long after this node has stopped. +""" + +import json + +import pytest + +from layout_mqtt import ( + POINT_QOS, POINT_RETAIN, POSITION_NORMAL, POSITION_REVERSE, POSITION_UNKNOWN, + REASSERT_INTERVAL_MS, LayoutMQTT, +) + +LAYOUT_ID = "c4e587aa-478d-46ab-a1df-9dd8359fc040" +P1 = "88d5527d-c7db-40ab-b27b-532503487c32" +P2 = "acc2150b-ffa1-4733-b2ff-1e4d2147dfe8" + +# Every field the backend's `.strict()` Zod schema accepts. An extra one is a malformed +# payload, which is a Fail-Safe Trigger. +ALLOWED_FIELDS = {"pointId", "position", "source", "updatedAt"} + + +class RecordingClient: + def __init__(self, succeed=True): + self.published = [] # (topic, payload, qos, retain) + self.succeed = succeed + + def publish(self, topic, message, qos=0, retain=0): + self.published.append((topic, message, qos, retain)) + return self.succeed + + +@pytest.fixture +def client(): + return RecordingClient() + + +@pytest.fixture +def layout(client): + layout = LayoutMQTT(client, LAYOUT_ID) + layout.register_point(P1) + layout.register_point(P2) + return layout + + +# --- topics ----------------------------------------------------------------- + +def test_point_topic_matches_the_contract(layout): + assert layout.point_topic(P1) == ( + "layout/%s/point/%s/reading" % (LAYOUT_ID, P1)) + + +def test_query_topic_matches_the_contract(layout): + assert layout.point_query_topic(P1) == ( + "layout/%s/point/%s/query" % (LAYOUT_ID, P1)) + + +# --- payload ---------------------------------------------------------------- + +def test_payload_carries_exactly_the_contract_fields(layout): + payload = json.loads(layout.point_reading_payload(P1, POSITION_NORMAL)) + + assert payload == {"pointId": P1, "position": "normal", "source": "sensor"} + assert set(payload) <= ALLOWED_FIELDS + + +def test_no_extra_field_is_ever_added(layout): + """The backend's schema is `.strict()` — an extra field Safe-Stops the layout.""" + for position in (POSITION_NORMAL, POSITION_REVERSE, POSITION_UNKNOWN): + payload = json.loads(layout.point_reading_payload(P1, position)) + assert set(payload) <= ALLOWED_FIELDS + + +def test_the_point_id_in_the_payload_matches_the_topic_segment(layout, client): + """A mismatch between the two is a Fail-Safe Trigger.""" + layout.publish_point(P1, POSITION_NORMAL, 0) + + topic, payload, _qos, _retain = client.published[-1] + assert json.loads(payload)["pointId"] == topic.split("/")[-2] + + +def test_source_is_always_sensor(layout): + """This node commands nothing, so it can never honestly claim `driver`. + + A `driver` reading is a delivery acknowledgement, not a position confirmation, and + never confirms a point configured as requiring feedback. + """ + assert json.loads(layout.point_reading_payload(P1, POSITION_NORMAL))["source"] == "sensor" + + +def test_unknown_is_a_publishable_position(layout, client): + """Unlike a command, a reading MAY be `unknown` — and must be, mid-throw.""" + layout.publish_point(P1, POSITION_UNKNOWN, 0) + + assert json.loads(client.published[-1][1])["position"] == "unknown" + + +def test_a_position_outside_the_vocabulary_is_refused(layout): + with pytest.raises(ValueError, match="vocabulary"): + layout.publish_point(P1, "mid-travel", 0) + + +def test_an_unregistered_point_is_refused(layout): + with pytest.raises(ValueError, match="never registered"): + layout.publish_point("00000000-0000-0000-0000-000000000000", POSITION_NORMAL, 0) + + +# --- QoS and retention ------------------------------------------------------ + +def test_a_reading_is_qos_1_and_never_retained(layout, client): + layout.publish_point(P1, POSITION_NORMAL, 0) + + _topic, _payload, qos, retain = client.published[-1] + assert qos == 1 + assert retain == 0, "a retained point position is a belief with no correction path" + + +def test_the_retain_constant_itself_is_off(): + assert POINT_RETAIN == 0 + assert POINT_QOS == 1 + + +# --- the re-assert ---------------------------------------------------------- + +def test_a_point_is_re_asserted_when_it_goes_quiet(layout, client): + layout.publish_point(P1, POSITION_NORMAL, 0) + before = len(client.published) + + layout.tick(REASSERT_INTERVAL_MS) + + assert len(client.published) > before + assert json.loads(client.published[-1][1])["position"] == "normal" + + +def test_a_point_is_not_re_asserted_early(layout, client): + layout.publish_point(P1, POSITION_NORMAL, 0) + before = len(client.published) + + layout.tick(REASSERT_INTERVAL_MS - 1) + + assert len(client.published) == before + + +def test_the_interval_leaves_room_inside_the_backends_window(): + """POINT_FRESHNESS_TIMEOUT_MS is 90 s, so two re-asserts can be lost.""" + assert REASSERT_INTERVAL_MS * 3 <= 90_000 + + +def test_a_point_never_read_is_not_asserted(layout, client): + """An unread pair would assert `unknown` for a point nothing is watching.""" + layout.tick(REASSERT_INTERVAL_MS * 10) + + assert client.published == [] + + +def test_a_failed_publish_is_retried_on_the_next_pass(): + client = RecordingClient(succeed=False) + layout = LayoutMQTT(client, LAYOUT_ID) + layout.register_point(P1) + + layout.publish_point(P1, POSITION_NORMAL, 0) + before = len(client.published) + + layout.tick(1) + + assert len(client.published) > before, \ + "a lost reading must not be held for a full interval" + + +def test_a_publish_failure_is_reported(): + seen = [] + client = RecordingClient(succeed=False) + layout = LayoutMQTT(client, LAYOUT_ID, + on_publish_failure=lambda topic_id, exc: seen.append(topic_id)) + layout.register_point(P1) + + layout.publish_point(P1, POSITION_NORMAL, 0) + + assert seen == [P1] + + +# --- answering a query ------------------------------------------------------ + +def test_a_query_is_answered_on_the_next_tick(layout, client): + layout.publish_point(P1, POSITION_REVERSE, 0) + before = len(client.published) + + assert layout.note_query(P1) is True + layout.tick(1) # far short of the re-assert interval + + assert len(client.published) == before + 1 + topic, payload, _qos, retain = client.published[-1] + assert topic == layout.point_topic(P1) + assert json.loads(payload)["position"] == "reverse" + assert retain == 0 + + +def test_noting_a_query_does_not_itself_publish(layout, client): + """Publishing from the message callback would re-enter the modem inside `poll()`.""" + layout.publish_point(P1, POSITION_NORMAL, 0) + before = len(client.published) + + layout.note_query(P1) + + assert len(client.published) == before + + +def test_a_query_for_an_unknown_point_is_ignored(layout, client): + """Answering for a point this node cannot see would be a fabricated reading.""" + assert layout.note_query("00000000-0000-0000-0000-000000000000") is False + + layout.tick(1) + assert client.published == [] + + +def test_a_query_before_the_first_read_is_not_answered(layout, client): + """There is nothing honest to say yet.""" + assert layout.note_query(P1) is False + + layout.tick(1) + assert client.published == [] + + +def test_answering_a_query_resets_the_re_assert_timer(layout, client): + layout.publish_point(P1, POSITION_NORMAL, 0) + layout.note_query(P1) + layout.tick(1) + before = len(client.published) + + layout.tick(REASSERT_INTERVAL_MS) # would be due measured from 0, not from the answer + + assert len(client.published) == before + + +def test_one_points_query_does_not_answer_for_another(layout, client): + layout.publish_point(P1, POSITION_NORMAL, 0) + layout.publish_point(P2, POSITION_REVERSE, 0) + before = len(client.published) + + layout.note_query(P1) + layout.tick(1) + + assert len(client.published) == before + 1 + assert client.published[-1][0] == layout.point_topic(P1) + + +# --- sensors and points share one tick -------------------------------------- + +def test_sensors_and_points_are_both_re_asserted_by_one_tick(client): + layout = LayoutMQTT(client, LAYOUT_ID) + layout.register_sensor("cs---goods-shed") + layout.register_point(P1) + layout.publish_sensor("cs---goods-shed", True, 0) + layout.publish_point(P1, POSITION_NORMAL, 0) + client.published.clear() + + published = layout.tick(REASSERT_INTERVAL_MS) + + assert published == 2 + topics = {topic for topic, _p, _q, _r in client.published} + assert topics == {layout.sensor_topic("cs---goods-shed"), layout.point_topic(P1)} diff --git a/tests/test_point_wiring.py b/tests/test_point_wiring.py new file mode 100644 index 0000000..0e4bd44 --- /dev/null +++ b/tests/test_point_wiring.py @@ -0,0 +1,139 @@ +"""Decoding a point's contact pair, and refusing an allocation that would lie. + +The hazards these guard are in `docs/point-position-feedback.md`. The one to keep in +mind: a point is commanded and read by two devices that know nothing about each other, +so a mis-allocation here means the orchestrator commands one motor and reads another — +with both ends looking healthy. +""" + +import pytest + +from layout_mqtt import POSITION_NORMAL, POSITION_REVERSE, POSITION_UNKNOWN +from point_wiring import ( + assert_no_pin_overlap, decode_position, is_closed, select_installed_points, +) + +P1 = "88d5527d-c7db-40ab-b27b-532503487c32" +P2 = "acc2150b-ffa1-4733-b2ff-1e4d2147dfe8" + +POINTS = ( + {"point_id": P1, "expander": 0x22, "normal_pin": 0, "reverse_pin": 1}, + {"point_id": P2, "expander": 0x22, "normal_pin": 2, "reverse_pin": 3}, +) + + +# --- decoding --------------------------------------------------------------- + +def test_one_contact_closed_gives_a_corroborated_position(): + assert decode_position(True, False) == (POSITION_NORMAL, False) + assert decode_position(False, True) == (POSITION_REVERSE, False) + + +def test_both_open_is_unknown_and_not_a_fault(): + """A break-before-make changeover passes through both-open on every throw. + + The honest sequence is normal -> unknown -> reverse. Suppressing it to keep the log + tidy would hide exactly the transition the backend is waiting for. + """ + assert decode_position(False, False) == (POSITION_UNKNOWN, False) + + +def test_both_closed_is_unknown_and_flagged_as_cross_wiring(): + """Impossible on an SPDT, so the wiring is not what config.py says it is.""" + position, cross_wired = decode_position(True, True) + + assert position == POSITION_UNKNOWN, "the safe answer, not a guess between the two" + assert cross_wired is True + + +def test_a_position_is_never_inferred_from_a_single_absence(): + """The whole argument for two inputs: absence is not evidence of the other throw.""" + assert decode_position(False, False)[0] != POSITION_REVERSE + assert decode_position(False, False)[0] != POSITION_NORMAL + + +def test_a_closed_contact_reads_low(): + """S2-C is wired to 0 V, so a closed throw pulls its input down.""" + assert is_closed(0, active_low=True) is True + assert is_closed(1, active_low=True) is False + assert is_closed(1, active_low=False) is True + + +# --- the allocation --------------------------------------------------------- + +def test_only_installed_points_are_selected(): + assert select_installed_points(POINTS, ()) == [] + assert select_installed_points(POINTS, (P2,)) == [POINTS[1]] + + +def test_installed_order_is_preserved(): + assert [e["point_id"] for e in select_installed_points(POINTS, (P2, P1))] == [P2, P1] + + +def test_a_point_cannot_read_one_pin_twice(): + """That is the one-input design wearing the two-input design's clothes. + + It would corroborate itself and always agree, so `unknown` could never be observed. + """ + bad = ({"point_id": P1, "expander": 0x22, "normal_pin": 4, "reverse_pin": 4},) + + with pytest.raises(ValueError, match="two separate contacts"): + select_installed_points(bad, (P1,)) + + +def test_two_points_cannot_share_a_pin(): + bad = ( + {"point_id": P1, "expander": 0x22, "normal_pin": 0, "reverse_pin": 1}, + {"point_id": P2, "expander": 0x22, "normal_pin": 1, "reverse_pin": 2}, + ) + + with pytest.raises(ValueError, match="both on expander"): + select_installed_points(bad, ()) + + +def test_a_duplicate_point_id_is_refused(): + bad = (POINTS[0], POINTS[0]) + + with pytest.raises(ValueError, match="appears twice"): + select_installed_points(bad, ()) + + +def test_an_installed_point_missing_from_the_table_is_refused(): + with pytest.raises(ValueError, match="not in POINTS"): + select_installed_points(POINTS, ("not-a-configured-point",)) + + +def test_a_pin_out_of_range_is_refused(): + bad = ({"point_id": P1, "expander": 0x22, "normal_pin": 0, "reverse_pin": 16},) + + with pytest.raises(ValueError, match="out of range"): + select_installed_points(bad, ()) + + +def test_a_placeholder_point_id_is_refused(): + bad = ({"point_id": "", "expander": 0x22, + "normal_pin": 0, "reverse_pin": 1},) + + with pytest.raises(ValueError, match="placeholder"): + select_installed_points(bad, ()) + + +def test_a_malformed_entry_names_what_it_needs(): + with pytest.raises(ValueError, match="normal_pin"): + select_installed_points(({"point_id": P1, "expander": 0x22},), ()) + + +# --- the two tables against each other -------------------------------------- + +def test_a_sensor_and_a_point_cannot_claim_the_same_pin(): + """Validated separately, so nothing else would notice both configuring one pin.""" + sensors = [{"sensor_id": "cs---goods-shed", "expander": 0x22, "pin": 3}] + + with pytest.raises(ValueError, match="both on expander"): + assert_no_pin_overlap(sensors, list(POINTS)) + + +def test_the_usual_case_has_no_overlap(): + sensors = [{"sensor_id": "cs---goods-shed", "expander": 0x20, "pin": 8}] + + assert_no_pin_overlap(sensors, list(POINTS)) is None