From 2ebf87272c8e0dac8f28ef405e4e34a78daeaee4 Mon Sep 17 00:00:00 2001
From: Alex Janka <alex@alexjanka.com>
Date: Thu, 7 Mar 2024 09:23:37 +1100
Subject: [PATCH] better error printing and handling of missing values in
 characteristic response

---
 .../src/pairing_data/characteristics.rs       | 11 +-----
 homekit-controller/src/pairing_data/mod.rs    | 37 +++++++++++--------
 homekit-exporter/src/main.rs                  |  9 +++--
 3 files changed, 28 insertions(+), 29 deletions(-)

diff --git a/homekit-controller/src/pairing_data/characteristics.rs b/homekit-controller/src/pairing_data/characteristics.rs
index d970300..e36ac48 100644
--- a/homekit-controller/src/pairing_data/characteristics.rs
+++ b/homekit-controller/src/pairing_data/characteristics.rs
@@ -3,15 +3,6 @@ use strum_macros::Display;
 
 // data from https://github.com/oznu/homebridge-gsh/blob/master/src/hap-types.ts
 
-pub(crate) fn deserialize_characteristic_type<'de, D>(
-    deserializer: D,
-) -> Result<CharacteristicType, D::Error>
-where
-    D: Deserializer<'de>,
-{
-    CharacteristicTypeInner::deserialize(deserializer).map(|v| v.into())
-}
-
 #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, Display)]
 #[strum(serialize_all = "snake_case")]
 
@@ -243,7 +234,7 @@ pub enum CharacteristicType {
 
 #[derive(Deserialize)]
 #[serde(field_identifier)]
-enum CharacteristicTypeInner {
+pub(super) enum CharacteristicTypeInner {
     #[serde(rename = "000000E5-0000-1000-8000-0026BB765291", alias = "E5")]
     AccessControlLevel,
     #[serde(rename = "000000A6-0000-1000-8000-0026BB765291", alias = "A6")]
diff --git a/homekit-controller/src/pairing_data/mod.rs b/homekit-controller/src/pairing_data/mod.rs
index 9f62121..22b6385 100644
--- a/homekit-controller/src/pairing_data/mod.rs
+++ b/homekit-controller/src/pairing_data/mod.rs
@@ -2,9 +2,11 @@ use std::collections::HashMap;
 
 use serde::{Deserialize, Deserializer, Serialize};
 
-use characteristics::{deserialize_characteristic_type, deserialize_service_type};
+use characteristics::deserialize_service_type;
 pub use characteristics::{CharacteristicType, ServiceType};
 
+use crate::pairing_data::characteristics::CharacteristicTypeInner;
+
 mod characteristics;
 
 #[derive(Deserialize, Debug, Clone)]
@@ -97,8 +99,8 @@ where
         iid: usize,
         #[serde(rename = "type", deserialize_with = "deserialize_service_type")]
         service_type: ServiceType,
-        primary: bool,
-        hidden: bool,
+        primary: Option<bool>,
+        hidden: Option<bool>,
         #[serde(deserialize_with = "deserialize_service_characteristics")]
         characteristics: HashMap<usize, ServiceCharacteristic>,
     }
@@ -107,8 +109,8 @@ where
             Self {
                 name: val.name,
                 service_type: val.service_type,
-                primary: val.primary,
-                hidden: val.hidden,
+                primary: val.primary.unwrap_or(false),
+                hidden: val.hidden.unwrap_or(false),
                 characteristics: val.characteristics,
                 name_for_accessory: val.name_for_accessory,
             }
@@ -209,9 +211,9 @@ where
     struct ServiceCharacteristicInner {
         #[serde(rename = "iid")]
         iid: usize,
-        #[serde(rename = "type", deserialize_with = "deserialize_characteristic_type")]
-        characteristic_type: CharacteristicType,
-        perms: Vec<CharacteristicPermissions>,
+        #[serde(rename = "type")]
+        characteristic_type: Option<CharacteristicTypeInner>,
+        perms: Option<Vec<CharacteristicPermissions>>,
         #[serde(flatten)]
         value: Option<Data>,
         #[serde(rename = "ev")]
@@ -227,11 +229,13 @@ where
         min_step: Option<f64>,
         unit: Option<Unit>,
     }
-    impl From<ServiceCharacteristicInner> for ServiceCharacteristic {
-        fn from(value: ServiceCharacteristicInner) -> Self {
-            Self {
-                characteristic_type: value.characteristic_type,
-                perms: value.perms,
+    impl ServiceCharacteristic {
+        fn maybe_from(value: ServiceCharacteristicInner) -> Option<Self> {
+            let characteristic_type = value.characteristic_type.map(|v| v.into())?;
+            let perms = value.perms?;
+            Some(Self {
+                characteristic_type,
+                perms,
                 value: value.value,
                 event_notifications_enabled: value.event_notifications_enabled,
                 enc: value.enc,
@@ -240,13 +244,16 @@ where
                 max_value: value.max_value,
                 min_step: value.min_step,
                 unit: value.unit,
-            }
+            })
         }
     }
     Vec::<ServiceCharacteristicInner>::deserialize(deserializer).map(|v| {
         v.into_iter()
             .fold(HashMap::new(), |mut map, characteristic| {
-                map.insert(characteristic.iid, characteristic.into());
+                let iid = characteristic.iid;
+                if let Some(c) = ServiceCharacteristic::maybe_from(characteristic) {
+                    map.insert(iid, c);
+                }
                 map
             })
     })
diff --git a/homekit-exporter/src/main.rs b/homekit-exporter/src/main.rs
index ce5d011..3bb6c34 100644
--- a/homekit-exporter/src/main.rs
+++ b/homekit-exporter/src/main.rs
@@ -49,8 +49,8 @@ async fn rocket() -> rocket::Rocket<rocket::Build> {
     let args = Args::parse();
     match args.command {
         Commands::Watch => match init(args.pairing_data).await {
-        Ok(paired) => launch(paired, args.port),
-        Err(e) => panic!("Error {e:#?}"),
+            Ok(paired) => launch(paired, args.port),
+            Err(e) => panic!("Error {e:#?}"),
         },
         Commands::Discover => {
             println!("discovering homekit devices via mdns");
@@ -82,8 +82,9 @@ async fn init(pairing_data: PathBuf) -> Result<HashMap<String, DeviceConnection>
         for (k, v) in devices {
             let mut num = 0;
             let connected = loop {
-                if let Ok(v) = v.connect(&discovered).await {
-                    break Some(v);
+                match v.connect(&discovered).await {
+                    Ok(v) => break Some(v),
+                    Err(e) => log::error!("error connecting to {k}: {e:#?}"),
                 }
                 num += 1;
                 if num > 10 {