Add Characteristic.OnValueGet hook

[josh]

This change proposes a characteristic hook that is invoked when /characteristics is accessed.

The is useful when the underlying accessory does not implement a change event system and must be polled. High frequency polling may also be excessive when the value is not changing all that often. Such a hook allows for a hybrid approach of low frequency polling and higher frequency checking when a user is actively engaging with the Home app.

acc.Switch.On.OnValueGet(func() {
  // sync value check before responding
  fetchLatestSwitchValue()
})

acc.Switch.On.OnValueGet(func() {
  // or kick off an async value check between polling intervals
  go fetchLatestSwitchValue()
})

The HAP-NodeJS library offers a similar getCharacteristic hook.

[josh]

@brutella thanks for the feedback! I pushed some revisions that should improve the API. Please have another review.

[brutella]

The changes look good. But there is still an issue which has to be resolved before it can be merged.

Calling GetValue() has some side effects because it may call UpdateValue(). In UpdateValue() we call updateValue(), which actually changes the value of the characteristic. The ip transport gets a callback when any characteristic value is changed – see ip_transport.go. There all connected clients get notified that the characteristic value was changed.

For example if there are 2 connected clients and one changes a value of a characteristic – like setting the power state of a light bulb to on – the other client gets notified. This is done by sending the notification to all connected clients, except the one which made the request to change a value.

Client A --- Set Power State to On --> | Server |
Client B <---- Send Notification ----  | Server |                                

Your changes now introduce the problem that if a client makes a request to get the value of a characteristic, we call GetValue(). There we call c.valueGetFunc() and update the characteristic value. By updating the value all connected clients get notified that the value changed – including the client which currently makes the request. So this is something we have to fix.

So how to fix that? We could introduce a new GetValueFromConnection(conn net.Conn) (similar to UpdateValueFromConnection) method, which could then call UpdateValueFromConnection() (instead of UpdateValue()).

Also we should add unit tests to verify that it works as expected.

[brutella]

Great changes. I've just added a couple of comments to finish things up.

[josh]

Thanks again @brutella, I've pushed those changes.

[ipstatic]

@josh I am trying to test this out and see if it will help me with IPCamera accessories, however I am running into an issue. When trying to use your code as documented above, I get a type mismatch:

cannot use func literal (type func()) as type characteristic.GetFunc in argument to acc.CameraRTPStreamManagement.SetupEndpoints.Bytes.String.Characteristic.OnValueGet

Here is how I am using the code:

acc.CameraRTPStreamManagement.SetupEndpoints.OnValueGet(func() {
  fetchLatestEndpointsValue(acc)                                
})

Any advice?

[ipstatic]

Also, I think I'll need the set event hook like HAP-NodeJS provides as well, any chance we could wire that up in this PR, or should we do a new one?

[josh]

@ipstatic the API has changed since the original PR has been opened.

Try:

acc.CameraRTPStreamManagement.SetupEndpoints.OnValueRemoteGet(func() bool {
  return fetchLatestEndpointsValue(acc)                                
})

Also, I think I'll need the set event hook like HAP-NodeJS provides as well, any chance we could wire that up in this PR, or should we do a new one?

Set event APIs are already supported.

acc.CameraRTPStreamManagement.SetupEndpoints.OnValueRemoteUpdate(func(value bool) {
})

[ipstatic]

@josh Thanks, I actually had to use string instead of bool as that was the type SetupEndpoints was expecting. However this is not firing. I either must be mis-understanding what this does or I am listening on the wrong character. Do you know of a way to catch all gets coming to the device? This is the perfect storm as I am a still a Go novice and the docs surrounding HomeKit APIs are sparse.

[josh]

@brutella I think everything should be good to go here. Let me know if you spot any additional typos. Thanks for all the feedback.

@ipstatic I do not know too much about the camera APIs, but I know they work a little different than simple switches. There maybe is some misunderstanding to when the setup endpoints are accessed from HomeKit clients. You may want to use something like an HTTP proxy to learn more about the camera setup HTTP request/response flow.

[ipstatic]

@josh I have been trying to do that, by dumping the form data we get from the clients. However one problem I have ran into is that the form data references the ad/id/iid instead of a human readable name (see #104). I have been trying to figure out a way to translate that with the internal database kept by HC with no luck so far.

[ipstatic]

@josh is there a way to get access to the session information via the OnValueRemoteUpdate callback? One of my responses requires the session ID. I am guessing from looking at this we would need to modify the callback to include both the session object as well as the function to call?

[brutella]

Thanks @josh

[ipstatic]

@brutella Would you have a suggest for my question above? Getting access to session data inside a callback.

[brutella]

@ipstatic Please post your question in a new issue.