From 0105b8620abec82a6dc3e3f7960784a7bc4a310b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 09:03:11 +0000 Subject: [PATCH 1/3] Clarify routing helper boundaries Co-authored-by: phieri <12006381+phieri@users.noreply.github.com> --- src/session.rs | 25 ++++++++++++++++++------- src/transport.rs | 40 +++++++++++++++++++++------------------- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/session.rs b/src/session.rs index 3478cf2..90d2248 100644 --- a/src/session.rs +++ b/src/session.rs @@ -31,6 +31,11 @@ use crate::worker::Worker; type Execute = Box Result>; +struct InspectionOutcome { + changed: bool, + error: Option, +} + pub(crate) fn guard_half_update( mode: Mode, update: &Result, @@ -362,13 +367,13 @@ impl Session { } let update = self.update_route_set(&sources, transport_ready); guard_half_update(self.config.mode, &update, || self.router.close())?; - let (changed, inspect_error) = self.inspect_and_confirm()?; - if changed { + let inspection = self.inspect_and_confirm()?; + if inspection.changed { self.router.close(); self.links.clear(); } - self.report_update(update, config_error, inspect_error); - self.last_update = (!changed).then(Instant::now); + self.report_update(update, config_error, inspection.error); + self.last_update = (!inspection.changed).then(Instant::now); Ok(()) } @@ -409,13 +414,16 @@ impl Session { } } - fn inspect_and_confirm(&mut self) -> Result<(bool, Option), String> { + fn inspect_and_confirm(&mut self) -> Result { let (current, active) = match self.router.inspect_owned() { Ok(snapshot) => snapshot, Err(error) => { self.headsets.clear(); self.links.clear(); - return Ok((false, Some(error))); + return Ok(InspectionOutcome { + changed: false, + error: Some(error), + }); } }; self.headsets = current; @@ -460,7 +468,10 @@ impl Session { &self.stopped, ); } - Ok((changed, None)) + Ok(InspectionOutcome { + changed, + error: None, + }) } fn report_update( diff --git a/src/transport.rs b/src/transport.rs index ee9a278..ecbc5ae 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -42,14 +42,11 @@ pub(crate) fn controller_warnings( devices: &BTreeMap, sco_limit: usize, ) -> Vec { - let mut controllers: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); - for (address, device) in devices { - controllers - .entry(&device.controller) - .or_default() - .push(address); - } - controllers + addresses_by_controller( + devices + .iter() + .map(|(address, device)| (address, device)), + ) .into_iter() .filter(|(_, addresses)| addresses.len() > sco_limit) .map(|(controller, addresses)| { @@ -142,6 +139,19 @@ impl Device { } } +fn addresses_by_controller<'a>( + devices: impl IntoIterator, +) -> BTreeMap<&'a str, Vec<&'a str>> { + let mut controllers = BTreeMap::new(); + for (address, device) in devices { + controllers + .entry(device.controller.as_str()) + .or_insert_with(Vec::new) + .push(address.as_str()); + } + controllers +} + fn identifier(value: &Value) -> Option { value .as_str() @@ -465,17 +475,9 @@ fn validate_source_capacity( devices: &BTreeMap, sco_limit: usize, ) -> Result<(), String> { - let mut controllers: BTreeMap<&str, Vec<&str>> = BTreeMap::new(); - for address in sources { - let Some(device) = devices.get(address) else { - continue; - }; - controllers - .entry(&device.controller) - .or_default() - .push(address); - } - for (controller, addresses) in controllers { + for (controller, addresses) in addresses_by_controller(sources.iter().filter_map(|address| { + devices.get(address).map(|device| (address, device)) + })) { if addresses.len() > sco_limit { return Err(format!( "SCO capacity conflict on {controller}: {} sources ({}) exceed configured --sco-limit {sco_limit}", From c8c85289401f1b5ae0a0314b8cf558c00b248ae2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 09:05:04 +0000 Subject: [PATCH 2/3] Extract talk-group editor model Co-authored-by: phieri <12006381+phieri@users.noreply.github.com> --- src/group_editor.rs | 126 ++++++++++++++++++++++++++++++++++++++++++++ src/main.rs | 1 + src/tui.rs | 126 +++++++++++++------------------------------- 3 files changed, 165 insertions(+), 88 deletions(-) create mode 100644 src/group_editor.rs diff --git a/src/group_editor.rs b/src/group_editor.rs new file mode 100644 index 0000000..788d1ba --- /dev/null +++ b/src/group_editor.rs @@ -0,0 +1,126 @@ +//! Talk-group edits and persistence, independent of terminal presentation. + +use std::collections::BTreeSet; +use std::path::PathBuf; + +use crate::groups::{TalkGroup, normalize_name, save}; + +pub(crate) struct GroupEditor { + pub(crate) groups: Vec, + pub(crate) persistence_error: Option, + pub(crate) error: Option, + groups_path: PathBuf, +} + +impl GroupEditor { + pub(crate) fn new(groups_path: PathBuf, groups: Vec) -> Self { + Self { + groups, + persistence_error: None, + error: None, + groups_path, + } + } + + pub(crate) fn add(&mut self, name: &str) -> bool { + let name = name.trim(); + if name.is_empty() { + self.error = Some("Talk-group name cannot be empty.".into()); + return false; + } + if self + .groups + .iter() + .any(|group| normalize_name(&group.name) == normalize_name(name)) + { + self.error = Some(format!("A talk group named {name:?} already exists.")); + return false; + } + self.groups.push(TalkGroup { + name: name.to_string(), + members: BTreeSet::new(), + }); + self.persist(); + true + } + + pub(crate) fn remove(&mut self, index: usize) -> bool { + if index >= self.groups.len() { + return false; + } + self.groups.remove(index); + self.persist(); + true + } + + pub(crate) fn toggle_member(&mut self, index: usize, address: String) -> bool { + let Some(group) = self.groups.get_mut(index) else { + return false; + }; + if !group.members.remove(&address) { + group.members.insert(address); + } + self.persist(); + true + } + + fn persist(&mut self) { + match save(&self.groups_path, &self.groups) { + Ok(()) => { + self.error = None; + self.persistence_error = None; + } + Err(error) => self.persistence_error = Some(error), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::groups::load; + + fn path(label: &str) -> PathBuf { + std::env::temp_dir() + .join(format!("bt-intercom-group-editor-{label}-{}", std::process::id())) + .join("talk-groups.json") + } + + #[test] + fn edits_persist_and_duplicate_names_use_persistence_normalization() { + let path = path("unicode"); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + let mut editor = GroupEditor::new( + path.clone(), + vec![TalkGroup { + name: "équipe".into(), + members: BTreeSet::new(), + }], + ); + + assert!(!editor.add("Équipe")); + assert!(editor.error.as_deref().unwrap().contains("already exists")); + assert!(editor.add(" Dispatch ")); + assert_eq!(editor.groups[1].name, "Dispatch"); + assert!(editor.toggle_member(1, "AA:BB:CC:DD:EE:01".into())); + assert!(editor.remove(0)); + assert_eq!(load(&path).unwrap(), editor.groups); + + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } + + #[test] + fn persistence_errors_are_reported_without_discarding_edits() { + let path = path("failure"); + let mut editor = GroupEditor::new( + path.join("not-a-directory").join("groups.json"), + Vec::new(), + ); + + assert!(editor.add("Dispatch")); + + assert_eq!(editor.groups.len(), 1); + assert!(editor.persistence_error.is_some()); + let _ = std::fs::remove_dir_all(path.parent().unwrap()); + } +} diff --git a/src/main.rs b/src/main.rs index 4083396..a4fc26f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,6 +10,7 @@ mod bluez; mod dashboard; mod device_status; mod groups; +mod group_editor; mod network; mod pair_button; mod pipewire; diff --git a/src/tui.rs b/src/tui.rs index ef42c7d..3e4b120 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -19,7 +19,8 @@ use ratatui::{DefaultTerminal, Frame}; use crate::bluez::{FAIR_RSSI, GOOD_RSSI}; use crate::device_status::{HeadsetStatus, StatusUpdate, start_polling}; -use crate::groups::{TalkGroup, load, normalize_name, save}; +use crate::group_editor::GroupEditor; +use crate::groups::load; use crate::terminal_style::{Meaning, Palette}; #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -30,15 +31,12 @@ enum Focus { struct ControlPanel { allowed: BTreeSet, - groups_path: PathBuf, - groups: Vec, + group_editor: GroupEditor, status: BTreeMap, selected_group: Option, selected_headset: usize, focus: Focus, new_group_name: Option, - error: Option, - persistence_error: Option, status_error: Option, last_updated: Option, palette: Palette, @@ -47,36 +45,24 @@ struct ControlPanel { impl ControlPanel { fn new(allowed: BTreeSet, groups_path: PathBuf) -> Result { let groups = load(&groups_path)?; + let selected_group = (!groups.is_empty()).then_some(0); Ok(Self { allowed, - groups_path, - selected_group: (!groups.is_empty()).then_some(0), - groups, + group_editor: GroupEditor::new(groups_path, groups), + selected_group, status: BTreeMap::new(), selected_headset: 0, focus: Focus::Groups, new_group_name: None, - error: None, - persistence_error: None, status_error: None, last_updated: None, palette: Palette::detect(std::io::stdout().is_terminal()), }) } - fn persist_groups(&mut self) { - match save(&self.groups_path, &self.groups) { - Ok(()) => { - self.error = None; - self.persistence_error = None; - } - Err(error) => self.persistence_error = Some(error), - } - } - fn apply_status_update(&mut self, update: StatusUpdate) { self.status = update.statuses; - self.error = None; + self.group_editor.error = None; self.status_error = update.error; self.last_updated = Some(Instant::now()); } @@ -88,63 +74,46 @@ impl ControlPanel { .unwrap_or_default() .trim() .to_string(); - if name.is_empty() { - self.error = Some("Talk-group name cannot be empty.".into()); - } else if self - .groups - .iter() - .any(|group| normalize_name(&group.name) == normalize_name(&name)) - { - self.error = Some(format!("A talk group named {name:?} already exists.")); - } else { - self.groups.push(TalkGroup { - name, - members: BTreeSet::new(), - }); - self.selected_group = Some(self.groups.len() - 1); + if self.group_editor.add(&name) { + self.selected_group = Some(self.group_editor.groups.len() - 1); self.focus = Focus::Members; - self.persist_groups(); } } fn remove_selected_group(&mut self) { if let Some(index) = self .selected_group - .filter(|index| *index < self.groups.len()) + .filter(|index| *index < self.group_editor.groups.len()) + .filter(|index| self.group_editor.remove(*index)) { - self.groups.remove(index); - self.selected_group = if self.groups.is_empty() { + self.selected_group = if self.group_editor.groups.is_empty() { None } else { - Some(index.min(self.groups.len() - 1)) + Some(index.min(self.group_editor.groups.len() - 1)) }; - self.persist_groups(); } } fn toggle_selected_member(&mut self) { - let Some(group) = self + let Some(group_index) = self .selected_group - .and_then(|index| self.groups.get_mut(index)) + .filter(|index| *index < self.group_editor.groups.len()) else { - self.error = Some("Create or select a talk group first.".into()); + self.group_editor.error = Some("Create or select a talk group first.".into()); return; }; let Some(address) = self.allowed.iter().nth(self.selected_headset).cloned() else { return; }; - if !group.members.remove(&address) { - group.members.insert(address); - } - self.persist_groups(); + self.group_editor.toggle_member(group_index, address); } fn move_selection(&mut self, down: bool) { match self.focus { - Focus::Groups if !self.groups.is_empty() => { + Focus::Groups if !self.group_editor.groups.is_empty() => { let current = self.selected_group.unwrap_or(0); let next = if down { - (current + 1).min(self.groups.len() - 1) + (current + 1).min(self.group_editor.groups.len() - 1) } else { current.saturating_sub(1) }; @@ -186,7 +155,7 @@ impl ControlPanel { KeyCode::Down => self.move_selection(true), KeyCode::Char('n') if self.focus == Focus::Groups => { self.new_group_name = Some(String::new()); - self.error = None; + self.group_editor.error = None; } KeyCode::Char('d') if self.focus == Focus::Groups => self.remove_selected_group(), KeyCode::Char(' ') if self.focus == Focus::Members => self.toggle_selected_member(), @@ -255,7 +224,7 @@ impl ControlPanel { .map(|address| { let included = self .selected_group - .and_then(|index| self.groups.get(index)) + .and_then(|index| self.group_editor.groups.get(index)) .is_some_and(|group| group.members.contains(address)); let status = self.status.get(address); let name = status @@ -319,6 +288,7 @@ impl ControlPanel { Layout::horizontal([Constraint::Percentage(40), Constraint::Percentage(60)]) .areas(sections[1]); let group_items: Vec> = self + .group_editor .groups .iter() .map(|group| ListItem::new(group.name.as_str())) @@ -351,7 +321,7 @@ impl ControlPanel { let members = self.member_items(); let group_name = self .selected_group - .and_then(|index| self.groups.get(index)) + .and_then(|index| self.group_editor.groups.get(index)) .map_or("Select a talk group", |group| group.name.as_str()); let members_list = List::new(members) .block( @@ -371,9 +341,10 @@ impl ControlPanel { frame.render_stateful_widget(members_list, lower[1], &mut member_state); let hint = self + .group_editor .persistence_error .as_deref() - .or(self.error.as_deref()) + .or(self.group_editor.error.as_deref()) .or(self.status_error.as_deref()) .unwrap_or({ if let Some(name) = &self.new_group_name { @@ -393,8 +364,8 @@ impl ControlPanel { frame.render_widget( Paragraph::new(format!("{hint} | {refreshed}")) .style( - if self.error.is_some() - || self.persistence_error.is_some() + if self.group_editor.error.is_some() + || self.group_editor.persistence_error.is_some() || self.status_error.is_some() { self.palette.style(Meaning::Error) @@ -483,58 +454,37 @@ mod tests { panel.handle_key(KeyCode::Char(character)); } panel.handle_key(KeyCode::Enter); - assert_eq!(panel.groups[0].name, "Dispatch"); + assert_eq!(panel.group_editor.groups[0].name, "Dispatch"); assert_eq!(panel.focus, Focus::Members); panel.handle_key(KeyCode::Char(' ')); - assert!(panel.groups[0].members.contains(&address)); - assert_eq!(load(&path).unwrap(), panel.groups); + assert!(panel.group_editor.groups[0].members.contains(&address)); + assert_eq!(load(&path).unwrap(), panel.group_editor.groups); panel.handle_key(KeyCode::Tab); panel.handle_key(KeyCode::Char('d')); - assert!(panel.groups.is_empty()); + assert!(panel.group_editor.groups.is_empty()); assert!(load(&path).unwrap().is_empty()); let _ = std::fs::remove_dir_all(path.parent().unwrap()); } - #[test] - fn group_name_uniqueness_matches_persistence_normalization() { - let path = std::env::temp_dir() - .join(format!("bt-intercom-tui-unicode-{}", std::process::id())) - .join("talk-groups.json"); - let _ = std::fs::remove_dir_all(path.parent().unwrap()); - save( - &path, - &[TalkGroup { - name: "équipe".into(), - members: BTreeSet::new(), - }], - ) - .unwrap(); - let mut panel = ControlPanel::new(BTreeSet::new(), path.clone()).unwrap(); - panel.new_group_name = Some("Équipe".into()); - - panel.add_group(); - - assert_eq!(panel.groups.len(), 1); - assert!(panel.error.as_deref().unwrap().contains("already exists")); - let _ = std::fs::remove_dir_all(path.parent().unwrap()); - } - #[test] fn status_updates_clear_transient_errors_but_keep_save_errors() { let path = std::env::temp_dir() .join(format!("bt-intercom-tui-errors-{}", std::process::id())) .join("talk-groups.json"); let mut panel = ControlPanel::new(BTreeSet::new(), path).unwrap(); - panel.error = Some("transient action error".into()); - panel.persistence_error = Some("save failed".into()); + panel.group_editor.error = Some("transient action error".into()); + panel.group_editor.persistence_error = Some("save failed".into()); panel.apply_status_update(StatusUpdate { statuses: BTreeMap::new(), error: None, }); - assert!(panel.error.is_none()); - assert_eq!(panel.persistence_error.as_deref(), Some("save failed")); + assert!(panel.group_editor.error.is_none()); + assert_eq!( + panel.group_editor.persistence_error.as_deref(), + Some("save failed") + ); } } From 245562243a0d8c3a5b225fe600b96ab3c1d89425 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Oct 2026 09:06:37 +0000 Subject: [PATCH 3/3] Fix group editor persistence test Co-authored-by: phieri <12006381+phieri@users.noreply.github.com> --- src/group_editor.rs | 10 ++++++++-- src/main.rs | 2 +- src/transport.rs | 14 ++++++-------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/group_editor.rs b/src/group_editor.rs index 788d1ba..bc1b8a3 100644 --- a/src/group_editor.rs +++ b/src/group_editor.rs @@ -82,7 +82,10 @@ mod tests { fn path(label: &str) -> PathBuf { std::env::temp_dir() - .join(format!("bt-intercom-group-editor-{label}-{}", std::process::id())) + .join(format!( + "bt-intercom-group-editor-{label}-{}", + std::process::id() + )) .join("talk-groups.json") } @@ -112,8 +115,11 @@ mod tests { #[test] fn persistence_errors_are_reported_without_discarding_edits() { let path = path("failure"); + let directory = path.parent().unwrap(); + std::fs::create_dir_all(directory).unwrap(); + std::fs::write(directory.join("not-a-directory"), "file").unwrap(); let mut editor = GroupEditor::new( - path.join("not-a-directory").join("groups.json"), + directory.join("not-a-directory").join("groups.json"), Vec::new(), ); diff --git a/src/main.rs b/src/main.rs index a4fc26f..a75d42a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -9,8 +9,8 @@ mod atomic_file; mod bluez; mod dashboard; mod device_status; -mod groups; mod group_editor; +mod groups; mod network; mod pair_button; mod pipewire; diff --git a/src/transport.rs b/src/transport.rs index ecbc5ae..aad36fe 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -42,11 +42,7 @@ pub(crate) fn controller_warnings( devices: &BTreeMap, sco_limit: usize, ) -> Vec { - addresses_by_controller( - devices - .iter() - .map(|(address, device)| (address, device)), - ) + addresses_by_controller(devices.iter()) .into_iter() .filter(|(_, addresses)| addresses.len() > sco_limit) .map(|(controller, addresses)| { @@ -475,9 +471,11 @@ fn validate_source_capacity( devices: &BTreeMap, sco_limit: usize, ) -> Result<(), String> { - for (controller, addresses) in addresses_by_controller(sources.iter().filter_map(|address| { - devices.get(address).map(|device| (address, device)) - })) { + for (controller, addresses) in addresses_by_controller( + sources + .iter() + .filter_map(|address| devices.get(address).map(|device| (address, device))), + ) { if addresses.len() > sco_limit { return Err(format!( "SCO capacity conflict on {controller}: {} sources ({}) exceed configured --sco-limit {sco_limit}",