From 7af0235736294e610c421d85768159f621bad468 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Fri, 29 May 2026 15:36:55 -0400 Subject: [PATCH] chore: senior-review cleanup pass - Remove Gemini's committed update_*.py regex-surgery scripts - Drop unused iroh-tickets dependency (hand-rolled ticket is used instead) - Replace ToString antipattern with Display impl on PeerSpeakTicket - Route debug log to XDG state/cache dir instead of hardcoded /home path - Clear all compiler + clippy warnings (unused imports, collapsible ifs, redundant pattern matching, missing Default) Builds clean with zero warnings. Co-Authored-By: Claude Opus 4.8 --- Cargo.lock | 65 +---------------- Cargo.toml | 1 - src/app/mod.rs | 4 +- src/audio/pipewire_impl.rs | 6 ++ src/bin/test_net.rs | 3 +- src/config.rs | 8 +-- src/lib.rs | 21 +++++- src/network/gossip.rs | 10 ++- src/network/iroh_impl.rs | 2 +- src/network/mod.rs | 15 ++-- update_app.py | 139 ------------------------------------- update_app_noisegate.py | 65 ----------------- update_core.py | 76 -------------------- 13 files changed, 48 insertions(+), 367 deletions(-) delete mode 100644 update_app.py delete mode 100644 update_app_noisegate.py delete mode 100644 update_core.py diff --git a/Cargo.lock b/Cargo.lock index ba03e9a..5edddaf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -324,15 +324,6 @@ dependencies = [ "rustc_version", ] -[[package]] -name = "atomic-polyfill" -version = "1.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8cf2bce30dfe09ef0bfaef228b9d414faaf7e563035494d7fe092dba54b300f4" -dependencies = [ - "critical-section", -] - [[package]] name = "atomic-waker" version = "1.1.2" @@ -529,12 +520,6 @@ dependencies = [ "syn", ] -[[package]] -name = "byteorder" -version = "1.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fd0f2584146f6f2ef48085050886acf353beff7305ebd1ae69500e27c67f64b" - [[package]] name = "bytes" version = "1.11.1" @@ -1601,7 +1586,7 @@ dependencies = [ "diatomic-waker", "futures-core", "pin-project-lite", - "spin 0.10.0", + "spin", ] [[package]] @@ -1950,15 +1935,6 @@ dependencies = [ "smallvec", ] -[[package]] -name = "hash32" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0c35f58762feb77d74ebe43bdbc3210f09be9fe6742234d573bacc26ed92b67" -dependencies = [ - "byteorder", -] - [[package]] name = "hashbrown" version = "0.15.5" @@ -1988,20 +1964,6 @@ dependencies = [ "foldhash 0.2.0", ] -[[package]] -name = "heapless" -version = "0.7.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cdc6457c0eb62c71aac4bc17216026d8410337c4126773b9c5daba343f17964f" -dependencies = [ - "atomic-polyfill", - "hash32", - "rustc_version", - "serde", - "spin 0.9.8", - "stable_deref_trait", -] - [[package]] name = "heck" version = "0.5.0" @@ -2812,20 +2774,6 @@ dependencies = [ "ws_stream_wasm", ] -[[package]] -name = "iroh-tickets" -version = "1.0.0-rc.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0a4b7fbfa10582f6b4f6b013eef1d21987d3df5fd42c0f7707d5de6abd34f8e9" -dependencies = [ - "data-encoding", - "derive_more", - "iroh-base", - "n0-error", - "postcard", - "serde", -] - [[package]] name = "irpc" version = "0.15.0" @@ -4180,7 +4128,6 @@ dependencies = [ "iced", "iroh", "iroh-gossip", - "iroh-tickets", "opus", "pipewire", "rand", @@ -4405,7 +4352,6 @@ dependencies = [ "cobs", "embedded-io 0.4.0", "embedded-io 0.6.1", - "heapless", "postcard-derive", "serde", ] @@ -5284,15 +5230,6 @@ dependencies = [ "syn", ] -[[package]] -name = "spin" -version = "0.9.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" -dependencies = [ - "lock_api", -] - [[package]] name = "spin" version = "0.10.0" diff --git a/Cargo.toml b/Cargo.toml index 3cf5606..11b678a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ dirs = "6.0.0" iced = "0.14.0" iroh = "1.0.0-rc.0" iroh-gossip = "0.99.0" -iroh-tickets = "1.0.0-rc.0" opus = "0.3.1" pipewire = "0.9" rand = "0.10.1" diff --git a/src/app/mod.rs b/src/app/mod.rs index 54a6fb0..72e661a 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -4,7 +4,7 @@ use crate::audio::pw_cli::{AudioDevice, enumerate_audio_devices}; use crate::config::AppConfig; use iced::widget::{ - container, column, row, text, button, text_input, scrollable, slider, checkbox, pick_list, Column, Space, + container, column, row, text, button, text_input, scrollable, slider, checkbox, pick_list, Column, }; use iced::{ Color, Background, Border, Element, Subscription, Task, Theme, Event, keyboard, @@ -143,7 +143,7 @@ pub fn run_gui() -> iced::Result { .run() } -fn subscription(state: &AppState) -> Subscription { +fn subscription(_state: &AppState) -> Subscription { let core_sub = Subscription::run(core_subscription).map(AppMessage::UiEventReceived); let event_sub = iced::event::listen().map(AppMessage::EventOccurred); Subscription::batch(vec![core_sub, event_sub]) diff --git a/src/audio/pipewire_impl.rs b/src/audio/pipewire_impl.rs index d50a0fb..6483cae 100644 --- a/src/audio/pipewire_impl.rs +++ b/src/audio/pipewire_impl.rs @@ -24,6 +24,12 @@ struct PlaybackState { thread: JoinHandle<()>, } +impl Default for PipeWireBackend { + fn default() -> Self { + Self::new() + } +} + impl PipeWireBackend { pub fn new() -> Self { pw::init(); diff --git a/src/bin/test_net.rs b/src/bin/test_net.rs index 1cd9866..f4945e2 100644 --- a/src/bin/test_net.rs +++ b/src/bin/test_net.rs @@ -1,10 +1,9 @@ use peerspeak::network::{ gossip::IrohGossipState, - RoomState, PeerState, RoomEvent, + RoomState, PeerState, }; use iroh::{Endpoint, endpoint::presets}; use iroh_gossip::net::Gossip; -use std::sync::Arc; use tokio::time::{self, Duration}; #[tokio::main] diff --git a/src/config.rs b/src/config.rs index a763ec0..d87e3da 100644 --- a/src/config.rs +++ b/src/config.rs @@ -29,13 +29,11 @@ impl AppConfig { } pub fn load() -> Self { - if let Some(path) = Self::config_path() { - if let Ok(contents) = fs::read_to_string(&path) { - if let Ok(config) = serde_json::from_str(&contents) { + if let Some(path) = Self::config_path() + && let Ok(contents) = fs::read_to_string(&path) + && let Ok(config) = serde_json::from_str(&contents) { return config; } - } - } Self::default() } diff --git a/src/lib.rs b/src/lib.rs index a4a131c..801729c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,11 +5,30 @@ pub mod core; pub mod app; pub mod config; +use std::path::PathBuf; +use std::sync::OnceLock; + +/// Resolves the log file path once: `$XDG_STATE_HOME/peerspeak/peerspeak.log` +/// (via `dirs::state_dir`), falling back to the system temp dir. Computed lazily +/// so we never hardcode a per-user path. +fn log_path() -> &'static PathBuf { + static LOG_PATH: OnceLock = OnceLock::new(); + LOG_PATH.get_or_init(|| { + let mut dir = dirs::state_dir() + .or_else(dirs::cache_dir) + .unwrap_or_else(std::env::temp_dir); + dir.push("peerspeak"); + let _ = std::fs::create_dir_all(&dir); + dir.push("peerspeak.log"); + dir + }) +} + pub fn log_msg(msg: &str) { if let Ok(mut file) = std::fs::OpenOptions::new() .create(true) .append(true) - .open("/home/mollusk/peerspeak.log") + .open(log_path()) { use std::io::Write; if let Ok(time) = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH) { diff --git a/src/network/gossip.rs b/src/network/gossip.rs index a32fdc3..c7b341b 100644 --- a/src/network/gossip.rs +++ b/src/network/gossip.rs @@ -114,12 +114,11 @@ impl RoomState for IrohGossipState { }) }; - if let Some(payload) = initial_payload { - if let Ok(bytes) = serde_json::to_vec(&payload) { + if let Some(payload) = initial_payload + && let Ok(bytes) = serde_json::to_vec(&payload) { crate::log_msg(&format!("Broadcasting initial state from self_id={:?}", self_id)); let _ = gossip_sender_clone.broadcast(bytes.into()).await; } - } // Stream topic messages while let Some(res) = gossip_receiver.next().await { @@ -186,12 +185,11 @@ impl RoomState for IrohGossipState { msg: GossipMessage::Announce(state.clone()), }) }; - if let Some(payload) = payload_opt { - if let Ok(bytes) = serde_json::to_vec(&payload) { + if let Some(payload) = payload_opt + && let Ok(bytes) = serde_json::to_vec(&payload) { crate::log_msg(&format!("Broadcasting state to new neighbor={:?}", peer_id)); let _ = gossip_sender_clone.broadcast(bytes.into()).await; } - } } Ok(iroh_gossip::api::Event::NeighborDown(peer_id)) => { crate::log_msg(&format!("Gossip event: NeighborDown={:?}", peer_id)); diff --git a/src/network/iroh_impl.rs b/src/network/iroh_impl.rs index 484356f..390004d 100644 --- a/src/network/iroh_impl.rs +++ b/src/network/iroh_impl.rs @@ -94,7 +94,7 @@ impl NetworkTransport for IrohTransport { loop { match conn_clone.read_datagram().await { Ok(bytes) => { - if let Err(_) = incoming_tx_inner.send((peer_id, bytes)).await { + if incoming_tx_inner.send((peer_id, bytes)).await.is_err() { break; } } diff --git a/src/network/mod.rs b/src/network/mod.rs index 3f71093..853f5d7 100644 --- a/src/network/mod.rs +++ b/src/network/mod.rs @@ -42,11 +42,16 @@ pub struct PeerSpeakTicket { pub topic_id: [u8; 32], } -impl ToString for PeerSpeakTicket { - fn to_string(&self) -> String { - let serialized = serde_json::to_vec(self).unwrap(); - // Convert to base64 URL-safe string - base64::Engine::encode(&base64::engine::general_purpose::URL_SAFE_NO_PAD, &serialized) +impl std::fmt::Display for PeerSpeakTicket { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // serde_json on a struct of String/[u8;32] fields is infallible in practice, + // but Display can't surface an error, so fall back to an empty ticket body. + let serialized = serde_json::to_vec(self).unwrap_or_default(); + let encoded = base64::Engine::encode( + &base64::engine::general_purpose::URL_SAFE_NO_PAD, + &serialized, + ); + f.write_str(&encoded) } } diff --git a/update_app.py b/update_app.py deleted file mode 100644 index 6ff2b83..0000000 --- a/update_app.py +++ /dev/null @@ -1,139 +0,0 @@ -import re - -with open("src/app/mod.rs", "r") as f: - text = f.read() - -# 1. Imports -text = text.replace( - "use crate::core::{CoreController, messages::{CoreCommand, UiEvent}};\nuse crate::network::PeerState;", - "use crate::core::{CoreController, messages::{CoreCommand, UiEvent}};\nuse crate::network::PeerState;\nuse crate::audio::pw_cli::{AudioDevice, enumerate_audio_devices};\nuse crate::config::AppConfig;" -) - -# 2. Iced imports -text = text.replace( - "container, column, row, text, button, text_input, scrollable, slider, checkbox, Column, Space,", - "container, column, row, text, button, text_input, scrollable, slider, checkbox, pick_list, Column, Space," -) - -# 3. AppMessage -text = text.replace( - "InputDeviceChanged(String),\n OutputDeviceChanged(String),", - "InputDeviceSelected(AudioDevice),\n OutputDeviceSelected(AudioDevice)," -) - -# 4. AppState -text = text.replace( - "input_device: String,\n output_device: String,", - "input_devices: Vec,\n output_devices: Vec,\n selected_input: Option,\n selected_output: Option,\n config: AppConfig," -) - -# 5. AppState::default() -default_start = text.find("let _ = UI_RX.set(Mutex::new(Some(ui_rx)));") -if default_start != -1: - default_end = default_start + len("let _ = UI_RX.set(Mutex::new(Some(ui_rx)));") - insertion = """ - let config = AppConfig::load(); - let all_devices = enumerate_audio_devices(); - let input_devices: Vec<_> = all_devices.iter().filter(|d| d.is_input).cloned().collect(); - let output_devices: Vec<_> = all_devices.iter().filter(|d| !d.is_input).cloned().collect(); - - let selected_input = input_devices.iter().find(|d| d.name == config.input_device).cloned(); - let selected_output = output_devices.iter().find(|d| d.name == config.output_device).cloned(); -""" - text = text[:default_end] + "\n" + insertion + text[default_end:] - -text = text.replace( - "input_device: \"\".to_string(),\n output_device: \"\".to_string(),", - "input_devices,\n output_devices,\n selected_input,\n selected_output,\n config," -) - -# 6. JoinPressed / CreatePressed -join_old = """ AppMessage::JoinPressed => { - state.status_message = "Connecting to room...".to_string(); - let _ = state.controller.send(CoreCommand::Join { - name: state.name.clone(), - ticket: state.ticket_input.clone(), - input_device: if state.input_device.is_empty() { None } else { Some(state.input_device.clone()) }, - output_device: if state.output_device.is_empty() { None } else { Some(state.output_device.clone()) }, - }); - }""" -join_new = """ AppMessage::JoinPressed => { - let input_device = state.selected_input.as_ref().map(|d| d.name.clone()); - let output_device = state.selected_output.as_ref().map(|d| d.name.clone()); - if !state.ticket_input.is_empty() { - state.status_message = "Joining room...".to_string(); - let _ = state.controller.send(CoreCommand::Join { - name: state.name.clone(), - ticket: state.ticket_input.clone(), - input_device, - output_device, - }); - } - }""" -text = text.replace(join_old, join_new) - -create_old = """ AppMessage::CreatePressed => { - state.status_message = "Creating room...".to_string(); - let _ = state.controller.send(CoreCommand::Join { - name: state.name.clone(), - ticket: "".to_string(), - input_device: if state.input_device.is_empty() { None } else { Some(state.input_device.clone()) }, - output_device: if state.output_device.is_empty() { None } else { Some(state.output_device.clone()) }, - }); - }""" -create_new = """ AppMessage::CreatePressed => { - let input_device = state.selected_input.as_ref().map(|d| d.name.clone()); - let output_device = state.selected_output.as_ref().map(|d| d.name.clone()); - state.status_message = "Creating room...".to_string(); - let _ = state.controller.send(CoreCommand::Join { - name: state.name.clone(), - ticket: "create".to_string(), - input_device, - output_device, - }); - }""" -text = text.replace(create_old, create_new) - -# 7. DeviceChanged -> DeviceSelected -text = text.replace( - "AppMessage::InputDeviceChanged(val) => {\n state.input_device = val;\n }", - "AppMessage::InputDeviceSelected(dev) => {\n state.config.input_device = dev.name.clone();\n state.config.save();\n state.selected_input = Some(dev);\n }" -) -text = text.replace( - "AppMessage::OutputDeviceChanged(val) => {\n state.output_device = val;\n }", - "AppMessage::OutputDeviceSelected(dev) => {\n state.config.output_device = dev.name.clone();\n state.config.save();\n state.selected_output = Some(dev);\n }" -) - -# 8. View Settings -settings_old = """ text("Device Settings (Optional Node Target IDs)").size(14).color(color_subtext), - row![ - text_input("Input Target", &state.input_device).on_input(AppMessage::InputDeviceChanged).style(t_style.clone()).padding(8), - horizontal_space(), - text_input("Output Target", &state.output_device).on_input(AppMessage::OutputDeviceChanged).style(t_style.clone()).padding(8), - ].spacing(10),""" - -settings_new = """ text("Device Settings").size(14).color(color_subtext), - row![ - column![ - text("Input Target").size(12).color(color_subtext), - pick_list( - &state.input_devices[..], - state.selected_input.clone(), - AppMessage::InputDeviceSelected, - ).width(iced::Length::Fixed(160.0)) - ].spacing(4), - horizontal_space(), - column![ - text("Output Target").size(12).color(color_subtext), - pick_list( - &state.output_devices[..], - state.selected_output.clone(), - AppMessage::OutputDeviceSelected, - ).width(iced::Length::Fixed(160.0)) - ].spacing(4), - ].spacing(10),""" - -text = text.replace(settings_old, settings_new) - -with open("src/app/mod.rs", "w") as f: - f.write(text) diff --git a/update_app_noisegate.py b/update_app_noisegate.py deleted file mode 100644 index 4976858..0000000 --- a/update_app_noisegate.py +++ /dev/null @@ -1,65 +0,0 @@ -import re - -with open("src/app/mod.rs", "r") as f: - text = f.read() - -# 1. Add to AppMessage -text = text.replace( - " OutputDeviceSelected(AudioDevice),\n EventOccurred(Event),", - " OutputDeviceSelected(AudioDevice),\n NoiseGateChanged(f32),\n EventOccurred(Event)," -) - -# 2. Add to AppState (already has config, so we can just read from config.noise_gate_threshold, but state needs it too if we don't want to use state.config everywhere) -# Let's just use state.config.noise_gate_threshold - -# 3. Add to update() match -update_old = """ AppMessage::OutputDeviceSelected(dev) => { - state.config.output_device = dev.name.clone(); - state.config.save(); - state.selected_output = Some(dev); - }""" -update_new = """ AppMessage::OutputDeviceSelected(dev) => { - state.config.output_device = dev.name.clone(); - state.config.save(); - state.selected_output = Some(dev); - } - AppMessage::NoiseGateChanged(val) => { - state.config.noise_gate_threshold = val; - state.config.save(); - let _ = state.controller.send(CoreCommand::SetNoiseGateThreshold(val)); - }""" -text = text.replace(update_old, update_new) - -# 4. We also need to send the initial noise gate value to the controller on Join/Create, -# but wait! CoreController receives it async. Maybe we send it right after controller is created? -# Or just in AppState::default() we can't send because it's async? No, controller.send() is async? Actually it's non-blocking channel send. -# Let's just add it to Join message or send it when RoomJoined occurs. -# Wait, let's just send it when creating the controller? No, `state.controller.send()` returns Result, it's non-blocking. -default_old = """ let controller = Arc::new(CoreController::new(ui_tx)); - let _ = UI_RX.set(Mutex::new(Some(ui_rx))); - - let config = AppConfig::load();""" -default_new = """ let controller = Arc::new(CoreController::new(ui_tx)); - let _ = UI_RX.set(Mutex::new(Some(ui_rx))); - - let config = AppConfig::load(); - let _ = controller.send(CoreCommand::SetNoiseGateThreshold(config.noise_gate_threshold));""" -text = text.replace(default_old, default_new) - -# 5. Add slider to view_settings -settings_old = """ ].spacing(10), - vertical_space(30.0), - button(""" - -settings_new = """ ].spacing(10), - vertical_space(20.0), - column![ - text(&format!("Mic Sensitivity (Noise Gate): {:.1}%", state.config.noise_gate_threshold * 100.0)).size(14).color(color_subtext), - slider(0.0..=0.1, state.config.noise_gate_threshold, AppMessage::NoiseGateChanged) - ].spacing(10).width(iced::Length::Fixed(320.0)), - vertical_space(30.0), - button(""" -text = text.replace(settings_old, settings_new) - -with open("src/app/mod.rs", "w") as f: - f.write(text) diff --git a/update_core.py b/update_core.py deleted file mode 100644 index 86129de..0000000 --- a/update_core.py +++ /dev/null @@ -1,76 +0,0 @@ -import re - -with open("src/core/mod.rs", "r") as f: - text = f.read() - -# 1. Add noise_gate_threshold to state -text = text.replace( - "let ptt_active = Arc::new(AtomicBool::new(false));", - "let ptt_active = Arc::new(AtomicBool::new(false));\n let noise_gate_threshold = Arc::new(std::sync::atomic::AtomicU32::new(0.01f32.to_bits()));" -) - -# 2. Add noise_gate_threshold to capture thread closure -text = text.replace( - "let ptt_active_clone = ptt_active.clone();", - "let ptt_active_clone = ptt_active.clone();\n let noise_gate_threshold_clone = noise_gate_threshold.clone();" -) - -# 3. Add noise gate logic to capture loop -old_capture_loop = """ while let Ok(pcm) = capture_rx.recv() { - if is_muted_clone.load(Ordering::Relaxed) { - continue; - } - if ptt_mode_clone.load(Ordering::Relaxed) && !ptt_active_clone.load(Ordering::Relaxed) { - continue; - } - if let Ok(encoded) = encoder.encode(&pcm) {""" - -new_capture_loop = """ while let Ok(pcm) = capture_rx.recv() { - if is_muted_clone.load(Ordering::Relaxed) { - continue; - } - if ptt_mode_clone.load(Ordering::Relaxed) && !ptt_active_clone.load(Ordering::Relaxed) { - continue; - } - - let ng_bits = noise_gate_threshold_clone.load(Ordering::Relaxed); - let ng_thresh = f32::from_bits(ng_bits); - if ng_thresh > 0.0001 { - let mut sum_sq = 0.0f32; - for &sample in &pcm { - let normalized = (sample as f32) / 32768.0; - sum_sq += normalized * normalized; - } - let rms = (sum_sq / pcm.len() as f32).sqrt(); - if rms < ng_thresh { - continue; - } - } - - if let Ok(encoded) = encoder.encode(&pcm) {""" -text = text.replace(old_capture_loop, new_capture_loop) - -# 4. Handle CoreCommand::SetNoiseGateThreshold -old_match_end = """ CoreCommand::SetPeerVolume(peer_id, vol) => { - let mut guard = peer_volumes.lock().await; - guard.insert(peer_id, vol); - } - } - } -""" - -new_match_end = """ CoreCommand::SetPeerVolume(peer_id, vol) => { - let mut guard = peer_volumes.lock().await; - guard.insert(peer_id, vol); - } - - CoreCommand::SetNoiseGateThreshold(threshold) => { - noise_gate_threshold.store(threshold.to_bits(), Ordering::Relaxed); - } - } - } -""" -text = text.replace(old_match_end, new_match_end) - -with open("src/core/mod.rs", "w") as f: - f.write(text)