Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[homekit] Add (partial) Doorbell Service #17130

Merged
merged 2 commits into from
Oct 28, 2024
Merged

Conversation

ccutrer
Copy link
Contributor

@ccutrer ccutrer commented Jul 22, 2024

This is not sufficient for HomeKit to present a doorbell, but it's a start. It just shows as an unsupported accessory with the house icon, and you can't have status or program actions on button presses.

Refs #9969

@ccutrer ccutrer requested review from andylintner and yfre as code owners July 22, 2024 19:12
@ccutrer
Copy link
Contributor Author

ccutrer commented Jul 22, 2024

This is dependent on #17129, even though it doesn't have conflicts. I'll rebase after that merges.

@lsiepel
Copy link
Contributor

lsiepel commented Jul 22, 2024

The DCO failed, previous commits did not, weird. Anyway, this is a step towards doorbell support but incomplete and therefore not adding any value to users yet? Do you know what needs to be done before the doorbell is functional?
It might be usefull to keep this PR as a draft while the doorbell is non-functional, WDYT?

@ccutrer
Copy link
Contributor Author

ccutrer commented Jul 22, 2024

Leaving as draft seems reasonable to me. The comments in the file detail what else needs to be implemented. The Speaker and Microphone services are trivial (the former is already implemented). But the Camera RTSP Service is quite large because it's far more than just a couple characteristics-- it's multiple additional protocols. I have no intentions of tackling it anytime soon, if ever.

@ccutrer ccutrer marked this pull request as draft July 22, 2024 20:04
@lolodomo lolodomo added the enhancement An enhancement or new feature for an existing add-on label Aug 8, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Oct 26, 2024

Can you outline the other parts that need to be worked on before this is functional? And if possible the technical difficulties?

@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 28, 2024

In HomeKit, a Doorbell is a both a Service and a Profile. An Accessory is composed of Services, typically an AccessoryInformationService and a single other service. A Profile is a common configuration of multiple services. The Video Doorbell Profile comprises the Doorbell Service (which is implemented in this PR), at the Camera RTP Stream Management Service, the Speaker Service, and the Microphone Service. The Home App on iOS won't do anything with an accessory that has a Doorbell service, but not those other services. The Speaker and Microphone Services are trivial (just mute and volume), so they are easily implemented. It's the Camera RTP Stream Management where things get complicated. The characteristics you publish are in a different format (TLV) from most other characteristics (simple JSON constructs), and they are various configuration type things describing what audio and video formats are supported, and parameters to negotiate an RTP connection with another service. openHAB doesn't really have the concept of a camera object - the IP Camera binding has a string channel for RTSP URL, but that's different from RTP. The RTP setup characteristic needs things like cryptography settings and encryption keys pre-negotiated. Not to mention that iOS only allows a very very small list of codecs. I have no interest in building all of this infrastructure in openHAB to do transcoding and such. Other projects such as go2rtc and scrypted are far better purpose built applications for handling the media-heavy pieces of exposing generic IP cameras to HomeKit.

@lsiepel
Copy link
Contributor

lsiepel commented Oct 28, 2024

ther projects such as go2rtc and scrypted are far better purpose built applications for handling the media-heavy pieces of exposing generic IP cameras to HomeKi

Totally understand that you don't want to spend all that time on transcoding etc to support just the doorbell. Especially as openHAB is juist moving the metadata around and not processing the actual stream and i don;t see that happening anytime soon. In that case i'll just merge it to atleast have this part of the puzzle.
Just wondering, the video part is mandatory, there are no doorbell lite variants that just do audio or just ring?

@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 28, 2024

Nope, no Doorbell lite. You can set up an accessory with just the Doorbell Service (which contains the critical Programmable Switch Event characteristic -- the button), but the Home App will just show it as "unsupported". Best you can do in that case is use a Stateless Programmable Switch accessory on its own.

@lsiepel lsiepel marked this pull request as ready for review October 28, 2024 20:02
@lsiepel
Copy link
Contributor

lsiepel commented Oct 28, 2024

Can you fix the build, then i'll merge it

@lsiepel lsiepel removed the enhancement An enhancement or new feature for an existing add-on label Oct 28, 2024
@lsiepel lsiepel changed the title [homekit] Add Doorbell Service [homekit] Add (partial) Doorbell Service Oct 28, 2024
this is _not_ sufficient for HomeKit to present a doorbell.
it just shows as an unsupported accessory with the house icon,
and you can't have status or program actions on button presses

refs openhab#9969

Signed-off-by: Cody Cutrer <cody@cutrer.us>
Signed-off-by: Cody Cutrer <cody@cutrer.us>
@ccutrer
Copy link
Contributor Author

ccutrer commented Oct 28, 2024

I rebased to fix the build, and also added another commit to add it to the readme, and fix the copyright year on the new file.

Copy link
Contributor

@lsiepel lsiepel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM.

@lsiepel lsiepel merged commit df365dc into openhab:main Oct 28, 2024
5 checks passed
@ccutrer ccutrer deleted the homekit-doorbell branch October 28, 2024 20:45
@lsiepel lsiepel added this to the 4.3 milestone Oct 28, 2024
@lsiepel
Copy link
Contributor

lsiepel commented Oct 28, 2024

Note: No label is added as this should not be listed on the release notes.

KaaNee pushed a commit to KaaNee/openhab-addons that referenced this pull request Nov 8, 2024
* [homekit] add Doorbell Service 
this is _not_ sufficient for HomeKit to present a doorbell.
it just shows as an unsupported accessory with the house icon,

Signed-off-by: Cody Cutrer <cody@cutrer.us>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Dec 16, 2024
* [homekit] add Doorbell Service 
this is _not_ sufficient for HomeKit to present a doorbell.
it just shows as an unsupported accessory with the house icon,

Signed-off-by: Cody Cutrer <cody@cutrer.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants