make the characteristic read & update callbacks fallible

- return `Result`s from `OnReadFn`, `OnUpdateFn`, `OnReadFuture` and `OnUpdateFuture` to be able to reflect errors getting and setting values from and to the real world to the requesting crontroller - update `tokio` dependency and remove unused feature flags
ewilken · Jul 12, 2021 · a368225 · a368225
1 parent 6515dda
commit a368225
Show file tree

Hide file tree

Showing 8 changed files with 56 additions and 21 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "hap"
-version = "0.1.0-pre.9"
+version = "0.1.0-pre.10"
 authors = ["Elias Wilken <[email protected]>"]
 edition = "2018"
 description = "Rust implementation of the Apple HomeKit Accessory Protocol (HAP)"
@@ -43,7 +43,7 @@ sha2 = "0.9"
 signature = "1.1"
 srp = "0.5"
 thiserror = "1.0"
-tokio = { version = "1.7", features = ["rt", "net", "time", "macros"] }
+tokio = "1.8"
 url = "2.1"
 uuid = { version = "0.8", features = ["v4", "serde"] }
 x25519-dalek = "0.6"
@@ -56,7 +56,7 @@ uuid = { version = "0.8", features = ["v4", "serde"] }
 
 [dev-dependencies]
 env_logger = "0.8"
-tokio = { version = "1.7", features = ["rt-multi-thread", "time"] }
+tokio = { version = "1.8", features = ["rt-multi-thread", "time", "macros"] }
 
 [workspace]
 members = ["codegen"]
diff --git a/codegen/Cargo.toml b/codegen/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "hap-codegen"
-version = "0.1.0-pre.9"
+version = "0.1.0-pre.10"
 authors = ["Elias Wilken <[email protected]>"]
 edition = "2018"
 description = "Rust implementation of the Apple HomeKit Accessory Protocol (HAP)"

diff --git a/examples/async_callbacks.rs b/examples/async_callbacks.rs
@@ -21,7 +21,7 @@ async fn main() -> Result<()> {
     lightbulb.lightbulb.power_state.on_read_async(Some(|| {
         async {
             println!("power_state characteristic read");
-            None
+            Ok(None)
         }
         .boxed()
     }));
@@ -31,6 +31,7 @@ async fn main() -> Result<()> {
         .on_update_async(Some(|current_val: bool, new_val: bool| {
             async move {
                 println!("power_state characteristic updated from {} to {}", current_val, new_val);
+                Ok(())
             }
             .boxed()
         }));

diff --git a/examples/bridged_accessories.rs b/examples/bridged_accessories.rs
@@ -37,6 +37,7 @@ async fn main() -> Result<()> {
                 "Lightbulb 1: power_state characteristic updated from {} to {}",
                 current_val, new_val
             );
+            Ok(())
         }));
     lightbulb_2
         .lightbulb
@@ -46,6 +47,7 @@ async fn main() -> Result<()> {
                 "Lightbulb 2: power_state characteristic updated from {} to {}",
                 current_val, new_val
             );
+            Ok(())
         }));
     lightbulb_3
         .lightbulb
@@ -55,6 +57,7 @@ async fn main() -> Result<()> {
                 "Lightbulb 3: power_state characteristic updated from {} to {}",
                 current_val, new_val
             );
+            Ok(())
         }));
 
     let mut storage = FileStorage::current_dir().await?;

diff --git a/examples/callbacks.rs b/examples/callbacks.rs
@@ -19,13 +19,14 @@ async fn main() -> Result<()> {
 
     lightbulb.lightbulb.power_state.on_read(Some(|| {
         println!("power_state characteristic read");
-        None
+        Ok(None)
     }));
     lightbulb
         .lightbulb
         .power_state
         .on_update(Some(|current_val: &bool, new_val: &bool| {
             println!("power_state characteristic updated from {} to {}", current_val, new_val);
+            Ok(())
         }));
 
     let mut storage = FileStorage::current_dir().await?;

diff --git a/src/characteristic/mod.rs b/src/characteristic/mod.rs
@@ -9,7 +9,7 @@ use serde::{
 use serde_json::json;
 use std::fmt;
 
-use crate::{event::Event, pointer, HapType, Result};
+use crate::{event::Event, pointer, Error, HapType, Result};
 
 mod generated;
 
@@ -146,10 +146,10 @@ where
     pub async fn get_value(&mut self) -> Result<T> {
         let mut val = None;
         if let Some(ref on_read) = self.on_read {
-            val = on_read();
+            val = on_read().map_err(|e| Error::ValueOnRead(e))?;
         }
         if let Some(ref on_read_async) = self.on_read_async {
-            val = on_read_async().await;
+            val = on_read_async().await.map_err(|e| Error::ValueOnRead(e))?;
         }
         if let Some(v) = val {
             self.set_value(v).await?;
@@ -174,10 +174,12 @@ where
 
         let old_val = self.value.clone();
         if let Some(ref on_update) = self.on_update {
-            on_update(&old_val, &val);
+            on_update(&old_val, &val).map_err(|e| Error::ValueOnUpdate(e))?;
         }
         if let Some(ref on_update_async) = self.on_update_async {
-            on_update_async(old_val, val.clone()).await;
+            on_update_async(old_val, val.clone())
+                .await
+                .map_err(|e| Error::ValueOnUpdate(e))?;
         }
 
         if self.event_notifications == Some(true) {
@@ -421,29 +423,44 @@ pub trait HapCharacteristicSetup {
 /// [`OnReadFn`](OnReadFn) represents a callback function to be set on a characteristic that is called every time a
 /// controller attempts to read its value. Returning a `Some(T)` from this function changes the value of the
 /// characteristic before the controller reads it so the controller reads the new value.
-pub trait OnReadFn<T: Default + Clone + Serialize + Send + Sync>: Fn() -> Option<T> + 'static + Send + Sync {}
+pub trait OnReadFn<T: Default + Clone + Serialize + Send + Sync>:
+    Fn() -> std::result::Result<Option<T>, Box<dyn std::error::Error + Send + Sync>> + 'static + Send + Sync
+{
+}
 impl<F, T: Default + Clone + Serialize + Send + Sync> OnReadFn<T> for F where
-    F: Fn() -> Option<T> + 'static + Send + Sync
+    F: Fn() -> std::result::Result<Option<T>, Box<dyn std::error::Error + Send + Sync>> + 'static + Send + Sync
 {
 }
 
 /// [`OnUpdateFn`](OnUpdateFn) represents a callback function to be set on a characteristic that is called every time a
 /// controller attempts to update its value. The first argument is a reference to the current value of the
 /// characteristic and the second argument is a reference to the value the controller attempts to change the
 /// characteristic's to.
-pub trait OnUpdateFn<T: Default + Clone + Serialize + Send + Sync>: Fn(&T, &T) + 'static + Send + Sync {}
-impl<F, T: Default + Clone + Serialize + Send + Sync> OnUpdateFn<T> for F where F: Fn(&T, &T) + 'static + Send + Sync {}
+pub trait OnUpdateFn<T: Default + Clone + Serialize + Send + Sync>:
+    Fn(&T, &T) -> std::result::Result<(), Box<dyn std::error::Error + Send + Sync>> + 'static + Send + Sync
+{
+}
+impl<F, T: Default + Clone + Serialize + Send + Sync> OnUpdateFn<T> for F where
+    F: Fn(&T, &T) -> std::result::Result<(), Box<dyn std::error::Error + Send + Sync>> + 'static + Send + Sync
+{
+}
 
 /// [`OnReadFuture`](OnReadFuture) represents an async callback function to be set on a characteristic that is driven to
 /// completion by the async runtime driving the HAP server every time a controller attempts to read its value. Returning
 /// a `Some(T)` from this function changes the value of the characteristic before the controller reads it so the
 /// controller reads the new value.
 pub trait OnReadFuture<T: Default + Clone + Serialize + Send + Sync>:
-    Fn() -> BoxFuture<'static, Option<T>> + 'static + Send + Sync
+    Fn() -> BoxFuture<'static, std::result::Result<Option<T>, Box<dyn std::error::Error + Send + Sync>>>
+    + 'static
+    + Send
+    + Sync
 {
 }
 impl<F, T: Default + Clone + Serialize + Send + Sync> OnReadFuture<T> for F where
-    F: Fn() -> BoxFuture<'static, Option<T>> + 'static + Send + Sync
+    F: Fn() -> BoxFuture<'static, std::result::Result<Option<T>, Box<dyn std::error::Error + Send + Sync>>>
+        + 'static
+        + Send
+        + Sync
 {
 }
 
@@ -452,11 +469,17 @@ impl<F, T: Default + Clone + Serialize + Send + Sync> OnReadFuture<T> for F wher
 /// value. The first argument is a reference to the current value of the characteristic and the second argument is a
 /// reference to the value the controller attempts to change the characteristic's to.
 pub trait OnUpdateFuture<T: Default + Clone + Serialize + Send + Sync>:
-    Fn(T, T) -> BoxFuture<'static, ()> + 'static + Send + Sync
+    Fn(T, T) -> BoxFuture<'static, std::result::Result<(), Box<dyn std::error::Error + Send + Sync>>>
+    + 'static
+    + Send
+    + Sync
 {
 }
 impl<F, T: Default + Clone + Serialize + Send + Sync> OnUpdateFuture<T> for F where
-    F: Fn(T, T) -> BoxFuture<'static, ()> + 'static + Send + Sync
+    F: Fn(T, T) -> BoxFuture<'static, std::result::Result<(), Box<dyn std::error::Error + Send + Sync>>>
+        + 'static
+        + Send
+        + Sync
 {
 }
 

diff --git a/src/error.rs b/src/error.rs
@@ -29,6 +29,10 @@ pub enum Error {
     InvalidValue(Format),
     #[error("Invalid HapType string value: `{0}`.")]
     InvalidHapTypeString(String),
+    #[error("Error on value read: {0}")]
+    ValueOnRead(Box<dyn std::error::Error + Send + Sync>),
+    #[error("Error on value update: {0}")]
+    ValueOnUpdate(Box<dyn std::error::Error + Send + Sync>),
 
     // converted errors
     #[error("IO Error: {0}")]

diff --git a/src/transport/http/handler/characteristics.rs b/src/transport/http/handler/characteristics.rs
@@ -1,5 +1,6 @@
 use futures::future::{BoxFuture, FutureExt};
 use hyper::{body::Buf, Body, Response, StatusCode, Uri};
+use log::error;
 use std::collections::HashMap;
 use url::form_urlencoded;
 
@@ -72,7 +73,8 @@ impl JsonHandlerExt for GetCharacteristics {
                             }
                             res_object
                         },
-                        Err(_) => {
+                        Err(e) => {
+                            error!("error reading characteristic: {:?}", e);
                             some_err = true;
                             ReadResponseObject {
                                 iid,
@@ -159,7 +161,8 @@ impl JsonHandlerExt for UpdateCharacteristics {
                         }
                         res_object
                     },
-                    Err(_) => {
+                    Err(e) => {
+                        error!("error updating characteristic: {:?}", e);
                         some_err = true;
                         WriteResponseObject {
                             iid,