From 044c38ba8631af7c1d8edb858c21672f578388f1 Mon Sep 17 00:00:00 2001 From: Dreaded_X Date: Fri, 18 Aug 2023 03:07:16 +0200 Subject: [PATCH] More refactoring --- Cargo.lock | 5 +- google-home/Cargo.toml | 1 + google-home/src/fullfillment.rs | 21 ++-- google-home/src/request/execute.rs | 2 +- google-home/src/request/query.rs | 2 +- google-home/src/request/sync.rs | 2 +- google-home/src/response.rs | 2 +- google-home/src/response/sync.rs | 8 +- src/config.rs | 6 +- src/device_manager.rs | 2 +- src/devices/contact_sensor.rs | 2 +- src/devices/debug_bridge.rs | 2 +- src/devices/hue_light.rs | 192 ++++++++++++++++++++--------- src/devices/ikea_outlet.rs | 15 ++- src/devices/ntfy.rs | 10 +- src/devices/presence.rs | 2 +- src/error.rs | 7 +- src/main.rs | 2 +- src/messages.rs | 26 ++++ src/traits.rs | 5 +- 20 files changed, 209 insertions(+), 105 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 29b81cd..ba99912 100644 --- a/Cargo.lock +++ b/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", diff --git a/google-home/Cargo.toml b/google-home/Cargo.toml index e36e6b4..a858d19 100644 --- a/google-home/Cargo.toml +++ b/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" diff --git a/google-home/src/fullfillment.rs b/google-home/src/fullfillment.rs index 7a65210..6beb324 100644 --- a/google-home/src/fullfillment.rs +++ b/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::>::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(); diff --git a/google-home/src/request/execute.rs b/google-home/src/request/execute.rs index ce95ef6..d76c969 100644 --- a/google-home/src/request/execute.rs +++ b/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] { diff --git a/google-home/src/request/query.rs b/google-home/src/request/query.rs index bb54d0b..2703490 100644 --- a/google-home/src/request/query.rs +++ b/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] { diff --git a/google-home/src/request/sync.rs b/google-home/src/request/sync.rs index 8e8f0f9..5d4033d 100644 --- a/google-home/src/request/sync.rs +++ b/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] { diff --git a/google-home/src/response.rs b/google-home/src/response.rs index 627b91c..191b07b 100644 --- a/google-home/src/response.rs +++ b/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, } } diff --git a/google-home/src/response/sync.rs b/google-home/src/response/sync.rs index 7697061..3b8efc1 100644 --- a/google-home/src/response/sync.rs +++ b/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()), - model: Some("hs1234".to_string()), - hw_version: Some("3.2".to_string()), - sw_version: Some("11.4".to_string()), + manufacturer: Some("lights-out-inc".into()), + model: Some("hs1234".into()), + hw_version: Some("3.2".into()), + sw_version: Some("11.4".into()), }); sync_resp.add_device(device); diff --git a/src/config.rs b/src/config.rs index 3beeb64..404e380 100644 --- a/src/config.rs +++ b/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() } } }); diff --git a/src/device_manager.rs b/src/device_manager.rs index eeb4441..ed4a89a 100644 --- a/src/device_manager.rs +++ b/src/device_manager.rs @@ -91,7 +91,7 @@ impl DeviceManager { } pub async fn add(&self, device: Box) { - let id = device.get_id().to_owned(); + let id = device.get_id().into(); debug!(id, "Adding device"); diff --git a/src/devices/contact_sensor.rs b/src/devices/contact_sensor.rs index a78735a..65fa3d1 100644 --- a/src/devices/contact_sensor.rs +++ b/src/devices/contact_sensor.rs @@ -178,7 +178,7 @@ impl OnMqtt for ContactSensor { if trigger.timeout.is_zero() && let Some(light) = As::::cast_mut(light.as_mut()) { light.set_on(false).await.ok(); } else if let Some(light) = As::::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 } diff --git a/src/devices/debug_bridge.rs b/src/devices/debug_bridge.rs index 71a03c5..c23a370 100644 --- a/src/devices/debug_bridge.rs +++ b/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| { diff --git a/src/devices/hue_light.rs b/src/devices/hue_light.rs index 153b78c..18fe16b 100644 --- a/src/devices/hue_light.rs +++ b/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::{debug, error, warn}; +use tracing::{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; - - let url = format!( - "http://{}/api/{}/lights/{}/state", - self.addr, self.login, self.light_id - ); + self.stop_timeout().await.unwrap(); + let message = message::State::new(on); let res = reqwest::Client::new() - .put(url) - .body(format!(r#"{{"on": {}}}"#, on)) + .put(self.url_set_state()) + .json(&message) .send() .await; @@ -90,12 +105,10 @@ impl OnOff for HueLight { } async fn is_on(&self) -> Result { - let url = format!( - "http://{}/api/{}/lights/{}", - self.addr, self.login, self.light_id - ); - - let res = reqwest::Client::new().get(url).send().await; + let res = reqwest::Client::new() + .get(self.url_get_state()) + .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(); - // TODO: This is not very nice - return Ok(v["state"]["on"].as_bool().unwrap()); + let on = match res.json::().await { + Ok(info) => info.is_on(), + 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; - - let url = format!( - "http://{}/api/{}/schedules/{}", - self.addr, self.login, self.timer_id - ); - - 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}"); - - debug!(id = self.identifier, "Starting timeout ({time})..."); + self.stop_timeout().await?; + let message = message::Timeout::new(Some(timeout)); let res = reqwest::Client::new() - .put(url) - .body(format!(r#"{{"status": "enabled", "localtime": "{time}"}}"#)) + .put(self.url_set_schedule()) + .json(&message) .send() - .await; + .await + .context("Failed to start timeout")?; - 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}"), + 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 } } } - async fn stop_timeout(&mut self) { - let url = format!( - "http://{}/api/{}/schedules/{}", - self.addr, self.login, self.timer_id - ); + #[derive(Debug, Serialize, Deserialize)] + pub struct Info { + state: State, + } - let res = reqwest::Client::new() - .put(url) - .body(format!(r#"{{"status": "disabled"}}"#)) - .send() - .await; + impl Info { + pub fn is_on(&self) -> bool { + self.state.on + } + } - match res { - Ok(res) => { - let status = res.status(); - if !status.is_success() { - warn!(id = self.identifier, "Status code is not success: {status}"); - } + #[derive(Debug)] + pub struct Timeout { + timeout: Option, + } + + impl Timeout { + pub fn new(timeout: Option) -> Self { + Self { timeout } + } + } + + impl Serialize for Timeout { + fn serialize(&self, serializer: S) -> Result + 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")?; } - Err(err) => error!(id = self.identifier, "Error: {err}"), + + 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)?; + }; + + state.end() } } } diff --git a/src/devices/ikea_outlet.rs b/src/devices/ikea_outlet.rs index 832f59e..e7cd471 100644 --- a/src/devices/ikea_outlet.rs +++ b/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(()) } } diff --git a/src/devices/ntfy.rs b/src/devices/ntfy.rs index 5eba3da..b3b0f76 100644 --- a/src/devices/ntfy.rs +++ b/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), }; diff --git a/src/devices/presence.rs b/src/devices/presence.rs index 01a8487..e1ec4a8 100644 --- a/src/devices/presence.rs +++ b/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 diff --git a/src/error.rs b/src/error.rs index 95b1dcb..6528be2 100644 --- a/src/error.rs +++ b/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::(&self.into()).unwrap(), + serde_json::to_string::(&self.into()) + .expect("Serialization should not fail"), ) .into_response() } diff --git a/src/main.rs b/src/main.rs index 31b8ecb..681d918 100644 --- a/src/main.rs +++ b/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 diff --git a/src/messages.rs b/src/messages.rs index f190f3b..1ab36fb 100644 --- a/src/messages.rs +++ b/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 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 for HueMessage { + type Error = ParseError; + + fn try_from(bytes: Bytes) -> Result { + serde_json::from_slice(&bytes).or(Err(ParseError::InvalidPayload(bytes.clone()))) + } +} diff --git a/src/traits.rs b/src/traits.rs index bd55a97..a2e5325 100644 --- a/src/traits.rs +++ b/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 stop_timeout(&mut self); + async fn start_timeout(&mut self, _timeout: Duration) -> Result<()>; + async fn stop_timeout(&mut self) -> Result<()>; }