Skip to content

Initial commit for integrating pure js Bonjour - #495

Closed
sokarovski wants to merge 2 commits into
homebridge:masterfrom
sokarovski:integrate_pure_js_bonjour
Closed

Initial commit for integrating pure js Bonjour#495
sokarovski wants to merge 2 commits into
homebridge:masterfrom
sokarovski:integrate_pure_js_bonjour

Conversation

@sokarovski

@sokarovski sokarovski commented Aug 23, 2017

Copy link
Copy Markdown
Contributor

Hey there, I did integration of pure JS mDNS library (Bonjour) so there is no need to depend on the libnss-mdns libavahi-compat-libdnssd-dev so it makes the install process easier also installing it on raspberry pi-w is faster by 11 mins because it doesn't build the C bindings and that was the main reason why I decided to do it. mDNS is simple UDP socket so I don't see any reason why it should be implemented in C and then wrapped rather than using pure JS to make things simpler, it's not something that is processor intensive and it will be a big load on node-js shoulders.

Not to mention that the ugly warning message Apple Bonjour compatibility layer of Avahi..... is no more.

The only thing that i noticed different is that when you unpublish a bridge it needs some time for the iOS to be aware that the bridge is no longer available. I tested the libavahi and it turns-out that avahi sends response with all services and ttl:0 right before exiting. So I may add this later in the Advertiser.prototype.stopAdvertising method or if it is bug in the Bonjour library and it should take care for it automatically i can send a pull request there with a fix too.

@pdlove

pdlove commented Aug 24, 2017

Copy link
Copy Markdown
Collaborator

I'll test with this tomorrow and see how it works on a few different OSs.

@KhaosT

KhaosT commented Aug 24, 2017

Copy link
Copy Markdown
Contributor

Thanks for the PR, it would be nice to get rid of binding. However, please be aware that HomeKit expects accessory to update TXT record when configuration changes (or when paired/unpaired), see updateAdvertisement. I briefly looked over the Bonjour library and it doesn't look like it can support update TXT record yet?

@sokarovski

Copy link
Copy Markdown
Contributor Author

@KhaosT after looking at the code a little bit better i do see that no (realtime) update is supported. If you change the txt record it will work but on the next query the mdns will not send the changes right away. However am still looking a way to fix this without intervention to the Bonjour library. If there is no way i will fork it and fix it there too.

@pdlove Thanks, I didnt test it on Windows, only macOS and Raspbian.

@sokarovski

Copy link
Copy Markdown
Contributor Author

I updated the bonjour library to support the txt update record and spent some time refactoring it. I tested it and it works good.

The issue with goodbye message is still present i will address it tomorrow.

Also not sure what happens with the avahi but i dont think this library does something if the IP address of the HAP changes (example new lease of DHCP server). I think I can also work around that with periodically looking for changes in the interfaces table.

@pdlove

pdlove commented Aug 25, 2017

Copy link
Copy Markdown
Collaborator

This is working in Windows. I had some trouble that adding accessories to the running bridge wasn't working, but that could be because of the TXT update record. When you've finished correcting the goodbye message issue I'll retest.

@yene

yene commented Sep 6, 2017

Copy link
Copy Markdown
Contributor

I think that bonjour library does also not resolve name conflict, and does not support '.' in the service name. We replaced it in our product with apples implementation.

@sokarovski

Copy link
Copy Markdown
Contributor Author

@yene what do you mean by "apples implementation" can you provide a link?

@yene

yene commented Sep 7, 2017

Copy link
Copy Markdown
Contributor

@piejanssens

Copy link
Copy Markdown

This would be the solution to many people's issues trying to run this project or a project depending on HAP-NodeJS inside a Docker container. What's the status?

@sokarovski

Copy link
Copy Markdown
Contributor Author

@piejanssens merging of this branch depends on another pull request watson/bonjour#41 . This pull request allows the watson/bonjour to be able to update TXT records. for now you can probably use my branch https://github.com/sokarovski/HAP-NodeJS/tree/integrate_pure_js_bonjour which pulls the bonjour from my repo but it's a bit outdated HAP.

@pdlove @KhaosT still interested in this? I have used it since august on my home automation system and it looks solid to me, i didn't had any problems. The problem with the "goodbye message" is because mdns library works as a separate daemon and can continue sending "goodbye messages" even if you kill the node while this JS implementation dies as soon as you kill node. Do you have any idea on how can we fix this? I was thinking of adding handler to process.on('SIGINT'...) to wait a couple of second before closing the node but this looks more like workaround then a solution.

@KhaosT

KhaosT commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

Definitely ^^ I think we can add a hook to give the library sometime to clear up (and send the message) before we actually quit…

@ebaauw

ebaauw commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

I think we can add a hook to give the library sometime to clear up (and send the message) before we actually quit…

Yes, please! I'd love such a hook for plugins as well, homebridge/homebridge#984.

@sokarovski

Copy link
Copy Markdown
Contributor Author

@KhaosT how will this effect projects that extend HAP-NodeJS? As @ebaauw I also have project on my own that is dependent on HAP-NodeJS what if my projects tries to handle cleanup on it's own then both of them will interrupt each other. My project can call process.exit(0) earlier than HAP-NodeJS requires to clean up. Also it can happen the other way around or maybe am wrong?

Maybe there is some library that handles this stuff or maybe we can use promises... have to do a little bit of a research.

@KhaosT

KhaosT commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

Ah that’s an interesting point... I guess the right way to do that is have caller to invoke Accessory.destroy() before quitting?

@piejanssens

Copy link
Copy Markdown

@sokarovski Looking at the lack of activity in the Bonjour repo since february even though there have been other PR's, I wouldn't count on watson/bonjour#41 getting merged any time soon. Once you find a solution for sending the goodbye messages I suggest to use your fork of Bonjour in HAP-NodeJS until mr Watson is on board.

@KhaosT

KhaosT commented Dec 20, 2017

Copy link
Copy Markdown
Contributor

Maybe we can do a fork and publish the package on npm with a postfix “-hap” for now?

@piejanssens

piejanssens commented Dec 20, 2017

Copy link
Copy Markdown

@KhaosT Indeed, simple as that. Once Watson is on board it's just a matter of changing the dependency.

@neophob

neophob commented Feb 8, 2018

Copy link
Copy Markdown
Contributor

why not simple use

    "bonjour": "sokarovski/bonjour#refactoring_and_update_txt_support",

in the package.json?

@oznu

oznu commented Feb 11, 2018

Copy link
Copy Markdown
Member

I wanted to test this out so I published the required packages under my own namespace.

It works really well, nice work @sokarovski. All my accessories still work and I didn't even need to set them up again.

I maintain a homebridge docker image and many users have had issues with avahi/mdns causing conflicts with the host or other containers. This PR should solve the problem.

@brandom

brandom commented Mar 13, 2018

Copy link
Copy Markdown

@KhaosT Any movement on this? I've been using a modified version of @oznu's docker image with the pure_js branch at home for quite some time now with no issues as well.

Also, @oznu, thanks for homebridge/docker-homebridge/pull/80! 🎉

@NorthernMan54

Copy link
Copy Markdown
Contributor

@sokarovski @pdlove @ebaauw @KhaosT @oznu @piejanssens Everyone, after watching some of the constant noise on the homebridge issues page, I think this is something we should push forward on. ( Also when I was launching my updated homebridge-alexa plugin, which also used MDNS, I found that about 30% of all users had challenges with installing due to MDNS ).

To get this ready for prime time I'm thinking we need to do this:

  1. @sokarovski can you publish your HAP branch of bonjour, as bonjour-hap on GitHub/npm

  2. Update the pull request based on the bonjour-hap branch, and do some final testing prior to merging it.

Once we can get these steps done, I think we are good to go.

@KhaosT

KhaosT commented May 5, 2018

Copy link
Copy Markdown
Contributor

As Node v10 introduces a lot of changes, I think it's worth push this forward. Since @sokarovski hasn't be active over the last few months, I decided to fork this branch and published bonjour-hap on npm. Additionally changes from this PR has been merged to the master, I will test it over this weekend and probably push it to npm by the end of this weekend.

Huge kudos to @sokarovski, thanks for the amazing change!

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.

10 participants