Refactor error types

firmware/src/buzzer.rs

@@ -1,3 +1,4 @@
+use core::fmt;
 use embassy_time::{Duration, Timer};
 use esp_hal::clock::Clocks;
 use esp_hal::gpio::AnyPin;
@@ -9,12 +10,10 @@ use log::{debug, info};
 
 /// Buzzer error
 #[derive(Debug)]
-#[non_exhaustive]
 pub enum Error {
     /// PWM timer error
     Timer(timer::Error),
     /// PWM channel error
-    #[allow(dead_code)]
     Channel(channel::Error),
 }
 
@@ -30,6 +29,15 @@ impl From<channel::Error> for Error {
     }
 }
 
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::Timer(_err) => write!(f, "PWM timer error"),
+            Self::Channel(_err) => write!(f, "PWM channel error"),
+        }
+    }
+}
+
 /// Passive buzzer (driven by PWM signal on GPIO)
 pub struct Buzzer<'a> {
     ledc: Ledc<'a>,

firmware/src/display.rs

@@ -1,4 +1,5 @@
 use crate::screen::{self, Screen};
+use core::fmt;
 use embedded_graphics::pixelcolor::BinaryColor;
 use embedded_graphics::prelude::*;
 use embedded_hal_async::i2c::I2c;
@@ -10,7 +11,53 @@ use ssd1306::size::DisplaySize128x64;
 use ssd1306::Ssd1306Async;
 
 /// Display error
-pub type Error = screen::Error<display_interface::DisplayError>;
+#[derive(Debug)]
+pub enum Error {
+    /// Display interface error
+    InterfaceError(display_interface::DisplayError),
+    /// Font render error
+    FontRenderError(screen::Error<()>),
+}
+
+impl From<display_interface::DisplayError> for Error {
+    fn from(err: display_interface::DisplayError) -> Self {
+        Self::InterfaceError(err)
+    }
+}
+
+impl From<screen::Error<display_interface::DisplayError>> for Error {
+    fn from(err: screen::Error<display_interface::DisplayError>) -> Self {
+        use screen::Error;
+
+        match err {
+            Error::BackgroundColorNotSupported => {
+                Self::FontRenderError(Error::BackgroundColorNotSupported)
+            }
+            Error::GlyphNotFound(ch) => Self::FontRenderError(Error::GlyphNotFound(ch)),
+            Error::DisplayError(err) => Self::InterfaceError(err),
+        }
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        use display_interface::DisplayError;
+
+        match self {
+            Self::InterfaceError(err) => match err {
+                DisplayError::InvalidFormatError => write!(f, "Invalid format"),
+                DisplayError::BusWriteError => write!(f, "Bus write error"),
+                DisplayError::DCError => write!(f, "DC error"),
+                DisplayError::CSError => write!(f, "CS error"),
+                DisplayError::DataFormatNotImplemented => write!(f, "Format not implemented"),
+                DisplayError::RSError => write!(f, "Reset error"),
+                DisplayError::OutOfBoundsError => write!(f, "Out of bounds"),
+                _ => write!(f, "Interface error"),
+            },
+            Self::FontRenderError(_err) => write!(f, "Font render error"),
+        }
+    }
+}
 
 /// Convenient hardware-agnostic display driver
 pub struct Display<I2C> {

firmware/src/error.rs

@@ -0,0 +1,42 @@
+use crate::{display, nfc};
+use core::fmt;
+
+/// Main error type
+#[derive(Debug)]
+#[allow(clippy::enum_variant_names)]
+pub enum Error {
+    /// Display output error
+    DisplayError(display::Error),
+    /// NFC reader error
+    NFCError(nfc::Error),
+    /// User cancel request
+    Cancel,
+    /// User interaction timeout
+    UserTimeout,
+    /// No network connection
+    NoNetwork,
+}
+
+impl From<display::Error> for Error {
+    fn from(err: display::Error) -> Self {
+        Self::DisplayError(err)
+    }
+}
+
+impl From<nfc::Error> for Error {
+    fn from(err: nfc::Error) -> Self {
+        Self::NFCError(err)
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            Self::DisplayError(err) => write!(f, "Display: {err}"),
+            Self::NFCError(err) => write!(f, "NFC: {err}"),
+            Self::Cancel => write!(f, "User cancelled"),
+            Self::UserTimeout => write!(f, "Timeout waiting for input"),
+            Self::NoNetwork => write!(f, "No network connection"),
+        }
+    }
+}

firmware/src/main.rs

@@ -41,6 +41,7 @@
 
 mod buzzer;
 mod display;
+mod error;
 mod keypad;
 mod nfc;
 mod pn532;
@@ -188,9 +189,9 @@ async fn main(spawner: Spawner) {
             // Success: start over again
             Ok(()) => (),
             // Cancel: start over again
-            Err(ui::Error::Cancel) => info!("User cancelled, starting over..."),
+            Err(error::Error::Cancel) => info!("User cancelled, starting over..."),
             // Timeout: start over again
-            Err(ui::Error::UserTimeout) => info!("Timeout waiting for user, starting over..."),
+            Err(error::Error::UserTimeout) => info!("Timeout waiting for user, starting over..."),
             // TODO: Display error to user and start over again
             Err(err) => panic!("Unhandled Error: {:?}", err),
         }

firmware/src/nfc.rs

@@ -16,27 +16,38 @@ const READ_TIMEOUT: Duration = Duration::from_millis(100);
 const READ_SLEEP: Duration = Duration::from_millis(400);
 
 /// NFC reader error
+// Basically a PN532 error with static interface error type to avoid generics in this type
 #[derive(Debug)]
-#[non_exhaustive]
-pub enum Error {
-    /// PN532 error (with static interface error type)
-    #[allow(dead_code)]
-    Pn532(Pn532Error<embedded_hal_async::i2c::ErrorKind>),
-}
+pub struct Error(Pn532Error<embedded_hal_async::i2c::ErrorKind>);
 
 impl<E: embedded_hal_async::i2c::Error> From<Pn532Error<E>> for Error {
     fn from(err: Pn532Error<E>) -> Self {
         // Convert generic Pn532Error::InterfaceError(E: embedded_hal::i2c::Error) to non-generic
         // Pn532Error::InterfaceError(embedded_hal::i2c::ErrorKind) to avoid generics in this type
         match err {
-            Pn532Error::BadAck => Self::Pn532(Pn532Error::BadAck),
-            Pn532Error::BadResponseFrame => Self::Pn532(Pn532Error::BadResponseFrame),
-            Pn532Error::Syntax => Self::Pn532(Pn532Error::Syntax),
-            Pn532Error::CrcError => Self::Pn532(Pn532Error::CrcError),
-            Pn532Error::BufTooSmall => Self::Pn532(Pn532Error::BufTooSmall),
-            Pn532Error::TimeoutAck => Self::Pn532(Pn532Error::TimeoutAck),
-            Pn532Error::TimeoutResponse => Self::Pn532(Pn532Error::TimeoutResponse),
-            Pn532Error::InterfaceError(e) => Self::Pn532(Pn532Error::InterfaceError(e.kind())),
+            Pn532Error::BadAck => Self(Pn532Error::BadAck),
+            Pn532Error::BadResponseFrame => Self(Pn532Error::BadResponseFrame),
+            Pn532Error::Syntax => Self(Pn532Error::Syntax),
+            Pn532Error::CrcError => Self(Pn532Error::CrcError),
+            Pn532Error::BufTooSmall => Self(Pn532Error::BufTooSmall),
+            Pn532Error::TimeoutAck => Self(Pn532Error::TimeoutAck),
+            Pn532Error::TimeoutResponse => Self(Pn532Error::TimeoutResponse),
+            Pn532Error::InterfaceError(e) => Self(Pn532Error::InterfaceError(e.kind())),
+        }
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self.0 {
+            Pn532Error::BadAck => write!(f, "Bad ACK"),
+            Pn532Error::BadResponseFrame => write!(f, "Bad response frame"),
+            Pn532Error::Syntax => write!(f, "Syntax error"),
+            Pn532Error::CrcError => write!(f, "CRC error"),
+            Pn532Error::BufTooSmall => write!(f, "Buffer too small"),
+            Pn532Error::TimeoutAck => write!(f, "ACK timeout"),
+            Pn532Error::TimeoutResponse => write!(f, "Response timeout"),
+            Pn532Error::InterfaceError(_err) => write!(f, "Bus error"),
         }
     }
 }

firmware/src/screen.rs

@@ -39,26 +39,7 @@ const SMALL_FONT: FontRenderer = FontRenderer::new::<fonts::u8g2_font_5x7_tf>();
 const FOOTER_FONT: FontRenderer = FontRenderer::new::<fonts::u8g2_font_5x7_tf>();
 
 /// Screen display error
-#[derive(Debug)]
-#[non_exhaustive]
-pub enum Error<E> {
-    /// Display error
-    DisplayError(E),
-    /// Font render error
-    FontRenderError(u8g2_fonts::Error<E>),
-}
-
-impl<E> From<E> for Error<E> {
-    fn from(err: E) -> Self {
-        Self::DisplayError(err)
-    }
-}
-
-impl<E> From<u8g2_fonts::Error<E>> for Error<E> {
-    fn from(err: u8g2_fonts::Error<E>) -> Self {
-        Self::FontRenderError(err)
-    }
-}
+pub type Error<E> = u8g2_fonts::Error<E>;
 
 /// Generic screen that can be displayed
 pub trait Screen {
@@ -76,7 +57,9 @@ impl Screen for Splash {
         &self,
         target: &mut D,
     ) -> Result<(), Error<D::Error>> {
-        Image::new(&LOGO, Point::new(0, 13)).draw(target)?;
+        Image::new(&LOGO, Point::new(0, 13))
+            .draw(target)
+            .map_err(Error::DisplayError)?;
         SPLASH_VERSION_FONT.render_aligned(
             VERSION_STR,
             Point::new(63, 30 + 12),

firmware/src/ui.rs

@@ -1,7 +1,8 @@
 use crate::buzzer::Buzzer;
-use crate::display::{self, Display};
+use crate::display::Display;
+use crate::error::Error;
 use crate::keypad::{Key, Keypad};
-use crate::nfc::{self, Nfc, Uid};
+use crate::nfc::{Nfc, Uid};
 use crate::screen;
 use crate::wifi::Wifi;
 use core::convert::Infallible;
@@ -32,43 +33,6 @@ const IDLE_TIMEOUT: Duration = Duration::from_secs(300);
 #[cfg(debug_assertions)]
 const IDLE_TIMEOUT: Duration = Duration::from_secs(10);
 
-/// User interface error
-#[derive(Debug)]
-#[non_exhaustive]
-#[allow(clippy::enum_variant_names)]
-pub enum Error {
-    /// Failed to output to display
-    #[allow(dead_code)]
-    DisplayError(display::Error),
-    /// NFC reader error
-    #[allow(dead_code)]
-    NFCError(nfc::Error),
-    /// User cancel request
-    Cancel,
-    /// User interaction timeout
-    UserTimeout,
-    /// No network connection
-    NoNetwork,
-}
-
-impl From<display::Error> for Error {
-    fn from(err: display::Error) -> Self {
-        Self::DisplayError(err)
-    }
-}
-
-impl From<nfc::Error> for Error {
-    fn from(err: nfc::Error) -> Self {
-        Self::NFCError(err)
-    }
-}
-
-impl From<TimeoutError> for Error {
-    fn from(_err: TimeoutError) -> Self {
-        Self::UserTimeout
-    }
-}
-
 /// User interface
 pub struct Ui<'a, I2C, IRQ> {
     display: Display<I2C>,
@@ -107,8 +71,12 @@ impl<'a, I2C: I2c, IRQ: Wait<Error = Infallible>> Ui<'a, I2C, IRQ> {
     /// Show splash screen and wait for keypress or timeout
     pub async fn show_splash_screen(&mut self) -> Result<(), Error> {
         self.display.screen(&screen::Splash).await?;
-        let _key = with_timeout(SPLASH_TIMEOUT, self.keypad.read()).await?;
-        Ok(())
+        match with_timeout(SPLASH_TIMEOUT, self.keypad.read()).await {
+            // Key pressed
+            Ok(_key) => Ok(()),
+            // User interaction timeout
+            Err(TimeoutError) => Err(Error::UserTimeout),
+        }
     }
 
     /// Wait for network to become available (if not already). Show a waiting screen and allow to
@@ -159,7 +127,8 @@ impl<'a, I2C: I2c, IRQ: Wait<Error = Infallible>> Ui<'a, I2C, IRQ> {
 }
 
 impl<'a, I2C: I2c, IRQ: Wait<Error = Infallible>> Ui<'a, I2C, IRQ> {
-    /// Wait for id card and verify identification
+    /// Wait for id card and read it. On idle timeout, enter power saving (turn off display).
+    /// Any key pressed leaves power saving (turn on display).
     async fn read_id_card(&mut self) -> Result<Uid, Error> {
         info!("UI: Waiting for NFC card...");
 
@@ -203,15 +172,17 @@ impl<'a, I2C: I2c, IRQ: Wait<Error = Infallible>> Ui<'a, I2C, IRQ> {
         self.display.screen(&screen::NumberOfDrinks).await?;
         loop {
             #[allow(clippy::match_same_arms)]
-            match with_timeout(USER_TIMEOUT, self.keypad.read()).await? {
+            match with_timeout(USER_TIMEOUT, self.keypad.read()).await {
                 // Any digit 1..=9 selects number of drinks
-                Key::Digit(n) if (1..=9).contains(&n) => return Ok(n as usize),
+                Ok(Key::Digit(n)) if (1..=9).contains(&n) => return Ok(n as usize),
                 // Ignore any other digit
-                Key::Digit(_) => (),
+                Ok(Key::Digit(_)) => (),
                 // Cancel key cancels
-                Key::Cancel => return Err(Error::Cancel),
+                Ok(Key::Cancel) => return Err(Error::Cancel),
                 // Ignore any other key
-                _ => (),
+                Ok(_) => (),
+                // User interaction timeout
+                Err(TimeoutError) => return Err(Error::UserTimeout),
             }
         }
     }
@@ -227,13 +198,15 @@ impl<'a, I2C: I2c, IRQ: Wait<Error = Infallible>> Ui<'a, I2C, IRQ> {
             .screen(&screen::Checkout::new(num_drinks, total_price))
             .await?;
         loop {
-            match with_timeout(USER_TIMEOUT, self.keypad.read()).await? {
+            match with_timeout(USER_TIMEOUT, self.keypad.read()).await {
                 // Enter key confirms purchase
-                Key::Enter => return Ok(()),
+                Ok(Key::Enter) => return Ok(()),
                 // Cancel key cancels
-                Key::Cancel => return Err(Error::Cancel),
+                Ok(Key::Cancel) => return Err(Error::Cancel),
                 // Ignore any other key
-                _ => (),
+                Ok(_) => (),
+                // User interaction timeout
+                Err(TimeoutError) => return Err(Error::UserTimeout),
             }
         }
     }