Change updateValue to return not responding status #556
Conversation
|
Looks good to me :) |
|
Tks, much appreciated
… On Feb 4, 2018, at 10:01 PM, Khaos Tian ***@***.***> wrote:
Looks good to me :)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <https://github.com/KhaosT/HAP-NodeJS/pull/556#issuecomment-362969295>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AS5CmCJFbH7qodlDYj6h1s3E8UhNUDEmks5tRm8NgaJpZM4R426a>.
|
* 'master' of github.com:KhaosT/HAP-NodeJS: (58 commits) 0.4.47 Add ability to pass in mdns config. Add setupURI to the TypeScript def for Accessory (homebridge#594) Update bonjour dependency to resolve a crash caused by state management of mdns announcement. Fix the issue where we incorrectly removed pairing information on closing connection. 0.4.44 Remove the duplicated `start` 0.4.43 Ensure the uniqueness of service name 0.4.42 Legacy Dictionary Fixes (homebridge#569) Create Sprinkler_accessory.js (homebridge#585) Fix Node v10 support Update Light_accessory.js (homebridge#583) Update SSRC generation. Use Int32 instead of UInt32 for ssrc. Changes SSRC to be random per prepareStream, not always 1 (homebridge#558) 0.4.41 Change updateValue to handle not responding status (homebridge#556) Add max, min and valid values to the new services (homebridge#551) ... # Conflicts: # accessories/Light_accessory.js # lib/gen/HomeKitTypes.js # package.json
I know this is an old post, but checking HAP Node git master and this commit, shows the changes are still there. I am trying to understand the changes and why there isnt any event/update to the accessory using this code update. The addition to If I follow correctly, there is nothing that is emitted, or sent to the accessory. I followed the example, added 'debug' to the 'characteristic.js' file but no change on console or the accessory itself on iOS. There is no change when using an instance of Any help would be greatly appreciated. Using HAP-Node master & iOS12.2. The only confusing part is the usage of "status" which is not clear how it causes a change to the accessory side. |
|
I can confirm that this feature is still working with iOS 12.2 and the latest version of homebridge/Hap-nodejs. Also, please keep in mind that the ‘not responding’ status is only sent to HomeKit as a response to the characteristic status request. And is not sent as an event. The HomeKit API for events does not support the setting of a not responding status. To sum it up, the response to a characteristic status pull request from the home app can include an error response that triggers ‘not responding’. But characteristic status push events from hap-nodejs can not, and this is a HomeKit API limitation. When your testing the feature, are you triggering a characteristic status request from the home app? Ie restart the home app or change rooms |
|
Thank you for the reply. Ok gotcha on the if (message == 'exit') {
debug("Received message %s", message);
sensor
.getService(Service.TemperatureSensor)
.getCharacteristic(Characteristic.CurrentTemperature)
.updateValue(new Error("No Response"));
hsensor
.setCharacteristic(Characteristic.CurrentRelativeHumidity, new Error("Offline"));
} |
|
The MQTT retain flag has nothing to do with it, as HAP-Node caches or uses the previous values. Here is a log that shows that a For some reason, this code section is never reached which I think is relevant. https://github.com/KhaosT/HAP-NodeJS/blob/1862f73d495c7f25d3ed812abac4be4c1d2dee34/lib/Characteristic.js#L166-L168 Hap-Node JS Log |
|
Looking at your log, are you actually setting the "Error" status? Here is a log from my environment, please note the line "Error getting value", this is the error being returned. This request was from a page reload with three devices on the page, and one of them was 'not responding' The plugin code behind this is here And the actual message is returned from here in Accessory.js |
|
Thanks again, “error status”? I thought the only thing required was to use updateValue as per the code snippet I included in post above. Is there another step? I’ve came across the code you linked but can’t yet deduce what is missing in my hap-node code. I may think it has to do with this snippet but unsure... https://github.com/KhaosT/HAP-NodeJS/blob/1862f73d495c7f25d3ed812abac4be4c1d2dee34/accessories/TemperatureSensor_accessory.js#L40 |
|
In your logging are you seeing this being executed? In that example your shared, the callback needs to be an error object and not 'null', the callback lands here And sets 'status' to be the first parameter of the callback. |
|
Yes, execution reaches that line, and you just confirmed my suspicion, the callback needs to be an error object. I just literally read your first post in the PR and thought the callback(null,value) is fine... I also saw that .emit code but couldn’t piece all together. Not obvious for noobs. Although this line says it ... I now have to understand how to do this ! |
|
ok that was easy ... All I did was initialize a var myerr to null, and set that error object accordingly based on the acc presence/reachability. But one confusing find, the Just setting the error in the callback is sufficient to trigger this code section https://github.com/KhaosT/HAP-NodeJS/blob/1862f73d495c7f25d3ed812abac4be4c1d2dee34/lib/Characteristic.js#L164 _H2OS currentime 07:28 received topic home/basement/h2os1/status message exit +43s Accessory [Water Sensor] Getting value for Characteristic "Leak Detected" +6s Accessory [Water Sensor] Got Characteristic "Leak Detected" value: undefined +1ms Accessory [Water Sensor] Error getting value for Characteristic "Leak Detected": Bye +1ms var myerr = null; // init the error object
// Add the actual Sensor Service.
sensor
.addService(lsensor)
.getCharacteristic(Characteristic.LeakDetected)
.on('get', function(callback) {
callback(myerr, ACC.getLeak());
});
isAccReachable ? myerr = null : myerr = new Error("Bye"); |
|
In my plugins I don’t implement the ‘get’ event, and strictly use update value to provide device status back to HomeKit.
By not implementing the ‘get’ event it increases the responsiveness of the home app. Hap-nodejs caches the value until requested in a get request and/or sends as an event. Errors can not be sent as an event, limitation of the Apple API.
… On Apr 25, 2019, at 7:48 AM, Fadi ***@***.***> wrote:
ok that was easy ... All I did was initialize a var myerr to null, and set that error object accordingly based on the acc presence/reachability.
But one confusing find, the .updateValue(myerr) is not needed as you explained that the error is only returned in response to a charac status request. So then what's the purpose of that .updateValue(myerr) ?
Just setting the error in the callback is sufficient to trigger this code section https://github.com/KhaosT/HAP-NodeJS/blob/1862f73d495c7f25d3ed812abac4be4c1d2dee34/lib/Characteristic.js#L164
_H2OS currentime 07:28 received topic home/basement/h2os1/status message exit +43s
Accessory [Water Sensor] Getting value for Characteristic "Leak Detected" +6s
Accessory [Water Sensor] Got Characteristic "Leak Detected" value: undefined +1ms
Accessory [Water Sensor] Error getting value for Characteristic "Leak Detected": Bye +1ms
var myerr = null; // init the error object
// Add the actual Sensor Service.
sensor
.addService(lsensor)
.getCharacteristic(Characteristic.LeakDetected)
.on('get', function(callback) {
callback(myerr, ACC.getLeak());
});
isAccReachable ? myerr = null : myerr = new Error("Bye");
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Thank you for that additional insight, that is very interesting. I always thought the .get event is required but I will experiment. And thanks again for all your replies and this PR ! |
|
Good luck, and if you have more questions, I’m around |
|
So I removed all I take your word about this increasing Homekit responsiveness, I have no way of testing it as I only have around 10 IoT devices running on HAP-Node. I did notice few times that Home app takes a while to update the HAP-Node devices but can't correlate this to the The reason for the But if the cached Characteristic.value(s) are the result of querying the devices through say MQTT and |
|
With my plugins, I prefill the cache during code startup to avoid using the default value. A minor thing but increases the usability.
If we look at this one level higher, how is your code interacting with your device, and how is your device telling your code about local events? In my opinion devices that can push status/state changes in real time are the most ideal pattern. As HomeKit and the home app support real time events, having a local status change passed up gives the best user experience. And allows automations to be triggered in real time based on state changes. Implementing the get event listener, does not enable this very desirable behaviour, as the get event is only triggered when the home app is opened or a room is changed. Imagine a motion sensor device that only updated in the home app, when a user opens the app. Not very useful, if you wanted to trigger a light etc.
Second best would be polling the device at regular intervals and updating HomeKit with the status.
Worst would be only updating when the home app is opened or a room changed.
… On Apr 26, 2019, at 7:55 AM, Fadi ***@***.***> wrote:
So I removed all get event handlers, and used updateValue for all updates incl errors, and indeed it works :-) Even when no cached values exist (start from scratch, clear mosquitto db) and device offline, HAP-Node will PUT charac defaults of 0 to Homekit (a safeguard).
I take your word about this increasing Homekit responsiveness, I have no way of testing it as I only have around 10 IoT devices running on HAP-Node. I did notice few times that Home app takes a while to update the HAP-Node devices but can't correlate this to the get event handlers.
The reason for the get handler is explained by @KhaosT in the example comments: https://github.com/KhaosT/HAP-NodeJS/blob/1862f73d495c7f25d3ed812abac4be4c1d2dee34/accessories/Light_accessory.js#L106-L109
But if HAP-Node cached values are the result of querying the devices through say MQTT and updateValue, then implementing that get handler is kinda redundant or not needed.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Indeed, coupling devices with MQTT (or other means) and the usage of It's too bad that Homekit doesn't support
This could have been a nice alternative to using the error status had Home app display those events on the device icon instead of embedding them in device settings ... (Apple being lazy) |
|
I managed to report accessory "not responding" with all the comments. Thanks !
You can try with my test file here: Regards |
I have been moving my plugins away from using getValue to return the current accessory state, and switching to a event based model, and having the plugin push state via updateValue or setValue. ( I'm switching to this event based model, to improve the responsiveness of the Home app and reduce the lagging caused by the callouts to remote devices to return state. ) While this works really well, what is lacking is the ability to deal with accessories that are not responding for various reasons, as the updateValue and setValue don't accept a value of 'Error'. And I'm not aware of any way to set a 'Not Responding' for an accessory either thru a characteristic or the HAP event interface, except as a response to "GET /characteristics".
Creates a characteristic level status 'this.status' - Value is null or the Error returned by the callback.
Changes updateValue and setValue to accept "Error" as a newValue, and update the status.
Changes getValue to include status as part of the response when there are no listeners to the get event
To use the feature, have the value updated by updateValue or setValue be an instance of Error. During the next get characteristics request, the accessory will display 'Not responding'. When the device starts responding again, sending a valid value, that is not an instance of 'Error' will clear the 'Not responding' message after the next get characteristics.
ie use this for updateValue
Accessory.getService(Service.Outlet).getCharacteristic(Characteristic.On).updateValue(new Error("Polling failed"));Optional additional functionality
If you want to set a particular HAP return code, have the error be the HAP return code.
ie use this instead for updateValue
Accessory.getService(Service.Outlet).getCharacteristic(Characteristic.On).updateValue(new Error(HAPServer.Status.SERVICE_COMMUNICATION_FAILURE));To gain access to the HAPServer Status codes, add these 2 HAPServer lines to the start of your plugin.
var Accessory, Service, Characteristic, UUIDGen, HAPServer;
module.exports = function(homebridge) {
Accessory = homebridge.platformAccessory;
Service = homebridge.hap.Service;
Characteristic = homebridge.hap.Characteristic;
UUIDGen = homebridge.hap.uuid;
HAPServer = homebridge.hap.HAPServer;
homebridge.registerPlatform("homebridge-ecoplugs", "EcoPlug", EcoPlugPlatform);
}