From: Thomas Karpiniec Date: Tue, 4 Feb 2025 09:54:56 +0000 (+1100) Subject: Add error handling X-Git-Url: https://code.octet-stream.net/m17rt/commitdiff_plain/2fb25de49daca6ddff6f5af13bcf7c314aafafb3 Add error handling --- diff --git a/m17app/src/error.rs b/m17app/src/error.rs index aeb58a5..634a531 100644 --- a/m17app/src/error.rs +++ b/m17app/src/error.rs @@ -2,7 +2,7 @@ use std::{fmt::Display, path::PathBuf}; use thiserror::Error; -/// Errors originating from the M17 Rust Toolkit core +/// Errors from the M17 Rust Toolkit #[derive(Debug, Error)] pub enum M17Error { #[error("given callsign contains at least one character invalid in M17: {0}")] @@ -39,10 +39,17 @@ pub enum M17Error { #[error("adapter error for id {0}: {1}")] Adapter(usize, #[source] AdapterError), + + #[error("soundmodem component error: {0}")] + Soundmodem(#[source] SoundmodemError), } +/// Arbitrary error type returned from adapters, which may be user-implemented pub type AdapterError = Box; +/// Arbitrary error type returned from soundmodem components, which may be user-implemented +pub type SoundmodemError = Box; + /// Iterator over potentially multiple errors #[derive(Debug, Error)] pub struct M17Errors(pub(crate) Vec); @@ -56,6 +63,13 @@ impl Iterator for M17Errors { impl Display for M17Errors { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:?}", self.0) + let mut displays = vec![]; + for e in &self.0 { + displays.push(e.to_string()); + } + write!(f, "[{}]", displays.join(", ")) } } + +#[derive(Debug, Error)] +pub enum M17SoundmodemError {} diff --git a/m17app/src/rtlsdr.rs b/m17app/src/rtlsdr.rs index 658aca4..7829f96 100644 --- a/m17app/src/rtlsdr.rs +++ b/m17app/src/rtlsdr.rs @@ -5,7 +5,7 @@ use std::{ }; use crate::{ - error::M17Error, + error::{M17Error, SoundmodemError}, soundmodem::{InputSource, SoundmodemEvent}, }; @@ -26,8 +26,7 @@ impl RtlSdr { } impl InputSource for RtlSdr { - fn start(&self, tx: SyncSender) { - // TODO: error handling + fn start(&self, tx: SyncSender) -> Result<(), SoundmodemError> { let mut cmd = Command::new("rtl_fm") .args([ "-E", @@ -40,8 +39,7 @@ impl InputSource for RtlSdr { "48k", ]) .stdout(Stdio::piped()) - .spawn() - .unwrap(); + .spawn()?; let mut stdout = cmd.stdout.take().unwrap(); let mut buf = [0u8; 1024]; let mut leftover: Option = None; @@ -72,11 +70,13 @@ impl InputSource for RtlSdr { } }); *self.rtlfm.lock().unwrap() = Some(cmd); + Ok(()) } - fn close(&self) { + fn close(&self) -> Result<(), SoundmodemError> { if let Some(mut process) = self.rtlfm.lock().unwrap().take() { let _ = process.kill(); } + Ok(()) } } diff --git a/m17app/src/serial.rs b/m17app/src/serial.rs index 8abb4b9..667d10a 100644 --- a/m17app/src/serial.rs +++ b/m17app/src/serial.rs @@ -1,6 +1,6 @@ use serialport::SerialPort; -use crate::soundmodem::Ptt; +use crate::{error::SoundmodemError, soundmodem::Ptt}; /// The pin on the serial port which is driving PTT pub enum PttPin { @@ -23,27 +23,27 @@ impl SerialPtt { .map(|i| i.port_name) } - pub fn new(port_name: &str, pin: PttPin) -> Self { + pub fn new(port_name: &str, pin: PttPin) -> Result { // TODO: error handling let port = serialport::new(port_name, 9600).open().unwrap(); let mut s = Self { port, pin }; - s.ptt_off(); - s + s.ptt_off()?; + Ok(s) } } impl Ptt for SerialPtt { - fn ptt_on(&mut self) { - let _ = match self.pin { + fn ptt_on(&mut self) -> Result<(), SoundmodemError> { + Ok(match self.pin { PttPin::Rts => self.port.write_request_to_send(true), PttPin::Dtr => self.port.write_data_terminal_ready(true), - }; + }?) } - fn ptt_off(&mut self) { - let _ = match self.pin { + fn ptt_off(&mut self) -> Result<(), SoundmodemError> { + Ok(match self.pin { PttPin::Rts => self.port.write_request_to_send(false), PttPin::Dtr => self.port.write_data_terminal_ready(false), - }; + }?) } } diff --git a/m17app/src/soundcard.rs b/m17app/src/soundcard.rs index 3914632..f89d9f4 100644 --- a/m17app/src/soundcard.rs +++ b/m17app/src/soundcard.rs @@ -12,7 +12,7 @@ use cpal::{ }; use crate::{ - error::M17Error, + error::{M17Error, SoundmodemError}, soundmodem::{InputSource, OutputBuffer, OutputSink, SoundmodemEvent}, }; @@ -118,12 +118,12 @@ pub struct SoundcardInputSource { } impl InputSource for SoundcardInputSource { - fn start(&self, samples: SyncSender) { - let _ = self.event_tx.send(SoundcardEvent::StartInput { samples }); + fn start(&self, samples: SyncSender) -> Result<(), SoundmodemError> { + Ok(self.event_tx.send(SoundcardEvent::StartInput { samples })?) } - fn close(&self) { - let _ = self.event_tx.send(SoundcardEvent::CloseInput); + fn close(&self) -> Result<(), SoundmodemError> { + Ok(self.event_tx.send(SoundcardEvent::CloseInput)?) } } @@ -132,14 +132,18 @@ pub struct SoundcardOutputSink { } impl OutputSink for SoundcardOutputSink { - fn start(&self, event_tx: SyncSender, buffer: Arc>) { - let _ = self + fn start( + &self, + event_tx: SyncSender, + buffer: Arc>, + ) -> Result<(), SoundmodemError> { + Ok(self .event_tx - .send(SoundcardEvent::StartOutput { event_tx, buffer }); + .send(SoundcardEvent::StartOutput { event_tx, buffer })?) } - fn close(&self) { - let _ = self.event_tx.send(SoundcardEvent::CloseOutput); + fn close(&self) -> Result<(), SoundmodemError> { + Ok(self.event_tx.send(SoundcardEvent::CloseOutput)?) } } diff --git a/m17app/src/soundmodem.rs b/m17app/src/soundmodem.rs index 16bb976..1cb871a 100644 --- a/m17app/src/soundmodem.rs +++ b/m17app/src/soundmodem.rs @@ -1,4 +1,4 @@ -use crate::error::M17Error; +use crate::error::{M17Error, SoundmodemError}; use crate::tnc::{Tnc, TncError}; use log::debug; use m17core::kiss::MAX_FRAME_LEN; @@ -170,11 +170,12 @@ fn spawn_soundmodem_worker( tnc.set_data_carrier_detect(demodulator.data_carrier_detect()); } SoundmodemEvent::Start => { - input.start(event_tx.clone()); - output.start(event_tx.clone(), out_buffer.clone()); + // TODO: runtime event handling + input.start(event_tx.clone()).unwrap(); + output.start(event_tx.clone(), out_buffer.clone()).unwrap(); } SoundmodemEvent::Close => { - ptt_driver.ptt_off(); + ptt_driver.ptt_off().unwrap(); break; } SoundmodemEvent::DidReadFromOutputBuffer { len, timestamp } => { @@ -200,9 +201,9 @@ fn spawn_soundmodem_worker( let new_ptt = tnc.ptt(); if new_ptt != ptt { if new_ptt { - ptt_driver.ptt_on(); + ptt_driver.ptt_on().unwrap(); } else { - ptt_driver.ptt_off(); + ptt_driver.ptt_off().unwrap(); } } ptt = new_ptt; @@ -236,8 +237,8 @@ fn spawn_soundmodem_worker( } pub trait InputSource: Send + Sync + 'static { - fn start(&self, samples: SyncSender); - fn close(&self); + fn start(&self, samples: SyncSender) -> Result<(), SoundmodemError>; + fn close(&self) -> Result<(), SoundmodemError>; } pub struct InputRrcFile { @@ -259,7 +260,7 @@ impl InputRrcFile { } impl InputSource for InputRrcFile { - fn start(&self, samples: SyncSender) { + fn start(&self, samples: SyncSender) -> Result<(), SoundmodemError> { let (end_tx, end_rx) = channel(); let baseband = self.baseband.clone(); std::thread::spawn(move || { @@ -291,10 +292,12 @@ impl InputSource for InputRrcFile { } }); *self.end_tx.lock().unwrap() = Some(end_tx); + Ok(()) } - fn close(&self) { + fn close(&self) -> Result<(), SoundmodemError> { let _ = self.end_tx.lock().unwrap().take(); + Ok(()) } } @@ -311,7 +314,7 @@ impl NullInputSource { } impl InputSource for NullInputSource { - fn start(&self, samples: SyncSender) { + fn start(&self, samples: SyncSender) -> Result<(), SoundmodemError> { let (end_tx, end_rx) = channel(); std::thread::spawn(move || { // assuming 48 kHz for now @@ -333,10 +336,12 @@ impl InputSource for NullInputSource { } }); *self.end_tx.lock().unwrap() = Some(end_tx); + Ok(()) } - fn close(&self) { + fn close(&self) -> Result<(), SoundmodemError> { let _ = self.end_tx.lock().unwrap().take(); + Ok(()) } } @@ -370,8 +375,12 @@ impl Default for OutputBuffer { } pub trait OutputSink: Send + Sync + 'static { - fn start(&self, event_tx: SyncSender, buffer: Arc>); - fn close(&self); + fn start( + &self, + event_tx: SyncSender, + buffer: Arc>, + ) -> Result<(), SoundmodemError>; + fn close(&self) -> Result<(), SoundmodemError>; } pub struct OutputRrcFile { @@ -389,13 +398,14 @@ impl OutputRrcFile { } impl OutputSink for OutputRrcFile { - fn start(&self, event_tx: SyncSender, buffer: Arc>) { + fn start( + &self, + event_tx: SyncSender, + buffer: Arc>, + ) -> Result<(), SoundmodemError> { let (end_tx, end_rx) = channel(); - let path = self.path.clone(); + let mut file = File::create(self.path.clone())?; std::thread::spawn(move || { - // TODO: error handling - let mut file = File::create(path).unwrap(); - // assuming 48 kHz for now const TICK: Duration = Duration::from_millis(25); const SAMPLES_PER_TICK: usize = 1200; @@ -437,10 +447,12 @@ impl OutputSink for OutputRrcFile { } }); *self.end_tx.lock().unwrap() = Some(end_tx); + Ok(()) } - fn close(&self) { + fn close(&self) -> Result<(), SoundmodemError> { let _ = self.end_tx.lock().unwrap().take(); + Ok(()) } } @@ -463,7 +475,11 @@ impl Default for NullOutputSink { } impl OutputSink for NullOutputSink { - fn start(&self, event_tx: SyncSender, buffer: Arc>) { + fn start( + &self, + event_tx: SyncSender, + buffer: Arc>, + ) -> Result<(), SoundmodemError> { let (end_tx, end_rx) = channel(); std::thread::spawn(move || { // assuming 48 kHz for now @@ -498,16 +514,18 @@ impl OutputSink for NullOutputSink { } }); *self.end_tx.lock().unwrap() = Some(end_tx); + Ok(()) } - fn close(&self) { + fn close(&self) -> Result<(), SoundmodemError> { let _ = self.end_tx.lock().unwrap().take(); + Ok(()) } } pub trait Ptt: Send + 'static { - fn ptt_on(&mut self); - fn ptt_off(&mut self); + fn ptt_on(&mut self) -> Result<(), SoundmodemError>; + fn ptt_off(&mut self) -> Result<(), SoundmodemError>; } /// There is no PTT because this TNC will never make transmissions on a real radio. @@ -526,6 +544,11 @@ impl Default for NullPtt { } impl Ptt for NullPtt { - fn ptt_on(&mut self) {} - fn ptt_off(&mut self) {} + fn ptt_on(&mut self) -> Result<(), SoundmodemError> { + Ok(()) + } + + fn ptt_off(&mut self) -> Result<(), SoundmodemError> { + Ok(()) + } } diff --git a/m17codec2/src/lib.rs b/m17codec2/src/lib.rs index c338f6a..2f05701 100755 --- a/m17codec2/src/lib.rs +++ b/m17codec2/src/lib.rs @@ -91,7 +91,7 @@ impl StreamAdapter for Codec2Adapter { std::thread::spawn(move || stream_thread(end_rx, setup_tx, state, output_card)); self.state.lock().unwrap().end_tx = Some(end_tx); // Propagate any errors arising in the thread - Ok(setup_rx.recv()??) + setup_rx.recv()? } fn close(&self) -> Result<(), AdapterError> { diff --git a/tools/m17rt-mod/src/main.rs b/tools/m17rt-mod/src/main.rs index 26b7aba..6a3c527 100644 --- a/tools/m17rt-mod/src/main.rs +++ b/tools/m17rt-mod/src/main.rs @@ -9,7 +9,7 @@ use std::path::PathBuf; pub fn mod_test() { let soundcard = Soundcard::new("plughw:CARD=Device,DEV=0").unwrap(); soundcard.set_tx_inverted(true); - let ptt = SerialPtt::new("/dev/ttyUSB0", PttPin::Rts); + let ptt = SerialPtt::new("/dev/ttyUSB0", PttPin::Rts).unwrap(); let soundmodem = Soundmodem::new(soundcard.input(), soundcard.output(), ptt); let app = M17App::new(soundmodem); app.start().unwrap(); diff --git a/tools/m17rt-txpacket/src/main.rs b/tools/m17rt-txpacket/src/main.rs index 37f86d9..fcac6f5 100644 --- a/tools/m17rt-txpacket/src/main.rs +++ b/tools/m17rt-txpacket/src/main.rs @@ -8,7 +8,7 @@ use m17core::protocol::PacketType; fn main() { let soundcard = Soundcard::new("plughw:CARD=Device,DEV=0").unwrap(); soundcard.set_tx_inverted(true); - let ptt = SerialPtt::new("/dev/ttyUSB0", PttPin::Rts); + let ptt = SerialPtt::new("/dev/ttyUSB0", PttPin::Rts).unwrap(); let soundmodem = Soundmodem::new(soundcard.input(), soundcard.output(), ptt); let app = M17App::new(soundmodem);