From 61d0416fc6c16a5c2b7bffd9a3289b7a6382a691 Mon Sep 17 00:00:00 2001 From: Markus Cozowicz Date: Tue, 8 Sep 2026 18:17:43 +0100 Subject: [PATCH 1/2] fix(sim): address input reference review gaps Select InputRef sources from graph connectivity and mirror values in both simulator engines. Make PushButton disable all peripheral inputs and add focused Reset, Disable, On, and JIT parity coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lox-sim/src/blocks/io.rs | 62 +++++++-- lox-sim/src/blocks/mod.rs | 8 +- lox-sim/src/blocks/state.rs | 87 +++++++++--- lox-sim/src/blocks/timers.rs | 17 +++ lox-sim/src/compiler.rs | 247 +++++++++++++++++++++++++++++++---- lox-sim/src/engine.rs | 12 +- lox-sim/src/parser.rs | 13 ++ 7 files changed, 387 insertions(+), 59 deletions(-) diff --git a/lox-sim/src/blocks/io.rs b/lox-sim/src/blocks/io.rs index a622c04..882d0d9 100644 --- a/lox-sim/src/blocks/io.rs +++ b/lox-sim/src/blocks/io.rs @@ -47,15 +47,44 @@ macro_rules! passthrough_io_block { /// Input reference — proxy that forwards named inputs to the block graph. /// -/// A ref is fed on exactly one side (I or AI) but the Miniserver mirrors the -/// signal on BOTH outputs: consumers routinely read Q from an AI-fed ref -/// (r50 corpus: `ref.AI <- mem.AQ` with `monoflop.InputTrigger: ref.Q`). -/// Q is the digital view (non-zero → 1), AQ the analog value; the unfed -/// side idles at 0, so combining the two inputs is lossless. +/// The Miniserver mirrors the fed input on BOTH outputs: consumers routinely +/// read Q from an AI-fed ref (r50 corpus: `ref.AI <- mem.AQ` with +/// `monoflop.InputTrigger: ref.Q`). Q is the digital view (non-zero -> 1), AQ +/// the analog value. When both inputs are wired, AI is authoritative. #[derive(Clone, Copy)] -pub struct InputRef; +pub struct InputRef { + source: InputRefSource, +} + +#[derive(Clone, Copy)] +enum InputRefSource { + Digital, + Analog, +} + +impl InputRef { + pub fn new() -> Self { + Self { + source: InputRefSource::Digital, + } + } +} + +impl Default for InputRef { + fn default() -> Self { + Self::new() + } +} impl Block for InputRef { + fn configure_input_connections(&mut self, connected: &[bool]) { + self.source = if connected.get(1).copied().unwrap_or(false) { + InputRefSource::Analog + } else { + InputRefSource::Digital + }; + } + fn eval( &mut self, inputs: &[Signal], @@ -63,11 +92,11 @@ impl Block for InputRef { _dt: f64, _prev: &[Signal], ) -> Vec { - let i = inputs.first().copied().unwrap_or(0.0); - let ai = inputs.get(1).copied().unwrap_or(0.0); - let q = if i != 0.0 || ai != 0.0 { 1.0 } else { 0.0 }; - let aq = if ai != 0.0 { ai } else { i }; - vec![q, aq] + let value = match self.source { + InputRefSource::Digital => inputs.first().copied().unwrap_or(0.0), + InputRefSource::Analog => inputs.get(1).copied().unwrap_or(0.0), + }; + vec![if value != 0.0 { 1.0 } else { 0.0 }, value] } fn block_type(&self) -> &str { @@ -268,13 +297,22 @@ mod tests { use crate::blocks::create_block; #[test] fn input_ref_mirrors_fed_side_to_both_outputs() { - let mut block = InputRef; + let mut block = InputRef::new(); // I=42, AI unfed → Q=1 (digital view), AQ=42 assert_eq!(block.eval(&[42.0], &[], 0.0, &[]), vec![1.0, 42.0]); + block.configure_input_connections(&[false, true]); // I unfed, AI=99 → Q=1, AQ=99 (AI-fed refs serve Q consumers) assert_eq!(block.eval(&[0.0, 99.0], &[], 0.0, &[]), vec![1.0, 99.0]); + // Both present, AI=0 → the analog zero remains authoritative. + assert_eq!(block.eval(&[1.0, 0.0], &[], 0.0, &[]), vec![0.0, 0.0]); // empty → Q=0, AQ=0 assert_eq!(block.eval(&[], &[], 0.0, &[]), vec![0.0, 0.0]); + + block.configure_input_connections(&[true, false]); + assert_eq!(block.eval(&[42.0, 99.0], &[], 0.0, &[]), vec![1.0, 42.0]); + + block.configure_input_connections(&[true, true]); + assert_eq!(block.eval(&[1.0, 0.0], &[], 0.0, &[]), vec![0.0, 0.0]); } #[test] diff --git a/lox-sim/src/blocks/mod.rs b/lox-sim/src/blocks/mod.rs index 5d1b753..aa7e049 100644 --- a/lox-sim/src/blocks/mod.rs +++ b/lox-sim/src/blocks/mod.rs @@ -221,6 +221,12 @@ impl Clone for Box { /// Core block trait — all simulation blocks implement this. pub trait Block: Send + Sync + BlockClone { + /// Configure which input connectors are wired before the first tick. + /// + /// Most blocks only depend on input values. Blocks whose semantics depend + /// on connector presence can override this hook. + fn configure_input_connections(&mut self, _connected: &[bool]) {} + /// Evaluate the block for one tick. fn eval( &mut self, @@ -591,7 +597,7 @@ pub fn create_block(block_type: &str) -> Box { "JoinWindowSensor" => Box::new(JoinWindowSensor), // Group D — I/O - "InputRef" => Box::new(InputRef), + "InputRef" => Box::new(InputRef::new()), "OutputRef" => Box::new(OutputRef), "OutputRefLM" => Box::new(OutputRefLM), "EIBPush" => Box::new(EIBPush), diff --git a/lox-sim/src/blocks/state.rs b/lox-sim/src/blocks/state.rs index 3a4eadd..4c8d4e5 100644 --- a/lox-sim/src/blocks/state.rs +++ b/lox-sim/src/blocks/state.rs @@ -257,14 +257,16 @@ impl Block for PushButton { let prev_trigger = prev_inputs.first().copied().unwrap_or(0.0); let previous = self.is_on; - // WARNING: Assumed behavior — not validated against Miniserver. - // Assumption: Reset dominates On; InputDisable gates only the trigger. - if is_high(reset) { - self.is_on = false; - } else if is_high(force_on) { - self.is_on = true; - } else if !is_high(disable) && !is_high(prev_trigger) && is_high(trigger) { - self.is_on = !self.is_on; + // InputDisable disables every peripheral input; Reset dominates On + // when peripheral control is enabled. + if !is_high(disable) { + if is_high(reset) { + self.is_on = false; + } else if is_high(force_on) { + self.is_on = true; + } else if !is_high(prev_trigger) && is_high(trigger) { + self.is_on = !self.is_on; + } } let qon = !previous && self.is_on; @@ -335,20 +337,23 @@ impl Block for PushButton2 { prev_inputs: &[Signal], ) -> Vec { let trigger = inputs.first().copied().unwrap_or(0.0); + let force_on = inputs.get(1).copied().unwrap_or(0.0); let reset = inputs.get(2).copied().unwrap_or(0.0); let disable = inputs.get(3).copied().unwrap_or(0.0); let prev_trigger = prev_inputs.first().copied().unwrap_or(0.0); let dc_window = params.first().copied().unwrap_or(0.4).max(0.0); let previous = self.is_on; - // WARNING: Assumed behavior — not validated against Miniserver. - // Assumption: Reset dominates and cancels a pending double-click; - // InputDisable gates only the trigger. - if is_high(reset) { - self.is_on = false; - self.awaiting_second = false; + let mut rising = false; + if !is_high(disable) { + if is_high(reset) { + self.is_on = false; + self.awaiting_second = false; + } else if is_high(force_on) { + self.is_on = true; + } else { + rising = !is_high(prev_trigger) && is_high(trigger); + } } - let rising = - !is_high(reset) && !is_high(disable) && !is_high(prev_trigger) && is_high(trigger); let mut double_click = false; if self.awaiting_second { @@ -940,6 +945,56 @@ mod tests { ); } + #[test] + fn pushbutton_reset_dominates_on_when_enabled() { + let mut block = PushButton::new(); + assert_eq!( + block.eval(&[1.0, 1.0, 1.0, 0.0], &[], 0.0, &[0.0, 0.0, 0.0, 0.0]), + vec![0.0, 0.0, 0.0] + ); + } + + #[test] + fn pushbutton_disable_blocks_all_inputs() { + let mut block = PushButton::new(); + block.eval(&[1.0, 0.0, 0.0, 0.0], &[], 0.0, &[0.0, 0.0, 0.0, 0.0]); + + assert_eq!( + block.eval(&[0.0, 0.0, 1.0, 1.0], &[], 0.0, &[1.0, 0.0, 0.0, 0.0]), + vec![1.0, 0.0, 0.0], + "disabled Reset must not change the state" + ); + + let mut off = PushButton::new(); + assert_eq!( + off.eval(&[1.0, 1.0, 0.0, 1.0], &[], 0.0, &[0.0, 0.0, 0.0, 1.0]), + vec![0.0, 0.0, 0.0], + "disabled Trigger and On must not change the state" + ); + } + + #[test] + fn pushbutton2_disable_blocks_reset_and_trigger() { + let mut block = PushButton2::new(); + block.eval(&[1.0, 0.0, 0.0, 0.0], &[0.5], 0.0, &[0.0, 0.0, 0.0, 0.0]); + let out = block.eval(&[0.0, 0.0, 1.0, 1.0], &[0.5], 0.1, &[1.0, 0.0, 0.0, 0.0]); + assert_eq!(out[0], 1.0); + assert_eq!(out[1], 0.0); + } + + #[test] + fn pushbutton2_on_forces_output_unless_disabled() { + let mut block = PushButton2::new(); + let out = block.eval(&[0.0, 1.0, 0.0, 0.0], &[0.5], 0.1, &[0.0; 4]); + assert_eq!(out[0], 1.0); + assert_eq!(out[2], 1.0); + + let mut disabled = PushButton2::new(); + let out = disabled.eval(&[0.0, 1.0, 0.0, 1.0], &[0.5], 0.1, &[0.0; 4]); + assert_eq!(out[0], 0.0); + assert_eq!(out[2], 0.0); + } + #[test] fn pushbutton2_detects_double_click() { let mut block = PushButton2::new(); diff --git a/lox-sim/src/blocks/timers.rs b/lox-sim/src/blocks/timers.rs index bacc3e5..c03117b 100644 --- a/lox-sim/src/blocks/timers.rs +++ b/lox-sim/src/blocks/timers.rs @@ -979,6 +979,23 @@ mod tests { assert_eq!(block.eval(&[0.0], &[1.0], 0.25, &[0.0]), vec![0.0]); } + #[test] + fn monoflop_reset_aborts_and_blocks_retrigger() { + let mut block = Monoflop::new(); + assert_eq!( + block.eval(&[1.0, 0.0], &[1.0], 0.25, &[0.0, 0.0]), + vec![1.0] + ); + assert_eq!( + block.eval(&[1.0, 1.0], &[1.0], 0.25, &[1.0, 0.0]), + vec![0.0] + ); + assert_eq!( + block.eval(&[1.0, 1.0], &[1.0], 0.25, &[0.0, 1.0]), + vec![0.0] + ); + } + #[test] fn on_pulse_delay_emits_delayed_pulse() { let mut block = OnPulseDelay::new(); diff --git a/lox-sim/src/compiler.rs b/lox-sim/src/compiler.rs index bc23353..6c3b311 100644 --- a/lox-sim/src/compiler.rs +++ b/lox-sim/src/compiler.rs @@ -246,9 +246,9 @@ pub enum EvalStep { dst: usize, }, InputRef { - digital: usize, - analog: usize, - outputs: [usize; 2], + src: usize, + output_q: Option, + output_aq: Option, }, // -- Constant (from parameter) -- @@ -708,6 +708,18 @@ impl CompiledGraph { state_idx: si, } } + "InputRef" => { + let analog_connected = info + .inputs + .get(1) + .is_some_and(|&cid| graph.input_source_of(cid).is_some()); + let source_index = usize::from(analog_connected); + EvalStep::InputRef { + src: resolved_inputs.get(source_index).copied().unwrap_or(0), + output_q: outputs.first().copied(), + output_aq: outputs.get(1).copied(), + } + } "Counter" => { let si = state.len(); state.push(BlockState::Counter { count: 0.0 }); @@ -839,14 +851,6 @@ impl CompiledGraph { state_idx: si, } } - "InputRef" => EvalStep::InputRef { - digital: resolved_inputs.first().copied().unwrap_or(0), - analog: resolved_inputs.get(1).copied().unwrap_or(0), - outputs: [ - *outputs.first().unwrap_or(&0), - *outputs.get(1).unwrap_or(&0), - ], - }, // Default: PassThrough / unknown → copy first input to first output _ => EvalStep::Copy { src: resolved_inputs.first().copied().unwrap_or(0), @@ -923,6 +927,19 @@ impl CompiledGraph { let high_count = inputs.iter().filter(|&&i| self.signals[i] >= 0.5).count(); self.signals[*output] = bool_f64(high_count % 2 == 1); } + EvalStep::InputRef { + src, + output_q, + output_aq, + } => { + let value = self.signals[*src]; + if let Some(output) = output_q { + self.signals[*output] = bool_f64(value != 0.0); + } + if let Some(output) = output_aq { + self.signals[*output] = value; + } + } // -- Comparisons -- EvalStep::GreaterEqual { @@ -1390,12 +1407,14 @@ impl CompiledGraph { if let BlockState::PushButton { is_on } = &mut self.state[si] { let previous = *is_on; - if rst >= 0.5 { - *is_on = false; - } else if force >= 0.5 { - *is_on = true; - } else if dis < 0.5 && prev_trig < 0.5 && trig >= 0.5 { - *is_on = !*is_on; + if dis < 0.5 { + if rst >= 0.5 { + *is_on = false; + } else if force >= 0.5 { + *is_on = true; + } else if prev_trig < 0.5 && trig >= 0.5 { + *is_on = !*is_on; + } } let qon = !previous && *is_on; let qoff = previous && !*is_on; @@ -1429,17 +1448,6 @@ impl CompiledGraph { EvalStep::Copy { src, dst } => { self.signals[*dst] = self.signals[*src]; } - EvalStep::InputRef { - digital, - analog, - outputs, - } => { - let digital = self.signals[*digital]; - let analog = self.signals[*analog]; - self.signals[outputs[0]] = bool_f64(digital != 0.0 || analog != 0.0); - self.signals[outputs[1]] = if analog != 0.0 { analog } else { digital }; - } - // -- Constant -- EvalStep::Const { param, output } => { self.signals[*output] = self.signals[*param]; @@ -1978,6 +1986,187 @@ mod tests { assert_eq!(c.get_output("Mono"), 0.0, "tick 5: should be low"); } + #[test] + fn compiled_monoflop_reset_aborts_and_blocks_retrigger() { + let mut g = SimGraph::new(); + let trig = g.add_block("Trig", pt(), &["I1"], &["Q"], &[]); + let reset = g.add_block("Reset", pt(), &["I1"], &["Q"], &[]); + let mono = g.add_block( + "Mono", + Box::new(blocks::Monoflop::new()), + &["InputTrigger", "Reset"], + &["Q"], + &["Time"], + ); + g.add_wire( + g.find_connector(trig, "Q").unwrap(), + g.find_connector(mono, "InputTrigger").unwrap(), + ) + .unwrap(); + g.add_wire( + g.find_connector(reset, "Q").unwrap(), + g.find_connector(mono, "Reset").unwrap(), + ) + .unwrap(); + + let mut c = CompiledGraph::from_graph(&g); + c.set_param("Mono", "Time", 1.0); + c.set_input("Trig", 1.0); + c.tick(0.25); + assert_eq!(c.get_output("Mono"), 1.0); + + c.set_input("Reset", 1.0); + c.tick(0.25); + assert_eq!(c.get_output("Mono"), 0.0); + c.tick(0.25); + assert_eq!(c.get_output("Mono"), 0.0); + } + + #[test] + fn compiled_pushbutton_disable_blocks_all_inputs() { + let mut g = SimGraph::new(); + let trigger = g.add_block("Trigger", pt(), &["I1"], &["Q"], &[]); + let on = g.add_block("On", pt(), &["I1"], &["Q"], &[]); + let reset = g.add_block("Reset", pt(), &["I1"], &["Q"], &[]); + let disable = g.add_block("Disable", pt(), &["I1"], &["Q"], &[]); + let button = g.add_block( + "Button", + Box::new(blocks::PushButton::new()), + &["InputTrigger", "On", "Reset", "InputDisable"], + &["Q", "Qoff", "Qon"], + &[], + ); + for (source, input) in [ + (trigger, "InputTrigger"), + (on, "On"), + (reset, "Reset"), + (disable, "InputDisable"), + ] { + g.add_wire( + g.find_connector(source, "Q").unwrap(), + g.find_connector(button, input).unwrap(), + ) + .unwrap(); + } + + let mut c = CompiledGraph::from_graph(&g); + c.set_input("Trigger", 1.0); + c.tick(0.1); + assert_eq!(c.get_output("Button"), 1.0); + + c.set_input("Trigger", 0.0); + c.set_input("Reset", 1.0); + c.set_input("Disable", 1.0); + c.tick(0.1); + assert_eq!(c.get_output("Button"), 1.0); + + c.set_input("Disable", 0.0); + c.tick(0.1); + assert_eq!(c.get_output("Button"), 0.0); + + c.set_input("Reset", 0.0); + c.set_input("On", 1.0); + c.set_input("Disable", 1.0); + c.tick(0.1); + assert_eq!(c.get_output("Button"), 0.0); + } + + #[test] + fn pushbutton2_on_matches_interpreter_and_compiler() { + let mut g = SimGraph::new(); + let on = g.add_block("On", pt(), &["I1"], &["Q"], &[]); + let button = g.add_block( + "Button", + Box::new(blocks::PushButton2::new()), + &["InputTrigger", "On", "Reset", "InputDisable"], + &["Q", "Qoff", "Qon", "QDoubleClick"], + &["DoubleClickTime"], + ); + g.add_wire( + g.find_connector(on, "Q").unwrap(), + g.find_connector(button, "On").unwrap(), + ) + .unwrap(); + + let mut engine = SimEngine::new(g.clone()); + let mut compiled = CompiledGraph::from_graph(&g); + engine.set_input("On", 1.0); + compiled.set_input("On", 1.0); + engine.tick(0.1); + compiled.tick(0.1); + + assert_eq!(engine.get_output("Button"), 1.0); + assert_eq!(compiled.get_output("Button"), 1.0); + } + + #[test] + fn input_ref_mirrors_ai_in_interpreter_and_compiler() { + let mut g = SimGraph::new(); + let analog = g.add_block("Analog", pt(), &["I1"], &["Q"], &[]); + let input_ref = g.add_block( + "Ref", + Box::new(blocks::InputRef::new()), + &["I", "AI"], + &["Q", "AQ"], + &[], + ); + g.add_wire( + g.find_connector(analog, "Q").unwrap(), + g.find_connector(input_ref, "AI").unwrap(), + ) + .unwrap(); + + let mut engine = SimEngine::new(g.clone()); + let mut compiled = CompiledGraph::from_graph(&g); + engine.set_input("Analog", 99.0); + compiled.set_input("Analog", 99.0); + engine.tick(0.1); + compiled.tick(0.1); + + assert_eq!(engine.get_output("Ref.Q"), 1.0); + assert_eq!(engine.get_output("Ref.AQ"), 99.0); + assert_eq!(compiled.get_output("Ref.Q"), 1.0); + assert_eq!(compiled.get_output("Ref.AQ"), 99.0); + } + + #[test] + fn input_ref_preserves_connected_analog_zero() { + let mut g = SimGraph::new(); + let digital = g.add_block("Digital", pt(), &["I1"], &["Q"], &[]); + let analog = g.add_block("Analog", pt(), &["I1"], &["Q"], &[]); + let input_ref = g.add_block( + "Ref", + Box::new(blocks::InputRef::new()), + &["I", "AI"], + &["Q", "AQ"], + &[], + ); + g.add_wire( + g.find_connector(digital, "Q").unwrap(), + g.find_connector(input_ref, "I").unwrap(), + ) + .unwrap(); + g.add_wire( + g.find_connector(analog, "Q").unwrap(), + g.find_connector(input_ref, "AI").unwrap(), + ) + .unwrap(); + + let mut engine = SimEngine::new(g.clone()); + let mut compiled = CompiledGraph::from_graph(&g); + engine.set_input("Digital", 1.0); + compiled.set_input("Digital", 1.0); + engine.set_input("Analog", 0.0); + compiled.set_input("Analog", 0.0); + engine.tick(0.1); + compiled.tick(0.1); + + assert_eq!(engine.get_output("Ref.Q"), 0.0); + assert_eq!(engine.get_output("Ref.AQ"), 0.0); + assert_eq!(compiled.get_output("Ref.Q"), 0.0); + assert_eq!(compiled.get_output("Ref.AQ"), 0.0); + } + // -- FlipFlop -- #[test] @@ -2351,7 +2540,7 @@ mod tests { let source = g.add_block("Source", pt(), &["I"], &["Q"], &[]); let input_ref = g.add_block( "Reference", - Box::new(blocks::InputRef), + Box::new(blocks::InputRef::new()), &["I", "AI"], &["Q", "AQ"], &[], diff --git a/lox-sim/src/engine.rs b/lox-sim/src/engine.rs index da0c909..9cc75f6 100644 --- a/lox-sim/src/engine.rs +++ b/lox-sim/src/engine.rs @@ -143,10 +143,20 @@ impl SimEngine { /// Build an engine from a fully-wired graph. pub fn new(mut graph: SimGraph) -> Self { let topo = graph.topological_order(); - let blocks = graph.take_block_impls(); + let mut blocks = graph.take_block_impls(); let n_conn = graph.connector_count(); let n_blocks = graph.block_count(); + for (bid, block) in blocks.iter_mut().enumerate() { + let connected: Vec = graph + .block_info(bid) + .inputs + .iter() + .map(|&cid| graph.input_source_of(cid).is_some()) + .collect(); + block.configure_input_connections(&connected); + } + // Initialise signals with connector defaults. let signals: Vec = (0..n_conn) .map(|i| graph.connector(i).default_value) diff --git a/lox-sim/src/parser.rs b/lox-sim/src/parser.rs index 2fe776b..5eb4ff2 100644 --- a/lox-sim/src/parser.rs +++ b/lox-sim/src/parser.rs @@ -2190,6 +2190,19 @@ fn is_structural_type(block_type: &str) -> bool { mod tests { use super::*; + #[test] + fn reset_and_disable_connectors_have_canonical_positions() { + assert_eq!(block_signature("Monoflop").0, &["InputTrigger", "Reset"]); + assert_eq!( + block_signature("PushButton").0, + &["InputTrigger", "On", "Reset", "InputDisable"] + ); + assert_eq!( + block_signature("PushButton2").0, + &["InputTrigger", "On", "Reset", "InputDisable"] + ); + } + fn parse_xml(xml: &str) -> SimGraph { parse_bytes(xml.as_bytes()).expect("parse failed") } From 78157e787e4f3db601fff49553df528ba3a34271 Mon Sep 17 00:00:00 2001 From: Markus Cozowicz Date: Tue, 8 Sep 2026 18:34:43 +0100 Subject: [PATCH 2/2] fix(sim): satisfy current Clippy slice lint Use fixed-size slice chunks when decoding serialized floating-point state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lox-sim/src/blocks/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lox-sim/src/blocks/mod.rs b/lox-sim/src/blocks/mod.rs index aa7e049..e8a1077 100644 --- a/lox-sim/src/blocks/mod.rs +++ b/lox-sim/src/blocks/mod.rs @@ -107,8 +107,8 @@ pub(crate) fn deserialize_f64s(bytes: &[u8], count: usize) -> Option> { return None; } let mut values = Vec::with_capacity(count); - for chunk in bytes[..count * 8].chunks_exact(8) { - values.push(f64::from_le_bytes(chunk.try_into().ok()?)); + for chunk in bytes[..count * 8].as_chunks::<8>().0 { + values.push(f64::from_le_bytes(*chunk)); } Some(values) }