-
Notifications
You must be signed in to change notification settings - Fork 0
fix: Enable MIDI Learn event streaming from daemon to GUI #11
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.
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
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes MIDI Learn functionality that wasn't capturing events from connected
MIDI devices. Three root causes addressed:
1. Added missing `isOpen` prop on MidiLearnDialog in MappingsView.svelte
2. Added missing `on:midiLearn` handler on TriggerSelector component
3. Implemented daemon→GUI event streaming via IPC polling
Architecture: GUI polls daemon every 100ms for captured MIDI events when
MIDI Learn is active. Events are displayed in a selectable list and
converted to trigger configurations.
New IPC commands: START_MIDI_LEARN, STOP_MIDI_LEARN, GET_MIDI_LEARN_EVENTS
New Tauri commands: start_daemon_midi_learn, stop_daemon_midi_learn,
get_daemon_midi_learn_events
Includes ADR-002 documenting the polling-based event streaming decision.
Co-Authored-By: Claude Opus 4.5 <[email protected]>
14 tasks
Fixes identified by LLM Council verification:
1. State Machine Transition (CRITICAL):
- DeviceReconnected handler now properly transitions through
Degraded → Reconnecting → Running instead of Degraded → Running
- Fixes reconnection failures due to invalid state transitions
2. Statistics Integer Arithmetic:
- Changed running average formula to use u128 for intermediate
calculations to prevent precision loss from integer division
- Prevents "frozen" averages when differences are small
3. Debug Logging for MIDI Learn:
- Added console.log statements to trace MIDI Learn flow
- Helps diagnose why buttons may not appear to work
Also includes v3.0 cleanup:
- Added InputMode enum to DeviceConfig for MIDI/Gamepad selection
- Updated conductorctl to show MIDI and gamepad devices separately
- Removed obsolete src/config.rs (replaced by config module)
- Re-exported InputMode from conductor-core
Co-Authored-By: Claude Opus 4.5 <[email protected]>
Fixes code quality issues identified by LLM Council: 1. Fixed dangling reference: Changed `unwrap_or(&"None".to_string())` to `unwrap_or_else(|| "None".to_string())` which returns an owned String instead of a reference to a temporary. 2. Fixed ownership issue: Changed `self.config_path` to `self.config_path.display().to_string()` to avoid moving the PathBuf out of a borrowed context. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Performance improvements based on LLM Council feedback: 1. Increased input event channel buffer from 100 to 1000 to handle high-frequency MIDI devices without dropping events during config reload or IPC processing. 2. Changed midi_learn_events from Vec to VecDeque for O(1) removal of oldest events (was O(n) with Vec::remove(0)). 3. Pre-allocated VecDeque with capacity 100 to avoid reallocations. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Introduces conductor-capture, a CLI tool for recording MIDI and gamepad input patterns with privacy controls. Part of the crowdsourced pattern platform (Issue #5). Features: - Start/stop/pause/resume capture sessions - Privacy levels: private, anonymous, public - Protocol selection: midi, gamepad, or both - Local storage with tagging and categorization - Pattern anonymization before sharing - Export/import for pattern sharing - Upload to pattern library (with consent) Modules: - main.rs: CLI interface with clap - capture.rs: Session state machine and event recording - storage.rs: Local pattern storage and retrieval - anonymization.rs: Data anonymization for sharing - privacy.rs: Privacy level definitions and enforcement Dependencies: - conductor-core for InputEvent abstraction - tokio for async I/O - uuid for capture session IDs - directories for platform-specific storage paths Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add comprehensive requirements traceability section that traces all 13 MIDI Learn detection patterns through 5 implementation layers: - Core (event_processor.rs) - Config (types.rs) - MIDI Learn (midi_learn.rs) - Mapping (mapping.rs) - Test coverage Key findings: - 6/13 patterns (46%) fully functional end-to-end - 7/13 patterns (54%) have gaps (silent failures) - Root cause: missing match cases in mapping.rs:205-274 Includes LLM Council review (2026-01-30): - Verdict: Conditional Acceptance - P0: Fix mapping.rs dispatch logic - P1: Update documentation accuracy - P2: Add gamepad UI forms Co-Authored-By: Claude Opus 4.5 <[email protected]>
Implement matching logic for 7 MIDI trigger types that were silently failing due to missing match cases in mapping.rs. Fixes: - DoubleTap: Matches ProcessedEvent::DoubleTap (note comparison) - LongPress: Matches ProcessedEvent::LongPress (duration threshold) - Aftertouch: Matches ProcessedEvent::AftertouchChanged (pressure threshold) - PitchBend: Matches ProcessedEvent::PitchBendMoved (value range) - VelocityRange: Matches ProcessedEvent::PadPressed (velocity level) - CC: Matches ProcessedEvent::EncoderTurned (value threshold, cc < 128) - EncoderTurn: Matches ProcessedEvent::EncoderTurned (direction filter, cc < 128) Implementation: - Added 6 new CompiledTrigger enum variants - Added 6 compile_mapping() arms to extract trigger parameters - Added 7 trigger_matches_processed() arms for event matching - Added tracing::warn! for unhandled combinations (Council safeguard) Testing: - 22 TDD tests in conductor-core/tests/trigger_matching_test.rs - All positive and negative test cases pass Closes #12, #13, #14, #15, #16, #17, #18, #19 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Bump version to 4.3.0 for ADR-002 trigger matching gap fixes. Features in this release: - Fixed 7 MIDI trigger types that were silently failing (#12-#19) - Added 22 TDD tests for trigger matching - Added Council safeguard (tracing::warn for unhandled triggers) - Fixed global mappings fallback in get_action() See CHANGELOG.md for full details. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Implements the previously stubbed get_config and save_config Tauri commands that were blocking the entire GUI configuration persistence layer (AMI-167). Changes: - get_config: Loads config.toml from standard location, returns JSON - save_config: Validates config, writes TOML with atomic writes - Both commands use conductor-core's existing Config::load/save - Added 9 TDD tests for config commands Fixes #20, #21, #22 The daemon's ConfigWatcher will automatically detect file changes and hot-reload within 500ms, so explicit reload is not required. Co-Authored-By: Claude Opus 4.5 <[email protected]>
- GUI Config Management: Implemented get_config and save_config commands - Added 9 TDD tests for config command validation - Closes #20, #21, #22, #23 Co-Authored-By: Claude Opus 4.5 <[email protected]>
This commit addresses technical debt identified by LLM Council during v4.4.0 verification, implementing ADR-003 GUI State Management Patterns: 1. validate_config unsafe defaults (lines 355, 383-387): - Changed unwrap_or(true) to explicit error on missing 'valid' field - Returns error when response.data is None instead of defaulting valid=true - ADR-003 Pattern 1: Fail-Safe Defaults Principle 2. get_daemon_status state inconsistency (lines 225-241): - Added state.set_daemon_connected(false) on daemon error response - Changed connected: true to connected: false on error - ADR-003 Pattern 2: AppState Synchronization Requirement 3. Unsafe u64→usize casts (lines 197, 536, 1307): - Replaced direct `as usize` casts with parse_port() helper - parse_port() validates MIDI port range (0-255) and returns None for overflow - ADR-003 Pattern 3: Type-Safe Casting Pattern 4. apply_connection_status stale flags (already fixed): - Uses atomic replacement pattern to set all flags in single pass - Prevents UI flickering and stale connected flags - ADR-003 Pattern 4: Atomic Replacement Pattern Test coverage: - Added 5 new TDD tests for parse_port() bounds checking - Added 2 new TDD tests for apply_connection_status stale flag clearing - All 10 commands tests pass Closes #26, #27, #28, #29 Part of Epic #25 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Extends ADR-003 Pattern 2 (AppState Synchronization) to additional IPC commands that were missing state updates on connection failure: - reload_config: Now updates state.set_daemon_connected(false) on IPC connection failure and request send failure - stop_daemon: Same treatment - ping_daemon: Same treatment, also updates state on ResponseStatus::Error - validate_config: Changed _state to state, now updates state on failure This ensures the UI accurately reflects daemon connectivity status regardless of which command encounters a connection error. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Adds state.set_daemon_connected(true) on successful response in validate_config for consistency with other IPC commands per ADR-003 Pattern 2 (AppState Synchronization Requirement). Co-Authored-By: Claude Opus 4.5 <[email protected]>
…achability) Per ADR-003 Pattern 2, error responses from the daemon prove that IPC succeeded, so AppState.daemon_connected should be true. The error only indicates an application-level failure, not a connectivity issue. Updated reload_config, stop_daemon, and get_daemon_status to set daemon_connected=true when receiving ResponseStatus::Error. Note: DaemonStatus.connected (device status) is distinct from AppState.daemon_connected (IPC reachability). Co-Authored-By: Claude Opus 4.5 <[email protected]>
Addresses LLM Council feedback about naming ambiguity: - AppState.daemon_connected = IPC reachability (can we reach the daemon?) - DaemonStatus.connected = device status (is a MIDI device connected?) Updated Pattern 2 to clarify that error responses prove connectivity, so daemon_connected should be true when we receive any response. Co-Authored-By: Claude Opus 4.5 <[email protected]>
Release v4.5.0 includes: - Fixed 4 state management issues in commands.rs (ADR-003) - Added ADR-003 documenting GUI state management patterns - 10 new TDD tests for state management helpers - Extended AppState synchronization to all IPC commands Closes #25, #26, #27, #28, #29 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Added complete requirements traceability from public documentation (getconductor.dev/getting-started/midi-learn.html) through implementation: Requirements traced: - 9 MIDI triggers: Note, VelocityRange, LongPress, DoubleTap, Chord, Encoder, CC, Aftertouch, PitchBend - 4 Gamepad triggers: GamepadButton, GamepadButtonChord, GamepadAnalogStick, GamepadTrigger Key findings: - Backend: 100% complete (all 13 triggers work) - UI: 69% complete (9/13 - MIDI 100%, Gamepad 0%) - Gamepad triggers display as raw JSON in MIDI Learn dialog - No manual configuration UI for gamepad triggers LLM Council Review (2026-01-30): - Verdict: Approved with Recommendations - Priority: P1 (High) due to documentation/product mismatch - Phase 1: Add display formatting and dropdown selection - Phase 2: Build full configuration forms with deadzone support Co-Authored-By: Claude Opus 4.5 <[email protected]>
Implements full UI support for 4 gamepad trigger types: TriggerSelector.svelte (#31, #32): - Add gamepad trigger types to dropdown - Add form sections for GamepadButton, GamepadButtonChord, GamepadAnalogStick, and GamepadTrigger - Add getGamepadButtonName() helper for human-readable labels MidiLearnDialog.svelte (#33): - Add formatTrigger() cases for all 4 gamepad trigger types - Human-readable display instead of JSON.stringify fallback stores.js (#34): - Add formatEvent() cases for gamepad events - Add eventToTrigger() cases for gamepad event conversion - Add getGamepadButtonName() helper function Documentation (#35): - Update ADR-002 with implementation completion status - Update CHANGELOG.md for v4.6.0 - Bump version: 4.5.0 → 4.6.0 Closes #30, #31, #32, #33, #34, #35 Co-Authored-By: Claude Opus 4.5 <[email protected]>
- Update MIDI Detection Patterns table: all 9 patterns now Complete (v4.3.0 fix) - Update Gamepad Detection Patterns table: add UI column, all 4 Complete (v4.6.0) - Update UI Layer Support table: add Test column, gamepad triggers U9-U12 Complete - Update Gap Summary: 13/13 backend (100%), 9/12 UI (75%) - Update Council Recommendations section: Phase 1 & 2 marked IMPLEMENTED - Document remaining limitations (multi-event pattern auto-detection) Co-Authored-By: Claude Opus 4.5 <[email protected]>
…, Chord (#36) MIDI Learn now auto-detects multi-event patterns when the daemon processes events through EventProcessor during learning mode. Changes: - Extended MidiLearnEvent and DaemonMidiLearnEvent with pattern fields: - pattern_type: "long_press", "double_tap", "chord", "gamepad_chord" - pattern_notes/pattern_buttons: For chord patterns - pattern_duration_ms/pattern_timeout_ms: For timing info - Daemon now emits enriched events with detected pattern information - stores.js eventToTrigger() converts detected patterns to triggers - stores.js formatEvent() displays human-readable pattern descriptions - Gamepad events properly emit as gamepad_button, gamepad_axis, gamepad_trigger ADR-002 Requirements: - U3 (Long Press form): Complete - U4 (Double-Tap form): Complete - U5 (Chord form): Complete - U10 (Gamepad Button Chord form): Complete Closes #36, #37, #38, #39, #40 Co-Authored-By: Claude Opus 4.5 <[email protected]>
Previously, DaemonStatus.connected was hardcoded to true on IPC success, causing the GUI to show "connected" even when no device was attached. Changes: - Derive connected from device.as_ref().map_or(false, |d| d.connected) - Added 3 TDD tests for derivation scenarios - Documented LLM Council v4.7.0 verification investigation in ADR-002 - Identified 5 false positives in Council findings - Bumped version to v4.8.0 GitHub Issue: #41 ADR: ADR-002 (MIDI Learn Event Streaming) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Documented LLM Council v4.8.0 verification results showing all flagged issues were false positives or pre-existing patterns: - File truncation: Council reviewed diff, not full file - parse_port 255 limit: Correct per MIDI specification - Silent JSON parsing: Pre-existing defensive pattern Co-Authored-By: Claude Opus 4.5 <[email protected]>
Type-Safe Event Schemas (ADR-004, #42, #47): - Create EventType enum (12 variants) and PatternType enum (4 variants) - Add serde snake_case serialization for JSON API compatibility - Add is_gamepad()/is_midi() helper methods - 11 TDD tests for serialization/deserialization VelocityRange Config Fix (#48): - Add classify_velocity() helper using config thresholds - VelocityRange trigger now respects soft_max/medium_max values - 3 new TDD tests verify custom threshold behavior load_from_config Stale Modes Fix (#49): - Add mode_mappings.clear() at start of function - Add mode_count() method for testability - Add defensive bounds check for >256 modes (Council feedback) - 2 regression tests verify mode count changes correctly Chord Comment Fix (#50): - Update misleading comment to match exact-match behavior Documentation: - Create ADR-004: Type-Safe Event Schemas - Update ADR-002 with v4.9.0 implementation status and Council feedback Co-Authored-By: Claude Opus 4.5 <[email protected]>
Daemon (GitHub #43): - MidiLearnEvent struct uses EventType and PatternType enums - All string assignments replaced with enum variants - Wire format unchanged (serde snake_case serialization) GUI Rust (GitHub #44): - DaemonMidiLearnEvent struct uses typed enums - Updated tests to use enum variants TypeScript (GitHub #45, #46): - Added EventType and PatternType type aliases - Added type guards: isValidEventType(), isValidPatternType() - Added helpers: isGamepadEventType(), isGamepadPatternType() - Existing stores.js works unchanged (JSON wire format compatible) Core (supporting changes): - Added Default derive to EventType and PatternType enums Co-Authored-By: Claude Opus 4.5 <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
isOpenprop on MidiLearnDialog in MappingsView.svelteon:midiLearnhandler on TriggerSelector componentArchitecture
The fix implements a polling-based event streaming pattern:
start_daemon_midi_learnTauri commandmidi_learn_activeflag and captures events to bufferget_daemon_midi_learn_eventsevery 100msstop_daemon_midi_learnwhen doneNew IPC Commands
START_MIDI_LEARN- Enable MIDI event capture in daemonSTOP_MIDI_LEARN- Disable capture and clear bufferGET_MIDI_LEARN_EVENTS- Retrieve and drain captured eventsNew Tauri Commands
start_daemon_midi_learnstop_daemon_midi_learnget_daemon_midi_learn_eventsFiles Changed
conductor-daemon/src/daemon/engine_manager.rsconductor-daemon/src/daemon/types.rsconductor-gui/src-tauri/src/commands.rsconductor-gui/src-tauri/src/main.rsconductor-gui/ui/src/lib/api.jsconductor-gui/ui/src/lib/stores.jsconductor-gui/ui/src/lib/components/MidiLearnDialog.svelteconductor-gui/ui/src/lib/views/MappingsView.sveltedocs/adrs/ADR-002-midi-learn-event-streaming.mdCHANGELOG.mdCargo.tomlTest Plan
cargo run -p conductor-daemoncd conductor-gui && npm run tauri devcargo test --workspace🤖 Generated with Claude Code