From b6bf8a82a22c190f4740d2a431c209737d146d7f Mon Sep 17 00:00:00 2001 From: Dreaded_X Date: Wed, 18 Jan 2023 20:03:33 +0100 Subject: [PATCH] Improved code --- google-home/src/device.rs | 37 +++++++++++----------- google-home/src/fullfillment.rs | 54 ++++++++++++++++----------------- src/config.rs | 48 ++++++++++++++--------------- src/debug_bridge.rs | 6 ++-- src/devices.rs | 10 +++--- src/devices/kasa_outlet.rs | 44 +++++++++++++++++---------- src/main.rs | 12 ++++---- src/ntfy.rs | 2 +- 8 files changed, 110 insertions(+), 103 deletions(-) diff --git a/google-home/src/device.rs b/google-home/src/device.rs index 2db542e..1841c41 100644 --- a/google-home/src/device.rs +++ b/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::(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::(DeviceError::ActionNotAvailable.into())?; + + scene.set_active(!deactivate)?; }, } diff --git a/google-home/src/fullfillment.rs b/google-home/src/fullfillment.rs index f9c77c0..bb525b6 100644 --- a/google-home/src/fullfillment.rs +++ b/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::, 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::, 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), diff --git a/src/config.rs b/src/config.rs index 5713806..674017d 100644 --- a/src/config.rs +++ b/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 { 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(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 { let device: Result = 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) }, }; diff --git a/src/debug_bridge.rs b/src/debug_bridge.rs index 38257d5..1cb210d 100644 --- a/src/debug_bridge.rs +++ b/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 { diff --git a/src/devices.rs b/src/devices.rs index d5dbbdb..92eb511 100644 --- a/src/devices.rs +++ b/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; +pub type DeviceBox = Box; #[derive(Clone)] pub struct DeviceHandle { diff --git a/src/devices/kasa_outlet.rs b/src/devices/kasa_outlet.rs index 08389ec..5e847e4 100644 --- a/src/devices/kasa_outlet.rs +++ b/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 { 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 { + fn is_on(&self) -> Result { let mut stream = TcpStream::connect(self.addr).or::(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::(Err(DeviceError::TransientError.into()))?; + let read = stream.read(&mut rx_bytes).or::(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::(Err(DeviceError::TransientError.into()))?; + let resp = Response::decrypt(received.into()).or::(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::(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::(Err(DeviceError::TransientError.into()))?; + let resp = Response::decrypt(received.into()).or::(Err(DeviceError::TransientError.into()))?; resp.check_set_relay_success().or(Err(DeviceError::TransientError.into())) } diff --git a/src/main.rs b/src/main.rs index 2300026..261bb0f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -91,18 +91,18 @@ async fn app() -> Result<(), Box> { ).await.into_iter().collect::>()?; // 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, diff --git a/src/ntfy.rs b/src/ntfy.rs index 6d06293..1c84204 100644 --- a/src/ntfy.rs +++ b/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 {