From 6a0f174a3f2d2fd558be2abb1a319961825ea047 Mon Sep 17 00:00:00 2001 From: Michael Wilson Date: Tue, 14 May 2024 12:26:38 -0400 Subject: [PATCH] Reduce use of unwrap/expect. Unwrap and expect have been reduced where possible to try to prevent the possibilities of panicking. --- src/audio/cpal.rs | 8 +++-- src/controller.rs | 4 +-- src/midi/midir.rs | 84 ++++++++++++++++++++++++++--------------------- src/midi/mock.rs | 14 ++++---- src/player.rs | 35 +++++++++++--------- src/playlist.rs | 25 +++++++++----- src/songs.rs | 7 ++-- 7 files changed, 103 insertions(+), 74 deletions(-) diff --git a/src/audio/cpal.rs b/src/audio/cpal.rs index 1cdb297..d317761 100644 --- a/src/audio/cpal.rs +++ b/src/audio/cpal.rs @@ -209,7 +209,9 @@ impl Device { // Stop only when we hit a frame boundary. This will prevent weird noises // when stopping a song. if cancel_handle.is_cancelled() && source.get_frame_position() == 0 { - tx.send(()).expect("error sending message"); + if tx.send(()).is_err() { + error!("Error sending message") + } true } else { false @@ -241,7 +243,9 @@ impl Device { data_pos += 1; } None => { - tx.send(()).expect("error sending message"); + if tx.send(()).is_err() { + error!("Error sending message") + } return; } } diff --git a/src/controller.rs b/src/controller.rs index 857f8f4..5cabf0e 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -168,7 +168,7 @@ mod test { /// Signals the next event to the monitor thread. fn next_event(&self, event: TestEvent) { { - let mut current_event = self.current_event.lock().unwrap(); + let mut current_event = self.current_event.lock().expect("failed to get lock"); *current_event = event; } // Wait until the thread goes to receive the event. @@ -187,7 +187,7 @@ mod test { loop { // Wait for next event to set the current event. barrier.wait(); - let current_event = current_event.lock().unwrap(); + let current_event = current_event.lock().expect("failed to get lock"); // Let next event know that we got the event. barrier.wait(); match *current_event { diff --git a/src/midi/midir.rs b/src/midi/midir.rs index 5c562c2..d44708e 100644 --- a/src/midi/midir.rs +++ b/src/midi/midir.rs @@ -46,21 +46,24 @@ impl super::Device for Device { let span = span!(Level::INFO, "wait for event (midir)"); let _enter = span.enter(); - let mut event_connection = self.event_connection.lock().expect("Unable to get lock"); + let mut event_connection = self.event_connection.lock().expect("unable to get lock"); if event_connection.is_some() { return Err("Already watching events.".into()); } info!("Watching MIDI events."); - if self.input_port.is_none() { - warn!("No MIDI output device configured, cannot listen for events."); - return Ok(()); - } + let input_port = match self.input_port.as_ref() { + Some(input_port) => input_port, + None => { + warn!("No MIDI output device configured, cannot listen for events."); + return Ok(()); + } + }; let input = MidiInput::new("mtrack player input")?; *event_connection = Some(input.connect( - self.input_port.as_ref().unwrap(), + input_port, "mtrack input watcher", move |_, raw_event, _| { if let Ok(event) = LiveEvent::parse(raw_event) { @@ -85,7 +88,7 @@ impl super::Device for Device { let event_connection = self .event_connection .lock() - .expect("Error getting mutex.") + .expect("error getting mutex") .take(); mem::drop(event_connection); @@ -101,20 +104,24 @@ impl super::Device for Device { let span = span!(Level::INFO, "play song (midir)"); let _enter = span.enter(); - if self.output_port.is_none() { - warn!( - song = song.name, - "No MIDI output device configured, cannot play song." - ); - return Ok(()); - } + let output_port = match self.output_port.as_ref() { + Some(output_port) => output_port, + None => { + warn!( + song = song.name, + "No MIDI output device configured, cannot play song." + ); + return Ok(()); + } + }; - let maybe_sheet = song.midi_sheet()?; - if maybe_sheet.is_none() { - info!(song = song.name, "Song has no MIDI sheet."); - return Ok(()); - } - let midi_sheet = maybe_sheet.unwrap(); + let midi_sheet = match song.midi_sheet()? { + Some(midi_sheet) => midi_sheet, + None => { + info!(song = song.name, "Song has no MIDI sheet."); + return Ok(()); + } + }; let output = MidiOutput::new("mtrack player output")?; info!( @@ -128,8 +135,7 @@ impl super::Device for Device { let cancel_handle = cancel_handle.clone(); // Wrap the midir connection in a cancel connection so that we can stop playback. - let midir_connection = - output.connect(self.output_port.as_ref().unwrap(), "mtrack player")?; + let midir_connection = output.connect(output_port, "mtrack player")?; let connection = CancelConnection { connection: midir_connection, cancel_handle: cancel_handle.clone(), @@ -243,18 +249,21 @@ fn list_midir_devices() -> Result, Box> { for port in output_ports { let name = output.port_name(&port)?; - if !devices.contains_key(&name) { - devices.insert( - name.clone(), - Device { - name: name.clone(), - input_port: None, - output_port: Some(port), - event_connection: Box::new(Mutex::new(None)), - }, - ); - } else { - devices.get_mut(&name).unwrap().output_port = Some(port); + match devices.get_mut(&name) { + Some(device) => { + device.output_port = Some(port); + } + None => { + devices.insert( + name.clone(), + Device { + name: name.clone(), + input_port: None, + output_port: Some(port), + event_connection: Box::new(Mutex::new(None)), + }, + ); + } } } @@ -319,9 +328,10 @@ impl Timer for AccurateTimer { self.last_instant = Some(last_instant.add(duration)); // Subtract the duration unless it would be an overflow. If so, use the original duration. - duration = duration - .checked_sub(Instant::now().duration_since(last_instant)) - .unwrap_or(duration); + duration = match duration.checked_sub(Instant::now().duration_since(last_instant)) { + Some(duration) => duration, + None => duration, + }; } None => self.last_instant = Some(time::Instant::now()), }; diff --git a/src/midi/mock.rs b/src/midi/mock.rs index f8eb0f4..29de93b 100644 --- a/src/midi/mock.rs +++ b/src/midi/mock.rs @@ -53,7 +53,7 @@ impl Device { /// Sends the mock event through to the sender. pub fn mock_event(&self, event: &Vec) { { - let mut mutex_event = self.event.lock().expect("Unable to get event lock."); + let mut mutex_event = self.event.lock().expect("unable to get event lock"); *mutex_event = event.to_vec(); } // Wait until the thread goes to receive the event. @@ -68,7 +68,7 @@ impl Device { let emit_called = self .emit_called .lock() - .expect("Unable to get emit called lock."); + .expect("unable to get emit called lock"); match emit_called.as_ref() { Some(event) => Some(event.to_vec()), None => None, @@ -81,7 +81,7 @@ impl Device { let mut emit_called = self .emit_called .lock() - .expect("Unable to get emit called lock."); + .expect("unable to get emit called lock"); *emit_called = None; } } @@ -94,7 +94,7 @@ impl super::Device for Device { /// Watches MIDI input for events and sends them to the given sender. fn watch_events(&self, sender: Sender>) -> Result<(), Box> { - let mut event_thread = self.event_thread.lock().expect("Unable to get lock"); + let mut event_thread = self.event_thread.lock().expect("unable to get lock"); if event_thread.is_some() { return Err("Already watching events.".into()); } @@ -109,10 +109,10 @@ impl super::Device for Device { if closed.load(Ordering::Relaxed) { return; } - let event = event.lock().expect("Unable to get event lock."); + let event = event.lock().expect("unable to get event lock"); sender .blocking_send(event.to_vec()) - .expect("Error sending event."); + .expect("error sending event"); } barrier.wait(); })); @@ -174,7 +174,7 @@ impl super::Device for Device { let mut emit_called = self .emit_called .lock() - .expect("Unable to get emit called lock."); + .expect("unable to get emit called lock"); let mut buf: Vec = Vec::with_capacity(8); midi_event.write(&mut buf)?; diff --git a/src/player.rs b/src/player.rs index 6bf8549..6d499a7 100644 --- a/src/player.rs +++ b/src/player.rs @@ -229,17 +229,18 @@ impl Player { let _enter = self.span.enter(); let mut join = self.join.lock().await; - if join.is_none() { - return Ok(false); - } - info!( - song = self.get_playlist().current().name, - "Waiting for song to finish.", - ); - - // Wait for the mutex to become available and immediately drop it. - (&mut join.as_mut().unwrap().join).await?; - Ok(true) + Ok(match join.as_mut() { + Some(join) => { + info!( + song = self.get_playlist().current().name, + "Waiting for song to finish.", + ); + // Wait for the mutex to become available and immediately drop it. + (&mut join.join).await?; + true + } + None => false, + }) } /// Next goes to the next entry in the playlist. @@ -276,10 +277,13 @@ impl Player { pub async fn stop(&mut self) -> Result<(), Box> { let mut join = self.join.lock().await; - if join.is_none() { - info!("Player is not active, nothing to stop."); - return Ok(()); - } + let join = match join.as_mut() { + Some(join) => join, + None => { + info!("Player is not active, nothing to stop."); + return Ok(()); + } + }; if self .stop_run @@ -296,7 +300,6 @@ impl Player { ); // Cancel the playback. - let join = join.as_mut().unwrap(); join.cancel.cancel(); Ok(()) } diff --git a/src/playlist.rs b/src/playlist.rs index 7d1e250..8527a18 100644 --- a/src/playlist.rs +++ b/src/playlist.rs @@ -34,8 +34,10 @@ impl fmt::Display for Playlist { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { writeln!(f, "Playlist ({} songs):", self.songs.len())?; for song_name in self.songs.iter() { - let song: Arc = self.registry.get(song_name).unwrap(); - writeln!(f, " - {} (Channels: {})", song.name, song.num_channels)?; + match self.registry.get(song_name) { + Ok(song) => writeln!(f, " - {} (Channels: {})", song.name, song.num_channels)?, + Err(_) => writeln!(f, " - {} (unable to find song)", song_name)?, + }; } Ok(()) @@ -45,8 +47,7 @@ impl fmt::Display for Playlist { impl Playlist { /// Creates a new playlist. pub fn new(song_names: Vec, registry: Arc) -> Result> { - // Verify that each song in the playlist exists in the registry. This will allow us - // to unwrap subsequent song gets later without worrying about panicking. + // Verify that each song in the playlist exists in the registry. for song_name in song_names.iter() { registry.get(song_name)?; } @@ -79,12 +80,15 @@ impl Playlist { pub fn next(&self) -> Arc { let _enter = self.span.enter(); - let mut position = self.position.write().unwrap(); + let mut position = self.position.write().expect("unable to get lock"); if *position < self.songs.len() - 1 { *position += 1; } - let current = &self.registry.get(&self.songs[*position]).unwrap(); + let current = &self + .registry + .get(&self.songs[*position]) + .expect("unable to get song from the registry"); info!( position = *position, @@ -116,8 +120,13 @@ impl Playlist { /// Return the song at the current position of the playlist. pub fn current(&self) -> Arc { - let position = self.position.read().unwrap(); - Arc::clone(&self.registry.get(&self.songs[*position]).unwrap()) + let position = self.position.read().expect("unable to get lock"); + Arc::clone( + &self + .registry + .get(&self.songs[*position]) + .expect("unable to find song in the registry"), + ) } } diff --git a/src/songs.rs b/src/songs.rs index 636021a..ca99e19 100644 --- a/src/songs.rs +++ b/src/songs.rs @@ -308,7 +308,10 @@ where files_to_tracks.insert(file_path.clone(), Vec::new()); } - files_to_tracks.get_mut(file_path).unwrap().push(track); + files_to_tracks + .get_mut(file_path) + .expect("unable to get tracks") + .push(track); }); let num_channels = *track_mapping @@ -502,7 +505,7 @@ where .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |val| { Some((val + 1) % self.channels) }) - .expect("Got none from frame position update function"); + .expect("got none from frame position update function"); } sample