fix(sim): one sensor-record merge, one ownership rule, and a board preference

Review pass over 8f148e8, closing what was still copied or only half true.

  - `upsertSensorRecords` is THE merge for sensor records, exported from the
    ESP32 bridge: same kind on a pin merges per field (the part and the store
    each know one half of an HC-SR04), a different kind replaces outright, so
    a DHT22 swapped for an HC-SR04 cannot inherit a stale echo_pin that the
    ownership guard would then keep. The seven overlay bridges carried their
    own copy of the old replace-by-pin loop — the exact copy-six-times trap
    this codebase has hit before — and now call this one.
  - `sensorRecordOwnsPin` is likewise the one answer to "does this record own
    that pad", so the overlay's pre-connect path and the QEMU bridge cannot
    drift apart.
  - connectDigitalInputsToMcu forgets its cached level for a pin a part owns.
    Keeping it meant that once the part let go (rewire, unmount, detach) the
    next solve could find the circuit agreeing with a stale memory and never
    re-emit, leaving the guest at whatever the part last drove.
  - traceBoardGpio's `boardId` was a filter applied AFTER the walk had already
    picked a pad, so a net reaching two boards' GPIOs answered null for the
    second board. It is now a preference the node collection honours, with a
    test that fails without it.
This commit is contained in:
David Montero 2026-08-23 01:47:23 +02:00
parent 8f148e836d
commit 6e543b5877
5 changed files with 170 additions and 57 deletions

View File

@ -108,3 +108,31 @@ describe('part pin ownership', () => {
expect(wrapped.readValue()).toBe(7); // `this` is the target, not the proxy
});
});
describe('sensor records on the ESP32 bridge', () => {
it('merges the two registration paths per field, same kind on the same pin', async () => {
const { upsertSensorRecords } = await import('../simulation/Esp32Bridge');
// The store knows the distance, the part knows the echo pin. Neither may
// erase what the other contributed.
const a = upsertSensorRecords([], [{ sensor_type: 'hc-sr04', pin: 21, echo_pin: 41 }]);
const b = upsertSensorRecords(a, [{ sensor_type: 'hc-sr04', pin: 21, distance: 30 }]);
expect(b).toEqual([{ sensor_type: 'hc-sr04', pin: 21, echo_pin: 41, distance: 30 }]);
});
it('replaces outright when a different kind lands on the pin', async () => {
const { upsertSensorRecords, sensorRecordOwnsPin } = await import('../simulation/Esp32Bridge');
const a = upsertSensorRecords([], [{ sensor_type: 'hc-sr04', pin: 4, echo_pin: 5 }]);
const b = upsertSensorRecords(a, [{ sensor_type: 'dht22', pin: 4, temperature: 22 }]);
expect(b).toEqual([{ sensor_type: 'dht22', pin: 4, temperature: 22 }]);
// ...so the old echo pin is handed back to the host, not guarded forever.
expect(sensorRecordOwnsPin(b[0], 5)).toBe(false);
expect(sensorRecordOwnsPin(b[0], 4)).toBe(true);
});
it('only single-wire kinds own pads', async () => {
const { sensorRecordOwnsPin } = await import('../simulation/Esp32Bridge');
// An ePaper panel registers real GPIOs too; the host must keep driving them.
expect(sensorRecordOwnsPin({ sensor_type: 'epaper-ssd168x', pin: 10, dc_pin: 9 }, 9)).toBe(false);
expect(sensorRecordOwnsPin({ sensor_type: 'hc-sr04', pin: 21, echo_pin: 41 }, 41)).toBe(true);
});
});

View File

@ -20,7 +20,7 @@
*/
import { describe, it, expect, beforeEach } from 'vitest';
import { useSimulatorStore } from '../store/useSimulatorStore';
import { traceDetailed } from '../simulation/PinTrace';
import { traceDetailed, traceBoardGpio } from '../simulation/PinTrace';
const BOARD = 'arduino-uno';
@ -232,3 +232,25 @@ describe('cost of walking the net', () => {
expect(perRail).toBeLessThan(8);
});
});
describe('a net shared by two boards', () => {
it('answers each board its own pad', () => {
// One sensor line wired to a GPIO on BOTH boards. Whoever pre-registers
// the sensor for board B must learn B's pad, whatever order the wires
// were drawn in — the walk must not stop at A's pad and report "not on B".
useSimulatorStore.setState({ boards: [], components: [], wires: [] } as never);
useSimulatorStore.getState().addBoard('arduino-uno' as never, 0, 0, 'uno-a');
useSimulatorStore.getState().addBoard('esp32' as never, 0, 0, 'esp-b');
const s = useSimulatorStore.getState();
s.setComponents([]);
s.setWires([
wire('x1', ['sensor', 'ECHO'], ['uno-a', '7']),
wire('x2', ['sensor', 'ECHO'], ['esp-b', '19']),
] as never);
const state = useSimulatorStore.getState();
expect(traceBoardGpio(state as never, 'sensor', 'ECHO', 'uno-a')).toBe(7);
expect(traceBoardGpio(state as never, 'sensor', 'ECHO', 'esp-b')).toBe(19);
// And a board the net never reaches is still "not here".
expect(traceBoardGpio(state as never, 'sensor', 'ECHO', 'nope')).toBeNull();
});
});

View File

@ -66,8 +66,52 @@ export function toQemuBoardType(kind: BoardKind): 'esp32' | 'esp32-s3' | 'esp32-
/** Sensor kinds whose model drives the pad itself for the whole exchange — a
* DHT22's DATA line, an HC-SR04's TRIG/ECHO pair. Anything else registered
* through the sensor channel (ePaper, keypad, I2C devices) leaves its GPIOs to
* the host. */
const SINGLE_WIRE_SENSOR_TYPES = new Set(['dht22', 'hc-sr04']);
* the host. Exported so an overlay engine answers the same question with the
* same list instead of a copy that drifts. */
export const SINGLE_WIRE_SENSOR_TYPES: ReadonlySet<string> = new Set(['dht22', 'hc-sr04']);
/** True for a sensor record (`{ sensor_type, pin, ... }`) of a single-wire kind. */
export function isSingleWireSensorRecord(rec: Record<string, unknown>): boolean {
return SINGLE_WIRE_SENSOR_TYPES.has(String(rec['sensor_type'] ?? ''));
}
/** Pads a single-wire sensor record owns: its data pin and any extra pin it
* names (`echo_pin` for an HC-SR04). */
export function sensorRecordOwnsPin(rec: Record<string, unknown>, gpioPin: number): boolean {
if (!isSingleWireSensorRecord(rec)) return false;
if (rec['pin'] === gpioPin) return true;
for (const [k, v] of Object.entries(rec)) {
if ((k.endsWith('_pin') || k.endsWith('Pin')) && v === gpioPin) return true;
}
return false;
}
/**
* Upsert sensor records by `pin` — the one merge every bridge uses.
*
* Same kind on the same pin merges per FIELD. Two registration paths describe
* one sensor knowing different halves: the part resolves its extra pins through
* the wire walk, the store's pre-registration knows the component's properties.
* Replacing the object let the coarser path silently drop `echo_pin`, and the
* QEMU worker then fell back to TRIG+1 and pulsed a pin nobody was reading
* (the 1k/2k2 divider report). Whoever writes last still wins per field.
*
* A DIFFERENT kind on the same pin replaces outright: a DHT22 swapped for an
* HC-SR04 must not inherit a stale `echo_pin` that ownsSensorPin would then
* keep guarding against the host.
*/
export function upsertSensorRecords(
existing: ReadonlyArray<Record<string, unknown>>,
incoming: ReadonlyArray<Record<string, unknown>>,
): Array<Record<string, unknown>> {
const merged = existing.slice();
for (const s of incoming) {
const idx = merged.findIndex((e) => e['pin'] === s['pin']);
if (idx < 0) merged.push(s);
else merged[idx] = merged[idx]['sensor_type'] === s['sensor_type'] ? { ...merged[idx], ...s } : s;
}
return merged;
}
const API_BASE = (): string => {
// The desktop shell injects the sidecar URL at runtime (random port) via
@ -622,21 +666,7 @@ export class Esp32Bridge {
* 5.65" UC8159c panel sat unresponsive while its firmware busy-waited.
*/
setSensors(sensors: Array<Record<string, unknown>>): void {
const merged = this._pendingSensors.slice();
for (const s of sensors) {
const pin = s['pin'];
const idx = merged.findIndex((e) => e['pin'] === pin);
// FIELD-level merge, not object replacement. The two registration paths
// see different amounts: the part resolves the sensor's extra pins
// through the full wire walk, while the store's pre-registration knows
// the component's properties. Replacing the whole entry let the coarser
// one silently drop `echo_pin` — the QEMU worker then fell back to
// TRIG+1 and pulsed a pin nobody was reading (the 1k/2k2 divider
// report). Whoever writes last still wins per FIELD.
if (idx >= 0) merged[idx] = { ...merged[idx], ...s };
else merged.push(s);
}
this._pendingSensors = merged;
this._pendingSensors = upsertSensorRecords(this._pendingSensors, sensors);
}
/** Returns true if a firmware has been loaded and is ready to send. */
@ -681,18 +711,13 @@ export class Esp32Bridge {
* the pin correctly. Same shape as the in-browser engines' ownsPin guard.
*/
ownsSensorPin(gpioPin: number): boolean {
for (const s of this._pendingSensors) {
// ONLY the single-wire sensors own a pad. The same channel registers
// plenty of other things — an ePaper panel's DC/BUSY pins, a membrane
// keypad's rows, every I2C device on a virtual 200+addr pin — and those
// still need the host to drive their real GPIOs. Blocking those was the
// difference between this guard and the in-browser engines' narrow
// SingleWireSensorHub.ownsPin, which is the behaviour to match.
if (!SINGLE_WIRE_SENSOR_TYPES.has(String(s['sensor_type']))) continue;
if (s['pin'] === gpioPin) return true;
if (s['echo_pin'] === gpioPin) return true;
}
return false;
// ONLY the single-wire sensors own a pad. The same channel registers plenty
// of other things — an ePaper panel's DC/BUSY pins, a membrane keypad's
// rows, every I2C device on a virtual 200+addr pin — and those still need
// the host to drive their real GPIOs. Blocking those was the difference
// between this guard and the in-browser engines' narrow
// SingleWireSensorHub.ownsPin, which is the behaviour to match.
return this._pendingSensors.some((s) => sensorRecordOwnsPin(s, gpioPin));
}
/** Drive a GPIO pin from an external source (e.g. connected Arduino) */
@ -800,12 +825,7 @@ export class Esp32Bridge {
// before the WebSocket opens (the common case when attachEvents fires
// before the user clicks Run).
const entry = { sensor_type: sensorType, pin, ...properties };
const existing = this._pendingSensors.findIndex((s) => s['pin'] === pin);
if (existing >= 0) {
this._pendingSensors[existing] = entry;
} else {
this._pendingSensors.push(entry);
}
this._pendingSensors = upsertSensorRecords(this._pendingSensors, [entry]);
// Also send immediately if already connected (re-attach on hot reload)
if (this._connected) {
this._send({ type: 'esp32_sensor_attach', data: entry });

View File

@ -271,18 +271,35 @@ export function traceDetailed(
state: TraceState,
fromId: string,
fromPin: string,
depth: number,
depth = 0,
activeSeen = false,
visited?: Map<string, number>,
): TraceResult {
// The ROOT call is the only one allowed to mint a chip-net / own-chip-pin
// fallback (see rule 5) — the guard `depth === 0` used to give that, and
// no longer can now that a node is collected rather than walked.
const isRoot = visited === undefined;
// Visits are recorded WITH their depth: a node first reached deep, where the
// hop budget cut its continuations short, must still be walkable when a
// shorter route arrives later.
const seen = visited ?? new Map<string, number>();
return walk(state, fromId, fromPin, depth, activeSeen, new Map(), true, undefined);
}
/**
* The recursion behind traceDetailed / traceBoardGpio.
*
* - `seen` records each visit WITH its depth: a node first reached deep,
* where the hop budget cut its continuations short, must still be walkable
* when a shorter route arrives later.
* - `isRoot` marks the outermost frame, the only one allowed to mint a
* chip-net / own-chip-pin fallback (rule 5) — `depth === 0` used to say
* that, and cannot now that a node is collected rather than walked.
* - `preferBoard` breaks a tie between driven pads of DIFFERENT boards on one
* node, so a caller asking about a specific board gets that board's pad
* when the net reaches both (a sensor shared by two MCUs).
*/
function walk(
state: TraceState,
fromId: string,
fromPin: string,
depth: number,
activeSeen: boolean,
seen: Map<string, number>,
isRoot: boolean,
preferBoard: string | undefined,
): TraceResult {
const selfKey = pinKey(fromId, fromPin);
const seenAt = seen.get(selfKey);
if (seenAt !== undefined && seenAt <= depth) {
@ -290,7 +307,7 @@ export function traceDetailed(
}
if (depth > MAX_HOPS) return { arduinoPin: null, crossedActiveDevice: activeSeen };
const node = collectNode(state, fromId, fromPin, seen, depth);
const node = collectNode(state, fromId, fromPin, seen, depth, preferBoard);
// ── Rules 1 and 2: the pads on this node ─────────────────────────────────
if (node.drivenHit) return { ...node.drivenHit, crossedActiveDevice: activeSeen };
@ -306,7 +323,7 @@ export function traceDetailed(
if (!pair || !p.comp) continue;
const farPin = p.pinName === pair[0] ? pair[1] : pair[0];
const nowActive = activeSeen || isActiveDevice(p.comp.metadataId);
const across = traceDetailed(state, p.comp.id, farPin, depth + 1, nowActive, seen);
const across = walk(state, p.comp.id, farPin, depth + 1, nowActive, seen, false, preferBoard);
if (isBoardPin(across.arduinoPin)) return across;
if (across.arduinoPin === RAIL) crossedRail ??= across;
else if (across.arduinoPin !== null) chipHit ??= across;
@ -359,11 +376,22 @@ function collectNode(
fromPin: string,
seen: Map<string, number>,
depth: number,
preferBoard: string | undefined,
): NodeInfo {
const pins: NodeInfo['pins'] = [];
const boardHits: TraceResult[] = [];
// A driven pad of the preferred board ends collection at once; one of any
// other board is kept and only wins if the preferred board never shows up.
let drivenHit: TraceResult | null = null;
let chipPin: NodeInfo['chipPin'] = null;
const takeDriven = (hit: TraceResult): boolean => {
if (preferBoard === undefined || hit.boardId === preferBoard) {
drivenHit = hit;
return true;
}
drivenHit ??= hit;
return false;
};
const local = new Set<string>();
const queue: Array<{ componentId: string; pinName: string }> = [
{ componentId: fromId, pinName: fromPin },
@ -391,8 +419,11 @@ function collectNode(
const socket = traceThroughSocket(state, cur.componentId, cur.pinName);
if (socket) {
const hit = { arduinoPin: socket.pin, crossedActiveDevice: false, boardId: socket.boardId };
if (isBoardPin(socket.pin)) return { pins, boardHits, drivenHit: hit, chipPin };
boardHits.push(hit);
if (isBoardPin(socket.pin)) {
if (takeDriven(hit)) return { pins, boardHits, drivenHit, chipPin };
} else {
boardHits.push(hit);
}
}
// Breadboards join N holes per internal group (5-hole strip / power rail):
@ -437,8 +468,11 @@ function collectNode(
};
// A driven pad ends the search for this node — and, on a big shared
// rail, ends a walk of every leg hanging off it.
if (isBoardPin(pin)) return { pins, boardHits, drivenHit: hit, chipPin };
boardHits.push(hit);
if (isBoardPin(pin)) {
if (takeDriven(hit)) return { pins, boardHits, drivenHit, chipPin };
} else {
boardHits.push(hit);
}
}
continue;
}
@ -459,6 +493,11 @@ function collectNode(
* supply pad, a custom-chip net, or nothing at all. This is the shape a caller
* that wants to talk to REAL hardware wants: a rail and a synthetic chip pin
* are both "not a GPIO on this board".
*
* `boardId` is a PREFERENCE the walk honours on the node, not just a filter on
* the way out: when one net reaches two boards' GPIOs (a sensor shared by two
* MCUs) the caller asking about board A gets A's pad, and only a net that
* truly never reaches A answers null for it.
*/
export function traceBoardGpio(
state: TraceState,
@ -466,7 +505,7 @@ export function traceBoardGpio(
pinName: string,
boardId?: string,
): number | null {
const hit = traceDetailed(state, componentId, pinName, 0);
const hit = walk(state, componentId, pinName, 0, false, new Map(), true, boardId);
if (hit.arduinoPin === null) return null;
if (hit.arduinoPin < 0 || hit.arduinoPin >= SYNTHETIC_CHIP_PIN_BASE) return null;
if (boardId !== undefined && hit.boardId !== undefined && hit.boardId !== boardId) return null;

View File

@ -98,10 +98,14 @@ export function connectDigitalInputsToMcu(): () => void {
// user adds the pull-up or the level divider real hardware needs, the
// net is component-backed and solves at whatever the passives say,
// which is not what the part is driving.
// Nothing is cached for a skipped pin on purpose: the connector must
// stay free to emit the moment the part lets go of the line.
if (isPartOwnedPin(board.id, gpio)) continue;
if (sim.ownsPin?.(gpio)) continue; // a backend sensor drives this pad
if (isPartOwnedPin(board.id, gpio) || sim.ownsPin?.(gpio)) {
// Forget what we last pushed here. While the part drove the line the
// guest's level was whatever the part made it, so the moment the
// part lets go (rewire, unmount, detach) the next solve must emit
// even if the circuit happens to agree with our stale memory.
lastLevel.delete(`${board.id}:${gpio}`);
continue;
}
// Only drive pins whose net is backed by a real source/element (rail,
// pull, button switch, divider, cross-board output, …). A net that is
// only floating (an event-driven part like a rotary encoder / keypad