More refactoring

Cargo.lock

@@ -43,9 +43,9 @@ dependencies = [
 
 [[package]]
 name = "anyhow"
-version = "1.0.72"
+version = "1.0.75"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3b13c32d80ecc7ab747b80c3784bce54ee8a7a0cc4fbda9bf4cda2cf6fe90854"
+checksum = "a4668cab20f66d8d020e1fbc0ebe47217433c1b6c8f2040faf858554e394ace6"
 
 [[package]]
 name = "arrayvec"
@@ -554,6 +554,7 @@ checksum = "b6c80984affa11d98d1b88b66ac8853f143217b399d3c74116778ff8fdb4ed2e"
 name = "google-home"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "async-trait",
  "futures",
  "impl_cast",

google-home/Cargo.toml

@@ -13,3 +13,4 @@ thiserror = "1.0.37"
 tokio = { version = "1", features = ["sync"] }
 async-trait = "0.1.61"
 futures = "0.3.25"
+anyhow = "1.0.75"

google-home/src/fullfillment.rs

@@ -195,9 +195,8 @@ impl GoogleHome {
 
         join_all(f).await;
 
-        // We await all the futures that use resp_payload so try_unwrap should never fail
         std::sync::Arc::<tokio::sync::Mutex<response::execute::Payload>>::try_unwrap(resp_payload)
-            .unwrap()
+            .expect("All futures are done, so there should only be one strong reference")
             .into_inner()
     }
 }
@@ -331,11 +330,11 @@ impl GoogleHome {
 //         let mut lamp = TestOutlet::new("living/lamp");
 //         let mut scene = TestScene::new();
 //         let mut devices: HashMap<&str, &mut dyn GoogleHomeDevice> = HashMap::new();
-//         let id = nightstand.get_id().to_owned();
+//         let id = nightstand.get_id().into();
 //         devices.insert(&id, &mut nightstand);
-//         let id = lamp.get_id().to_owned();
+//         let id = lamp.get_id().into();
 //         devices.insert(&id, &mut lamp);
-//         let id = scene.get_id().to_owned();
+//         let id = scene.get_id().into();
 //         devices.insert(&id, &mut scene);
 //
 //         let resp = gh.handle_request(req, &mut devices).unwrap();
@@ -374,11 +373,11 @@ impl GoogleHome {
 //         let mut lamp = TestOutlet::new("living/lamp");
 //         let mut scene = TestScene::new();
 //         let mut devices: HashMap<&str, &mut dyn GoogleHomeDevice> = HashMap::new();
-//         let id = nightstand.get_id().to_owned();
+//         let id = nightstand.get_id().into();
 //         devices.insert(&id, &mut nightstand);
-//         let id = lamp.get_id().to_owned();
+//         let id = lamp.get_id().into();
 //         devices.insert(&id, &mut lamp);
-//         let id = scene.get_id().to_owned();
+//         let id = scene.get_id().into();
 //         devices.insert(&id, &mut scene);
 //
 //         let resp = gh.handle_request(req, &mut devices).unwrap();
@@ -429,11 +428,11 @@ impl GoogleHome {
 //         let mut lamp = TestOutlet::new("living/lamp");
 //         let mut scene = TestScene::new();
 //         let mut devices: HashMap<&str, &mut dyn GoogleHomeDevice> = HashMap::new();
-//         let id = nightstand.get_id().to_owned();
+//         let id = nightstand.get_id().into();
 //         devices.insert(&id, &mut nightstand);
-//         let id = lamp.get_id().to_owned();
+//         let id = lamp.get_id().into();
 //         devices.insert(&id, &mut lamp);
-//         let id = scene.get_id().to_owned();
+//         let id = scene.get_id().into();
 //         devices.insert(&id, &mut scene);
 //
 //         let resp = gh.handle_request(req, &mut devices).unwrap();

google-home/src/request/execute.rs

@@ -83,7 +83,7 @@ mod tests {
 
         assert_eq!(
             req.request_id,
-            "ff36a3cc-ec34-11e6-b1a0-64510650abcf".to_owned()
+            "ff36a3cc-ec34-11e6-b1a0-64510650abcf".to_string()
         );
         assert_eq!(req.inputs.len(), 1);
         match &req.inputs[0] {

google-home/src/request/query.rs

@@ -54,7 +54,7 @@ mod tests {
 
         assert_eq!(
             req.request_id,
-            "ff36a3cc-ec34-11e6-b1a0-64510650abcf".to_owned()
+            "ff36a3cc-ec34-11e6-b1a0-64510650abcf".to_string()
         );
         assert_eq!(req.inputs.len(), 1);
         match &req.inputs[0] {

google-home/src/request/sync.rs

@@ -19,7 +19,7 @@ mod tests {
 
         assert_eq!(
             req.request_id,
-            "ff36a3cc-ec34-11e6-b1a0-64510650abcf".to_owned()
+            "ff36a3cc-ec34-11e6-b1a0-64510650abcf".to_string()
         );
         assert_eq!(req.inputs.len(), 1);
         match req.inputs[0] {

google-home/src/response.rs

@@ -14,7 +14,7 @@ pub struct Response {
 impl Response {
     pub fn new(request_id: &str, payload: ResponsePayload) -> Self {
         Self {
-            request_id: request_id.to_owned(),
+            request_id: request_id.into(),
             payload,
         }
     }

google-home/src/response/sync.rs

@@ -86,10 +86,10 @@ mod tests {
 
         device.room_hint = Some("kitchen".into());
         device.device_info = Some(device::Info {
-            manufacturer: Some("lights-out-inc".to_string()),
+            manufacturer: Some("lights-out-inc".into()),
-            model: Some("hs1234".to_string()),
+            model: Some("hs1234".into()),
-            hw_version: Some("3.2".to_string()),
+            hw_version: Some("3.2".into()),
-            sw_version: Some("11.4".to_string()),
+            sw_version: Some("11.4".into()),
         });
 
         sync_resp.add_device(device);

src/config.rs

@@ -122,16 +122,16 @@ impl Config {
         let file = fs::read_to_string(filename)?;
 
         // Substitute in environment variables
-        let re = Regex::new(r"\$\{(.*)\}").unwrap();
+        let re = Regex::new(r"\$\{(.*)\}").expect("Regex should be valid");
         let mut missing = MissingEnv::new();
         let file = re.replace_all(&file, |caps: &Captures| {
-            let key = caps.get(1).unwrap().as_str();
+            let key = caps.get(1).expect("Capture group should exist").as_str();
             debug!("Substituting '{key}' in config");
             match std::env::var(key) {
                 Ok(value) => value,
                 Err(_) => {
                     missing.add_missing(key);
-                    "".to_string()
+                    "".into()
                 }
             }
         });

src/device_manager.rs

@@ -91,7 +91,7 @@ impl DeviceManager {
     }
 
     pub async fn add(&self, device: Box<dyn Device>) {
-        let id = device.get_id().to_owned();
+        let id = device.get_id().into();
 
         debug!(id, "Adding device");
 

src/devices/contact_sensor.rs

@@ -178,7 +178,7 @@ impl OnMqtt for ContactSensor {
                         if trigger.timeout.is_zero() && let Some(light) = As::<dyn OnOff>::cast_mut(light.as_mut()) {
                             light.set_on(false).await.ok();
                         } else if let Some(light) = As::<dyn Timeout>::cast_mut(light.as_mut()) {
-                            light.start_timeout(trigger.timeout).await;
+                            light.start_timeout(trigger.timeout).await.unwrap();
                         }
                         // TODO: Put a warning/error on creation if either of this has to option to fail
                     }

src/devices/debug_bridge.rs

@@ -60,7 +60,7 @@ impl OnPresence for DebugBridge {
                 topic,
                 rumqttc::QoS::AtLeastOnce,
                 true,
-                serde_json::to_string(&message).unwrap(),
+                serde_json::to_string(&message).expect("Serialization should not fail"),
             )
             .await
             .map_err(|err| {

src/devices/hue_light.rs

@@ -3,11 +3,11 @@ use std::{
     time::Duration,
 };
 
+use anyhow::{anyhow, Context, Result};
 use async_trait::async_trait;
 use google_home::{errors::ErrorCode, traits::OnOff};
 use serde::Deserialize;
-use serde_json::Value;
+use tracing::{error, warn};
-use tracing::{debug, error, warn};
 
 use crate::{
     device_manager::{ConfigExternal, DeviceConfig},
@@ -53,6 +53,25 @@ struct HueLight {
     pub timer_id: isize,
 }
 
+// Couple of helper function to get the correct urls
+impl HueLight {
+    fn url_base(&self) -> String {
+        format!("http://{}/api/{}", self.addr, self.login)
+    }
+
+    fn url_set_schedule(&self) -> String {
+        format!("{}/schedules/{}", self.url_base(), self.timer_id)
+    }
+
+    fn url_set_state(&self) -> String {
+        format!("{}/lights/{}/state", self.url_base(), self.light_id)
+    }
+
+    fn url_get_state(&self) -> String {
+        format!("{}/lights/{}", self.url_base(), self.light_id)
+    }
+}
+
 impl Device for HueLight {
     fn get_id(&self) -> &str {
         &self.identifier
@@ -63,16 +82,12 @@ impl Device for HueLight {
 impl OnOff for HueLight {
     async fn set_on(&mut self, on: bool) -> Result<(), ErrorCode> {
         // Abort any timer that is currently running
-        self.stop_timeout().await;
+        self.stop_timeout().await.unwrap();
-
-        let url = format!(
-            "http://{}/api/{}/lights/{}/state",
-            self.addr, self.login, self.light_id
-        );
 
+        let message = message::State::new(on);
         let res = reqwest::Client::new()
-            .put(url)
+            .put(self.url_set_state())
-            .body(format!(r#"{{"on": {}}}"#, on))
+            .json(&message)
             .send()
             .await;
 
@@ -90,12 +105,10 @@ impl OnOff for HueLight {
     }
 
     async fn is_on(&self) -> Result<bool, ErrorCode> {
-        let url = format!(
+        let res = reqwest::Client::new()
-            "http://{}/api/{}/lights/{}",
+            .get(self.url_get_state())
-            self.addr, self.login, self.light_id
+            .send()
-        );
+            .await;
-
-        let res = reqwest::Client::new().get(url).send().await;
 
         match res {
             Ok(res) => {
@@ -104,9 +117,16 @@ impl OnOff for HueLight {
                     warn!(id = self.identifier, "Status code is not success: {status}");
                 }
 
-                let v: Value = serde_json::from_slice(res.bytes().await.unwrap().as_ref()).unwrap();
+                let on = match res.json::<message::Info>().await {
-                // TODO: This is not very nice
+                    Ok(info) => info.is_on(),
-                return Ok(v["state"]["on"].as_bool().unwrap());
+                    Err(err) => {
+                        error!(id = self.identifier, "Failed to parse message: {err}");
+                        // TODO: Error code
+                        return Ok(false);
+                    }
+                };
+
+                return Ok(on);
             }
             Err(err) => error!(id = self.identifier, "Error: {err}"),
         }
@@ -117,59 +137,109 @@ impl OnOff for HueLight {
 
 #[async_trait]
 impl Timeout for HueLight {
-    async fn start_timeout(&mut self, timeout: Duration) {
+    async fn start_timeout(&mut self, timeout: Duration) -> Result<()> {
         // Abort any timer that is currently running
-        self.stop_timeout().await;
+        self.stop_timeout().await?;
 
-        let url = format!(
+        let message = message::Timeout::new(Some(timeout));
-            "http://{}/api/{}/schedules/{}",
+        let res = reqwest::Client::new()
-            self.addr, self.login, self.timer_id
+            .put(self.url_set_schedule())
-        );
+            .json(&message)
+            .send()
+            .await
+            .context("Failed to start timeout")?;
 
+        let status = res.status();
+        if !status.is_success() {
+            return Err(anyhow!(
+                "Hue bridge returned unsuccessful status '{status}'"
+            ));
+        }
+
+        Ok(())
+    }
+
+    async fn stop_timeout(&mut self) -> Result<()> {
+        let message = message::Timeout::new(None);
+        let res = reqwest::Client::new()
+            .put(self.url_set_schedule())
+            .json(&message)
+            .send()
+            .await
+            .context("Failed to stop timeout")?;
+
+        let status = res.status();
+        if !status.is_success() {
+            return Err(anyhow!(
+                "Hue bridge returned unsuccessful status '{status}'"
+            ));
+        }
+
+        Ok(())
+    }
+}
+
+mod message {
+    use std::time::Duration;
+
+    use serde::{ser::SerializeStruct, Deserialize, Serialize};
+
+    #[derive(Debug, Serialize, Deserialize)]
+    pub struct State {
+        on: bool,
+    }
+
+    impl State {
+        pub fn new(on: bool) -> Self {
+            Self { on }
+        }
+    }
+
+    #[derive(Debug, Serialize, Deserialize)]
+    pub struct Info {
+        state: State,
+    }
+
+    impl Info {
+        pub fn is_on(&self) -> bool {
+            self.state.on
+        }
+    }
+
+    #[derive(Debug)]
+    pub struct Timeout {
+        timeout: Option<Duration>,
+    }
+
+    impl Timeout {
+        pub fn new(timeout: Option<Duration>) -> Self {
+            Self { timeout }
+        }
+    }
+
+    impl Serialize for Timeout {
+        fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+        where
+            S: serde::Serializer,
+        {
+            let len = if self.timeout.is_some() { 2 } else { 1 };
+            let mut state = serializer.serialize_struct("TimerMessage", len)?;
+            if self.timeout.is_some() {
+                state.serialize_field("status", "enabled")?;
+            } else {
+                state.serialize_field("status", "disabled")?;
+            }
+
+            if let Some(timeout) = self.timeout {
                 let seconds = timeout.as_secs() % 60;
                 let minutes = (timeout.as_secs() / 60) % 60;
                 let hours = timeout.as_secs() / 3600;
+
                 let time = format!("PT{hours:<02}:{minutes:<02}:{seconds:<02}");
+                state.serialize_field("localtime", &time)?;
+            };
 
-        debug!(id = self.identifier, "Starting timeout ({time})...");
+            state.end()
-
-        let res = reqwest::Client::new()
-            .put(url)
-            .body(format!(r#"{{"status": "enabled", "localtime": "{time}"}}"#))
-            .send()
-            .await;
-
-        match res {
-            Ok(res) => {
-                let status = res.status();
-                if !status.is_success() {
-                    warn!(id = self.identifier, "Status code is not success: {status}");
-                }
-            }
-            Err(err) => error!(id = self.identifier, "Error: {err}"),
-        }
-    }
-
-    async fn stop_timeout(&mut self) {
-        let url = format!(
-            "http://{}/api/{}/schedules/{}",
-            self.addr, self.login, self.timer_id
-        );
-
-        let res = reqwest::Client::new()
-            .put(url)
-            .body(format!(r#"{{"status": "disabled"}}"#))
-            .send()
-            .await;
-
-        match res {
-            Ok(res) => {
-                let status = res.status();
-                if !status.is_success() {
-                    warn!(id = self.identifier, "Status code is not success: {status}");
-                }
-            }
-            Err(err) => error!(id = self.identifier, "Error: {err}"),
         }
     }
 }

src/devices/ikea_outlet.rs

@@ -1,3 +1,4 @@
+use anyhow::Result;
 use async_trait::async_trait;
 use google_home::errors::ErrorCode;
 use google_home::{
@@ -135,14 +136,14 @@ impl OnMqtt for IkeaOutlet {
         }
 
         // Abort any timer that is currently running
-        self.stop_timeout().await;
+        self.stop_timeout().await.unwrap();
 
         debug!(id = self.identifier, "Updating state to {state}");
         self.last_known_state = state;
 
         // If this is a kettle start a timeout for turning it of again
         if state && let Some(timeout) = self.timeout {
-			self.start_timeout(timeout).await;
+			self.start_timeout(timeout).await.unwrap();
         }
     }
 }
@@ -205,9 +206,9 @@ impl traits::OnOff for IkeaOutlet {
 
 #[async_trait]
 impl crate::traits::Timeout for IkeaOutlet {
-    async fn start_timeout(&mut self, timeout: Duration) {
+    async fn start_timeout(&mut self, timeout: Duration) -> Result<()> {
         // Abort any timer that is currently running
-        self.stop_timeout().await;
+        self.stop_timeout().await?;
 
         // Turn the kettle of after the specified timeout
         // TODO: Impl Drop for IkeaOutlet that will abort the handle if the IkeaOutlet
@@ -224,11 +225,15 @@ impl crate::traits::Timeout for IkeaOutlet {
             // I don't think we can really get around calling outside function
             set_on(client, &topic, false).await;
         }));
+
+        Ok(())
     }
 
-    async fn stop_timeout(&mut self) {
+    async fn stop_timeout(&mut self) -> Result<()> {
         if let Some(handle) = self.handle.take() {
             handle.abort();
         }
+
+        Ok(())
     }
 }

src/devices/ntfy.rs

@@ -81,17 +81,17 @@ impl Notification {
     }
 
     pub fn set_title(mut self, title: &str) -> Self {
-        self.title = Some(title.to_owned());
+        self.title = Some(title.into());
         self
     }
 
     pub fn set_message(mut self, message: &str) -> Self {
-        self.message = Some(message.to_owned());
+        self.message = Some(message.into());
         self
     }
 
     pub fn add_tag(mut self, tag: &str) -> Self {
-        self.tags.push(tag.to_owned());
+        self.tags.push(tag.into());
         self
     }
 
@@ -107,7 +107,7 @@ impl Notification {
 
     fn finalize(self, topic: &str) -> NotificationFinal {
         NotificationFinal {
-            topic: topic.to_owned(),
+            topic: topic.into(),
             inner: self,
         }
     }
@@ -168,7 +168,7 @@ impl OnPresence for Ntfy {
         // Create broadcast action
         let action = Action {
             action: ActionType::Broadcast { extras },
-            label: if presence { "Set away" } else { "Set home" }.to_owned(),
+            label: if presence { "Set away" } else { "Set home" }.into(),
             clear: Some(true),
         };
 

src/devices/presence.rs

@@ -59,7 +59,7 @@ impl OnMqtt for Presence {
             .find('+')
             .or(self.mqtt.topic.find('#'))
             .expect("Presence::create fails if it does not contain wildcards");
-        let device_name = message.topic[offset..].to_owned();
+        let device_name = message.topic[offset..].into();
 
         if message.payload.is_empty() {
             // Remove the device from the map

src/error.rs

@@ -18,7 +18,7 @@ impl MissingEnv {
     }
 
     pub fn add_missing(&mut self, key: &str) {
-        self.keys.push(key.to_owned());
+        self.keys.push(key.into());
     }
 
     pub fn has_missing(self) -> result::Result<(), Self> {
@@ -84,7 +84,7 @@ pub struct MissingWildcard {
 impl MissingWildcard {
     pub fn new(topic: &str) -> Self {
         Self {
-            topic: topic.to_owned(),
+            topic: topic.into(),
         }
     }
 }
@@ -145,7 +145,8 @@ impl IntoResponse for ApiError {
     fn into_response(self) -> axum::response::Response {
         (
             self.status_code,
-            serde_json::to_string::<ApiErrorJson>(&self.into()).unwrap(),
+            serde_json::to_string::<ApiErrorJson>(&self.into())
+                .expect("Serialization should not fail"),
         )
             .into_response()
     }

src/main.rs

@@ -50,7 +50,7 @@ async fn app() -> anyhow::Result<()> {
     info!("Starting automation_rs...");
 
     let config_filename =
-        std::env::var("AUTOMATION_CONFIG").unwrap_or("./config/config.toml".to_owned());
+        std::env::var("AUTOMATION_CONFIG").unwrap_or("./config/config.toml".into());
     let config = Config::parse_file(&config_filename)?;
 
     // Create a mqtt client

src/messages.rs

@@ -1,5 +1,6 @@
 use std::time::{SystemTime, UNIX_EPOCH};
 
+use bytes::Bytes;
 use rumqttc::Publish;
 use serde::{Deserialize, Serialize};
 
@@ -215,3 +216,28 @@ impl TryFrom<Publish> for PowerMessage {
             .or(Err(ParseError::InvalidPayload(message.payload.clone())))
     }
 }
+
+// Message used to report the power state of a hue light
+#[derive(Debug, Deserialize)]
+pub struct HueState {
+    on: bool,
+}
+
+#[derive(Debug, Deserialize)]
+pub struct HueMessage {
+    state: HueState,
+}
+
+impl HueMessage {
+    pub fn is_on(&self) -> bool {
+        self.state.on
+    }
+}
+
+impl TryFrom<Bytes> for HueMessage {
+    type Error = ParseError;
+
+    fn try_from(bytes: Bytes) -> Result<Self, Self::Error> {
+        serde_json::from_slice(&bytes).or(Err(ParseError::InvalidPayload(bytes.clone())))
+    }
+}

src/traits.rs

@@ -1,11 +1,12 @@
 use std::time::Duration;
 
+use anyhow::Result;
 use async_trait::async_trait;
 use impl_cast::device_trait;
 
 #[async_trait]
 #[device_trait]
 pub trait Timeout {
-    async fn start_timeout(&mut self, _timeout: Duration);
+    async fn start_timeout(&mut self, _timeout: Duration) -> Result<()>;
-    async fn stop_timeout(&mut self);
+    async fn stop_timeout(&mut self) -> Result<()>;
 }