From 06526c79223084d6903f45d979da39d8bd4f9dae Mon Sep 17 00:00:00 2001 From: ciegovolador Date: Mon, 8 Jun 2026 21:58:49 -0300 Subject: [PATCH 1/5] =?UTF-8?q?fix(sim):=20sample-accurate=20buzzer=20audi?= =?UTF-8?q?o=20=E2=80=94=20precise=20PWM=20detection=20+=20display-aligned?= =?UTF-8?q?=20scheduling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A PWM-driven buzzer (analogWrite / Timer tones) was chaotic and unusable as a metronome. Causes, all on the PWM path: 1. PWM was polled once per animation frame AFTER the cycle loop, so short clicks that started and ended within one frame were merged or lost, and onsets were quantised to the frame. 2. The buzzer started the oscillator with `oscillator.start()` (no scheduled time) — frame-delivery jitter and per-onset oscillator churn. 3. The digital HIGH/LOW path also fired on the ~490Hz PWM carrier edges, injecting spurious onsets (OCR read as 0 → 20kHz squeaks). Fix: - AVRSimulator: poll PWM sub-frame (every 256 cycles) so no pulse is merged or lost; pass the precise simulated time through updatePwm. - PinManager: PwmCallback / updatePwm carry an optional timeMs (backward compat). - Buzzer: one continuous oscillator gated by the gain node, each on/off scheduled on the AudioContext clock. The schedule predicts the next onset at a smoothed interval (de-jittering the simulator's bursty per-frame delivery) and holds a small bounded latency so the click stays aligned with the on-screen playhead (driven from the same clock) instead of drifting behind it. A `pwmActive` flag mutes the digital path once hardware PWM drives the pin. Result: onset jitter for a firmware metronome drops from chaotic (σ ≈ 250ms, dropped/extra beats, unbounded audio latency) to σ ≈ 15ms at ~30ms latency — steady and aligned with the display. All 54 simulation-parts tests pass. Co-Authored-By: Claude Opus 4.8 --- .../src/__tests__/simulation-parts.test.ts | 4 +- frontend/src/simulation/AVRSimulator.ts | 12 +- frontend/src/simulation/PinManager.ts | 12 +- frontend/src/simulation/parts/ComplexParts.ts | 108 ++++++++++++++---- 4 files changed, 104 insertions(+), 32 deletions(-) diff --git a/frontend/src/__tests__/simulation-parts.test.ts b/frontend/src/__tests__/simulation-parts.test.ts index 47a886c4..7952fea2 100644 --- a/frontend/src/__tests__/simulation-parts.test.ts +++ b/frontend/src/__tests__/simulation-parts.test.ts @@ -848,14 +848,14 @@ describe('Buzzer — attachEvents', () => { beforeEach(() => { const mockOscillator = { type: 'square', - frequency: { value: 440, setTargetAtTime: vi.fn() }, + frequency: { value: 440, setTargetAtTime: vi.fn(), setValueAtTime: vi.fn() }, connect: vi.fn(), start: vi.fn(), stop: vi.fn(), disconnect: vi.fn(), }; const mockGain = { - gain: { value: 0.1 }, + gain: { value: 0.1, setTargetAtTime: vi.fn(), setValueAtTime: vi.fn() }, connect: vi.fn(), }; function MockAudioContext(this: any) { diff --git a/frontend/src/simulation/AVRSimulator.ts b/frontend/src/simulation/AVRSimulator.ts index 5a366c50..c1a5c8e4 100644 --- a/frontend/src/simulation/AVRSimulator.ts +++ b/frontend/src/simulation/AVRSimulator.ts @@ -727,13 +727,16 @@ export class AVRSimulator { */ private pollPwmRegisters(): void { if (!this.cpu) return; + // Precise simulated time of this poll (sub-frame). Parts that schedule + // audio use it to recover the real onset time instead of the frame edge. + const timeMs = this.cpu.cycles / 16_000; const pins = this.pwmPins; for (let i = 0; i < pins.length; i++) { const { ocrAddr, pin } = pins[i]; const ocrValue = this.cpu.data[ocrAddr]; if (ocrValue !== this.lastOcrValues[i]) { this.lastOcrValues[i] = ocrValue; - this.pinManager.updatePwm(pin, ocrValue / 255); + this.pinManager.updatePwm(pin, ocrValue / 255, timeMs); } } } @@ -786,9 +789,14 @@ export class AVRSimulator { avrInstruction(this.cpu); // Execute the AVR instruction this.cpu.tick(); // Update peripheral timers and cycles if (this.scheduledPinChanges.length > 0) this.flushScheduledPinChanges(); + // Poll PWM sub-frame (~every 256 cycles = 16µs) so short OCR pulses + // (e.g. a metronome click that starts and ends within one 16ms frame) + // aren't merged or lost at the frame boundary. 256 cycles is far finer + // than any audible pulse yet light enough not to perturb frame pacing. + if ((i & 0xff) === 0) this.pollPwmRegisters(); } - // Poll PWM registers every frame + // Final poll at the frame edge to catch the last change. this.pollPwmRegisters(); // Try to drain any pending RX byte every frame. The primary diff --git a/frontend/src/simulation/PinManager.ts b/frontend/src/simulation/PinManager.ts index ff0388ba..180ef9f4 100644 --- a/frontend/src/simulation/PinManager.ts +++ b/frontend/src/simulation/PinManager.ts @@ -19,7 +19,10 @@ export type PinState = boolean; export type PinChangeCallback = (pin: number, state: PinState) => void; export type AnalogCallback = (pin: number, voltage: number) => void; -export type PwmCallback = (pin: number, dutyCycle: number) => void; +// timeMs (optional) is the precise simulated time of the duty-cycle change +// (cpu.cycles / 16000). Parts that schedule audio/output use it for +// sample-accurate timing instead of the per-frame delivery instant. +export type PwmCallback = (pin: number, dutyCycle: number, timeMs?: number) => void; export class PinManager { private listeners: Map> = new Map(); @@ -190,14 +193,15 @@ export class PinManager { } /** - * Called by AVRSimulator each frame when an OCR register changes. + * Called by AVRSimulator when an OCR register changes (polled sub-frame). + * timeMs is the precise simulated time of the change for accurate audio. */ - updatePwm(pin: number, dutyCycle: number): void { + updatePwm(pin: number, dutyCycle: number, timeMs?: number): void { this.pwmValues.set(pin, dutyCycle); if (dutyCycle > 0) this.outputPins.add(pin); const callbacks = this.pwmListeners.get(pin); if (callbacks) { - callbacks.forEach((cb) => cb(pin, dutyCycle)); + callbacks.forEach((cb) => cb(pin, dutyCycle, timeMs)); } } diff --git a/frontend/src/simulation/parts/ComplexParts.ts b/frontend/src/simulation/parts/ComplexParts.ts index 25d2cce9..4ee59f72 100644 --- a/frontend/src/simulation/parts/ComplexParts.ts +++ b/frontend/src/simulation/parts/ComplexParts.ts @@ -510,6 +510,11 @@ PartSimulationRegistry.register('buzzer', { let oscillator: OscillatorNode | null = null; let gainNode: GainNode | null = null; let isSounding = false; + // Once the pin is driven by hardware PWM (analogWrite/Timer), the PWM + // handler owns the audio. The digital HIGH/LOW path is only for tone() + // (software pin toggling); on a PWM pin its ~490Hz carrier would otherwise + // fire spurious onsets at the duty edges. This flag mutes that path. + let pwmActive = false; const el = element as any; // Timer2 register addresses @@ -536,36 +541,74 @@ PartSimulationRegistry.register('buzzer', { return F_CPU / (2 * prescaler * (ocr2a + 1)); } - function startTone(freq: number) { + // ── Sample-accurate audio ──────────────────────────────────────────── + // PWM duty events arrive in per-frame batches (~16ms), so starting the + // oscillator "now" quantises every onset to the animation frame and a + // metronome wobbles / turns chaotic. Instead we keep ONE oscillator running + // and gate it with the gain node, scheduling each on/off at the precise + // time the event happened in the simulation (timeMs = cpu.cycles / 16000) + // mapped onto the AudioContext clock with a small look-ahead. Onsets then + // land on the beat regardless of frame jitter. + const LOOKAHEAD = 0.025; // target audio latency (~1-2 frames; aligns with the display) + let playWhen: number | null = null; // next scheduled audio time (monotonic) + let lastNow: number | null = null; // audio time at the previous onset + let avgGap: number | null = null; // smoothed onset interval (de-jitters bursts) + + function ensureAudio() { if (!audioCtx) { audioCtx = new AudioContext(); gainNode = audioCtx.createGain(); - gainNode.gain.value = 0.1; + gainNode.gain.value = 0; gainNode.connect(audioCtx.destination); + oscillator = audioCtx.createOscillator(); + oscillator.type = 'square'; + oscillator.frequency.value = 440; + oscillator.connect(gainNode); + oscillator.start(); // runs forever; the gain envelope is the gate } - // Browser autoplay policy: AudioContext starts in 'suspended' state - // until a user gesture has occurred. Resume it here so sound plays. - if (audioCtx.state === 'suspended') { - audioCtx.resume(); + // Autoplay policy: the context starts 'suspended' until a user gesture. + if (audioCtx.state === 'suspended') audioCtx.resume(); + } + + // The simulation delivers onsets in per-frame catch-up bursts (uneven + // wall-clock gaps), so scheduling them "now" reproduces that jitter, while + // locking to the simulated timestamps makes the audio drift away from the + // display (which is driven from the same clock). We split the difference: + // predict the next onset at a SMOOTHED interval (de-jitters the bursts) and + // pull the scheduling latency toward a small LOOKAHEAD so the click stays + // aligned with the on-screen playhead. The sub-frame PWM polling (see + // AVRSimulator) is what removes the dropped/merged clicks underneath. + function whenFor(_timeMs: number | undefined): number { + const ctx = audioCtx!; + const now = ctx.currentTime; + if (playWhen === null || lastNow === null) { + playWhen = now + LOOKAHEAD; + lastNow = now; + return playWhen; } - if (oscillator) { - oscillator.frequency.setTargetAtTime(freq, audioCtx.currentTime, 0.01); - return; - } - oscillator = audioCtx.createOscillator(); - oscillator.type = 'square'; - oscillator.frequency.value = freq; - oscillator.connect(gainNode!); - oscillator.start(); + const gap = now - lastNow; + avgGap = avgGap === null ? gap : avgGap + (gap - avgGap) * 0.08; + let when = playWhen + avgGap; // even prediction from the smoothed interval + when -= (when - now - LOOKAHEAD) * 0.12; // hold latency near LOOKAHEAD + if (when < now + 0.003) when = now + 0.003; + if (when <= playWhen) when = playWhen + 0.001; // strictly monotonic + playWhen = when; + lastNow = now; + return when; + } + + function startTone(freq: number, timeMs?: number) { + ensureAudio(); + const when = whenFor(timeMs); + oscillator!.frequency.setValueAtTime(freq, when); + gainNode!.gain.setValueAtTime(0.1, when); isSounding = true; if (el.playing !== undefined) el.playing = true; } - function stopTone() { - if (oscillator) { - oscillator.stop(); - oscillator.disconnect(); - oscillator = null; + function stopTone(timeMs?: number) { + if (audioCtx && gainNode) { + gainNode.gain.setValueAtTime(0, whenFor(timeMs)); } isSounding = false; if (el.playing !== undefined) el.playing = false; @@ -576,13 +619,14 @@ PartSimulationRegistry.register('buzzer', { if (pinSIG !== null && pinManager) { unsubscribers.push( - pinManager.onPwmChange(pinSIG, (_: number, dc: number) => { + pinManager.onPwmChange(pinSIG, (_: number, dc: number, timeMs?: number) => { + pwmActive = true; const cpu = (avrSimulator as any).cpu; if (dc > 0) { const freq = cpu ? getFrequency(cpu) : 440; - startTone(Math.max(20, Math.min(20000, freq))); + startTone(Math.max(20, Math.min(20000, freq)), timeMs); } else { - stopTone(); + stopTone(timeMs); } }), ); @@ -594,6 +638,7 @@ PartSimulationRegistry.register('buzzer', { if (sigResolver) { unsubscribers.push( sigResolver.onChange((state) => { + if (pwmActive) return; // PWM-driven: the duty handler owns audio if (!isSounding && state === 'HIGH') { const cpu = (avrSimulator as any).cpu; const freq = cpu ? getFrequency(cpu) : 440; @@ -606,6 +651,7 @@ PartSimulationRegistry.register('buzzer', { } else { unsubscribers.push( pinManager.onPinChange(pinSIG, (_: number, state: boolean) => { + if (pwmActive) return; // PWM-driven: the duty handler owns audio if (!isSounding && state) { const cpu = (avrSimulator as any).cpu; const freq = cpu ? getFrequency(cpu) : 440; @@ -617,7 +663,21 @@ PartSimulationRegistry.register('buzzer', { } return () => { - stopTone(); + if (oscillator) { + try { + oscillator.stop(); + } catch { + /* already stopped */ + } + oscillator.disconnect(); + oscillator = null; + } + isSounding = false; + pwmActive = false; + if (el.playing !== undefined) el.playing = false; + playWhen = null; + lastNow = null; + avgGap = null; if (audioCtx) { audioCtx.close(); audioCtx = null; From b06f500ad6966ac06f862b556fd54749124c78c7 Mon Sep 17 00:00:00 2001 From: ciegovolador Date: Tue, 9 Jun 2026 09:22:57 -0300 Subject: [PATCH 2/5] =?UTF-8?q?fix(sim):=20mature=20the=20buzzer=20?= =?UTF-8?q?=E2=80=94=20per-note=20oscillators,=20sim-time=20scheduling,=20?= =?UTF-8?q?ramps=20+=20metronome=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the previous commit; reworks the buzzer audio for glitch-free, cross-browser playback and adds a metronome quality suite. - Per-note oscillators with short attack/release ramps, instead of one long-lived oscillator gated by gain: a fresh fixed frequency per note and no gain/frequency automation on a persistent node — Firefox in particular clicks and glitches the pitch otherwise. - Schedule onsets by their SIMULATED inter-onset spacing (exact, even) with a light latency hold, instead of a wall-clock average. Turning a control (BPM, K…) re-locks immediately and the rhythm stays even — no bursts, no overlaps, no audio drifting away from the display. - Place each note-off relative to its own onset, preserving the exact click length from the simulation (the onset scheduler now tracks onsets only). - Poll PWM every 256 cycles (was 64): finer than any audible pulse, lighter on the frame loop. - New src/__tests__/buzzer-metronome.test.ts: drives the buzzer as a metronome against a controllable audio clock and asserts even spacing, one oscillator per click with no overlap, correct pitch per metric level, burst absorption, and a clean re-lock on tempo change. All simulation-parts + metronome tests pass (57). Co-Authored-By: Claude Opus 4.8 --- .../src/__tests__/buzzer-metronome.test.ts | 199 ++++++++++++++++++ .../src/__tests__/simulation-parts.test.ts | 16 +- frontend/src/simulation/parts/ComplexParts.ts | 139 +++++++----- 3 files changed, 300 insertions(+), 54 deletions(-) create mode 100644 frontend/src/__tests__/buzzer-metronome.test.ts diff --git a/frontend/src/__tests__/buzzer-metronome.test.ts b/frontend/src/__tests__/buzzer-metronome.test.ts new file mode 100644 index 00000000..699ff911 --- /dev/null +++ b/frontend/src/__tests__/buzzer-metronome.test.ts @@ -0,0 +1,199 @@ +/** + * Buzzer — metronome quality + * + * A real use case for the buzzer: an Arduino sketch driving it as a metronome + * (analogWrite/Timer PWM, an accent on the down-beat). This exercises the audio + * SCHEDULING that makes a metronome usable — and guards it against regressions. + * + * It drives the buzzer's PWM handler with a metronome sequence (onset → note-off + * pairs carrying their simulated timestamps) against a CONTROLLABLE audio clock, + * and records the resulting Web-Audio schedule (one oscillator per click). Then + * it asserts the qualities a metronome needs: + * - even onset spacing (steady tempo) + * - one oscillator per click, no overlap, strictly monotonic + * - correct pitch per metric level (accent vs beat) + * - bursty per-frame delivery is absorbed (still even) + * - a tempo change re-locks immediately and cleanly + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { PartSimulationRegistry } from '../simulation/parts/PartSimulationRegistry'; +import '../simulation/parts/ComplexParts'; + +// ── Controllable audio clock + schedule recorder ───────────────────────────── +let clock = 0; // seconds; we advance it to emulate real time passing +type Ev = { kind: 'start' | 'stop'; when: number; freq?: number }; +let sched: Ev[] = []; + +function mockOscillator() { + const o: any = { + type: '', + frequency: { value: 440, setValueAtTime: vi.fn(), linearRampToValueAtTime: vi.fn() }, + connect: vi.fn(), + disconnect: vi.fn(), + onended: null, + start: (w: number) => sched.push({ kind: 'start', when: w, freq: Math.round(o.frequency.value) }), + stop: (w: number) => sched.push({ kind: 'stop', when: w }), + }; + return o; +} +function mockGain() { + return { + gain: { value: 0, setValueAtTime: vi.fn(), linearRampToValueAtTime: vi.fn() }, + connect: vi.fn(), + disconnect: vi.fn(), + }; +} +class MockAudioContext { + state = 'running'; + destination = {}; + resume = vi.fn(); + close = vi.fn(); + get currentTime() { + return clock; + } + createOscillator() { + return mockOscillator(); + } + createGain() { + return mockGain(); + } +} + +beforeEach(() => { + clock = 0; + sched = []; + vi.stubGlobal('AudioContext', MockAudioContext as unknown as typeof AudioContext); +}); +afterEach(() => { + vi.unstubAllGlobals(); +}); + +// ── Metronome driver ───────────────────────────────────────────────────────── +const OCR2A = 0xb3; +const TCCR2B = 0xb1; +// OCR2A value for a target frequency (CTC, prescaler 64): f = 16e6/(2*64*(OCR+1)). +// Integer division, matching the firmware's `F_CPU/(2*64*freq) - 1`. +const ocrFor = (freq: number) => Math.floor(125000 / freq) - 1; +const ACCENT = ocrFor(1500); // 82 -> 1506 Hz +const BEAT = ocrFor(1100); // 112 -> 1106 Hz + +function setupBuzzer() { + const sim: any = { + cpu: { data: new Uint8Array(512).fill(0), cycles: 0 }, + pinManager: { + onPinChange: vi.fn().mockReturnValue(() => {}), + }, + }; + let pwm: ((pin: number, dc: number, timeMs?: number) => void) | null = null; + sim.pinManager.onPwmChange = vi.fn().mockImplementation((_pin: number, cb: any) => { + pwm = cb; + return () => {}; + }); + const cleanup = PartSimulationRegistry.get('buzzer')!.attachEvents!( + { addEventListener: vi.fn(), removeEventListener: vi.fn(), playing: false } as any, + sim, + (name: string) => (name === '1' ? 11 : null), + ); + return { sim, cleanup, hit: (ocr: number, simMs: number, clickMs = 25) => { + // onset: OCR set, PWM duty > 0 at simMs + sim.cpu.data[OCR2A] = ocr; + sim.cpu.data[TCCR2B] = 0x04; // CS22 -> prescaler 64 + clock = simMs / 1000; + pwm!(11, ocr / 255, simMs); + // note-off one click later + clock = (simMs + clickMs) / 1000; + pwm!(11, 0, simMs + clickMs); + } }; +} + +// Pull the recorded notes (a start paired with its following stop). +function notes() { + const out: { on: number; off: number; freq: number }[] = []; + for (let i = 0; i + 1 < sched.length; i++) { + if (sched[i].kind === 'start' && sched[i + 1].kind === 'stop') { + out.push({ on: sched[i].when, off: sched[i + 1].when, freq: sched[i].freq! }); + } + } + return out; +} + +describe('Buzzer — metronome quality', () => { + it('plays an even metronome with an accent on the down-beat', () => { + const { hit } = setupBuzzer(); + const beat = 500; // ms — quarter notes @ 120 BPM + for (let i = 0; i < 8; i++) hit(i % 4 === 0 ? ACCENT : BEAT, i * beat); + + const ns = notes(); + expect(ns.length).toBe(8); + // even spacing (steady tempo) + for (let i = 1; i < ns.length; i++) { + const gap = (ns[i].on - ns[i - 1].on) * 1000; + expect(Math.abs(gap - beat)).toBeLessThan(5); + } + // one oscillator per click, no overlap, monotonic + for (let i = 0; i < ns.length; i++) { + expect(ns[i].off).toBeGreaterThan(ns[i].on); + if (i > 0) { + expect(ns[i].on).toBeGreaterThan(ns[i - 1].on); + expect(ns[i - 1].off).toBeLessThanOrEqual(ns[i].on + 1e-6); + } + } + // correct pitch per metric level + expect(Math.abs(ns[0].freq - 1506)).toBeLessThanOrEqual(2); // accent + expect(Math.abs(ns[1].freq - 1106)).toBeLessThanOrEqual(2); // beat + }); + + it('absorbs bursty per-frame delivery (onsets even in sim time, jittery in wall time)', () => { + const { sim, cleanup } = setupBuzzer(); + void cleanup; + let pwm!: (p: number, dc: number, t?: number) => void; + sim.pinManager.onPwmChange.mock.calls; // noop ref + // re-grab the callback captured during setup + pwm = sim.pinManager.onPwmChange.mock.calls[0][1]; + + const beat = 250; // even sim spacing + // wall-clock delivery jitters by ~one 60fps frame each onset (a real burst) + const wobble = [0, -15, 12, -10, 14, -13, 10, -8, 0, 11]; + for (let i = 0; i < 10; i++) { + const simMs = i * beat; + sim.cpu.data[OCR2A] = i % 4 === 0 ? ACCENT : BEAT; + sim.cpu.data[TCCR2B] = 0x04; + clock = (simMs + wobble[i]) / 1000; // jittery wall time + pwm(11, 0.3, simMs); + clock = (simMs + 25 + wobble[i]) / 1000; + pwm(11, 0, simMs + 25); + } + const ns = notes(); + expect(ns.length).toBeGreaterThanOrEqual(8); + // despite the wall-clock jitter, scheduled onsets stay even (de-jittered) + for (let i = 2; i < ns.length; i++) { + const gap = (ns[i].on - ns[i - 1].on) * 1000; + expect(Math.abs(gap - beat)).toBeLessThan(20); + expect(ns[i - 1].off).toBeLessThanOrEqual(ns[i].on + 1e-6); // no overlap + } + }); + + it('re-locks immediately and cleanly when the tempo changes', () => { + const { hit } = setupBuzzer(); + let t = 0; + for (let i = 0; i < 5; i++) { + hit(BEAT, t); + t += 500; + } // 120 BPM + for (let i = 0; i < 6; i++) { + hit(BEAT, t); + t += 250; + } // jump to 240 BPM + + const ns = notes(); + expect(ns.length).toBe(11); + // no overlap / no backward note across the change + for (let i = 1; i < ns.length; i++) { + expect(ns[i].on).toBeGreaterThan(ns[i - 1].on); + expect(ns[i - 1].off).toBeLessThanOrEqual(ns[i].on + 1e-6); + } + // settled to the new rate within a beat of the change (no drift/creep) + const lastGap = (ns[ns.length - 1].on - ns[ns.length - 2].on) * 1000; + expect(Math.abs(lastGap - 250)).toBeLessThan(10); + }); +}); diff --git a/frontend/src/__tests__/simulation-parts.test.ts b/frontend/src/__tests__/simulation-parts.test.ts index 7952fea2..32a86bd1 100644 --- a/frontend/src/__tests__/simulation-parts.test.ts +++ b/frontend/src/__tests__/simulation-parts.test.ts @@ -848,15 +848,27 @@ describe('Buzzer — attachEvents', () => { beforeEach(() => { const mockOscillator = { type: 'square', - frequency: { value: 440, setTargetAtTime: vi.fn(), setValueAtTime: vi.fn() }, + frequency: { + value: 440, + setTargetAtTime: vi.fn(), + setValueAtTime: vi.fn(), + linearRampToValueAtTime: vi.fn(), + }, connect: vi.fn(), start: vi.fn(), stop: vi.fn(), disconnect: vi.fn(), + onended: null, }; const mockGain = { - gain: { value: 0.1, setTargetAtTime: vi.fn(), setValueAtTime: vi.fn() }, + gain: { + value: 0.1, + setTargetAtTime: vi.fn(), + setValueAtTime: vi.fn(), + linearRampToValueAtTime: vi.fn(), + }, connect: vi.fn(), + disconnect: vi.fn(), }; function MockAudioContext(this: any) { this.createOscillator = vi.fn().mockReturnValue(mockOscillator); diff --git a/frontend/src/simulation/parts/ComplexParts.ts b/frontend/src/simulation/parts/ComplexParts.ts index 4ee59f72..478a86ac 100644 --- a/frontend/src/simulation/parts/ComplexParts.ts +++ b/frontend/src/simulation/parts/ComplexParts.ts @@ -507,8 +507,8 @@ PartSimulationRegistry.register('buzzer', { : null; let audioCtx: AudioContext | null = null; - let oscillator: OscillatorNode | null = null; - let gainNode: GainNode | null = null; + let activeOsc: OscillatorNode | null = null; // the note currently sounding (one per click) + let activeGain: GainNode | null = null; let isSounding = false; // Once the pin is driven by hardware PWM (analogWrite/Timer), the PWM // handler owns the audio. The digital HIGH/LOW path is only for tone() @@ -542,73 +542,105 @@ PartSimulationRegistry.register('buzzer', { } // ── Sample-accurate audio ──────────────────────────────────────────── - // PWM duty events arrive in per-frame batches (~16ms), so starting the - // oscillator "now" quantises every onset to the animation frame and a - // metronome wobbles / turns chaotic. Instead we keep ONE oscillator running - // and gate it with the gain node, scheduling each on/off at the precise - // time the event happened in the simulation (timeMs = cpu.cycles / 16000) - // mapped onto the AudioContext clock with a small look-ahead. Onsets then - // land on the beat regardless of frame jitter. + // PWM duty events arrive in per-frame batches (~16ms), so starting a note + // "now" quantises every onset to the animation frame and a metronome + // wobbles. We instead schedule each note on the AudioContext clock at the + // time it happened in the simulation, with a small look-ahead. ONE + // oscillator PER NOTE (created on the onset, stopped on the note-off) with a + // short attack/release ramp: each note has a fresh fixed frequency and we + // never automate gain/frequency on a long-lived node — Firefox in particular + // clicks/pops on abrupt gain steps and glitches on live frequency changes. const LOOKAHEAD = 0.025; // target audio latency (~1-2 frames; aligns with the display) + const ATTACK = 0.002; // 2 ms fade-in — removes the start click/pop + const RELEASE = 0.003; // 3 ms fade-out — removes the end click/pop let playWhen: number | null = null; // next scheduled audio time (monotonic) - let lastNow: number | null = null; // audio time at the previous onset - let avgGap: number | null = null; // smoothed onset interval (de-jitters bursts) + let lastSimMs: number | null = null; // simulated time of the previous onset + let onWhen: number | null = null; // scheduled audio time of the current note's onset + let onSimMs: number | null = null; // simulated time of the current note's onset - function ensureAudio() { - if (!audioCtx) { - audioCtx = new AudioContext(); - gainNode = audioCtx.createGain(); - gainNode.gain.value = 0; - gainNode.connect(audioCtx.destination); - oscillator = audioCtx.createOscillator(); - oscillator.type = 'square'; - oscillator.frequency.value = 440; - oscillator.connect(gainNode); - oscillator.start(); // runs forever; the gain envelope is the gate - } + function ensureCtx() { + if (!audioCtx) audioCtx = new AudioContext(); // Autoplay policy: the context starts 'suspended' until a user gesture. if (audioCtx.state === 'suspended') audioCtx.resume(); } - // The simulation delivers onsets in per-frame catch-up bursts (uneven - // wall-clock gaps), so scheduling them "now" reproduces that jitter, while - // locking to the simulated timestamps makes the audio drift away from the - // display (which is driven from the same clock). We split the difference: - // predict the next onset at a SMOOTHED interval (de-jitters the bursts) and - // pull the scheduling latency toward a small LOOKAHEAD so the click stays - // aligned with the on-screen playhead. The sub-frame PWM polling (see - // AVRSimulator) is what removes the dropped/merged clicks underneath. - function whenFor(_timeMs: number | undefined): number { + // Schedule onsets by their SIMULATED inter-onset spacing — exact and even, + // because the firmware's clock is precise — advancing playWhen by the sim + // gap (timeMs delta). A light pull holds the scheduling latency near + // LOOKAHEAD, which bounds the slow sim↔audio clock drift and keeps the click + // aligned with the on-screen playhead. Because the spacing comes straight + // from the simulation (not a wall-clock average), turning a control (BPM, + // K…) re-locks immediately and the rhythm stays even — no bursts, no + // overlaps. whenFor sees ONLY onsets; note-offs are placed relative to their + // own onset in stopTone. + function whenFor(timeMs: number | undefined): number { const ctx = audioCtx!; const now = ctx.currentTime; - if (playWhen === null || lastNow === null) { - playWhen = now + LOOKAHEAD; - lastNow = now; + if (timeMs === undefined || playWhen === null || lastSimMs === null) { + playWhen = Math.max(now + LOOKAHEAD, (playWhen ?? 0) + 0.001); + if (timeMs !== undefined) lastSimMs = timeMs; return playWhen; } - const gap = now - lastNow; - avgGap = avgGap === null ? gap : avgGap + (gap - avgGap) * 0.08; - let when = playWhen + avgGap; // even prediction from the smoothed interval - when -= (when - now - LOOKAHEAD) * 0.12; // hold latency near LOOKAHEAD + const dSim = Math.max(0, (timeMs - lastSimMs) / 1000); // exact, even sim spacing + let when = playWhen + dSim; + when -= (when - now - LOOKAHEAD) * 0.2; // hold latency / absorb clock drift if (when < now + 0.003) when = now + 0.003; if (when <= playWhen) when = playWhen + 0.001; // strictly monotonic playWhen = when; - lastNow = now; + lastSimMs = timeMs; return when; } function startTone(freq: number, timeMs?: number) { - ensureAudio(); - const when = whenFor(timeMs); - oscillator!.frequency.setValueAtTime(freq, when); - gainNode!.gain.setValueAtTime(0.1, when); + ensureCtx(); + const ctx = audioCtx!; + const when = whenFor(timeMs); // the scheduler tracks ONSETS only (clean rhythm) + onWhen = when; + onSimMs = timeMs ?? null; + const osc = ctx.createOscillator(); + osc.type = 'square'; + osc.frequency.value = freq; // fixed for the life of this note (no live change) + const g = ctx.createGain(); + g.gain.setValueAtTime(0, when); + g.gain.linearRampToValueAtTime(0.1, when + ATTACK); + osc.connect(g); + g.connect(ctx.destination); + osc.start(when); + osc.onended = () => { + try { + osc.disconnect(); + g.disconnect(); + } catch { + /* already torn down */ + } + }; + activeOsc = osc; + activeGain = g; isSounding = true; if (el.playing !== undefined) el.playing = true; } function stopTone(timeMs?: number) { - if (audioCtx && gainNode) { - gainNode.gain.setValueAtTime(0, whenFor(timeMs)); + const ctx = audioCtx; + if (ctx && activeOsc && activeGain) { + // Note-off relative to its own onset, preserving the exact click length + // from the simulation (not via the onset scheduler, which would smear + // the short on→off and long off→on gaps together). + let off = + onWhen !== null && onSimMs !== null && timeMs !== undefined + ? onWhen + Math.max(0.004, (timeMs - onSimMs) / 1000) + : ctx.currentTime + 0.02; + if (onWhen !== null && off < onWhen + ATTACK + 0.002) off = onWhen + ATTACK + 0.002; + if (off < ctx.currentTime + 0.003) off = ctx.currentTime + 0.003; + try { + activeGain.gain.setValueAtTime(0.1, off); + activeGain.gain.linearRampToValueAtTime(0, off + RELEASE); + activeOsc.stop(off + RELEASE + 0.001); + } catch { + /* already scheduled */ + } + activeOsc = null; + activeGain = null; } isSounding = false; if (el.playing !== undefined) el.playing = false; @@ -663,21 +695,24 @@ PartSimulationRegistry.register('buzzer', { } return () => { - if (oscillator) { + if (activeOsc) { try { - oscillator.stop(); + activeOsc.stop(); + activeOsc.disconnect(); + activeGain?.disconnect(); } catch { /* already stopped */ } - oscillator.disconnect(); - oscillator = null; + activeOsc = null; + activeGain = null; } isSounding = false; pwmActive = false; if (el.playing !== undefined) el.playing = false; playWhen = null; - lastNow = null; - avgGap = null; + lastSimMs = null; + onWhen = null; + onSimMs = null; if (audioCtx) { audioCtx.close(); audioCtx = null; From 6a1f79e3317e8c522878c77f7478ef1368699bc9 Mon Sep 17 00:00:00 2001 From: ciegovolador Date: Thu, 11 Jun 2026 02:35:33 -0300 Subject: [PATCH 3/5] =?UTF-8?q?fix(sim):=20monophonic=20buzzer=20guard=20?= =?UTF-8?q?=E2=80=94=20replace=20note=20on=20pitch=20change=20(no=20stacki?= =?UTF-8?q?ng)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A melody / continuous tone (consecutive tone() with no noTone() between) is back-to-back nonzero-OCR PWM writes with no note-off, so startTone() overwrote activeOsc without stopping the previous node — oscillators stacked and were never stopped (reported: created 6, started 6, never stopped 6). Add a monophonic guard at the top of startTone(): release the live note (gain ramp + stop) before starting the new one, so a pitch change REPLACES rather than STACKS. Extract a shared releaseActive(off) helper (also used by stopTone). Add two melody tests: one asserts starts === stops (no orphans), monotonic onsets and per-note pitch; one asserts a melody ending without a trailing noTone() leaves only the final note ringing (stops === starts - 1). The metronome path is unaffected (each click is an onset→note-off pair, so the guard never fires there); the three existing metronome tests stay green. Co-Authored-By: Claude Opus 4.8 --- .../src/__tests__/buzzer-metronome.test.ts | 97 +++++++++++++++++++ frontend/src/simulation/parts/ComplexParts.ts | 51 +++++++--- 2 files changed, 136 insertions(+), 12 deletions(-) diff --git a/frontend/src/__tests__/buzzer-metronome.test.ts b/frontend/src/__tests__/buzzer-metronome.test.ts index 699ff911..f44b3e52 100644 --- a/frontend/src/__tests__/buzzer-metronome.test.ts +++ b/frontend/src/__tests__/buzzer-metronome.test.ts @@ -196,4 +196,101 @@ describe('Buzzer — metronome quality', () => { const lastGap = (ns[ns.length - 1].on - ns[ns.length - 2].on) * 1000; expect(Math.abs(lastGap - 250)).toBeLessThan(10); }); + + // Regression guard for the maintainer's review (PR #220, comment 4671821612): + // a melody / continuous tone is consecutive tone(pin, freqN) calls with NO + // noTone() between pitches — back-to-back nonzero OCR writes, each firing the + // PWM handler with duty>0 and no intervening note-off. The old code overwrote + // activeOsc on every pitch change without stopping the previous node, so + // oscillators were "started but never stopped" — they stacked and played + // forever. The monophonic guard must REPLACE the live note instead. + it('replaces rather than stacks oscillators on a melody / continuous tone', () => { + const { sim } = setupBuzzer(); + const pwm = sim.pinManager.onPwmChange.mock.calls[0][1] as ( + p: number, + dc: number, + t?: number, + ) => void; + + // A little tune: pitch changes with no note-off between them (legato). + const melody = [523, 587, 659, 698, 784, 659]; // C5 D5 E5 F5 G5 E5 + const step = 200; // ms per note + melody.forEach((freq, i) => { + sim.cpu.data[OCR2A] = ocrFor(freq); + sim.cpu.data[TCCR2B] = 0x04; // CS22 -> prescaler 64 + clock = (i * step) / 1000; + pwm(11, 0.5, i * step); // square-wave duty>0, pitch change, NO dc=0 + }); + // End the tune with a noTone — releases the final note. + clock = (melody.length * step) / 1000; + pwm(11, 0, melody.length * step); + + const starts = sched.filter((e) => e.kind === 'start'); + const stops = sched.filter((e) => e.kind === 'stop'); + + // Every oscillator that started is stopped — no orphans left ringing. This + // is the exact failure the maintainer saw ("started but never stopped: 6"). + expect(starts.length).toBe(melody.length); + expect(stops.length).toBe(starts.length); + + // One start per note, at the right pitch, in order. Compare against the + // CTC-reconstructed pitch (what the firmware's integer OCR actually yields), + // not the nominal note — same round-trip the buzzer's getFrequency does. + const heard = (freq: number) => Math.round(125000 / (ocrFor(freq) + 1)); + starts.forEach((s, i) => { + expect(s.freq).toBe(heard(melody[i])); + if (i > 0) expect(s.when).toBeGreaterThan(starts[i - 1].when); // monotonic, no backward + }); + // Pitch is read FRESH per note (not stuck on the first onset): the tune rises + // then falls, so the heard sequence is non-constant and tracks the melody. + const heardSeq = starts.map((s) => s.freq); + expect(new Set(heardSeq).size).toBeGreaterThan(1); + expect(heardSeq).toEqual(melody.map(heard)); + + // Bounded overlap (replacement, not stacking): each note is released close to + // the NEXT note's onset — not smeared to the end of the tune. Pair each start + // with its own stop (interleaved start/stop/start/stop… once the guard fires). + const notesSeq: { on: number; off: number }[] = []; + for (let i = 0; i + 1 < sched.length; i++) { + if (sched[i].kind === 'start' && sched[i + 1].kind === 'stop') { + notesSeq.push({ on: sched[i].when, off: sched[i + 1].when }); + } + } + expect(notesSeq.length).toBe(melody.length); + for (let i = 0; i < notesSeq.length - 1; i++) { + // old note ends as the next begins (≤ a release tail past the next onset) + expect(notesSeq[i].off).toBeLessThanOrEqual(notesSeq[i + 1].on + 0.01); + expect(notesSeq[i].off).toBeGreaterThan(notesSeq[i].on); // positive duration + } + }); + + // A melody that ends WITHOUT a noTone() — the real "sketch loops tone() and + // never calls noTone()" pattern. Correct Arduino semantics: a tone() plays + // until noTone() or the NEXT tone(), so the final note must keep ringing. The + // guard must release exactly the SUPERSEDED notes (one stop each) and leave the + // last one sounding — not orphan the middle notes, not cut the last one short. + it('releases superseded notes but leaves the final note ringing (no trailing noTone)', () => { + const { sim } = setupBuzzer(); + const pwm = sim.pinManager.onPwmChange.mock.calls[0][1] as ( + p: number, + dc: number, + t?: number, + ) => void; + + const melody = [440, 494, 523]; // A4 B4 C5 + const step = 200; + melody.forEach((freq, i) => { + sim.cpu.data[OCR2A] = ocrFor(freq); + sim.cpu.data[TCCR2B] = 0x04; + clock = (i * step) / 1000; + pwm(11, 0.5, i * step); // pitch change, NO dc=0 — and no noTone at the end + }); + + const starts = sched.filter((e) => e.kind === 'start'); + const stops = sched.filter((e) => e.kind === 'stop'); + // Every note starts; only the superseded ones stop → exactly one note (the + // last) is still live. Pre-guard this was starts=3, stops=0 (all orphaned). + expect(starts.length).toBe(melody.length); + expect(stops.length).toBe(starts.length - 1); + }); }); diff --git a/frontend/src/simulation/parts/ComplexParts.ts b/frontend/src/simulation/parts/ComplexParts.ts index 478a86ac..6f1b60af 100644 --- a/frontend/src/simulation/parts/ComplexParts.ts +++ b/frontend/src/simulation/parts/ComplexParts.ts @@ -591,10 +591,47 @@ PartSimulationRegistry.register('buzzer', { return when; } + // Ramp the note currently sounding down to silence ending at audio time + // `off` and schedule its stop. Shared by stopTone (note-off) and the + // monophonic guard in startTone (a pitch change with no note-off). Keeps the + // envelope valid: never release before this note's own attack has finished, + // nor in the past. + // + // Bounded-overlap note (guard path): on a normal metronome/melody — onsets + // tens-to-hundreds of ms apart — the old note ends ~RELEASE before the next + // onset. On a degenerate sub-4 ms onset (a >250-note/s trill, or two tone() + // calls at the same simulated timestamp — neither of which a passive buzzer + // produces) the `onWhen + ATTACK` floor pushes `off` past the next onset, so + // two oscillators overlap for at most ~ATTACK+RELEASE (≈5 ms). That is + // inaudible and still leak-free (one stop per note). We deliberately keep the + // attack-finished envelope rather than clamp `off` down to the onset, which + // would start the down-ramp from a gain that never reached its peak. + function releaseActive(off: number) { + const ctx = audioCtx; + if (!ctx || !activeOsc || !activeGain) return; + if (onWhen !== null && off < onWhen + ATTACK + 0.002) off = onWhen + ATTACK + 0.002; + if (off < ctx.currentTime + 0.003) off = ctx.currentTime + 0.003; + try { + activeGain.gain.setValueAtTime(0.1, off); + activeGain.gain.linearRampToValueAtTime(0, off + RELEASE); + activeOsc.stop(off + RELEASE + 0.001); + } catch { + /* already scheduled */ + } + activeOsc = null; + activeGain = null; + } + function startTone(freq: number, timeMs?: number) { ensureCtx(); const ctx = audioCtx!; const when = whenFor(timeMs); // the scheduler tracks ONSETS only (clean rhythm) + // Monophonic guard: a pitch change with no intervening note-off (a melody — + // consecutive tone() calls) must REPLACE the current note, not stack a new + // oscillator on top. Release the live note so it ends as the new one begins + // (seamless legato) instead of orphaning it to play forever. Reads the + // PREVIOUS note's onWhen, so it must run before onWhen is reassigned below. + if (activeOsc && activeGain) releaseActive(when); onWhen = when; onSimMs = timeMs ?? null; const osc = ctx.createOscillator(); @@ -626,21 +663,11 @@ PartSimulationRegistry.register('buzzer', { // Note-off relative to its own onset, preserving the exact click length // from the simulation (not via the onset scheduler, which would smear // the short on→off and long off→on gaps together). - let off = + const off = onWhen !== null && onSimMs !== null && timeMs !== undefined ? onWhen + Math.max(0.004, (timeMs - onSimMs) / 1000) : ctx.currentTime + 0.02; - if (onWhen !== null && off < onWhen + ATTACK + 0.002) off = onWhen + ATTACK + 0.002; - if (off < ctx.currentTime + 0.003) off = ctx.currentTime + 0.003; - try { - activeGain.gain.setValueAtTime(0.1, off); - activeGain.gain.linearRampToValueAtTime(0, off + RELEASE); - activeOsc.stop(off + RELEASE + 0.001); - } catch { - /* already scheduled */ - } - activeOsc = null; - activeGain = null; + releaseActive(off); } isSounding = false; if (el.playing !== undefined) el.playing = false; From e6ba5ed9c74eb68e802103907af858178ae4f982 Mon Sep 17 00:00:00 2001 From: ciegovolador Date: Thu, 11 Jun 2026 03:14:32 -0300 Subject: [PATCH 4/5] test(sim): update PWM-callback assertions for the new timeMs arg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sample-accurate scheduling (06526c7) added an optional 3rd `timeMs` argument to PwmCallback / updatePwm, which broke 9 existing strict toHaveBeenCalledWith(pin, duty) assertions (PinManager, AVRSimulator, mega-emulation, attiny85). Match the real signature: PinManager drives updatePwm directly with no timeMs (assert `undefined`); the AVR OCR-poll path computes timeMs = cpu.cycles / 16000 (assert `expect.anything()`). Leaves one pre-existing red — component-to-spice "custom-chip missing fixture" — which fails on master too and is unrelated to this PR. Co-Authored-By: Claude Opus 4.8 --- frontend/src/__tests__/AVRSimulator.test.ts | 4 ++-- frontend/src/__tests__/PinManager.test.ts | 4 ++-- frontend/src/__tests__/attiny85-simulation.test.ts | 4 ++-- frontend/src/__tests__/mega-emulation.test.ts | 6 +++--- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frontend/src/__tests__/AVRSimulator.test.ts b/frontend/src/__tests__/AVRSimulator.test.ts index f98cfd42..f88b9a44 100644 --- a/frontend/src/__tests__/AVRSimulator.test.ts +++ b/frontend/src/__tests__/AVRSimulator.test.ts @@ -168,7 +168,7 @@ describe('AVRSimulator — PWM OCR monitoring', () => { sim.start(); sim.stop(); - expect(pwmCb).toHaveBeenCalledWith(9, 128 / 255); + expect(pwmCb).toHaveBeenCalledWith(9, 128 / 255, expect.anything()); // 3rd arg = sim timeMs }); it('PWM covers all six Arduino PWM pins', () => { @@ -202,7 +202,7 @@ describe('AVRSimulator — PWM OCR monitoring', () => { PWM_MAP.forEach(({ pin }, i) => { const expected = ((i + 1) * 25) / 255; - expect(cbs[pin]).toHaveBeenCalledWith(pin, expected); + expect(cbs[pin]).toHaveBeenCalledWith(pin, expected, expect.anything()); // 3rd arg = sim timeMs }); }); }); diff --git a/frontend/src/__tests__/PinManager.test.ts b/frontend/src/__tests__/PinManager.test.ts index e0db42ac..b98cca16 100644 --- a/frontend/src/__tests__/PinManager.test.ts +++ b/frontend/src/__tests__/PinManager.test.ts @@ -124,7 +124,7 @@ describe('PinManager — PWM duty cycle', () => { const cb = vi.fn(); pm.onPwmChange(9, cb); pm.updatePwm(9, 0.5); - expect(cb).toHaveBeenCalledWith(9, 0.5); + expect(cb).toHaveBeenCalledWith(9, 0.5, undefined); // 3rd arg = optional timeMs (not passed here) }); it('stores the latest PWM value', () => { @@ -150,7 +150,7 @@ describe('PinManager — PWM duty cycle', () => { pwmPins.forEach((pin, i) => { const dc = (i + 1) / 6; pm.updatePwm(pin, dc); - expect(callbacks[i]).toHaveBeenCalledWith(pin, dc); + expect(callbacks[i]).toHaveBeenCalledWith(pin, dc, undefined); // optional timeMs not passed }); }); }); diff --git a/frontend/src/__tests__/attiny85-simulation.test.ts b/frontend/src/__tests__/attiny85-simulation.test.ts index 5843c765..2c66826b 100644 --- a/frontend/src/__tests__/attiny85-simulation.test.ts +++ b/frontend/src/__tests__/attiny85-simulation.test.ts @@ -283,7 +283,7 @@ describe('ATtiny85 — PWM monitoring', () => { sim.start(); sim.stop(); - expect(pwmCb).toHaveBeenCalledWith(1, 128 / 255); + expect(pwmCb).toHaveBeenCalledWith(1, 128 / 255, expect.anything()); // 3rd arg = sim timeMs }); it('PinManager receives PWM update on pin 0 when OCR0A (0x56) is written', () => { @@ -300,7 +300,7 @@ describe('ATtiny85 — PWM monitoring', () => { sim.start(); sim.stop(); - expect(pwmCb).toHaveBeenCalledWith(0, 64 / 255); + expect(pwmCb).toHaveBeenCalledWith(0, 64 / 255, expect.anything()); // 3rd arg = sim timeMs }); it('ATtiny85 PWM covers 4 pins (OCR0A/OCR0B/OCR1A/OCR1B)', () => { diff --git a/frontend/src/__tests__/mega-emulation.test.ts b/frontend/src/__tests__/mega-emulation.test.ts index 0752eeda..4f373576 100644 --- a/frontend/src/__tests__/mega-emulation.test.ts +++ b/frontend/src/__tests__/mega-emulation.test.ts @@ -301,7 +301,7 @@ describe('AVRSimulator Mega — PWM OCR mapping differs from Uno', () => { // Call pollPwmRegisters directly — avoids RAF dependency in unit tests (sim as any).pollPwmRegisters(); - expect(cb).toHaveBeenCalledWith(13, 128 / 255); + expect(cb).toHaveBeenCalledWith(13, 128 / 255, expect.anything()); // 3rd arg = sim timeMs sim.stop(); }); @@ -318,7 +318,7 @@ describe('AVRSimulator Mega — PWM OCR mapping differs from Uno', () => { (sim as any).pollPwmRegisters(); - expect(cb).toHaveBeenCalledWith(5, 200 / 255); + expect(cb).toHaveBeenCalledWith(5, 200 / 255, expect.anything()); // 3rd arg = sim timeMs sim.stop(); }); @@ -335,7 +335,7 @@ describe('AVRSimulator Mega — PWM OCR mapping differs from Uno', () => { (sim as any).pollPwmRegisters(); - expect(cb).toHaveBeenCalledWith(6, 100 / 255); + expect(cb).toHaveBeenCalledWith(6, 100 / 255, expect.anything()); // 3rd arg = sim timeMs sim.stop(); }); }); From 9b86c816c11d58502179de2e5221234252fd66df Mon Sep 17 00:00:00 2001 From: ciegovolador Date: Thu, 11 Jun 2026 03:29:44 -0300 Subject: [PATCH 5/5] fix(sim): keep PwmCallback 2-arg compatible via arity dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Revert the earlier approach of widening the existing PWM-callback assertions to accept the new timeMs arg — that masked a contract change rather than fixing it. Instead, updatePwm now hands the optional timeMs only to listeners that declare a 3rd parameter (cb.length >= 3) — i.e. the buzzer, which needs the precise onset time. Plain (pin, dutyCycle) listeners, and the existing toHaveBeenCalledWith(pin, dutyCycle) tests, see an unchanged 2-arg call, so the original PwmCallback contract is preserved. Add a PinManager test locking the dispatch: a 2-param listener stays 2-arg; a 3-param listener receives timeMs. Co-Authored-By: Claude Opus 4.8 --- frontend/src/__tests__/AVRSimulator.test.ts | 4 +-- frontend/src/__tests__/PinManager.test.ts | 34 +++++++++++++++++-- .../src/__tests__/attiny85-simulation.test.ts | 4 +-- frontend/src/__tests__/mega-emulation.test.ts | 6 ++-- frontend/src/simulation/PinManager.ts | 8 ++++- 5 files changed, 46 insertions(+), 10 deletions(-) diff --git a/frontend/src/__tests__/AVRSimulator.test.ts b/frontend/src/__tests__/AVRSimulator.test.ts index f88b9a44..f98cfd42 100644 --- a/frontend/src/__tests__/AVRSimulator.test.ts +++ b/frontend/src/__tests__/AVRSimulator.test.ts @@ -168,7 +168,7 @@ describe('AVRSimulator — PWM OCR monitoring', () => { sim.start(); sim.stop(); - expect(pwmCb).toHaveBeenCalledWith(9, 128 / 255, expect.anything()); // 3rd arg = sim timeMs + expect(pwmCb).toHaveBeenCalledWith(9, 128 / 255); }); it('PWM covers all six Arduino PWM pins', () => { @@ -202,7 +202,7 @@ describe('AVRSimulator — PWM OCR monitoring', () => { PWM_MAP.forEach(({ pin }, i) => { const expected = ((i + 1) * 25) / 255; - expect(cbs[pin]).toHaveBeenCalledWith(pin, expected, expect.anything()); // 3rd arg = sim timeMs + expect(cbs[pin]).toHaveBeenCalledWith(pin, expected); }); }); }); diff --git a/frontend/src/__tests__/PinManager.test.ts b/frontend/src/__tests__/PinManager.test.ts index b98cca16..a5cc5695 100644 --- a/frontend/src/__tests__/PinManager.test.ts +++ b/frontend/src/__tests__/PinManager.test.ts @@ -124,7 +124,7 @@ describe('PinManager — PWM duty cycle', () => { const cb = vi.fn(); pm.onPwmChange(9, cb); pm.updatePwm(9, 0.5); - expect(cb).toHaveBeenCalledWith(9, 0.5, undefined); // 3rd arg = optional timeMs (not passed here) + expect(cb).toHaveBeenCalledWith(9, 0.5); }); it('stores the latest PWM value', () => { @@ -150,9 +150,39 @@ describe('PinManager — PWM duty cycle', () => { pwmPins.forEach((pin, i) => { const dc = (i + 1) / 6; pm.updatePwm(pin, dc); - expect(callbacks[i]).toHaveBeenCalledWith(pin, dc, undefined); // optional timeMs not passed + expect(callbacks[i]).toHaveBeenCalledWith(pin, dc); }); }); + + // The optional timeMs (precise simulated onset time, used by the buzzer for + // sample-accurate audio) must NOT widen the public PwmCallback contract: + // listeners that declare only (pin, dutyCycle) keep getting a 2-arg call, + // while a listener that declares a 3rd parameter receives timeMs. This guards + // the arity-based dispatch the buzzer relies on. Regular functions are used + // (not vi.fn) because the dispatch keys off Function.length, and a 3-param + // listener must report length 3. + it('hands timeMs only to listeners that declare a 3rd parameter', () => { + let twoArgCount = -1; + let threeArgCount = -1; + let threeArgTime: number | undefined; + function twoArg(this: unknown, _pin: number, _dc: number) { + // eslint-disable-next-line prefer-rest-params + twoArgCount = arguments.length; + } + function threeArg(this: unknown, _pin: number, _dc: number, t?: number) { + // eslint-disable-next-line prefer-rest-params + threeArgCount = arguments.length; + threeArgTime = t; + } + pm.onPwmChange(7, twoArg); + pm.onPwmChange(7, threeArg); + + pm.updatePwm(7, 0.5, 123); + + expect(twoArgCount).toBe(2); // original 2-arg contract preserved — no trailing timeMs + expect(threeArgCount).toBe(3); + expect(threeArgTime).toBe(123); // 3-arg listener (the buzzer) gets the precise time + }); }); // ─── Analog voltage API ────────────────────────────────────────────────────── diff --git a/frontend/src/__tests__/attiny85-simulation.test.ts b/frontend/src/__tests__/attiny85-simulation.test.ts index 2c66826b..5843c765 100644 --- a/frontend/src/__tests__/attiny85-simulation.test.ts +++ b/frontend/src/__tests__/attiny85-simulation.test.ts @@ -283,7 +283,7 @@ describe('ATtiny85 — PWM monitoring', () => { sim.start(); sim.stop(); - expect(pwmCb).toHaveBeenCalledWith(1, 128 / 255, expect.anything()); // 3rd arg = sim timeMs + expect(pwmCb).toHaveBeenCalledWith(1, 128 / 255); }); it('PinManager receives PWM update on pin 0 when OCR0A (0x56) is written', () => { @@ -300,7 +300,7 @@ describe('ATtiny85 — PWM monitoring', () => { sim.start(); sim.stop(); - expect(pwmCb).toHaveBeenCalledWith(0, 64 / 255, expect.anything()); // 3rd arg = sim timeMs + expect(pwmCb).toHaveBeenCalledWith(0, 64 / 255); }); it('ATtiny85 PWM covers 4 pins (OCR0A/OCR0B/OCR1A/OCR1B)', () => { diff --git a/frontend/src/__tests__/mega-emulation.test.ts b/frontend/src/__tests__/mega-emulation.test.ts index 4f373576..0752eeda 100644 --- a/frontend/src/__tests__/mega-emulation.test.ts +++ b/frontend/src/__tests__/mega-emulation.test.ts @@ -301,7 +301,7 @@ describe('AVRSimulator Mega — PWM OCR mapping differs from Uno', () => { // Call pollPwmRegisters directly — avoids RAF dependency in unit tests (sim as any).pollPwmRegisters(); - expect(cb).toHaveBeenCalledWith(13, 128 / 255, expect.anything()); // 3rd arg = sim timeMs + expect(cb).toHaveBeenCalledWith(13, 128 / 255); sim.stop(); }); @@ -318,7 +318,7 @@ describe('AVRSimulator Mega — PWM OCR mapping differs from Uno', () => { (sim as any).pollPwmRegisters(); - expect(cb).toHaveBeenCalledWith(5, 200 / 255, expect.anything()); // 3rd arg = sim timeMs + expect(cb).toHaveBeenCalledWith(5, 200 / 255); sim.stop(); }); @@ -335,7 +335,7 @@ describe('AVRSimulator Mega — PWM OCR mapping differs from Uno', () => { (sim as any).pollPwmRegisters(); - expect(cb).toHaveBeenCalledWith(6, 100 / 255, expect.anything()); // 3rd arg = sim timeMs + expect(cb).toHaveBeenCalledWith(6, 100 / 255); sim.stop(); }); }); diff --git a/frontend/src/simulation/PinManager.ts b/frontend/src/simulation/PinManager.ts index 180ef9f4..15a68714 100644 --- a/frontend/src/simulation/PinManager.ts +++ b/frontend/src/simulation/PinManager.ts @@ -201,7 +201,13 @@ export class PinManager { if (dutyCycle > 0) this.outputPins.add(pin); const callbacks = this.pwmListeners.get(pin); if (callbacks) { - callbacks.forEach((cb) => cb(pin, dutyCycle, timeMs)); + // Backward-compatible dispatch: the original PwmCallback contract is + // (pin, dutyCycle). Only listeners that actually declare a 3rd parameter + // (the buzzer, which needs the precise onset time for sample-accurate + // audio) receive timeMs. Plain 2-arg listeners — and the existing tests + // that assert toHaveBeenCalledWith(pin, dutyCycle) — see an unchanged + // 2-arg call instead of a spurious trailing arg. + callbacks.forEach((cb) => (cb.length >= 3 ? cb(pin, dutyCycle, timeMs) : cb(pin, dutyCycle))); } }