ebaauw / homebridge-nb

Homebridge plugin for Nuki Bridge
Apache License 2.0
51 stars 3 forks source link

why is battery the primary service and not the lock mechanism? #116

Closed justme-1968 closed 1 year ago

justme-1968 commented 1 year ago

after quite a lot of frustration with the initial installation and configuration of the 3.0 pro and the nuki homekit integration i bought the bridge and installed your plugin. what a night and day difference. it 'just works' as hoped and expected. without this plugin the lock would be completely useless. thanks!

just a question: is there a reason the battery service is the primary service and not the lock mechanism ?

and another short one: i could not find any description/documentation for the programmable switch that is also exposed by the plugin. what is it used for?

and a third (sorry): is there a reason the accessory itself is exposed as other instead HMAccessoryCategoryTypeDoorLock (that might be a (home)bridge limitation. i don't remember)?

thanks andre

ebaauw commented 1 year ago

the battery service is the primary service and not the lock mechanism ?

What makes you think it would be? I set the lock mechanism as primary service on the lock accessory. I don't set the battery service as primary service anywhere.

I don't think I set a primary service on the opener nor on the door sensor accessory - that would be an improvement point.

Looking at the code, I do create the battery service before the lock mechanism service. Maybe the HomeKit app you're using is confused by that?

Note that primary service has no inherent meaning in HomeKit. It's just a hint to HomeKit applications how they could visualise the accessory. I've only seen Apple's Home app using/requiring this for particular accessories, like televisions.

i could not find any description/documentation for the programmable switch that is also exposed by the plugin. what is it used for?

It's non-functional. It is used to prevent Home form showing an unsupported tile for the bridge accessory.

is there a reason the accessory itself is exposed as other instead HMAccessoryCategoryTypeDoorLock

Again, what makes you think that? The smart lock accessory is exposed as door lock, as is the opener accessory. The door sensor accessory as door, and the bridge accessory as range extender.

Note that HomeKit only shows the accessory type when pairing the accessory. For bridged accessories, it is never shown, as they aren't paired individually.

justme-1968 commented 1 year ago

this might get a little off topic as it probably in deed not specific to this plugin but homekit and homebridge in general.

What makes you think it would be?

my own project. which is, besides some other things, a homekit browser that displays all low level information that is accessible with the homekit api. see the attached screenshot.

the 'other' in the accessory section on the right is the accessory category, the battery icon next to it (and in the summary on the left) is the icon for the primary service of this accessory also the the star next to a service means primary. but i think i just remember that i stumbled over this problem already about a year ago. homebridge has a problem with setting the accessory category. it is always other. i think i even opened a issue or at least mentioned it somewhere. but i can't find it...

maybe there is also a problem with setting the primary characteristic and it is always the first one created. that i don't know. another nitpicky detail: maybe it would be nice to create the lock service first and the latch service last. then the home app might display the lock left and the latch right which would be the order of operation. using the labelIndex characteristic might be even better.

yes, it is only a hint. but my browser tries to take the hint (next to some other heuristics) and in this instance it is the first time it is completely off.

so there are quite a few unknowns:

i also just noticed that the battery service exposes the chargingState as notChargable. but i think it should be none as the power pack in my nuki pro is definitely chargeable.

another unrelated observation: the door sensor has no battery service or battery characteristic. if the nuki api provides battery information for the sensor it would be nice to include it.

Bildschirmfoto 2023-07-21 um 12 13 39
justme-1968 commented 1 year ago

i also just now found the range extender you mentioned. it is also an accessory with type other.

it has the switch as primary characteristic and the range extender with an characteristic uuid i could not find anywhere. the range extender part is nowhere to be seen.

justme-1968 commented 1 year ago

i'm noticing more things that i'm curious about. i really love your plugin and i don't want to talk anything down. just the opposite and i'm curious as i'm noticing quite a few things that are different from ever other accessories and services that i have encountered till now. if you prefer we can take this discussion somewhere else ?

some of the things i'm noticing is surely from the fact that the home app is partly very strict and not very indulging with things that are deemed unnecessary. the eve app on the other hand shows nearly everything and sometimes ignores the standard.

the last updated characteristic of the door sensor is hidden. is this intentional? this would not be noticeable in home or eve as the former will just not show anything custom and the latter will mostly ignore the hidden flag.

ebaauw commented 1 year ago

maybe it would be nice to create the lock service first and the latch service last

Way ahead of you, see second commit above.

but i think it should be none as the power pack in my nuki pro is definitely chargeable.

I don't have a charging pack, so Homebridge NB currently doesn't support this. Can you charge it while it's in the lock, or do you need to take it out? What's the output of nb list when it's charging?

using the labelIndex characteristic might be even better.

One of my many love/hate relationships. Originally it was supported only for Stateless Programmable Switch, but Eve are honouring it (sometimes) for other services as well. But sometimes it completely screws up. Will do some testing.

the door sensor has no battery service or battery characteristic. if the nuki api provides battery information for the sensor it would be nice to include it.

I have the v2 Smart Lock, with integrated door sensor, so it doesn't have a separate battery. Please provide the output of nb list, so I can see if the Nuki bridge reports the battery for the separate door sensor of the v3 smart lock. My guess is, it only exposes whether the battery is critical, not the battery level. Of course I can map that to 90%/10%, just as for the Opener.

it has the switch as primary characteristic and the range extender with an characteristic uuid i could not find anywhere

Service, not characteristic. I defined it myself, see https://github.com/ebaauw/homebridge-lib/blob/main/lib/MyHomeKitTypes.js.

if you prefer we can take this discussion somewhere else ?

Sure, PM me on Discord.

some of the things i'm noticing is surely from the fact that the home app is partly very strict and not very indulging with things that are deemed unnecessary. the eve app on the other hand shows nearly everything and sometimes ignores the standard.

The Apple Home app is probably the worst that ever happened to HomeKit. It only supports a limited subset of HomeKit; you really need a decent HomeKit app, like Eve, to leverage the full functionality of my plugins. On the other hand, Home supports stuff (like the Television accessories, you already mentioned, but also integration of AirPlay 2 speakers and the temperature/humidity sensor in the HomePod) that isn't available over the HomeKit framework. It gets worse with Matter; the HomeKit framework maps standard Matter clusters and attributes to HomeKit services and characteristics, but not the custom defined ones. E.g. Home+ can show Eve history for Eve devices with native HomeKit firmware, but not with Matter firmware. The Eve app uses Matter directly to get that info.

the last updated characteristic of the door sensor is hidden

No. It's a custom characteristic alright, but it doesn't have the hidden permission set.

justme-1968 commented 1 year ago

just some short answers, i will gather the requested data over the weekend or on monday.

i missed your commit while doing my edits above. will check if this works better.

yes, the power pack can be charged while in the lock. it has a usb-c socket on the bottom. per manual it can even permanently wired. but this would be ugly :).

yes. the nuki app also seems to show only ok/low for the sensor battery. instead of calculating the values for BatteryLevel i think it is better to use only StatusLowBattery. that avoids the confusion of a perfectly fine 90% battery dropping suddenly to 10%. and StatusLowBattery alone is enough to give a (red) indicator in the apps.

i had already found your characteristics definitions.they should work fine with my app.

you are right. it is indeed not hidden. i skipped a row. configuredname is hidden. but i double checked the other stance things and they are indeed there. at least on my side of the api.

you are right about the home app. eve is much more forgiving. but on the other hand: it will display some things that are cleary not optimal and other apps struggle with. so it might not be the best solution to check compatibility.

all the matter stuff maybe also coming to homekit as in principle it can/should give full access to matter with maybe a few necessary extension. at least apple intends homekit to integrate more in this way. let's see if this will really work one day... if philips gets around to activate matter on their hub this might be a good test for compatibility....

ebaauw commented 1 year ago

i think it is better to use only StatusLowBattery.

No can do. Battery Level is a mandatory characteristic in the Battery service.

configuredname is hidden.

Yes, intentionally.

in principle it can/should give full access to matter

No, only to the standard part, not to manufacturer-specific extensions (the Matter equivalent of custom services and characteristics). Had I known this beforehand, I wouldn't have upgraded my Eve Energy to Matter.

if philips gets around to activate matter on their hub this might be a good test for compatibility....

If Hue had good compatibility, I wouldn't have had the need to create Homebridge Hue.

The Hue bridge already supports Matter, but you might need to activate this from the Hue developer portal. I have it running on my Hue bridge, but haven't yet tried pairing it over Matter. I think they only use one or two custom characteristics in their HomeKit implementation. These are used by the Hue app, which is a HomeKit app (albeit with limited HomeKit functionality), to sync room assignments on the Hue bridge with HomeKit rooms. I think all device types supported by the Hue bridge are defined in the current Matter standard, so the functionality should be at par with their HomeKit function. I wonder if they expose non-Hue lights over Matter, that would eliminate one of the main needs for Homebridge Hue.

justme-1968 commented 1 year ago

i think it is better to use only StatusLowBattery.

No can do. Battery Level is a mandatory characteristic in the Battery service.

are you sure about this? i have at least two devices with 'official' homekit connectivity that expos only StatusLowBattery without BatteryLevel. i also have done it myself and it works perfectly fine.

edit: just saw it in the hap specification. silly.

justme-1968 commented 1 year ago

just had another look into homebridge/HAP-NodeJS/blob/master/src/lib/definitions/ServiceDefinitions.ts and there only StatusLowBattery is required. BatteryLevel and ChargingState are optional. the same is mentioned for example here: https://nrchkb.github.io/wiki/service/battery/

for me this makes sense as not every device is necessarily capable of providing an accurate battery percentage. especially if it allows different kinds of batteries.

ebaauw commented 1 year ago

Section 9.25 of the HAP Specification (non-commercial version from 2017) lists Battery Level, Charging State, and Status Low Battery as required characteristics of the Battery Service.

Same in section 9.26 of the HAP Specification (R13, from 2018).

The HomeKit Accessory Simulator (v4.0 (135.3), from 2018 v4.1.3 (135.4.3), current version just downloaded) creates a Battery Service with these characteristics as mandatory as well.

I don't have any more recent official documentation from Apple. I'm not impressed by any unofficial documentation. I'm willing to accept that HomeKit currently seems to accept a Battery Service with only Status Low Battery, but if that's accidental, I fear a next release of HomeKit might kick out the accessory (and bridge that provides it) for non-compliance. Apple seems to be tightening that net.

I'll do some testing to see if it works for me.

for me this makes sense

I stopped trying to make sense of HomeKit a long time ago. I always hated having to expose Charging State, for accessories that don't support charging. Instead of a Not Chargeable value, the absence of that characteristic would make more sense (and reduce the size of your HomeKit configuration).

ebaauw commented 1 year ago

Could you try v1.4.5?

And please list the output of nb list, so I can see if and how the battery status of the door sensor is exposed. Preferably while charging the battery pack in the smart lock, so I can double-check that as well.

justme-1968 commented 1 year ago

sorry for the delay. i have installed your update but when i wanted to start testing i noticed the door was blocked. after disassembling and cleaning everything i found the problem. one of the axels for the gears inside the lockbox inside the door was broken and blocked everything. as it is weekend the service was not available and i had to get a non matching replacement from a box box store. after a second try ingot one 3mm shorter. but still to deep. after chiseling out parts of the door and lots of trial and error it is now mounted and should work till monday. i hope to get the real replacement then. hopefully it has a short delivery time as we will go on holiday next saturday.

that was about 5 hours of hardware trouble. i will hopefully get back to software next week and send you everything.

sorry again andre

justme-1968 commented 1 year ago

here is a short update:

with the plugin updated i can see the labelindex and it is used by my app in eve the lock now also comes before the latch. there the order in home is still not opimal with the latch before the lock which also results in a strange display of a locked icon with an "unlocked" text on the mac and an locked icon with an "1 on 1 off" text on ios17 beta.

also the battery service is still primary on the lock accessory.

the door sensor does not show battery ok. i think you said it will show only the low warning? maybe having ok also would be nice.

in the mean time i have also connected a 2.0 keypad. maybe you could at it as an accessory even if it would only show the battery ok/low

i will make a second list while charging in the evening.

thanks! andre

justme-1968 commented 1 year ago

it just came to my mind that it might be better to combine the latch and the door sensor into an accessory type door with current state open and closed and a single target state limited to open to represent the latch instead of a second lock. maybe the current door state and target door state even work as optional characteristics in the lock service or accessory?

this might then even work in the home app as it would use accessory/service/characteristics that are more in line with what apple has intended for them instead of using a second lock for something it is not intended.

is this something you have already tried?

ebaauw commented 1 year ago

the door sensor does not show battery ok. i think you said it will show only the low warning?

I doens't even expose the critical state, so no low warning either. I issued a feature request for this on the Nuki developer forum, see https://developer.nuki.io/t/feature-request-expose-battery-status-for-external-door-sensor-to-smart-lock/22286.

the order in home is still not opimal with the latch before the lock

Did you remove the accessory from HomeKit and re-expose it? Home won't reflect changes on an accessory that has already paired, I fear. I think Home probably uses the order in which the services are exposed. I could also be a "feature" if the iOS 17 beta, I suppose. You might also try to set Show as Separate Tiles, so the two Lock Mechanism services are shown separately.

also the battery service is still primary on the lock accessory.

Not my doing. Again, did you remove and re-add the accessory to HomeKit?

in the mean time i have also connected a 2.0 keypad. maybe you could at it as an accessory even if it would only show the battery ok/low

I think Home would show an Unsupported tile for an accessory with only a Battery service (and that's hoping it won't kick the bridge for non-compliance). I could do another dummy switch to have a "normal" tile, but the Nuki bridge doesn't report any keypad events, so the switch would indeed be a dummy.

it just came to my mind that it might be better to combine the latch and the door sensor into an accessory type door with current state open and closed and a single target state limited to open to represent the latch instead of a second lock.

Haven't tried that; I think Door is relatively new in HomeKit. It would break Eve History on the door sensor, and it might be confusing that the battery reflects the sensor (if Nuki comes through), instead of the latch.

Note that the Unlatch is exposed as a custom characteristic on the Lock Mechanism service for the lock, so you might simply not expose the latch as separate Lock Mechanism service (through config.json setting). You can still use unlatch from Home or Siri, through a HomeKit scene (created in Eve or another decent app).

justme-1968 commented 1 year ago

the door sensor does not show battery ok. i think you said it will show only the low warning?

I doens't even expose the critical state, so no low warning either. I issued a feature request for this on the Nuki developer forum, see https://developer.nuki.io/t/feature-request-expose-battery-status-for-external-door-sensor-to-smart-lock/22286.

good. let's see if it happens.

the order in home is still not opimal with the latch before the lock

Did you remove the accessory from HomeKit and re-expose it? Home won't reflect changes on an accessory that has already paired, I fear. I think Home probably uses the order in which the services are exposed. I could also be a "feature" if the iOS 17 beta, I suppose. You might also try to set Show as Separate Tiles, so the two Lock Mechanism services are shown separately.

i will try. but that is not my experience. I can modify nearly everything on an accessory and I will be picked up after a little while. home itself has to be force quit sometimes to pick up the changes and read only characteristics are often only read on startup, but the api itself delivers nearly everything near realtime.

also the battery service is still primary on the lock accessory.

Not my doing. Again, did you remove and re-add the accessory to HomeKit?

will try.

in the mean time i have also connected a 2.0 keypad. maybe you could at it as an accessory even if it would only show the battery ok/low

I think Home would show an Unsupported tile for an accessory with only a Battery service (and that's hoping it won't kick the bridge for non-compliance). I could do another dummy switch to have a "normal" tile, but the Nuki bridge doesn't report any keypad events, so the switch would indeed be a dummy.

it just came to my mind that it might be better to combine the latch and the door sensor into an accessory type door with current state open and closed and a single target state limited to open to represent the latch instead of a second lock.

Haven't tried that; I think Door is relatively new in HomeKit. It would break Eve History on the door sensor, and it might be confusing that the battery reflects the sensor (if Nuki comes through), instead of the latch.

door (and also window) are there quite from the beginning.

Note that the Unlatch is exposed as a custom characteristic on the Lock Mechanism service for the lock, so you might simply not expose the latch as separate Lock Mechanism service (through config.json setting). You can still use unlatch from Home or Siri, through a HomeKit scene (created in Eve or another decent app).

ebaauw commented 1 year ago

v1.4.6 adds an accessory for a keypad, with a (dummy) stateless programmable switch and a Battery service.

ebaauw commented 1 year ago

it just came to my mind that it might be better to combine the latch and the door sensor into an accessory type door with current state open and closed and a single target state limited to open to represent the latch instead of a second lock.

That's not going to work. Door has current and target state, much like Window Covering, but the latch function doesn't set a target state. It just sends a pulse to unlatch the door, which might or might not open. After the pulse is done, the target state would become close, but there's no action to close the door.

justme-1968 commented 1 year ago

the idea was to combine the door sensor and the latch into the door. the sensor would give correct open/close for current and with validValues for targetState set only to open the only command would be to send open.

ebaauw commented 1 year ago

Yes, I understood that. Won’t be doing that.