Returning "comm failure" for 1 characteristic renders entire bridge unresponsive about hap HOT 5 CLOSED

The Apple Home app shows "No Response" when it has no network connection to a specific accessory.

If an accessory is managed by a bridge, and HomeKit doesn't get a network connection to one of the bridged accessories, it marks the whole bridge as not accessible. (I haven't tested this myself but it seems reasonable.)

One way to handle the case where a specific accessory is not accessible is to remove the accessory from the bridge. But there are some things you have to consider.

• If you add and remove accessories over time, you have to make sure that their corresponding IDs are the same. HomeKit identifies accessory by their ids and expects that an accessory has the same id the next time the bridge is accessible. You will have to do some kind of mapping between an accessories and ids.
• hap currently doesn't support removing accessory dynamically while the bridge is running. You will have to recreate the list of accessory and use a new bridge instance programmatically when removing an accessory. I'm doing the same in my projects and it works perfectly.

from hap.

Alright I've managed to trace through the code using a runtime/debug.PrintStack() at ValueRequestFunc and figured it out. I found it hard to believe that something like a Philips Hue hub would not be able to signal to iOS that only a few bulbs are unreachable and takes down the entire hub with it.

Apparently there was a call to ValueRequestFunc as part of C.MarshalJSON(). The reason why iOS stopped at GET /accessories was because the returned JSON was "malformed", at least according to Apple's parsing. /accessories will call *A.MarshalJSON then *S.MarshalJSON, then ultimately to *C.MarshalJSON. Here, "value" is filled only on success:

But in testing, "value" needs to be present and cannot be null, so it can only be a dummy value.

After that is fixed, iOS continues on to call PUT /characteristics as I described above.

Next, the multi-status 207 return is also malformed. "status": 0 must be explicit, it cannot be omitted. Additionally, "value" must be omitted here, since it is an error.

Applying these 2 fixes solved the issue:

$ git diff
diff --git a/characteristic/c.go b/characteristic/c.go
index 0f1c61c..8b3b415 100644
--- a/characteristic/c.go
+++ b/characteristic/c.go
@@ -287,6 +287,8 @@ func (c *C) MarshalJSON() ([]byte, error) {
                // 2022-03-21 (mah) FIXME provide a http request instead of nil
                if v, s := c.ValueRequest(nil); s == 0 {
                        d.Value = &V{v}
+               } else {
+                       d.Value = &V{c.Val} // dummy "zero" value
                }
        }

diff --git a/characteristics.go b/characteristics.go
index 1f6f72d..3ed757c 100644
--- a/characteristics.go
+++ b/characteristics.go
@@ -14,7 +14,7 @@ import (
 type characteristicData struct {
        Aid   uint64      `json:"aid"`
        Iid   uint64      `json:"iid"`
-       Value interface{} `json:"value"`
+       Value interface{} `json:"value,omitempty"`

        // optional values
        Type        *string     `json:"type,omitempty"`
@@ -89,6 +89,7 @@ func (srv *Server) getCharacteristics(res http.ResponseWriter, req *http.Request
                        cdata.Status = &s
                } else {
                        cdata.Value = v
+                       cdata.Status = new(int) // 0
                }

                if meta {

As I'm not too familiar with the code, I don't know if there will be any side effects from these changes, but it works now. Only 1 sensor will be marked as "no response", while all other accessories function normally.

from hap.

Thanks for clarifying your findings.
We might also want to remove the "omitempty" json option for the Value field in the MarshalJSON() function.

from hap.

Do you want me to submit a PR that fixes the bugs? I would probably also add some tests that checks for the marshaled JSON results to verify they are conformant.

from hap.

Do you want me to submit a PR that fixes the bugs?

That would be great. 👍

from hap.