From 33721514054ea837dc9f0d2deb17bfe15fa1d879 Mon Sep 17 00:00:00 2001 From: David Montero Date: Wed, 17 Jun 2026 20:37:32 +0200 Subject: [PATCH] fix(sim): circuit verifier was silently blind to current faults in prod The pre-flight circuit verifier reads branch currents via runNetlist -> readAllCurrentVectors() (ngSpice_AllVecs enumeration). The production Web-Worker ngspice WASM build does not surface voltage-source #branch vectors through that enumeration for an .op plot, so branchCurrents came back empty and every current rule (short-circuit, LED over-current) read ?? 0 -> no fault. The live solver avoided this by requesting each current explicitly by name; the Node test build enumerates them, so the gap was invisible to the suite. Net effect: a 9V battery wired straight to an LED ran with no warning (reported on project 2840fd12). - runNetlist: request every V_* source branch current explicitly by name and merge with the enumeration, so source/LED currents are always present regardless of the worker WASM's AllVecs behaviour. - circuitVerifier: non-finite source/LED current -> blocking unstable-solve fault ("could not solve a stable current - likely a short or a part with no current limit, e.g. an LED with no series resistor"). - LED runtime (BasicParts): burn out on a non-finite current instead of falling through to the digital fallback and glowing; raise burnout threshold 20mA -> 100mA so high-power/RGB channels are not falsely destroyed; clear the burnt latch on Reset (resetBoard bumps hexEpoch). Tests: real-data repro, mocked non-finite verifier test, runtime non-finite / high-power / latch-recovery tests. --- .../circuit-verifier-nonfinite.test.ts | 64 ++++++++++++ .../src/__tests__/simulation-parts.test.ts | 86 ++++++++++++++++ .../src/components/editor/EditorToolbar.tsx | 11 +++ frontend/src/simulation/parts/BasicParts.ts | 97 +++++++++++++++++-- frontend/src/simulation/spice/runNetlist.ts | 40 +++++++- .../src/simulation/verify/circuitVerifier.ts | 34 ++++++- frontend/src/store/useSimulatorStore.ts | 6 ++ 7 files changed, 324 insertions(+), 14 deletions(-) create mode 100644 frontend/src/__tests__/circuit-verifier-nonfinite.test.ts diff --git a/frontend/src/__tests__/circuit-verifier-nonfinite.test.ts b/frontend/src/__tests__/circuit-verifier-nonfinite.test.ts new file mode 100644 index 00000000..fade88d9 --- /dev/null +++ b/frontend/src/__tests__/circuit-verifier-nonfinite.test.ts @@ -0,0 +1,64 @@ +/** + * circuitVerifier — the "cannot emulate" path. + * + * When ngspice cannot find a stable operating point it returns NaN/Infinity + * for the offending branch current (the classic case: an LED with no series + * resistor — a near-short across the supply). The verifier must surface that + * as a BLOCKING `unstable-solve` fault, NOT silently treat it as 0 A and wave + * the circuit through (the production bug behind the 9V→LED report). + * + * The real Node ngspice build always converges this circuit, so we mock the + * solver to deterministically return non-finite currents. + */ +import { describe, it, expect, vi } from 'vitest'; + +vi.mock('../simulation/spice/runNetlist', () => ({ + runNetlist: vi.fn(async () => ({ + variableNames: ['v(n0)', 'v(n1)', 'i(v_bat)', 'i(v_led1_sense)'], + dcValue: (name: string) => { + switch (name) { + case 'v(n0)': + return 1.05; + case 'v(n1)': + return -1.05; + case 'i(v_bat)': + return NaN; // source current — no stable solution + case 'i(v_led1_sense)': + return Infinity; // LED forward current — no stable solution + default: + return 0; + } + }, + vec: () => [], + vAtLast: () => 0, + findVar: () => -1, + })), +})); + +import { verifyCircuit } from '../simulation/verify/circuitVerifier'; +import type { BuildNetlistInput } from '../simulation/spice/types'; + +describe('verifyCircuit — non-finite solve → unstable-solve fault', () => { + it('blocks a 9V battery wired straight to an LED (no resistor)', async () => { + const input: BuildNetlistInput = { + components: [ + { id: 'bat', metadataId: 'battery-9v', properties: {} }, + { id: 'led1', metadataId: 'led', properties: { color: 'red' } }, + ], + wires: [ + { id: 'w1', start: { componentId: 'bat', pinName: '+' }, end: { componentId: 'led1', pinName: 'A' } }, + { id: 'w2', start: { componentId: 'led1', pinName: 'C' }, end: { componentId: 'bat', pinName: '−' } }, + ], + boards: [], + analysis: { kind: 'op' }, + }; + const result = await verifyCircuit(input); + const codes = result.errors.map((e) => e.code); + // Both the source and the LED report the unstable solve — either is enough + // to block, and both must be `unstable-solve` (not silently dropped to 0 A). + expect(codes, JSON.stringify(result.errors)).toContain('unstable-solve'); + expect(result.errors.length).toBeGreaterThan(0); + const ledFault = result.errors.find((e) => e.componentId === 'led1'); + expect(ledFault?.code).toBe('unstable-solve'); + }); +}); diff --git a/frontend/src/__tests__/simulation-parts.test.ts b/frontend/src/__tests__/simulation-parts.test.ts index 32a86bd1..76bdb5d9 100644 --- a/frontend/src/__tests__/simulation-parts.test.ts +++ b/frontend/src/__tests__/simulation-parts.test.ts @@ -17,6 +17,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { PartSimulationRegistry } from '../simulation/parts/PartSimulationRegistry'; +import { useElectricalStore } from '../store/useElectricalStore'; // Side-effect imports — register all parts import '../simulation/parts/BasicParts'; @@ -184,6 +185,91 @@ describe('LED — attachEvents (anode + cathode check)', () => { }); }); +// ─── LED overcurrent burnout (SPICE forward current) ────────────────────────── +describe('LED — overcurrent burnout', () => { + afterEach(() => { + useElectricalStore.setState({ branchCurrents: {}, timeWaveforms: undefined }); + }); + + function attachLed(id: string) { + const logic = PartSimulationRegistry.get('led')!; + const el = makeElement({ value: false, brightness: 0 }); + const sim = makeSimulator(); + logic.attachEvents!(el, sim as any, pinMap({ A: 13, C: -1 }), id); + const anode = sim.pinManager.onPinChange.mock.calls.find((c: any) => c[0] === 13)![1]; + return { el: el as any, trigger: () => anode(13, true) }; + } + + it('burns out (goes dark) at destructive forward current', () => { + const { el, trigger } = attachLed('led-burn'); + useElectricalStore.setState({ branchCurrents: { 'v_led-burn_sense': 4.6 } }); // 9V/no resistor + trigger(); + expect(el.brightness).toBe(0); + expect(el.value).toBe(false); + // latched: stays dark even if the current later drops to a safe value + useElectricalStore.setState({ branchCurrents: { 'v_led-burn_sense': 0.01 } }); + trigger(); + expect(el.brightness).toBe(0); + expect(el.value).toBe(false); + }); + + it('does NOT burn out at a normal bright current (15 mA)', () => { + const { el, trigger } = attachLed('led-ok'); + useElectricalStore.setState({ branchCurrents: { 'v_led-ok_sense': 0.015 } }); + trigger(); + expect(el.value).toBe(true); + expect(el.brightness).toBeCloseTo(0.75, 2); // 15 mA / 20 mA rated + }); + + it('does NOT burn out just over the rated max (25 mA) — only destructive current', () => { + const { el, trigger } = attachLed('led-warm'); + useElectricalStore.setState({ branchCurrents: { 'v_led-warm_sense': 0.025 } }); + trigger(); + expect(el.value).toBe(true); + expect(el.brightness).toBe(1); // bright (clamped), still alive + }); + + it('does NOT burn out at a legitimate high-power current (80 mA)', () => { + // High-power / RGB channels can pull ~100-150 mA legitimately; the burnout + // threshold (100 mA) must sit above the bright-but-fine range. + const { el, trigger } = attachLed('led-hp'); + useElectricalStore.setState({ branchCurrents: { 'v_led-hp_sense': 0.08 } }); + trigger(); + expect(el.value).toBe(true); + expect(el.brightness).toBe(1); + }); + + it('burns out when the solver returns a non-finite current (no-resistor short)', () => { + // A diode straight across a supply with no series resistor often has no + // stable operating point → ngspice returns NaN/Infinity. That must burn + // the LED out, NOT fall through to the digital fallback and glow. + const { el, trigger } = attachLed('led-nan'); + useElectricalStore.setState({ branchCurrents: { 'v_led-nan_sense': NaN } }); + trigger(); + expect(el.value).toBe(false); + expect(el.brightness).toBe(0); + // latched + useElectricalStore.setState({ branchCurrents: { 'v_led-nan_sense': 0.01 } }); + trigger(); + expect(el.value).toBe(false); + }); + + it('recovers after a fresh re-attach (Reset bumps hexEpoch → new closure)', () => { + // Burn one instance... + const first = attachLed('led-fix'); + useElectricalStore.setState({ branchCurrents: { 'v_led-fix_sense': 4.6 } }); + first.trigger(); + expect(first.el.value).toBe(false); + // ...fix the circuit, then re-attach (what a Reset does via hexEpoch). + // The new closure starts un-burnt, so the now-safe circuit lights again. + useElectricalStore.setState({ branchCurrents: { 'v_led-fix_sense': 0.015 } }); + const second = attachLed('led-fix'); + second.trigger(); + expect(second.el.value).toBe(true); + expect(second.el.brightness).toBeCloseTo(0.75, 2); + }); +}); + // ─── Pushbutton ────────────────────────────────────────────────────────────── describe('Pushbutton — attachEvents', () => { diff --git a/frontend/src/components/editor/EditorToolbar.tsx b/frontend/src/components/editor/EditorToolbar.tsx index dec9a7e8..36feaeb4 100644 --- a/frontend/src/components/editor/EditorToolbar.tsx +++ b/frontend/src/components/editor/EditorToolbar.tsx @@ -242,6 +242,17 @@ export const EditorToolbar = ({ return () => window.removeEventListener('velxio-open-library-manager', open); }, []); + // Surface a runtime circuit fault (e.g. an LED that burnt out from + // overcurrent during the live SPICE solve) as an inline message. + useEffect(() => { + const onFault = (e: Event) => { + const detail = (e as CustomEvent).detail as { message?: string } | undefined; + if (detail?.message) setMessage({ type: 'error', text: detail.message }); + }; + window.addEventListener('velxio-circuit-fault', onFault); + return () => window.removeEventListener('velxio-circuit-fault', onFault); + }, []); + useEffect(() => { if (!moreMenuOpen) return; const onClickOutside = (e: MouseEvent) => { diff --git a/frontend/src/simulation/parts/BasicParts.ts b/frontend/src/simulation/parts/BasicParts.ts index c0e342ab..76428465 100644 --- a/frontend/src/simulation/parts/BasicParts.ts +++ b/frontend/src/simulation/parts/BasicParts.ts @@ -150,6 +150,51 @@ PartSimulationRegistry.register('dip-switch-8', { * connected to GND (or a LOW GPIO). If the cathode is not wired at all the * LED stays off regardless of the anode state. */ + +// A real 5mm indicator LED survives ~20 mA (datasheet absolute max ~30 mA). +// A sustained forward current well above that destroys it within moments — +// the classic "LED straight across a 9V battery with no series resistor" +// mistake. A professional simulator must model that, not glow happily. Once +// the solved forward current crosses LED_BURNOUT_A the LED burns out (goes +// dark and stays dark for the rest of the run) and a fault message is shown. +// +// The burnout threshold (100 mA) sits well above both the 20 mA rating AND the +// ~100-150 mA a high-power / RGB channel may legitimately draw, so a merely +// bright LED is never falsely destroyed — only a missing or grossly-undersized +// series resistor trips it. The pre-flight circuitVerifier already warns at the +// 20 mA datasheet limit BEFORE the run starts (the primary, professional +// check); this runtime burnout is the last-resort net for users who click +// "Run Anyway" past that warning, or for faults that only appear mid-run. +const LED_RATED_MAX_A = 0.02; +const LED_BURNOUT_A = 0.1; + +/** Surface a circuit fault for an LED — console + a UI event the toolbar shows. */ +function reportLedFault(componentId: string, kind: string, message: string): void { + console.warn(`[led] ${componentId}: ${message}`); + if (typeof window !== 'undefined' && typeof window.dispatchEvent === 'function') { + try { + window.dispatchEvent( + new CustomEvent('velxio-circuit-fault', { + detail: { componentId, kind, message }, + }), + ); + } catch { + /* CustomEvent unavailable (test env) — the console.warn is enough */ + } + } +} + +function reportLedBurnout(componentId: string, current: number): void { + const mA = current * 1000; + const amount = mA >= 1000 ? `${(mA / 1000).toFixed(1)} A` : `${mA.toFixed(0)} mA`; + reportLedFault( + componentId, + 'led-burnout', + `LED burnt out — it drew ${amount}, far above its ~20 mA limit. ` + + `Add a series resistor between the supply and the LED.`, + ); +} + PartSimulationRegistry.register('led', { attachEvents: (element, simulator, getArduinoPinHelper, componentId, getPinResolver) => { const pinManager = (simulator as any).pinManager; @@ -177,9 +222,19 @@ PartSimulationRegistry.register('led', { // dies for good (useful diagnostic). let lastSpiceBrightness = 0; let lastSpiceTs = 0; + // Latches once the LED is destroyed by overcurrent; reset only when the + // part re-attaches (a fresh Run / reset re-arms it). + let burnt = false; const HOLD_MS = 500; const update = () => { + // A burnt-out LED stays dark for the rest of the run, no matter what + // the solver reports next. + if (burnt) { + el.value = false; + el.brightness = 0; + return; + } // SPICE is always active. Use real branch current for analog // brightness (0..1). The SPICE mapper emits a V-sense zero-volt // source in series with the diode (`V__sense`) so @@ -207,18 +262,40 @@ PartSimulationRegistry.register('led', { raw = sum / samples.length; } } - // Guard against NaN / Infinity coming back from ngspice. They - // happen on degenerate circuits (a forward-biased diode with no - // series resistor — the textbook "missing 220Ω" mistake — is - // the most common case). Without this guard the LED would mark - // `el.value = NaN > 1e-6 = false` and stay visually dark even - // when the user clicks "Run Anyway" past the verifier warning. - // Treat non-finite branch currents as "SPICE has nothing useful - // to say" → fall through to the digital fallback so at least the - // LED visually lights when its driver pin is HIGH. + // A non-finite branch current (NaN / Infinity) that ngspice actually + // returned is NOT "no data" — it means the solver could not find a + // stable operating point for this LED. In practice that is the textbook + // degenerate circuit: a forward-biased diode with no series resistor (a + // near-short across the supply). Burn the LED out rather than silently + // glowing via the digital fallback (the old behaviour, which let the + // "missing 220Ω" mistake light up as if it were fine). `raw === undefined` + // is different — that is the engine warming up, handled by the HOLD / + // digital-fallback path below. + if (raw !== undefined && !Number.isFinite(raw)) { + burnt = true; + el.value = false; + el.brightness = 0; + reportLedFault( + componentId, + 'led-burnout', + `LED destroyed — the circuit has no stable solution (the solver returned ` + + `an undefined current). This almost always means the LED has no series ` + + `resistor. Add a resistor between the supply and the LED.`, + ); + return; + } if (raw !== undefined && Number.isFinite(raw)) { const current = Math.abs(raw); - lastSpiceBrightness = Math.min(1, current / 0.02); + // Destructive overcurrent → burn the LED out (and tell the user why). + // Latches; the top-of-update guard keeps it dark from here on. + if (current > LED_BURNOUT_A) { + burnt = true; + el.value = false; + el.brightness = 0; + reportLedBurnout(componentId, current); + return; + } + lastSpiceBrightness = Math.min(1, current / LED_RATED_MAX_A); lastSpiceTs = Date.now(); el.value = current > 1e-6; el.brightness = lastSpiceBrightness; diff --git a/frontend/src/simulation/spice/runNetlist.ts b/frontend/src/simulation/spice/runNetlist.ts index 6687dda2..39227c09 100644 --- a/frontend/src/simulation/spice/runNetlist.ts +++ b/frontend/src/simulation/spice/runNetlist.ts @@ -112,18 +112,52 @@ export async function runNetlist(netlist: string): Promise { await adapter.init(); await adapter.loadCircuit(netlist); const analysis = detectAnalysis(netlist); + + // Every voltage source `V_` exposes its branch current as the + // ngspice vector `v_#branch` (legacy form `i(v_)`). These are + // the currents the circuit verifier relies on (short-circuit, LED + // overcurrent, …). We request them EXPLICITLY by name rather than + // depending on `readAllCurrentVectors`/`ngSpice_AllVecs` enumeration: + // the production Web-Worker WASM build does NOT surface source `#branch` + // vectors through `AllVecs` for an `.op` plot, so an enumeration-only + // read leaves every branch current missing in prod (the live solver + // works around this the same way — see CircuitSimulationService / + // MixedModeScheduler.setExtraVectorsOfInterest). Node test builds DO + // enumerate them, which is why this gap was invisible to the suite. + const branchVectorsOfInterest = Array.from( + new Set( + Array.from(netlist.matchAll(/^[ \t]*(V\S+)/gim)).map( + (m) => `i(${m[1]!.toLowerCase()})`, + ), + ), + ); + // Single solve — populate the plot, then enumerate + read every // vector via `readAllCurrentVectors` so the pointers stay valid. // (Re-running the analysis to read vectors would create a new // plot and invalidate everything.) - await adapter.solve(analysis, { vectorsOfInterest: [] }); + const solved = await adapter.solve(analysis, { + vectorsOfInterest: branchVectorsOfInterest, + }); const all = await (adapter as unknown as AdapterWithRead).readAllCurrentVectors(); + + // Merge: the enumeration is the base (node voltages, time axis, …) and + // the explicit branch reads are layered on top so source/LED currents + // are present even when `AllVecs` omits them. `solved.vectors` keys are + // the requested legacy names (`i(v_x)`); normalise to the ngspice raw + // key (`v_x#branch`) so `legacyNameFor`/`getVec` resolve consistently. + const mergedVectors = new Map(all.vectors); + for (const [k, v] of solved.vectors) { + const ngKey = ngspiceNameFor(k); + if (!mergedVectors.has(ngKey)) mergedVectors.set(ngKey, v); + } + const result = { analysis, - vectors: all.vectors, + vectors: mergedVectors, timeAxis: analysis.kind === 'tran' - ? all.vectors.get('time')?.real ?? new Float64Array(0) + ? mergedVectors.get('time')?.real ?? new Float64Array(0) : new Float64Array(0), solveMs: 0, warnings: [] as string[], diff --git a/frontend/src/simulation/verify/circuitVerifier.ts b/frontend/src/simulation/verify/circuitVerifier.ts index 7ce172d6..8ec2f03a 100644 --- a/frontend/src/simulation/verify/circuitVerifier.ts +++ b/frontend/src/simulation/verify/circuitVerifier.ts @@ -28,6 +28,7 @@ import type { BuildNetlistInput, ElectricalSolveResult } from '../spice/types'; export type WarningSeverity = 'error' | 'warning'; export type WarningCode = | 'solver-failed' + | 'unstable-solve' | 'short-circuit' | 'source-overload' | 'led-overcurrent' @@ -85,6 +86,14 @@ export async function verifyCircuit( const errors: CircuitWarning[] = []; const warnings: CircuitWarning[] = []; + // Branch-current vectors that the solver returned as NaN / Infinity. + // A non-finite branch current is not "no current" — it means ngspice + // could not find a stable operating point for that source (the classic + // case: a forward-biased LED with no series resistor, or a dead short). + // We must NOT silently treat these as 0 A; they get surfaced as a + // blocking "cannot emulate" fault below. + const nonFiniteBranches = new Set(); + // Run a forced .op solve so currents are scalar and deterministic. const opInput: BuildNetlistInput = { ...input, analysis: { kind: 'op' } }; const { netlist } = buildNetlist(opInput); @@ -101,7 +110,9 @@ export async function verifyCircuit( if (Number.isFinite(v)) nodeVoltages[name.slice(2, -1)] = v; } else if (name.startsWith('i(')) { const v = cooked.dcValue(name); - if (Number.isFinite(v)) branchCurrents[name.slice(2, -1)] = v; + const key = name.slice(2, -1); + if (Number.isFinite(v)) branchCurrents[key] = v; + else nonFiniteBranches.add(key); } } solve = { @@ -141,6 +152,18 @@ export async function verifyCircuit( /^(battery|signal-generator|power-supply)/.test(c.metadataId), ); for (const src of sourceComponents) { + // A non-finite source current means ngspice could not find a stable + // operating point — treat it as a blocking "cannot emulate" fault + // rather than waving the circuit through as 0 A. + if (nonFiniteBranches.has(`v_${src.id}`)) { + errors.push({ + severity: 'error', + code: 'unstable-solve', + componentId: src.id, + message: `Could not solve a stable current for ${src.metadataId} ${src.id} — the circuit has no stable operating point. This usually means a short circuit, or a part driven with no current limit (for example an LED with no series resistor). Check the wiring or add a series resistor.`, + }); + continue; + } const i = Math.abs(branchCurrents[`v_${src.id}`] ?? 0); const perInstanceLimit = src.metadataId === 'power-supply' @@ -168,6 +191,15 @@ export async function verifyCircuit( // that source is the LED forward current. const leds = input.components.filter((c) => c.metadataId === 'led'); for (const led of leds) { + if (nonFiniteBranches.has(`v_${led.id}_sense`)) { + errors.push({ + severity: 'error', + code: 'unstable-solve', + componentId: led.id, + message: `LED ${led.id} could not be solved — its forward current has no stable value. This almost always means the LED is wired with no series resistor (a near-short across the supply). Add a series resistor between the supply and the LED.`, + }); + continue; + } const i = Math.abs(branchCurrents[`v_${led.id}_sense`] ?? 0); if (i > config.ledMaxAmps) { errors.push({ diff --git a/frontend/src/store/useSimulatorStore.ts b/frontend/src/store/useSimulatorStore.ts index d220f757..d3823e32 100644 --- a/frontend/src/store/useSimulatorStore.ts +++ b/frontend/src/store/useSimulatorStore.ts @@ -1914,8 +1914,14 @@ export const useSimulatorStore = create((set, get) => { b.id === boardId ? { ...b, running: false, serialOutput: '', serialBaudRate: 0 } : b, ); const isActive = s.activeBoardId === boardId; + // Bump hexEpoch so every component part re-attaches with a fresh + // closure. Without this, latched per-part state (e.g. an LED's + // `burnt` flag after overcurrent) would survive a Reset and the + // part would stay dead even after the user fixes the circuit — + // only a recompile would clear it. Mirrors restartParts(). return { boards, + hexEpoch: s.hexEpoch + 1, ...(isActive ? { running: false, serialOutput: '', serialBaudRate: 0 } : {}), }; });