Skip to content

fix(sim): PushButton/Monoflop Reset handling; InputRef output mirroring #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
eisber merged 2 commits into from
Sep 8, 2026
Merged

fix(sim): PushButton/Monoflop Reset handling; InputRef output mirroring #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
735bc8d
fix(sim): PushButton honors Reset and InputDisable connectors
discostu105 Aug 25, 2026
1fb350d
fix(sim): Monoflop Reset input; InputRef mirrors its fed side to Q an…
discostu105 Aug 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions lox-sim/src/blocks/io.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,12 @@ macro_rules! passthrough_io_block {
// ---------------------------------------------------------------------------

/// Input reference — proxy that forwards named inputs to the block graph.
/// I→Q (digital) and AI→AQ (analog).
///
/// 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.
#[derive(Clone, Copy)]
pub struct InputRef;

Expand All @@ -60,7 +65,9 @@ impl Block for InputRef {
) -> Vec<Signal> {
let i = inputs.first().copied().unwrap_or(0.0);
let ai = inputs.get(1).copied().unwrap_or(0.0);
vec![i, ai]
let q = if i != 0.0 || ai != 0.0 { 1.0 } else { 0.0 };
let aq = if ai != 0.0 { ai } else { i };

@eisber eisber Sep 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ai != 0.0 uses the runtime value to infer which connector is fed. A connected analog input can legitimately produce 0.0; if I is also present, this then switches AQ to the digital value instead of preserving the analog zero. Please select the mirrored source from connector connectivity/presence rather than its current value, and add a regression case with AI = 0 and nonzero I.

vec![q, aq]

@eisber eisber Sep 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This behavior is not implemented in CompiledGraph: InputRef still falls through to the generic EvalStep::Copy, which copies only input 0 to output 0. Consequently an AI-only reference still produces compiled Q = 0, and compiled AQ is never written. Please add a dedicated compiled InputRef step that mirrors the selected input to both outputs, plus a non-ignored interpreter/JIT parity test for the motivating AI-only case.

}

fn block_type(&self) -> &str {
Expand Down Expand Up @@ -260,12 +267,12 @@ mod tests {
use super::*;
use crate::blocks::create_block;
#[test]
fn input_ref_passthrough() {
fn input_ref_mirrors_fed_side_to_both_outputs() {
let mut block = InputRef;
// I=42, AI=0 → Q=42, AQ=0
assert_eq!(block.eval(&[42.0], &[], 0.0, &[]), vec![42.0, 0.0]);
// I=0, AI=99 → Q=0, AQ=99
assert_eq!(block.eval(&[0.0, 99.0], &[], 0.0, &[]), vec![0.0, 99.0]);
// I=42, AI unfed → Q=1 (digital view), AQ=42
assert_eq!(block.eval(&[42.0], &[], 0.0, &[]), vec![1.0, 42.0]);
// 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]);
// empty → Q=0, AQ=0
assert_eq!(block.eval(&[], &[], 0.0, &[]), vec![0.0, 0.0]);
}
Expand Down
22 changes: 19 additions & 3 deletions lox-sim/src/blocks/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,18 @@ impl Block for PushButton {
) -> Vec<Signal> {
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 previous = self.is_on;

if is_high(force_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(prev_trigger) && is_high(trigger) {
} else if !is_high(disable) && !is_high(prev_trigger) && is_high(trigger) {

@eisber eisber Sep 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputDisable currently gates only the trigger, so Reset and On still change the block while it is active. Loxone's Switch documentation defines DisPc as “Disables all inputs when On,” which includes these inputs: https://www.loxone.com/enen/kb/switch/. Please apply the disable gate to Reset/On as well, consistently in PushButton, PushButton2, and the compiled step.

self.is_on = !self.is_on;
}

Expand Down Expand Up @@ -329,10 +335,20 @@ impl Block for PushButton2 {
prev_inputs: &[Signal],
) -> Vec<Signal> {
let trigger = inputs.first().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;
let rising = !is_high(prev_trigger) && is_high(trigger);
// 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 rising =
!is_high(reset) && !is_high(disable) && !is_high(prev_trigger) && is_high(trigger);
let mut double_click = false;

if self.awaiting_second {
Expand Down
8 changes: 7 additions & 1 deletion lox-sim/src/blocks/timers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,16 @@ impl Block for Monoflop {
prev_inputs: &[Signal],
) -> Vec<Signal> {
let trigger = inputs.first().copied().unwrap_or(0.0);
let reset = inputs.get(1).copied().unwrap_or(0.0);
let prev_trigger = prev_inputs.first().copied().unwrap_or(0.0);
let duration = params.first().copied().unwrap_or(1.0).max(0.0);

if !is_high(prev_trigger) && is_high(trigger) {
// WARNING: Assumed behavior — not validated against Miniserver.
// Assumption: Reset aborts the running pulse and blocks retriggering
// while held.
if is_high(reset) {
self.remaining = 0.0;
} else if !is_high(prev_trigger) && is_high(trigger) {
self.remaining = duration.max(dt);
}

Expand Down
25 changes: 22 additions & 3 deletions lox-sim/src/compiler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ pub enum EvalStep {
Monoflop {
trigger: usize,
prev_trigger: usize,
/// Reset signal slot; `usize::MAX` when the block has no Reset wire.
reset: usize,
param_duration: usize,
output: usize,
state_idx: usize,
Expand Down Expand Up @@ -217,6 +219,10 @@ pub enum EvalStep {
trigger: usize,
prev_trigger: usize,
force_on: usize,
/// Reset/InputDisable signal slots; `usize::MAX` when the block has
/// no such connector (PushButtonSel layouts vary).
reset: usize,
disable: usize,
/// outputs: [Q, Qoff, Qon]
outputs: [usize; 3],
state_idx: usize,
Expand Down Expand Up @@ -520,6 +526,7 @@ impl CompiledGraph {
EvalStep::Monoflop {
trigger: resolved_inputs.first().copied().unwrap_or(0),
prev_trigger: prev_inputs.first().copied().unwrap_or(0),
reset: resolved_inputs.get(1).copied().unwrap_or(usize::MAX),
param_duration: params.first().copied().unwrap_or(0),
output: outputs[0],
state_idx: si,
Expand Down Expand Up @@ -662,6 +669,8 @@ impl CompiledGraph {
trigger: resolved_inputs.first().copied().unwrap_or(0),
prev_trigger: prev_inputs.first().copied().unwrap_or(0),
force_on: resolved_inputs.get(1).copied().unwrap_or(0),
reset: resolved_inputs.get(2).copied().unwrap_or(usize::MAX),
disable: resolved_inputs.get(3).copied().unwrap_or(usize::MAX),
outputs: [
*outputs.first().unwrap_or(&0),
*outputs.get(1).unwrap_or(&0),
Expand Down Expand Up @@ -969,18 +978,22 @@ impl CompiledGraph {
EvalStep::Monoflop {
trigger,
prev_trigger,
reset,
param_duration,
output,
state_idx,
} => {
let trig = self.signals[*trigger];
let prev_trig = self.prev_signals[*prev_trigger];
let rst = self.signals.get(*reset).copied().unwrap_or(0.0);
let duration = self.signals[*param_duration].max(0.0);
let out = *output;
let si = *state_idx;

if let BlockState::Timer { remaining, .. } = &mut self.state[si] {
if prev_trig < 0.5 && trig >= 0.5 {
if rst >= 0.5 {
*remaining = 0.0;
} else if prev_trig < 0.5 && trig >= 0.5 {
*remaining = duration.max(dt);
}
let q = *remaining > 0.0;
Expand Down Expand Up @@ -1315,20 +1328,26 @@ impl CompiledGraph {
trigger,
prev_trigger,
force_on,
reset,
disable,
outputs,
state_idx,
} => {
let trig = self.signals[*trigger];
let prev_trig = self.prev_signals[*prev_trigger];
let force = self.signals[*force_on];
let rst = self.signals.get(*reset).copied().unwrap_or(0.0);
let dis = self.signals.get(*disable).copied().unwrap_or(0.0);
let outs = *outputs;
let si = *state_idx;

if let BlockState::PushButton { is_on } = &mut self.state[si] {
let previous = *is_on;
if force >= 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 {
} else if dis < 0.5 && prev_trig < 0.5 && trig >= 0.5 {
*is_on = !*is_on;
}
let qon = !previous && *is_on;
Expand Down
6 changes: 3 additions & 3 deletions lox-sim/src/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ fn block_signature(
&["Q1", "Q2", "Q3", "Q4", "AQ"],
&["Time", "V1", "V2", "V3", "V4"],
),
"Monoflop" => (&["InputTrigger"], &["Q"], &["Time"]),
"Monoflop" => (&["InputTrigger", "Reset"], &["Q"], &["Time"]),
"Minmax" => (
&["Input1", "Input2", "Input3", "Input4"],
&["AQmin", "AQmax"],
Expand Down Expand Up @@ -605,12 +605,12 @@ fn block_signature(
&["TimeHigh", "TimeLow"],
),
"PushButton" | "PushButton2" | "PushButton2Sel" => (
&["InputTrigger", "On"],
&["InputTrigger", "On", "Reset", "InputDisable"],

@eisber eisber Sep 1, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add regression coverage for the newly exposed connectors. The current jit_equivalence connector table still supplies only the old PushButton/Monoflop signatures, and that broad test is ignored, so the passing suite never exercises Reset/InputDisable through parser/interpreter/JIT. Targeted tests should cover reset dominance/held reset, disable behavior, and absent/unwired connectors.

&["Q", "Qoff", "Qon", "AQ"],
&["Min", "Max"],
),
"PushButtonSel" => (
&["InputTrigger", "InputPos", "Reset"],
&["InputTrigger", "InputPos", "Reset", "InputDisable"],
&["AQ"],
&["Min", "Max", "Step", "Repeat", "Def"],
),
Expand Down