Fix and display metronome

Cargo.lock

@@ -2540,6 +2540,7 @@ dependencies = [
 name = "osc"
 version = "0.1.0"
 dependencies = [
+ "log",
  "rosc",
  "strum",
  "thiserror",

audio_engine/src/metronome.rs

@@ -23,6 +23,16 @@ pub struct BufferTiming {
     pub beat_in_missed: Option<u32>,
 }
 
+#[derive(Debug, Clone, PartialEq)]
+pub struct PpqnTick {
+    /// Sample offset within the buffer where this tick occurs
+    pub sample_offset: usize,
+    /// PPQN tick index (0-23)
+    pub tick_index: usize,
+    /// Position within the beat (0.0 - 1.0)
+    pub position: f32,
+}
+
 impl Metronome {
     pub fn new(click_samples: Arc<AudioChunk>, state: &State) -> Self {
         Self {
@@ -44,6 +54,9 @@ impl Metronome {
         let buffer_size = ps.n_frames();
         let current_frame_time = ps.last_frame_time();
 
+        // Process PPQN ticks for this buffer
+        self.process_ppqn_ticks(ps, ports, osc_controller)?;
+
         // Calculate timing for this buffer
         let timing = self.calculate_timing(current_frame_time, buffer_size)?;
 
@@ -52,9 +65,6 @@ impl Metronome {
 
         self.render_click(buffer_size, &timing, click_output);
 
-        // Process PPQN ticks for this buffer
-        self.process_ppqn_ticks(ps, ports, osc_controller)?;
-
         Ok(timing)
     }
 
@@ -66,63 +76,24 @@ impl Metronome {
     ) -> Result<()> {
         let buffer_size = ps.n_frames() as usize;
 
-        // Calculate the starting position for this buffer
-        let start_position = (self.frames_since_last_beat + self.frames_per_beat - buffer_size)
-            % self.frames_per_beat;
-
-        // Process PPQN ticks in the current buffer only
-        self.send_buffer_ppqn_ticks(ps, ports, osc_controller, start_position, buffer_size)?;
-
-        Ok(())
-    }
-
-    fn send_buffer_ppqn_ticks(
-        &self,
-        ps: &jack::ProcessScope,
-        ports: &mut JackPorts,
-        osc_controller: &OscController,
-        buffer_start_position: usize,
-        buffer_size: usize,
-    ) -> Result<()> {
-        let frames_per_ppqn_tick = self.frames_per_beat / 24;
-        let mut current_position = buffer_start_position;
-        let mut frames_processed = 0;
-
-        // Get MIDI output writer
+        // Use the pure function to find tick (allocation-free)
+        if let Some(tick) = Self::find_next_ppqn_tick_in_buffer(
+            self.frames_since_last_beat,
+            self.frames_per_beat,
+            buffer_size,
+        ) {
+            // Send MIDI clock
+            let midi_clock_message = [0xF8];
             let mut midi_writer = ports.midi_out.writer(ps);
-
-        while frames_processed < buffer_size {
-            let frames_to_next_ppqn =
-                frames_per_ppqn_tick - (current_position % frames_per_ppqn_tick);
-            let frames_remaining_in_buffer = buffer_size - frames_processed;
-
-            if frames_remaining_in_buffer >= frames_to_next_ppqn {
-                // We hit a PPQN tick within this buffer
-                let tick_frame_offset = frames_processed + frames_to_next_ppqn;
-
-                // Ensure we don't hit the buffer boundary (JACK expects [0, buffer_size))
-                if tick_frame_offset >= buffer_size {
-                    break;
-                }
-
-                current_position = (current_position + frames_to_next_ppqn) % self.frames_per_beat;
-                frames_processed = tick_frame_offset;
-
-                // Send sample-accurate MIDI clock
-                let midi_clock_message = [0xF8]; // MIDI Clock byte
             midi_writer
                 .write(&jack::RawMidi {
-                        time: tick_frame_offset as u32,
+                    time: tick.sample_offset as u32,
                     bytes: &midi_clock_message,
                 })
                 .map_err(|_| LooperError::Midi(std::panic::Location::caller()))?;
 
             // Send OSC position message
-                let ppqn_position = (current_position as f32) / (self.frames_per_beat as f32);
-                osc_controller.metronome_position_changed(ppqn_position)?;
-            } else {
-                break;
-            }
+            osc_controller.metronome_position_changed(tick.position)?;
         }
 
         Ok(())
@@ -257,6 +228,40 @@ impl Metronome {
             beat_in_missed,
         })
     }
+
+    /// Pure function to find the next PPQN tick in a buffer (allocation-free)
+    /// Returns Some(tick) if there's a tick in this buffer, None otherwise
+    pub fn find_next_ppqn_tick_in_buffer(
+        frames_since_last_beat: usize,
+        frames_per_beat: usize,
+        buffer_size: usize,
+    ) -> Option<PpqnTick> {
+        const TICKS_PER_BEAT: usize = 24;
+        let frames_per_tick = frames_per_beat / TICKS_PER_BEAT;
+
+        // Find distance to next tick boundary
+        let frames_since_last_tick = frames_since_last_beat % frames_per_tick;
+        let frames_to_next_tick = if frames_since_last_tick == 0 {
+            0 // We're exactly on a tick
+        } else {
+            frames_per_tick - frames_since_last_tick
+        };
+
+        // Check if this tick occurs within our buffer
+        if frames_to_next_tick < buffer_size {
+            let absolute_tick_position = frames_since_last_beat + frames_to_next_tick;
+            let tick_index = (absolute_tick_position / frames_per_tick) % TICKS_PER_BEAT;
+            let position = (tick_index as f32) / (TICKS_PER_BEAT as f32);
+
+            Some(PpqnTick {
+                sample_offset: frames_to_next_tick,
+                tick_index,
+                position,
+            })
+        } else {
+            None
+        }
+    }
 }
 
 #[cfg(test)]
@@ -451,4 +456,77 @@ mod tests {
         assert_eq!(result.missed_frames, 0);
         assert_eq!(metronome.frames_since_last_beat, 256);
     }
+
+    // Tests for the pure PPQN calculation function
+    #[test]
+    fn test_ppqn_no_tick_in_buffer() {
+        let tick = Metronome::find_next_ppqn_tick_in_buffer(100, 960000, 128);
+        assert_eq!(tick, None);
+    }
+
+    #[test]
+    fn test_ppqn_tick_at_buffer_start() {
+        let frames_per_beat = 960000;
+
+        // Position exactly at tick boundary
+        let tick = Metronome::find_next_ppqn_tick_in_buffer(0, frames_per_beat, 128).unwrap();
+        assert_eq!(tick.sample_offset, 0);
+        assert_eq!(tick.tick_index, 0);
+        assert_eq!(tick.position, 0.0);
+    }
+
+    #[test]
+    fn test_ppqn_tick_in_middle_of_buffer() {
+        let frames_per_beat = 960000;
+        let frames_per_tick = frames_per_beat / 24; // 40000
+
+        // Position 64 frames before tick 1
+        let frames_since_last_beat = frames_per_tick - 64;
+        let tick =
+            Metronome::find_next_ppqn_tick_in_buffer(frames_since_last_beat, frames_per_beat, 128)
+                .unwrap();
+
+        assert_eq!(tick.sample_offset, 64);
+        assert_eq!(tick.tick_index, 1);
+        assert_eq!(tick.position, 1.0 / 24.0);
+    }
+
+    #[test]
+    fn test_ppqn_tick_wrapping() {
+        let frames_per_beat = 960000;
+        let frames_per_tick = frames_per_beat / 24; // 40000
+
+        // Position near end of beat cycle
+        let frames_since_last_beat = 23 * frames_per_tick - 64; // 64 frames before tick 23
+        let tick =
+            Metronome::find_next_ppqn_tick_in_buffer(frames_since_last_beat, frames_per_beat, 128)
+                .unwrap();
+
+        assert_eq!(tick.sample_offset, 64);
+        assert_eq!(tick.tick_index, 23);
+        assert_eq!(tick.position, 23.0 / 24.0);
+    }
+
+    #[test]
+    fn test_ppqn_real_world_scenario() {
+        // Your actual values
+        let frames_per_beat = 960000;
+        let frames_since_last_beat = 39936;
+        let buffer_size = 128;
+
+        let tick = Metronome::find_next_ppqn_tick_in_buffer(
+            frames_since_last_beat,
+            frames_per_beat,
+            buffer_size,
+        );
+
+        if let Some(tick) = tick {
+            println!(
+                "Tick {} at offset {} (position {})",
+                tick.tick_index, tick.sample_offset, tick.position
+            );
+            assert_eq!(tick.sample_offset, 64); // 40000 - 39936 = 64
+            assert_eq!(tick.tick_index, 1);
+        }
+    }
 }

audio_engine/src/notification_handler.rs

@@ -76,4 +76,9 @@ impl jack::NotificationHandler for NotificationHandler {
                 .expect("Could not send port registration notification");
         };
     }
+
+    fn xrun(&mut self, _: &jack::Client) -> jack::Control {
+        log::error!("XRUN");
+        jack::Control::Continue
+    }
 }

gui/src/main.rs

@@ -28,7 +28,7 @@ async fn main() -> Result<()> {
 
     // Logger
     simple_logger::SimpleLogger::new()
-        .with_level(log::LevelFilter::Error)
+        .with_level(log::LevelFilter::Info)
         .with_module_level("gui::osc_client", log::LevelFilter::Off)
         .init()
         .expect("Could not initialize logger");

gui/src/ui/app.rs

@@ -11,23 +11,24 @@ impl App {
 impl eframe::App for App {
     fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) {
         let state = self.state_receiver.borrow_and_update().clone();
+
         ctx.style_mut(|style| {
             style.visuals.panel_fill = egui::Color32::from_rgb(17, 17, 17);
             style.visuals.window_fill = egui::Color32::from_rgb(17, 17, 17);
             style.spacing.item_spacing = egui::vec2(0.0, 0.0);
         });
+
         egui::CentralPanel::default().show(ctx, |ui| {
-            ui.columns(state.columns, |ui| {
-                for (i, column) in state.cells.iter().enumerate() {
-                    let selected_row = if i == state.selected_column {
-                        Some(state.selected_row)
-                    } else {
-                        None
-                    };
-                    ui[i].add(super::Column::new(column, selected_row));
-                }
+            ui.vertical(|ui| {
+                ui.add(super::Metronome::new(state.metronome_position));
+                ui.add(super::Matrix::new(
+                    &state.cells,
+                    state.selected_column,
+                    state.selected_row,
+                ));
             });
         });
+
         ctx.request_repaint();
     }
 }

gui/src/ui/matrix.rs

@@ -0,0 +1,35 @@
+pub struct Matrix<'a> {
+    cells: &'a Vec<Vec<osc::Track>>,
+    selected_column: usize,
+    selected_row: usize,
+}
+
+impl<'a> Matrix<'a> {
+    pub fn new(
+        cells: &'a Vec<Vec<osc::Track>>,
+        selected_column: usize,
+        selected_row: usize,
+    ) -> Self {
+        Self {
+            cells,
+            selected_column,
+            selected_row,
+        }
+    }
+}
+
+impl<'a> egui::Widget for Matrix<'a> {
+    fn ui(self, ui: &mut egui::Ui) -> egui::Response {
+        ui.columns(self.cells.len(), |ui| {
+            for (i, column) in self.cells.iter().enumerate() {
+                let selected_row = if i == self.selected_column {
+                    Some(self.selected_row)
+                } else {
+                    None
+                };
+                ui[i].add(super::Column::new(column, selected_row));
+            }
+        });
+        ui.response()
+    }
+}

gui/src/ui/metronome.rs

@@ -0,0 +1,65 @@
+pub struct Metronome {
+    position: f32, // 0.0 - 1.0
+}
+
+impl Metronome {
+    pub fn new(position: f32) -> Self {
+        Self {
+            position: position.clamp(0.0, 1.0),
+        }
+    }
+}
+
+impl egui::Widget for Metronome {
+    fn ui(self, ui: &mut egui::Ui) -> egui::Response {
+        let sf = ui.available_width() / 100.0;
+
+        // Simple scaling values
+        let rail_height = sf * 1.0;
+        let circle_radius = sf * 2.0;
+        let h_padding = sf * 5.0;
+        let v_padding = sf * 3.0;
+
+        // Colors
+        let rail_color = egui::Color32::from_rgb(51, 51, 51); // #333333
+        let circle_color = egui::Color32::from_rgb(0, 255, 136); // #00ff88
+
+        // Calculate widget size
+        let width = ui.available_width();
+        let height = rail_height + 2.0 * v_padding;
+        let size = egui::vec2(width, height);
+
+        // Allocate the space we need
+        let (rect, response) = ui.allocate_exact_size(size, egui::Sense::hover());
+
+        // Calculate rail position (centered)
+        let rail_left = rect.min.x + h_padding;
+        let rail_right = rect.max.x - h_padding;
+        let rail_width = rail_right - rail_left;
+        let rail_center_y = rect.center().y;
+        let rail_top = rail_center_y - rail_height / 2.0;
+        let rail_bottom = rail_center_y + rail_height / 2.0;
+
+        let rail_rect = egui::Rect::from_min_max(
+            egui::pos2(rail_left, rail_top),
+            egui::pos2(rail_right, rail_bottom),
+        );
+
+        // Calculate circle position
+        let circle_x = rail_left + self.position * rail_width;
+        let circle_center = egui::pos2(circle_x, rail_center_y);
+
+        // Draw rail
+        ui.painter().rect_filled(
+            rail_rect,
+            egui::Rounding::same(rail_height / 2.0),
+            rail_color,
+        );
+
+        // Draw circle
+        ui.painter()
+            .circle_filled(circle_center, circle_radius, circle_color);
+
+        response
+    }
+}

gui/src/ui/mod.rs

@@ -1,8 +1,12 @@
 mod app;
 mod column;
+mod matrix;
+mod metronome;
 mod track;
 mod ui_rows_ext;
 
 pub use app::App;
 use column::Column;
+use matrix::Matrix;
+use metronome::Metronome;
 use track::Track;

gui/src/ui/track.rs

@@ -72,19 +72,16 @@ impl<'a> egui::Widget for Track<'a> {
         let volume_bar_rect = egui::Rect::from_min_max(
             egui::pos2(
                 inner_rect.min.x + margin,
-                inner_rect.max.y - volume_bar_height - margin
+                inner_rect.max.y - volume_bar_height - margin,
             ),
-            egui::pos2(
-                inner_rect.max.x - margin,
-                inner_rect.max.y - margin
-            )
+            egui::pos2(inner_rect.max.x - margin, inner_rect.max.y - margin),
         );
 
         // Draw volume bar background
         ui.painter().rect_filled(
             volume_bar_rect,
             egui::Rounding::same(1.0 * sf),
-            egui::Color32::from_rgb(51, 51, 51) // #333333
+            egui::Color32::from_rgb(51, 51, 51), // #333333
         );
 
         // Draw volume bar fill (only if volume > 0)
@@ -93,13 +90,13 @@ impl<'a> egui::Widget for Track<'a> {
             if fill_width > 0.0 {
                 let fill_rect = egui::Rect::from_min_size(
                     volume_bar_rect.min,
-                    egui::vec2(fill_width, volume_bar_rect.height())
+                    egui::vec2(fill_width, volume_bar_rect.height()),
                 );
 
                 ui.painter().rect_filled(
                     fill_rect,
                     egui::Rounding::same(1.0 * sf),
-                    egui::Color32::from_rgb(0, 255, 136) // #00ff88
+                    egui::Color32::from_rgb(0, 255, 136), // #00ff88
                 );
             }
         }

osc/Cargo.toml

@@ -6,5 +6,6 @@ edition = "2021"
 [dependencies]
 strum = { version = "0.23", features = ["derive"] }
 
+log.workspace = true
 rosc.workspace = true
 thiserror.workspace = true

osc/src/error.rs

@@ -10,8 +10,13 @@ pub trait AddressParseResult<T> {
     fn address_part_result(self, context: &'static str) -> Result<T>;
 }
 
-impl <T> AddressParseResult <T> for std::result::Result<T, std::num::ParseIntError> {
+impl<T> AddressParseResult<T> for std::result::Result<T, std::num::ParseIntError> {
     fn address_part_result(self, context: &'static str) -> Result<T> {
-        self.map_err(|e| Error::AddressParseError(format!("failed to parse number in address part {}: {}", context, e)))
+        self.map_err(|e| {
+            Error::AddressParseError(format!(
+                "failed to parse number in address part {}: {}",
+                context, e
+            ))
+        })
     }
 }

osc/src/message.rs

@@ -77,6 +77,7 @@ impl Message {
 
             if let Some(rosc::OscType::String(state_str)) = args.first() {
                 let state = state_str.parse::<TrackState>().unwrap_or(TrackState::Empty);
+                log::trace!("TrackStateChanged: column={column}, row={row}, state={state}");
                 return Ok(Some(Message::TrackStateChanged { column, row, state }));
             }
         } else if addr.starts_with("/looper/cell/") && addr.ends_with("/volume") {
@@ -90,26 +91,30 @@ impl Message {
             let row: usize = parts[4].parse::<usize>().address_part_result("row")? - 1; // Convert to 0-based
 
             if let Some(rosc::OscType::Float(volume)) = args.first() {
+                log::trace!("TrackVolumeChanged: column={column}, row={row}, volume={volume}");
                 return Ok(Some(Message::TrackVolumeChanged {
                     column,
                     row,
-                    volume: *volume 
+                    volume: *volume,
                 }));
             }
         } else if addr == "/looper/selected/column" {
             if let Some(rosc::OscType::Int(column_1based)) = args.first() {
+                log::trace!("SelectedColumnChanged: column={column_1based}");
                 let column = (*column_1based as usize).saturating_sub(1); // Convert to 0-based
                 return Ok(Some(Message::SelectedColumnChanged { column }));
             }
         } else if addr == "/looper/selected/row" {
             if let Some(rosc::OscType::Int(row_1based)) = args.first() {
+                log::trace!("SelectedRowChanged: row={row_1based}");
                 let row = (*row_1based as usize).saturating_sub(1); // Convert to 0-based
                 return Ok(Some(Message::SelectedRowChanged { row }));
             }
         } else if addr == "/looper/metronome/position" {
             if let Some(rosc::OscType::Float(position)) = args.first() {
+                log::trace!("MetronomePosition: position={position}");
                 return Ok(Some(Message::MetronomePosition {
-                    position: *position 
+                    position: *position,
                 }));
             }
         }
@@ -127,7 +132,11 @@ impl Message {
                     args: vec![rosc::OscType::String(state.to_string())],
                 })
             }
-            Message::TrackVolumeChanged { column, row, volume } => {
+            Message::TrackVolumeChanged {
+                column,
+                row,
+                volume,
+            } => {
                 let address = format!("/looper/cell/{}/{}/volume", column + 1, row + 1); // 1-based indexing
 
                 rosc::OscPacket::Message(rosc::OscMessage {

osc/src/state.rs

@@ -28,7 +28,15 @@ pub struct State {
 impl State {
     pub fn new(columns: usize, rows: usize) -> Self {
         let cells = (0..columns)
-            .map(|_| vec![Track { state: TrackState::Empty, volume: 1.0 }; rows])
+            .map(|_| {
+                vec![
+                    Track {
+                        state: TrackState::Empty,
+                        volume: 1.0
+                    };
+                    rows
+                ]
+            })
             .collect();
 
         Self {
@@ -49,7 +57,11 @@ impl State {
                     self.cells[*column][*row].state = state.clone();
                 }
             }
-            Message::TrackVolumeChanged { column, row, volume } => {
+            Message::TrackVolumeChanged {
+                column,
+                row,
+                volume,
+            } => {
                 if *column < self.columns && *row < self.rows {
                     self.cells[*column][*row].volume = *volume;
                 }

xtask/src/args.rs

@@ -1,4 +1,4 @@
- use clap::*;
+use clap::*;
 
 #[derive(Parser)]
 #[command(name = "fcb-looper")]
@@ -6,7 +6,6 @@
 pub struct Args {
     #[command(subcommand)]
     pub command: Option<Command>,
-
 }
 
 #[derive(Subcommand)]

xtask/src/main.rs

@@ -9,7 +9,9 @@ use clap::Parser;
 async fn main() {
     let args = args::Args::parse();
 
-    workspace::change_to_workspace_dir().await.expect("Failed to change to workspace directory");
+    workspace::change_to_workspace_dir()
+        .await
+        .expect("Failed to change to workspace directory");
 
     match args.command {
         Some(args::Command::Run) | None => {

xtask/src/run.rs

@@ -23,7 +23,9 @@ pub async fn run() {
     let cleanup_token = cancel_token.clone();
 
     tokio::spawn(async move {
-        tokio::signal::ctrl_c().await.expect("Failed to listen for ctrl+c");
+        tokio::signal::ctrl_c()
+            .await
+            .expect("Failed to listen for ctrl+c");
         println!("Received Ctrl+C, shutting down...");
         cleanup_token.cancel();
         cleanup_processes(cleanup_handles).await;
@@ -109,7 +111,11 @@ async fn stop_jack_daemon() {
     #[cfg(target_os = "linux")]
     let (jack, args) = ("pkill", vec!["-f", "jackd"]);
 
-    Command::new(jack).args(args).output().await.expect("Failed to stop JACK daemon");
+    Command::new(jack)
+        .args(args)
+        .output()
+        .await
+        .expect("Failed to stop JACK daemon");
 }
 
 async fn spawn_qjackctl() -> Child {
@@ -146,24 +152,14 @@ async fn spawn_audio_engine() -> Child {
 
 async fn spawn_gui() -> Child {
     Command::new("cargo")
-        .args([
-            "run",
-            "--release",
-            "--bin",
-            "gui",
-        ])
+        .args(["run", "--release", "--bin", "gui"])
         .spawn()
         .expect("Could not start gui")
 }
 
 async fn spawn_simulator() -> Child {
     Command::new("cargo")
-        .args([
-            "run",
-            "--release",
-            "--bin",
-            "simulator",
-        ])
+        .args(["run", "--release", "--bin", "simulator"])
         .spawn()
         .expect("Could not start simulator")
 }

xtask/src/workspace.rs

@@ -12,7 +12,8 @@ pub async fn change_to_workspace_dir() -> Result<(), Box<dyn std::error::Error>>
         return Err(format!(
             "Failed to locate workspace: {}",
             String::from_utf8_lossy(&output.stderr)
-        ).into());
+        )
+        .into());
     }
 
     let workspace_cargo_toml = String::from_utf8(output.stdout)?;
@@ -24,7 +25,10 @@ pub async fn change_to_workspace_dir() -> Result<(), Box<dyn std::error::Error>>
         .ok_or("Failed to get workspace directory")?
         .to_path_buf();
 
-    println!("Changing to workspace directory: {}", workspace_dir.display());
+    println!(
+        "Changing to workspace directory: {}",
+        workspace_dir.display()
+    );
     std::env::set_current_dir(&workspace_dir)?;
 
     Ok(())