diff --git a/conductor-core/src/config/types.rs b/conductor-core/src/config/types.rs index b291887d..a4696e66 100644 --- a/conductor-core/src/config/types.rs +++ b/conductor-core/src/config/types.rs @@ -685,13 +685,7 @@ impl DeviceIdentityConfig { /// - `channel` is `None` = pass through (non-MIDI events like HID have no channel) /// - Otherwise, checks if the channel is in the configured list pub fn is_channel_in_scope(&self, channel: Option) -> bool { - if self.channels.is_empty() { - return true; // No scope restriction - } - match channel { - None => true, // Non-MIDI events bypass channel filtering - Some(ch) => self.channels.contains(&ch), - } + crate::rule_set::channel_in_scope(&self.channels, channel) } } diff --git a/conductor-core/src/event_processor.rs b/conductor-core/src/event_processor.rs index f801133f..84c38079 100644 --- a/conductor-core/src/event_processor.rs +++ b/conductor-core/src/event_processor.rs @@ -285,6 +285,28 @@ pub enum ProcessedEvent { Raw(InputEvent), } +impl ProcessedEvent { + /// Extract the MIDI channel from any variant (#751). + /// Returns None for Raw passthrough (and gamepad events which set channel=None). + pub fn channel(&self) -> Option { + match self { + Self::ShortPress { channel, .. } + | Self::MediumPress { channel, .. } + | Self::LongPress { channel, .. } + | Self::HoldDetected { channel, .. } + | Self::PadPressed { channel, .. } + | Self::PadReleased { channel, .. } + | Self::DoubleTap { channel, .. } + | Self::EncoderTurned { channel, .. } + | Self::CCReceived { channel, .. } + | Self::AftertouchChanged { channel, .. } + | Self::PitchBendMoved { channel, .. } + | Self::ChordDetected { channel, .. } => *channel, + Self::Raw(_) => None, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq)] pub enum VelocityLevel { Soft, diff --git a/conductor-core/src/rule_compiler.rs b/conductor-core/src/rule_compiler.rs index 4a4a17dc..9e68e597 100644 --- a/conductor-core/src/rule_compiler.rs +++ b/conductor-core/src/rule_compiler.rs @@ -32,11 +32,22 @@ pub fn compile(config: &Config, version: u64) -> CompiledRuleSet { let (global_device_rules, global_any_device_rules) = compile_mappings_split(&config.global_mappings); + // Build channel scopes from device identity configs (#751). + // Only devices with non-empty channels are included; missing key = all channels. + // This is intentional: is_channel_in_scope() treats missing key as "no restriction". + let channel_scopes: HashMap> = config + .devices + .iter() + .filter(|d| !d.channels.is_empty()) + .map(|d| (d.alias.clone(), d.channels.clone())) + .collect(); + CompiledRuleSet::new( mode_rules, global_device_rules, global_any_device_rules, version, + channel_scopes, ) } diff --git a/conductor-core/src/rule_set.rs b/conductor-core/src/rule_set.rs index 124e6b33..9336ef06 100644 --- a/conductor-core/src/rule_set.rs +++ b/conductor-core/src/rule_set.rs @@ -24,6 +24,9 @@ pub struct CompiledRuleSet { global_rules: GlobalRuleSet, /// Monotonic version for debugging/logging version: u64, + /// Channel scopes per device alias (#751). + /// Missing key = no restriction (all channels). Only non-empty scopes are stored. + channel_scopes: HashMap>, } /// Rules for a single mode, with per-device indexing for O(1) lookup @@ -68,6 +71,7 @@ impl CompiledRuleSet { global_device_rules: HashMap>, global_any_device_rules: Vec, version: u64, + channel_scopes: HashMap>, ) -> Self { Self { mode_rules, @@ -76,6 +80,15 @@ impl CompiledRuleSet { any_device_rules: global_any_device_rules, }, version, + channel_scopes, + } + } + + /// Check if an event's channel is in scope for a device (#751) + fn is_channel_in_scope(&self, device_id: &str, event: &ProcessedEvent) -> bool { + match self.channel_scopes.get(device_id) { + Some(channels) => channel_in_scope(channels, event.channel()), + None => true, // Missing key = no restriction (all channels) } } @@ -92,10 +105,16 @@ impl CompiledRuleSet { mode_index: usize, device_id: Option<&str>, ) -> Option { + // Channel scope check (#751) — mirrored in match_event_with_provenance(). + // If either changes, update both. + let device_in_scope = + device_id.is_none_or(|device| self.is_channel_in_scope(device, event)); + // Check mode rules if mode_index is valid if let Some(mode) = self.mode_rules.get(mode_index) { - // 1. Device-specific rules for this mode - if let Some(device) = device_id + // 1. Device-specific rules for this mode (skip if channel out of scope) + if device_in_scope + && let Some(device) = device_id && let Some(rules) = mode.device_rules.get(device) && let Some(action) = find_matching_action(rules, event) { @@ -108,8 +127,9 @@ impl CompiledRuleSet { } } - // 3. Global device-specific rules - if let Some(device) = device_id + // 3. Global device-specific rules (skip if channel out of scope) + if device_in_scope + && let Some(device) = device_id && let Some(rules) = self.global_rules.device_rules.get(device) && let Some(action) = find_matching_action(rules, event) { @@ -152,10 +172,15 @@ impl CompiledRuleSet { ) -> Option { let mode_name = self.mode_name(mode_index).map(String::from); + // Channel scope check (#751) — mirrored in match_event(). If either changes, update both. + let device_in_scope = + device_id.is_none_or(|device| self.is_channel_in_scope(device, event)); + // Check mode rules if mode_index is valid if let Some(mode) = self.mode_rules.get(mode_index) { - // 1. Device-specific rules for this mode - if let Some(device) = device_id + // 1. Device-specific rules for this mode (skip if channel out of scope) + if device_in_scope + && let Some(device) = device_id && let Some(rules) = mode.device_rules.get(device) && let Some(envelope) = find_matching_envelope(rules, event, device_id, &mode_name) { @@ -170,8 +195,9 @@ impl CompiledRuleSet { } } - // 3. Global device-specific rules - if let Some(device) = device_id + // 3. Global device-specific rules (skip if channel out of scope) + if device_in_scope + && let Some(device) = device_id && let Some(rules) = self.global_rules.device_rules.get(device) && let Some(envelope) = find_matching_envelope(rules, event, device_id, &mode_name) { @@ -217,3 +243,16 @@ fn find_matching_envelope( } None } + +/// Check if a MIDI channel is within a channel scope (#751). +/// Shared logic used by both CompiledRuleSet and DeviceIdentityConfig. +/// Empty scope = all channels. None channel (non-MIDI) = pass through. +pub(crate) fn channel_in_scope(channels: &[u8], channel: Option) -> bool { + if channels.is_empty() { + return true; + } + match channel { + Some(ch) => channels.contains(&ch), + None => true, // Non-MIDI events bypass channel filtering + } +} diff --git a/conductor-core/tests/multi_device_rule_routing_test.rs b/conductor-core/tests/multi_device_rule_routing_test.rs index 08c7058c..21b23a94 100644 --- a/conductor-core/tests/multi_device_rule_routing_test.rs +++ b/conductor-core/tests/multi_device_rule_routing_test.rs @@ -324,3 +324,202 @@ fn test_multi_mode_device_routing() { let action_1 = rule_set.match_event(&event, 1, Some("pads")); assert!(matches!(action_1, Some(Action::Shell(_)))); } + +// #751: Channel scope filtering in rule matching +#[test] +fn test_channel_scope_filters_events() { + // Device "drums" only responds to channel 9 (drum channel) + let config = Config { + device: None, + devices: vec![DeviceIdentityConfig { + alias: "drums".to_string(), + matchers: vec![DeviceMatcher::name_contains("Drums")], + description: None, + enabled: true, + input: None, + output: None, + protocol: None, + channels: vec![9], // Only channel 9 (0-indexed, i.e., MIDI channel 10) + }], + modes: vec![Mode { + name: "Default".to_string(), + color: None, + mappings: vec![Mapping { + trigger: Trigger::Note { + note: 36, + velocity_min: None, + channel: None, // Trigger matches any channel + device: Some("drums".to_string()), + }, + action: ActionConfig::Keystroke { + keys: "d".to_string(), + modifiers: vec![], + }, + description: Some("Drum hit".to_string()), + }], + }], + global_mappings: vec![], + advanced_settings: Default::default(), + led: None, + event_console: None, + default_mode: None, + last_selected_mode: None, + logging: None, + }; + + let rule_set = conductor_core::rule_compiler::compile(&config, 1); + + // Event on channel 9 → should match (in scope) + let event_ch9 = ProcessedEvent::PadPressed { + note: 36, + velocity: 100, + velocity_level: VelocityLevel::Hard, + channel: Some(9), + }; + let action = rule_set.match_event(&event_ch9, 0, Some("drums")); + assert!(action.is_some(), "Channel 9 should match (in scope)"); + + // Event on channel 0 → should NOT match (out of scope) + let event_ch0 = ProcessedEvent::PadPressed { + note: 36, + velocity: 100, + velocity_level: VelocityLevel::Hard, + channel: Some(0), + }; + let action = rule_set.match_event(&event_ch0, 0, Some("drums")); + assert!( + action.is_none(), + "Channel 0 should NOT match (out of scope)" + ); +} + +#[test] +fn test_empty_channel_scope_matches_all() { + // Device with no channel scope → matches all channels + let config = Config { + device: None, + devices: vec![DeviceIdentityConfig { + alias: "pads".to_string(), + matchers: vec![DeviceMatcher::name_contains("Pads")], + description: None, + enabled: true, + input: None, + output: None, + protocol: None, + channels: vec![], // Empty = all channels + }], + modes: vec![Mode { + name: "Default".to_string(), + color: None, + mappings: vec![Mapping { + trigger: Trigger::Note { + note: 60, + velocity_min: None, + channel: None, + device: Some("pads".to_string()), + }, + action: ActionConfig::Keystroke { + keys: "p".to_string(), + modifiers: vec![], + }, + description: None, + }], + }], + global_mappings: vec![], + advanced_settings: Default::default(), + led: None, + event_console: None, + default_mode: None, + last_selected_mode: None, + logging: None, + }; + + let rule_set = conductor_core::rule_compiler::compile(&config, 1); + + // Any channel should match + let event = ProcessedEvent::PadPressed { + note: 60, + velocity: 80, + velocity_level: VelocityLevel::Medium, + channel: Some(5), + }; + assert!(rule_set.match_event(&event, 0, Some("pads")).is_some()); +} + +#[test] +fn test_channel_out_of_scope_still_matches_any_device_rules() { + // Device "drums" scoped to channel 9, but there's an any-device rule for note 36. + // An event on channel 0 should skip the device-specific rule but still match + // the any-device rule. + let config = Config { + device: None, + devices: vec![DeviceIdentityConfig { + alias: "drums".to_string(), + matchers: vec![DeviceMatcher::name_contains("Drums")], + description: None, + enabled: true, + input: None, + output: None, + protocol: None, + channels: vec![9], + }], + modes: vec![Mode { + name: "Default".to_string(), + color: None, + mappings: vec![ + // Device-specific rule — should NOT fire for ch.0 + Mapping { + trigger: Trigger::Note { + note: 36, + velocity_min: None, + channel: None, + device: Some("drums".to_string()), + }, + action: ActionConfig::Shell { + command: "device-action".to_string(), + }, + description: Some("Device rule".to_string()), + }, + // Any-device rule — SHOULD fire for ch.0 + Mapping { + trigger: Trigger::Note { + note: 36, + velocity_min: None, + channel: None, + device: None, // No device filter + }, + action: ActionConfig::Keystroke { + keys: "fallback".to_string(), + modifiers: vec![], + }, + description: Some("Any-device rule".to_string()), + }, + ], + }], + global_mappings: vec![], + advanced_settings: Default::default(), + led: None, + event_console: None, + default_mode: None, + last_selected_mode: None, + logging: None, + }; + + let rule_set = conductor_core::rule_compiler::compile(&config, 1); + + // Event on channel 0 (out of scope for "drums") + let event_ch0 = ProcessedEvent::PadPressed { + note: 36, + velocity: 100, + velocity_level: VelocityLevel::Hard, + channel: Some(0), + }; + + // Should match the any-device rule (Keystroke), NOT the device-specific rule (Shell) + let action = rule_set.match_event(&event_ch0, 0, Some("drums")); + assert!(action.is_some(), "Any-device rule should still match"); + assert!( + matches!(action, Some(Action::Keystroke { .. })), + "Should be Keystroke (any-device), not Shell (device-specific)" + ); +}