Improved code

google-home/src/device.rs

@@ -35,16 +35,16 @@ pub trait GoogleHomeDevice: AsOnOff + AsScene {
 
         let mut traits = Vec::new();
         // OnOff
-        if let Some(d) = AsOnOff::cast(self) {
+        if let Some(on_off) = AsOnOff::cast(self) {
             traits.push(Trait::OnOff);
-            device.attributes.command_only_on_off = d.is_command_only();
-            device.attributes.query_only_on_off = d.is_query_only();
+            device.attributes.command_only_on_off = on_off.is_command_only();
+            device.attributes.query_only_on_off = on_off.is_query_only();
         }
 
         // Scene
-        if let Some(d) = AsScene::cast(self) {
+        if let Some(scene) = AsScene::cast(self) {
             traits.push(Trait::Scene);
-            device.attributes.scene_reversible = d.is_scene_reversible();
+            device.attributes.scene_reversible = scene.is_scene_reversible();
         }
 
         device.traits = traits;
@@ -59,11 +59,10 @@ pub trait GoogleHomeDevice: AsOnOff + AsScene {
         }
 
         // OnOff
-        if let Some(d) = AsOnOff::cast(self) {
-            match d.is_on() {
-                Ok(state) => device.state.on = Some(state),
-                Err(err) => device.set_error(err),
-            }
+        if let Some(on_off) = AsOnOff::cast(self) {
+            device.state.on = on_off.is_on()
+                .map_err(|err| device.set_error(err))
+                .ok();
         }
 
         return device;
@@ -72,18 +71,16 @@ pub trait GoogleHomeDevice: AsOnOff + AsScene {
     fn execute(&mut self, command: &CommandType) -> Result<(), ErrorCode> {
         match command {
             CommandType::OnOff { on } => {
-                if let Some(d) = AsOnOff::cast_mut(self) {
-                    d.set_on(*on)?;
-                } else {
-                    return Err(DeviceError::ActionNotAvailable.into());
-                }
+                let on_off = AsOnOff::cast_mut(self)
+                    .ok_or::<ErrorCode>(DeviceError::ActionNotAvailable.into())?;
+
+                on_off.set_on(*on)?;
             },
             CommandType::ActivateScene { deactivate } => {
-                if let Some(d) = AsScene::cast_mut(self) {
-                    d.set_active(!deactivate)?;
-                } else {
-                    return Err(DeviceError::ActionNotAvailable.into());
-                }
+                let scene = AsScene::cast_mut(self)
+                    .ok_or::<ErrorCode>(DeviceError::ActionNotAvailable.into())?;
+
+                scene.set_active(!deactivate)?;
             },
         }
 

google-home/src/fullfillment.rs

@@ -47,16 +47,16 @@ impl GoogleHome {
             .into_iter()
             .map(|device| device.id)
             .map(|id| {
-                let mut d: query::Device;
-                if let Some(device) = devices.get(id.as_str()) {
-                    d = device.query();
-                } else {
-                    d = query::Device::new();
-                    d.set_offline();
-                    d.set_error(DeviceError::DeviceNotFound.into());
-                }
+                let device = devices.get(id.as_str())
+                    .map_or_else(|| {
+                        let mut device = query::Device::new();
+                        device.set_offline();
+                        device.set_error(DeviceError::DeviceNotFound.into());
 
-                return (id, d);
+                        device
+                    }, |device| device.query());
+
+                return (id, device);
             }).collect();
 
         return resp_payload;
@@ -79,25 +79,25 @@ impl GoogleHome {
                     .into_iter()
                     .map(|device| device.id)
                     .map(|id| {
-                        if let Some(device) = devices.get_mut(id.as_str()) {
-                            if !device.is_online() {
-                                return (id, Ok(false));
-                            }
-                            let results = command.execution.iter().map(|cmd| {
-                                // @TODO We should also return the state after update in the state
-                                // struct, however that will make things WAY more complicated
-                                device.execute(cmd)
-                            }).collect::<Result<Vec<_>, ErrorCode>>();
+                        devices.get_mut(id.as_str())
+                            .map_or((id.clone(), Err(DeviceError::DeviceNotFound.into())), |device| {
+                                if !device.is_online() {
+                                    return (id, Ok(false));
+                                }
 
-                            // @TODO We only get one error not all errors
-                            if let Err(err) = results {
-                                return (id, Err(err));
-                            } else {
-                                return (id, Ok(true));
-                            }
-                        } else {
-                            return (id, Err(DeviceError::DeviceNotFound.into()));
-                        }
+                                let results = command.execution.iter().map(|cmd| {
+                                    // @TODO We should also return the state after update in the state
+                                    // struct, however that will make things WAY more complicated
+                                    device.execute(cmd)
+                                }).collect::<Result<Vec<_>, ErrorCode>>();
+
+                                // @TODO We only get one error not all errors
+                                if let Err(err) = results {
+                                    return (id, Err(err));
+                                } else {
+                                    return (id, Ok(true));
+                                }
+                            })
                     }).for_each(|(id, state)| {
                         match state {
                             Ok(true) => success.add_id(&id),

src/config.rs

@@ -7,7 +7,7 @@ use rumqttc::{AsyncClient, has_wildcards};
 use serde::Deserialize;
 use eui48::MacAddress;
 
-use crate::{devices::{DeviceBox, IkeaOutlet, WakeOnLAN, AudioSetup, ContactSensor, KasaOutlet}, error::{FailedToParseConfig, MissingEnv, MissingWildcard, Error, FailedToCreateDevice}};
+use crate::{devices::{DeviceBox, IkeaOutlet, WakeOnLAN, AudioSetup, ContactSensor, KasaOutlet, self}, error::{FailedToParseConfig, MissingEnv, MissingWildcard, Error, FailedToCreateDevice}};
 
 #[derive(Debug, Deserialize)]
 pub struct Config {
@@ -132,7 +132,7 @@ 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) -> Result<(), MissingWildcard> {
+    fn generate_topic(mut self, class: &str, identifier: &str, config: &Config) -> Result<PresenceDeviceConfig, MissingWildcard> {
         if self.mqtt.is_none() {
             if !has_wildcards(&config.presence.topic) {
                 return Err(MissingWildcard::new(&config.presence.topic).into());
@@ -145,7 +145,7 @@ impl PresenceDeviceConfig {
             self.mqtt = Some(MqttDeviceConfig { topic });
         }
 
-        Ok(())
+        Ok(self)
     }
 }
 
@@ -213,23 +213,27 @@ impl Config {
     }
 }
 
+// Quick helper function to box up the devices,
+// passing in Box::new would be ideal, however the return type is incorrect
+// Maybe there is a better way to solve this?
+fn device_box<T: devices::Device + 'static>(device: T) -> DeviceBox {
+    let a: DeviceBox = Box::new(device);
+    a
+}
+
 impl Device {
     #[async_recursion]
     pub async fn create(self, identifier: &str, 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);
-                match IkeaOutlet::build(&identifier, info, mqtt, kettle, client).await {
-                    Ok(device) => Ok(Box::new(device)),
-                    Err(err) => Err(err),
-                }
+                IkeaOutlet::build(&identifier, info, mqtt, kettle, client).await
+                    .map(device_box)
             },
             Device::WakeOnLAN { info, mqtt, mac_address } => {
                 trace!(id = identifier, "WakeOnLan [{} in {:?}]", info.name, info.room);
-                match WakeOnLAN::build(&identifier, info, mqtt, mac_address, client).await {
-                    Ok(device) => Ok(Box::new(device)),
-                    Err(err) => Err(err),
-                }
+                WakeOnLAN::build(&identifier, info, mqtt, mac_address, client).await
+                    .map(device_box)
             },
             Device::KasaOutlet { ip } => {
                 trace!(id = identifier, "KasaOutlet [{}]", identifier);
@@ -243,22 +247,18 @@ impl Device {
                 let speakers_id = identifier.to_owned() + ".speakers";
                 let speakers = (*speakers).create(&speakers_id, config, client.clone()).await?;
 
-                match AudioSetup::build(&identifier, mqtt, mixer, speakers, client).await {
-                    Ok(device) => Ok(Box::new(device)),
-                    Err(err) => Err(err),
-                }
+                AudioSetup::build(&identifier, mqtt, mixer, speakers, client).await
+                    .map(device_box)
             },
-            Device::ContactSensor { mqtt, mut presence } => {
+            Device::ContactSensor { mqtt, presence } => {
                 trace!(id = identifier, "ContactSensor [{}]", identifier);
-                if let Some(presence) = &mut presence {
-                    presence.generate_topic("contact", &identifier, &config)
-                        .map_err(|err| FailedToCreateDevice::new(&identifier, err.into()))?;
-                }
+                let presence = presence
+                    .map(|p| p.generate_topic("contact", &identifier, &config))
+                    .transpose()
+                    .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),
-                }
+                ContactSensor::build(&identifier, mqtt, presence, client).await
+                    .map(device_box)
             },
         };
 

src/debug_bridge.rs

@@ -10,13 +10,13 @@ struct DebugBridge {
 }
 
 impl DebugBridge {
-    pub fn new(topic: String, client: AsyncClient) -> Self {
-        Self { topic, client }
+    pub fn new(topic: &str, client: AsyncClient) -> Self {
+        Self { topic: topic.to_owned(), client }
     }
 }
 
 pub fn start(mut presence_rx: presence::Receiver, mut light_sensor_rx: light_sensor::Receiver, config: DebugBridgeConfig, client: AsyncClient) {
-    let mut debug_bridge = DebugBridge::new(config.topic, client);
+    let mut debug_bridge = DebugBridge::new(&config.topic, client);
 
     tokio::spawn(async move {
         loop {

src/devices.rs

@@ -26,7 +26,7 @@ impl_cast::impl_cast!(Device, OnDarkness);
 impl_cast::impl_cast!(Device, GoogleHomeDevice);
 impl_cast::impl_cast!(Device, OnOff);
 
-pub trait Device: AsGoogleHomeDevice + AsOnMqtt + AsOnPresence + AsOnDarkness + AsOnOff + std::fmt::Debug {
+pub trait Device: AsGoogleHomeDevice + AsOnMqtt + AsOnPresence + AsOnDarkness + AsOnOff + std::fmt::Debug + Sync + Send {
     fn get_id(&self) -> &str;
 }
 
@@ -43,10 +43,8 @@ macro_rules! get_cast {
                 self.devices
                     .iter_mut()
                     .filter_map(|(id, device)| {
-                        if let Some(listener) = [< As $trait >]::cast_mut(device.as_mut()) {
-                            return Some((id.as_str(), listener));
-                        };
-                        return None;
+                        [< As $trait >]::cast_mut(device.as_mut())
+                            .map(|listener| (id.as_str(), listener))
                     }).collect()
             }
         }
@@ -66,7 +64,7 @@ enum Command {
     }
 }
 
-pub type DeviceBox = Box<dyn Device + Sync + Send>;
+pub type DeviceBox = Box<dyn Device>;
 
 #[derive(Clone)]
 pub struct DeviceHandle {

src/devices/kasa_outlet.rs

@@ -1,7 +1,7 @@
 use std::{net::{SocketAddr, Ipv4Addr, TcpStream}, io::{Write, Read}};
 
 use bytes::{Buf, BufMut};
-use google_home::{traits, errors::{ErrorCode, DeviceError}};
+use google_home::{traits, errors::{self, DeviceError}};
 use serde::{Serialize, Deserialize};
 
 use super::Device;
@@ -84,13 +84,30 @@ impl Request {
 }
 
 #[derive(Debug, Deserialize)]
-struct ResponseSetRelayState {
+struct ErrorCode {
     err_code: isize,
 }
 
+impl ErrorCode {
+    fn ok(&self) -> Result<(), anyhow::Error> {
+        if self.err_code != 0 {
+            Err(anyhow::anyhow!("Error code: {}", self.err_code))
+        } else {
+            Ok(())
+        }
+    }
+}
+
+#[derive(Debug, Deserialize)]
+struct ResponseSetRelayState {
+    #[serde(flatten)]
+    err_code: ErrorCode,
+}
+
 #[derive(Debug, Deserialize)]
 struct ResponseGetSysinfo {
-    err_code: isize,
+    #[serde(flatten)]
+    err_code: ErrorCode,
     relay_state: isize,
 }
 
@@ -108,10 +125,8 @@ struct Response {
 impl Response {
     fn get_current_relay_state(&self) -> Result<bool, anyhow::Error> {
         if let Some(sysinfo) = &self.system.get_sysinfo {
-            if sysinfo.err_code != 0 {
-                return Err(anyhow::anyhow!("Error code: {}", sysinfo.err_code));
-            }
-            return Ok(sysinfo.relay_state == 1);
+            return sysinfo.err_code.ok()
+                .map(|_| sysinfo.relay_state == 1);
         }
 
         return Err(anyhow::anyhow!("No sysinfo found in response"));
@@ -119,10 +134,7 @@ impl Response {
 
     fn check_set_relay_success(&self) -> Result<(), anyhow::Error> {
         if let Some(set_relay_state) = &self.system.set_relay_state {
-            if set_relay_state.err_code != 0 {
-                return Err(anyhow::anyhow!("Error code: {}", set_relay_state.err_code));
-            }
-            return Ok(());
+            return set_relay_state.err_code.ok();
         }
 
         return Err(anyhow::anyhow!("No relay_state found in response"));
@@ -148,7 +160,7 @@ impl Response {
 }
 
 impl traits::OnOff for KasaOutlet {
-    fn is_on(&self) -> Result<bool, ErrorCode> {
+    fn is_on(&self) -> Result<bool, errors::ErrorCode> {
         let mut stream = TcpStream::connect(self.addr).or::<DeviceError>(Err(DeviceError::DeviceOffline.into()))?;
 
         let body = Request::get_sysinfo().encrypt();
@@ -157,7 +169,7 @@ impl traits::OnOff for KasaOutlet {
         let mut received = Vec::new();
         let mut rx_bytes = [0; 1024];
         loop {
-            let read = stream.read(&mut rx_bytes).or::<ErrorCode>(Err(DeviceError::TransientError.into()))?;
+            let read = stream.read(&mut rx_bytes).or::<errors::ErrorCode>(Err(DeviceError::TransientError.into()))?;
 
             received.extend_from_slice(&rx_bytes[..read]);
 
@@ -166,12 +178,12 @@ impl traits::OnOff for KasaOutlet {
             }
         }
 
-        let resp = Response::decrypt(received.into()).or::<ErrorCode>(Err(DeviceError::TransientError.into()))?;
+        let resp = Response::decrypt(received.into()).or::<errors::ErrorCode>(Err(DeviceError::TransientError.into()))?;
 
         resp.get_current_relay_state().or(Err(DeviceError::TransientError.into()))
     }
 
-    fn set_on(&mut self, on: bool) -> Result<(), ErrorCode> {
+    fn set_on(&mut self, on: bool) -> Result<(), errors::ErrorCode> {
         let mut stream = TcpStream::connect(self.addr).or::<DeviceError>(Err(DeviceError::DeviceOffline.into()))?;
 
         let body = Request::set_relay_state(on).encrypt();
@@ -192,7 +204,7 @@ impl traits::OnOff for KasaOutlet {
             }
         }
 
-        let resp = Response::decrypt(received.into()).or::<ErrorCode>(Err(DeviceError::TransientError.into()))?;
+        let resp = Response::decrypt(received.into()).or::<errors::ErrorCode>(Err(DeviceError::TransientError.into()))?;
 
         resp.check_set_relay_success().or(Err(DeviceError::TransientError.into()))
     }

src/main.rs

@@ -91,18 +91,18 @@ async fn app() -> Result<(), Box<dyn std::error::Error>> {
     ).await.into_iter().collect::<Result<_, _>>()?;
 
     // Start the ntfy service if it is configured
-    if let Some(ntfy_config) = config.ntfy {
-        ntfy::start(presence.clone(), &ntfy_config);
+    if let Some(config) = config.ntfy {
+        ntfy::start(presence.clone(), config);
     }
 
     // Start the hue bridge if it is configured
-    if let Some(hue_bridge_config) = config.hue_bridge {
-        hue_bridge::start(presence.clone(), light_sensor.clone(), hue_bridge_config);
+    if let Some(config) = config.hue_bridge {
+        hue_bridge::start(presence.clone(), light_sensor.clone(), config);
     }
 
     // Start the debug bridge if it is configured
-    if let Some(debug_bridge_config) = config.debug_bridge {
-        debug_bridge::start(presence.clone(), light_sensor.clone(), debug_bridge_config, client.clone());
+    if let Some(config) = config.debug_bridge {
+        debug_bridge::start(presence.clone(), light_sensor.clone(), config, client.clone());
     }
 
     // Actually start listening for mqtt message,

src/ntfy.rs

@@ -93,7 +93,7 @@ impl Ntfy {
     }
 }
 
-pub fn start(mut rx: presence::Receiver, config: &NtfyConfig) {
+pub fn start(mut rx: presence::Receiver, config: NtfyConfig) {
     let mut ntfy = Ntfy::new(&config.url, &config.topic);
 
     tokio::spawn(async move {