Improved error handling

src/config.rs

@@ -1,14 +1,13 @@
-use std::{fs, error::Error, net::{Ipv4Addr, SocketAddr}, collections::HashMap};
+use std::{fs, net::{Ipv4Addr, SocketAddr}, collections::HashMap};
 
 use async_recursion::async_recursion;
 use regex::{Regex, Captures};
-use tracing::{debug, trace, error};
+use tracing::{debug, trace};
 use rumqttc::{AsyncClient, has_wildcards};
 use serde::Deserialize;
+use eui48::MacAddress;
 
-use crate::devices::{DeviceBox, IkeaOutlet, WakeOnLAN, AudioSetup, ContactSensor, KasaOutlet, AsOnOff};
-
-// @TODO Configure more defaults
+use crate::{devices::{DeviceBox, IkeaOutlet, WakeOnLAN, AudioSetup, ContactSensor, KasaOutlet}, error::{FailedToParseConfig, MissingEnv, MissingWildcard, Error, FailedToCreateDevice}};
 
 #[derive(Debug, Deserialize)]
 pub struct Config {
@@ -124,14 +123,20 @@ pub struct PresenceDeviceConfig {
 
 impl PresenceDeviceConfig {
     /// Set the mqtt topic to an appropriate value if it is not already set
-    fn generate_topic(&mut self, class: &str, identifier: &str, config: &Config) {
+    fn generate_topic(&mut self, class: &str, identifier: &str, config: &Config) -> Result<(), MissingWildcard> {
         if self.mqtt.is_none() {
+            if !has_wildcards(&config.presence.topic) {
+                return Err(MissingWildcard::new(&config.presence.topic).into());
+            }
+
             // @TODO This is not perfect, if the topic is some/+/thing/# this will fail
             let offset = config.presence.topic.find('+').or(config.presence.topic.find('#')).unwrap();
             let topic = config.presence.topic[..offset].to_owned() + class + "/" + identifier;
             trace!("Setting presence mqtt topic: {topic}");
             self.mqtt = Some(MqttDeviceConfig { topic });
         }
+
+        Ok(())
     }
 }
 
@@ -150,7 +155,7 @@ pub enum Device {
         info: InfoConfig,
         #[serde(flatten)]
         mqtt: MqttDeviceConfig,
-        mac_address: String,
+        mac_address: MacAddress,
     },
     KasaOutlet {
         ip: Ipv4Addr,
@@ -169,39 +174,31 @@ pub enum Device {
 }
 
 impl Config {
-    pub fn build(filename: &str) -> Result<Self, Box<dyn Error>> {
+    pub fn parse_file(filename: &str) -> Result<Self, FailedToParseConfig> {
         debug!("Loading config: {filename}");
-        let file = fs::read_to_string(filename)?;
+        let file = fs::read_to_string(filename)
+            .map_err(|err| FailedToParseConfig::new(filename, err.into()))?;
 
         // Substitute in environment variables
         let re = Regex::new(r"\$\{(.*)\}").unwrap();
-        let mut failure = false;
+        let mut missing = MissingEnv::new();
         let file = re.replace_all(&file, |caps: &Captures| {
             let key = caps.get(1).unwrap().as_str();
             debug!("Substituting '{key}' in config");
             match std::env::var(key) {
                 Ok(value) => value,
                 Err(_) => {
-                    failure = true;
-                    error!("Environment variable '{key}' is not set");
+                    missing.add_missing(key);
                     "".to_string()
                 }
             }
         });
 
-        if failure {
-            return Err("Missing environment variables".into());
-        }
+        missing.has_missing()
+            .map_err(|err| FailedToParseConfig::new(filename, err.into()))?;
 
-        let config: Config = toml::from_str(&file)?;
-
-        // Some extra config validation
-        if !has_wildcards(&config.presence.topic) {
-            return Err(format!("Invalid presence topic '{}', needs to contain a wildcard (+/#) in order to listen to presence devices", config.presence.topic).into());
-        }
-
-        // @TODO It would be nice it was possible to add validation to serde,
-        // that way we can check that the provided mqtt topics are actually valid
+        let config: Config = toml::from_str(&file)
+            .map_err(|err| FailedToParseConfig::new(filename, err.into()))?;
 
         Ok(config)
     }
@@ -209,48 +206,51 @@ impl Config {
 
 impl Device {
     #[async_recursion]
-    pub async fn into(self, identifier: String, config: &Config, client: AsyncClient) -> DeviceBox {
-        let device: DeviceBox = match self {
+    pub async fn create(self, identifier: String, config: &Config, client: AsyncClient) -> Result<DeviceBox, FailedToCreateDevice> {
+        let device: Result<DeviceBox, Error> = match self {
             Device::IkeaOutlet { info, mqtt, kettle } => {
                 trace!(id = identifier, "IkeaOutlet [{} in {:?}]", info.name, info.room);
-                Box::new(IkeaOutlet::new(identifier, info, mqtt, kettle, client).await)
+                match IkeaOutlet::build(&identifier, info, mqtt, kettle, client).await {
+                    Ok(device) => Ok(Box::new(device)),
+                    Err(err) => Err(err),
+                }
             },
             Device::WakeOnLAN { info, mqtt, mac_address } => {
                 trace!(id = identifier, "WakeOnLan [{} in {:?}]", info.name, info.room);
-                Box::new(WakeOnLAN::new(identifier, info, mqtt, mac_address, client).await)
+                match WakeOnLAN::build(&identifier, info, mqtt, mac_address, client).await {
+                    Ok(device) => Ok(Box::new(device)),
+                    Err(err) => Err(err),
+                }
             },
             Device::KasaOutlet { ip } => {
                 trace!(id = identifier, "KasaOutlet [{}]", identifier);
-                Box::new(KasaOutlet::new(identifier, ip))
+                Ok(Box::new(KasaOutlet::new(&identifier, ip)))
             }
             Device::AudioSetup { mqtt, mixer, speakers } => {
                 trace!(id = identifier, "AudioSetup [{}]", identifier);
                 // Create the child devices
-                let mixer = (*mixer).into(identifier.clone() + ".mixer", config, client.clone()).await;
-                let speakers = (*speakers).into(identifier.clone() + ".speakers", config, client.clone()).await;
+                let mixer = (*mixer).create(identifier.clone() + ".mixer", config, client.clone()).await?;
+                let speakers = (*speakers).create(identifier.clone() + ".speakers", config, client.clone()).await?;
 
-                // The AudioSetup expects the children to be something that implements the OnOff trait
-                // So let's convert the children and make sure OnOff is implemented
-                let mixer = match AsOnOff::consume(mixer) {
-                    Some(mixer) => mixer,
-                    None => todo!("Handle this properly"),
-                };
-                let speakers = match AsOnOff::consume(speakers) {
-                    Some(speakers) => speakers,
-                    None => todo!("Handle this properly"),
-                };
-
-                Box::new(AudioSetup::new(identifier, mqtt, mixer, speakers, client).await)
+                match AudioSetup::build(&identifier, mqtt, mixer, speakers, client).await {
+                    Ok(device) => Ok(Box::new(device)),
+                    Err(err) => Err(err),
+                }
             },
             Device::ContactSensor { mqtt, mut presence } => {
                 trace!(id = identifier, "ContactSensor [{}]", identifier);
                 if let Some(presence) = &mut presence {
-                    presence.generate_topic("contact", &identifier, &config);
+                    presence.generate_topic("contact", &identifier, &config)
+                        .map_err(|err| FailedToCreateDevice::new(&identifier, err.into()))?;
+                }
+
+                match ContactSensor::build(&identifier, mqtt, presence, client).await {
+                    Ok(device) => Ok(Box::new(device)),
+                    Err(err) => Err(err),
                 }
-                Box::new(ContactSensor::new(identifier, mqtt, presence, client).await)
             },
         };
 
-        return device;
+        return device.map_err(|err| FailedToCreateDevice::new(&identifier, err));
     }
 }