From 5c95ade35f76789fd4e325898ade231aef00a728 Mon Sep 17 00:00:00 2001 From: David Montero Crespo Date: Sun, 16 Aug 2026 20:02:58 +0200 Subject: [PATCH] =?UTF-8?q?fix(sim):=20stop=20dropping=20sibling=20pin=20e?= =?UTF-8?q?dges=20=E2=80=94=20only=20bit=200=20of=20a=20multi-pin=20write?= =?UTF-8?q?=20reached=20the=20canvas?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A sketch that drives several pins between yields lost every pin but the first. The 4-bit binary counter example (the most-viewed one in the gallery) blinked bit 0 and left bits 1-3 dark while the firmware counted 0-15 correctly on serial. Sequence: pin 2's edge arrives first, finds no V-source (the pin was not in outputPins when the netlist was built), self-heals via handleMcuEdge and rebuilds. The pin 3/4/5 edges of that same iteration land while the rebuild solve is in flight, so they queue in pendingMcuEdges. The drain then looked them up in the loaded context, found no V_uno_3/4/5, and dropped them. Those pins never got a source, so their LEDs kept the build-time current (~0) forever — and since the LED part prefers SPICE current over the digital fallback, they stayed dark for good. The drop was deliberate: it stops the self-heal -> tick -> self-heal loop for a pin wired to nothing. Replace the blanket drop with a per-pin guard: heal a pin at most once per netlist, then drop as before. The guard re-arms whenever a rebuild changes the V-source list, so a pin that becomes drivable later still heals. Verified in the browser on binary-counter-leds; two regression tests cover both sides (sibling pin heals, unwired pin does not loop). --- .../circuit-simulation-service.test.ts | 89 +++++++++++++++++++ .../spice/CircuitSimulationService.ts | 49 +++++++++- 2 files changed, 137 insertions(+), 1 deletion(-) diff --git a/frontend/src/__tests__/circuit-simulation-service.test.ts b/frontend/src/__tests__/circuit-simulation-service.test.ts index 2394b3a6..443781c1 100644 --- a/frontend/src/__tests__/circuit-simulation-service.test.ts +++ b/frontend/src/__tests__/circuit-simulation-service.test.ts @@ -471,6 +471,95 @@ describe('handleMcuEdge (Phase 1c D1)', () => { expect(fake.calls.loadCircuit.length).toBe(1); expect(fake.calls.alterSource).toEqual([]); }); + + it('heals a sibling pin that was classified as an output mid-solve', async () => { + // Regression (binary-counter-leds, the most-viewed gallery example): + // a sketch writing several pins between yields lost every pin but the + // first. Sequence: pin 2's edge lands first, self-heals, and rebuilds + // the netlist; pins 3-5 arrive while that solve is in flight, so they + // queue; the drain then found no V_uno_3 in the loaded context and + // DROPPED them. Those pins never got a source and stayed at their + // build-time voltage forever — bit 0 blinked, bits 1-3 stayed dark + // while the firmware counted correctly on serial. + // + // The drain must self-heal (re-queue + rebuild) like handleMcuEdge + // does, once per pin per netlist. + const fake = new FakeSolverAdapter({ + vectors: { 'v(vcc_rail)': 5 }, + solveDelayMs: 30, + }); + __setSchedulerSolverFactoryForTests(() => fake); + // Pin 3 is NOT an MCU output at build time — it becomes one only once + // the firmware first drives it, exactly like PinManager.outputPins. + let pin3IsOutput = false; + const sim = makeSimStore({ + components: [ + { id: 'r2', metadataId: 'resistor', properties: { value: '220' } }, + { id: 'r3', metadataId: 'resistor', properties: { value: '220' } }, + ], + wires: [ + { id: 'w2a', start: { componentId: 'uno', pinName: '2' }, end: { componentId: 'r2', pinName: '1' } }, + { id: 'w2b', start: { componentId: 'r2', pinName: '2' }, end: { componentId: 'uno', pinName: 'GND' } }, + { id: 'w3a', start: { componentId: 'uno', pinName: '3' }, end: { componentId: 'r3', pinName: '1' } }, + { id: 'w3b', start: { componentId: 'r3', pinName: '2' }, end: { componentId: 'uno', pinName: 'GND' } }, + ], + boards: [{ id: 'uno', boardKind: 'arduino-uno' }], + }); + const elec = makeElectricalStore(); + const service = new CircuitSimulationService( + sim.port, + elec.port, + getMixedModeScheduler() as unknown as MixedModeSchedulerPort, + { + collectBoardPinStates: () => + pin3IsOutput + ? { '2': { type: 'digital', v: 0 }, '3': { type: 'digital', v: 0 } } + : { '2': { type: 'digital', v: 0 } }, + }, + ); + startTracked(service); + // Let the initial netlist get built (without pin 3) while its solve + // is still in flight. + await new Promise((r) => setTimeout(r, 5)); + pin3IsOutput = true; + void service.handleMcuEdge('uno', '2', true, 5); + void service.handleMcuEdge('uno', '3', true, 5); + // Drain gap (33 ms) + rebuild + the healed edge's own solve. + await new Promise((r) => setTimeout(r, 300)); + + const altered = fake.calls.alterSource.map(([name]) => name); + expect(altered).toContain('V_uno_3'); + }); + + it('does not loop forever healing a pin that can never be sourced', async () => { + // The flip side of the heal above: an unwired GPIO never gets a + // V-source no matter how many rebuilds run. The per-pin guard must + // let it through at most once, then go back to dropping — otherwise + // heal -> tick -> heal spins the solver at 100% duty. + const fake = new FakeSolverAdapter({ + vectors: { 'v(vcc_rail)': 5 }, + solveDelayMs: 5, + }); + __setSchedulerSolverFactoryForTests(() => fake); + const sim = makeSimStore(simpleBoardWithBoard); + const elec = makeElectricalStore(); + const service = new CircuitSimulationService( + sim.port, + elec.port, + getMixedModeScheduler() as unknown as MixedModeSchedulerPort, + // Pin 7 is claimed as an output but is wired to nothing, so + // buildNetlist never emits V_uno_7. + { collectBoardPinStates: () => ({ '7': { type: 'digital', v: 0 } }) }, + ); + startTracked(service); + await new Promise((r) => setTimeout(r, 20)); + const before = fake.calls.solve.length; + void service.handleMcuEdge('uno', '7', true, 5); + await new Promise((r) => setTimeout(r, 400)); + // A bounded number of extra solves — not one per 33 ms drain gap + // (~12 over this window) and nowhere near a runaway. + expect(fake.calls.solve.length - before).toBeLessThanOrEqual(4); + }); }); describe('CircuitSimulationService — error handling', () => { diff --git a/frontend/src/simulation/spice/CircuitSimulationService.ts b/frontend/src/simulation/spice/CircuitSimulationService.ts index b155cfc7..a54756bb 100644 --- a/frontend/src/simulation/spice/CircuitSimulationService.ts +++ b/frontend/src/simulation/spice/CircuitSimulationService.ts @@ -150,6 +150,13 @@ export class CircuitSimulationService { private drainTimer: ReturnType | null = null; private static readonly EDGE_DRAIN_GAP_MS = 33; + /** Pins the drain has already forced one netlist rebuild for, keyed + * `${boardId}|${pinName}`. See `scheduleEdgeDrain` — this is what keeps + * the self-heal from looping forever on a pin that can never get a + * V-source (unwired GPIO). Cleared whenever a rebuild changes the + * V-source list, so a pin that becomes wired later heals again. */ + private drainHealAttempted = new Set(); + /** Drain pendingMcuEdges after a short gap (coalescing: one timer). */ private scheduleEdgeDrain(): void { if (this.stopped || this.drainTimer !== null) return; @@ -159,14 +166,43 @@ export class CircuitSimulationService { const edges = Array.from(this.pendingMcuEdges.values()); this.pendingMcuEdges.clear(); const ctx = this.loadedContext; + let needsRebuild = false; for (const edge of edges) { + const pinKey = `${edge.boardId}|${edge.pinName}`; const expected = `v_${sanitizeSpiceId(edge.boardId)}_${sanitizeSpiceId(edge.pinName)}`.toLowerCase(); const hasSource = ctx?.voltageSources.some( (vs) => vs.toLowerCase() === expected, ); - if (!hasSource) continue; + if (!hasSource) { + // Same self-heal `handleMcuEdge` does for a first edge: the pin + // wasn't in `outputPins` when this netlist was built, so there is + // no V-source to alter and `alterSource` would be a silent no-op. + // Re-queue and rebuild instead of dropping the edge. + // + // Dropping was deliberate (it stops the self-heal → tick → + // self-heal loop for a pin that is wired to nothing), but it also + // silently killed pins that only LOOK unsourced because they were + // classified as outputs after this netlist was built. That is the + // common case for any sketch driving several pins between yields: + // the first pin's edge triggers the rebuild, every sibling edge + // lands mid-solve, and the drain then threw them away — so those + // pins stayed at their build-time voltage forever. Symptom: the + // 4-bit binary counter example blinked bit 0 and left bits 1-3 + // dark while the firmware counted correctly on serial. + // + // The loop guard is now per-pin instead of blanket: heal a given + // pin at most once per netlist, then fall back to dropping. + if (this.drainHealAttempted.has(pinKey)) continue; + this.drainHealAttempted.add(pinKey); + this.pendingMcuEdges.set(pinKey, edge); + needsRebuild = true; + continue; + } + this.drainHealAttempted.delete(pinKey); void this.handleMcuEdge(edge.boardId, edge.pinName, edge.state, edge.vcc); } + // One rebuild for the whole drain, not one per edge. + if (needsRebuild) void this.tick(); }, CircuitSimulationService.EDGE_DRAIN_GAP_MS); } @@ -198,6 +234,7 @@ export class CircuitSimulationService { this.stopped = true; this.pending = false; this.pendingMcuEdges.clear(); + this.drainHealAttempted.clear(); if (this.drainTimer !== null) { clearTimeout(this.drainTimer); this.drainTimer = null; @@ -347,6 +384,16 @@ export class CircuitSimulationService { // Cache the load context so handleMcuEdge can publish without // re-running buildInputFromStore + buildNetlist. + // + // A rebuild that changed the V-source list is a new topology as far as + // the drain's per-pin heal guard is concerned: re-arm it so a pin that + // has just become drivable (newly wired, or newly classified as an + // output) can heal again instead of staying permanently written off. + const sourcesChanged = + this.loadedContext === null || + this.loadedContext.voltageSources.length !== voltageSources.length || + this.loadedContext.voltageSources.some((vs, i) => vs !== voltageSources[i]); + if (sourcesChanged) this.drainHealAttempted.clear(); this.loadedContext = { pinNetMap, nets,