homebridge-ping-hosts
homebridge-ping-hosts copied to clipboard
lightbulb service does not support onSet method
If I click on the lightbulb in Apple Home or in Homebridge then the lightbulb turns off immediately... and stays off until the interval timer expires and it is reset to the actual state. Looking at the code I see that there are no handlers set for onGet or onSet. I think you should implement both.
onGet should return state of the "lightbulb"
onSet should probably just be ignored... but should call updateCharacteristic to immediately put the bulb into the state it should be in.
An alternative for onSet would be to interpret it as a request to do an immediate health check on the target host... cancel the interval timer, run the ping test immediately, restart the interval timer. Or something like that.
Yeah I'm confused on this. Someone asked for me to add support for the accessory being either a contact sensor (which it was initially) or a motion sensor or a lightbulb. Neither of the last two made much sense to me but I didn't think there was any harm supporting a choice.
Of course the problem with lightbulb is that you can turn it on or off in Apple Home... so now somehow my plugin homebridge-ping-hosts
has become a light bulb manager.... :-)
Anyway, you're right I should implement onGet at the least as it would be nice to see the state immediately (even if it is just a contact sensor) rather than waiting for the poll interval...
If I understand you correctly (also bear in mind I'm not expert on the HAP API usage....) you are saying I should do the following (taking the lightbulb scenario):
First of all store the 'state' on the accessory and update this with the result within the doPing
method:
...
this.state = result.alive;
Then provide this value for onGet
:
this.services.sensor.getCharacteristic(Characteristic.On).on(CharacteristicEventTypes.GET,
(callback) => {
callback(this.state);
});
and also set it for onSet (taking it account it might be overridden next time the ping poll timeout occurs:
this.services.sensor.getCharacteristic(Characteristic.On).on(CharacteristicEventTypes.SET
(value) => {
this.services.sensor.updateCharacteristic(Characteristic.On, value);
});
?
Suggest you model after the Homebridge documentation for lightbulb.
// create handlers for required characteristics
this.service.getCharacteristic(this.Characteristic.On)
.onGet(this.handleOnGet.bind(this))
.onSet(this.handleOnSet.bind(this));
In the set handler I would not update it to value (which is what HomeKit is asking you to set it to, on/off) but rather update it to what the actual health state is... which may not be the same as what is requested. In other words we're ignoring what we're being asked to do and forcing the lightbulb into state that reflects health of the link.
You can do the get/set handler functions in-line if you don't want to create separate function. Like...
.onGet( () => {
return(whatever);
})
.onSet( (value: CharacteristicValue) => {
return;
});
Would be a good idea to log that we're ignoring set request...
.onGet( () => {
return(whatever);
})
.onSet( (value: CharacteristicValue) => {
this.log.warn(`Ignoring request to set XXX to value ${value} current health is ${whatever}`);
this.services.sensor.updateCharacteristic(Characteristic.On, whatever);
return;
});
have a look at the latest code (version 3.2.4) and see what you think...
It is now working as desired. Clicking on the "lightbulb" is ignored.
An observation on your code... You are treating the lightbulb, motion sensor and contact sensor as if they are all the same. They are not. For example, lightbulb and motion sensor are documented as returning a boolean onGet. But contact sensor returns a unsigned 8-bit integer, with contact detected as 0 and not detected as 1, which seems backwards to until you think of the intended use case (door contact sensor, no contact means door opened, so non zero value makes sense).
Also, only the lightbulb supports onSet, the others are not settable, only gettable. Homebridge doesn't seem to be objecting to adding the handlers.
Now for this simple use case the above is probably harmless, Your implementation certainly simplifies the code, but something to be aware of.
With respect to onGet(), the values that this.state
can take are defined here for contact sensor:
https://github.com/vectronic/homebridge-ping-hosts/blob/master/index.js#L81-L87
so I think this should be OK.
I am committing a change to address the fact only lightbulb supports onSet()