Apollon77 / hap-controller-node

Node.js library to implement a HAP (HomeKit) controller
Mozilla Public License 2.0
54 stars 16 forks source link

Only receiving 1 address #192

Closed ejb1123 closed 1 year ago

ejb1123 commented 1 year ago

I only seem to be getting 1 of the addresses from the discovery. I found that devices can have both v4 and v6 addresses. sometimes I get a v4 address and other times I get a v6 address. here is where I found it happening at.

Apollon77 commented 1 year ago

On first check that seems to be a limitation of the used dnssd library which uses the instance ale as identifier. So it seems which ever up Type is announced "first" after start is used. And the other one is ignored kind of.

In which case this is an issue? Should be irrelevant if we connect to ipv4 or ipv6 unless you use the up as "key" of the device ?!

ejb1123 commented 1 year ago

So I have homekit bridges/accessories that do not properly grab ipv6 addresses on my network. they only support ipv6 link local addresses and do not grab addresses being handed out by by router. I do have other bridges/accessories that grab proper Global Unicast Addresses. I do not use IP as the default key as IPs are not guaranteed to be the same. If you run the code below you will see that more the 1 IP is returned on the serviceUp event.

const dnssd = require('dnssd');
const browser = new dnssd.Browser(new dnssd.ServiceType('_hap._tcp'))
browser.on('serviceUp', service => console.log("Device up: ", service))
browser.on('serviceDown', service => console.log("Device down: ", service))
browser.start();
Apollon77 commented 1 year ago

Ok then the reason us easy ... https://github.com/Apollon77/hap-controller-node/blob/62dbbb75063473404e9be58bdcf39935f7282226/src/transport/ip/ip-discovery.ts#L167 ... need to think how to adjust this in a compatible way ...

Apollon77 commented 1 year ago

In fact I only see two options ...

Opinions?

ejb1123 commented 1 year ago

I prefer taking the route of not breaking compatibility

compatible: we still return the first address in property address (maybe then favoring IPv4) and add a new allAddresses with the array

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within the next 7 days. Please check if the issue is still relevant in the most current version of the adapter and tell us. Also check that all relevant details, logs and reproduction steps are included and update them if needed. Thank you for your contributions. Dieses Problem wurde automatisch als veraltet markiert, da es in letzter Zeit keine Aktivitäten gab. Es wird geschlossen, wenn nicht innerhalb der nächsten 7 Tage weitere Aktivitäten stattfinden. Bitte überprüft, ob das Problem auch in der aktuellsten Version des Adapters noch relevant ist, und teilt uns dies mit. Überprüft auch, ob alle relevanten Details, Logs und Reproduktionsschritte enthalten sind bzw. aktualisiert diese. Vielen Dank für Eure Unterstützung.