Repository navigation
Conversation
175f81a to
fb1842f
Compare
Pluggable HomekitAdvertiser lets callers use DnssdHomekitAdvertiser (JNA DNSServiceRegister) instead of JmDNS when mDNSResponder owns UDP 5353. Fixes hap-java#212.
fb1842f to
9c032ca
Compare
|
@yfre Eugen, this PR is necessary for a downstream fix in openHab openhab/openhab-addons#22013 |
|
@kgoderis how to reproduce the issue?
it does work without this PR. both Java HAP and mDNSResponder are bounded to the same port without any issues and i see Java HAP in mdns |
| int err = | ||
| dnsSd.DNSServiceRegister( | ||
| refOut, | ||
| 0, |
There was a problem hiding this comment.
0 means all interfaces, e.g. on my machine it was bounded to 13(!) interface including 127.0.0.1, ::1, and some virtual one. do we want to limit to one address / host or make it configurable? see also line 162 (host = null)
| public static HomekitAdvertiser auto(InetAddress localAddress) throws IOException { | ||
| if (DnssdHomekitAdvertiser.isSupported()) { | ||
| return new DnssdHomekitAdvertiser(); | ||
| } | ||
| return new JmdnsHomekitAdvertiser(localAddress); |
There was a problem hiding this comment.
i would prefer to able to force jmDNS usage, even on linux or macosx library. i.e. i could use share jmDNS instance from openHab. it should be configurable in openHAB and not automatic decision in Java HAP.
in addition, we only check whether library can be loaded but not whether it works.
| </dependency> | ||
|
|
||
| <!-- Native Bonjour / DNS-SD (macOS libSystem, Linux libdns_sd) --> | ||
| <dependency> |
There was a problem hiding this comment.
This makes JNA (~1.9 MB, native access) a compile-scope transitive dependency for every HAP-Java consumer, including everyone who only uses the JmDNS path. mvn dependency:tree shows net.java.dev.jna:jna:jar:5.14.0:compile.
Since native DNS-SD is opt-in, can this be optional? Consumers who want the native advertiser then declare JNA themselves. openhab-addons#22013 already embeds JNA in the bundle, so nothing changes for openHAB.
i.e.
net.java.dev.jna
jna
5.14.0
true
| try { | ||
| DnsSdLibrary.load(); | ||
| return true; | ||
| } catch (UnsatisfiedLinkError | Exception e) { |
There was a problem hiding this comment.
When JNA isn’t on the classpath, DnsSdLibrary.load() fails with NoClassDefFoundError: com/sun/jna/Library, which this catch doesn’t handle. I tested it: HomekitAdvertisers.isNativeDnssdSupported() throws instead of returning false.
Catching LinkageError covers both a missing JNA and a missing native library.
} catch (LinkageError | Exception e) {
| * Optional live registration against the host DNS-SD daemon (macOS mDNSResponder). Skipped when | ||
| * native DNS-SD is unavailable. Run with {@code mvn test} on macOS to exercise Bonjour IPC. | ||
| */ | ||
| public class DnssdLiveAdvertiserIT { |
There was a problem hiding this comment.
This test never runs. Surefire only picks up Test/Test/*Tests/*TestCase, and the pom has no failsafe plugin. It’s missing from target/surefire-reports after mvn test. Even if it ran, line 24 only checks isSupported() right after assumeTrue(isSupported()), so it can’t fail. It doesn’t confirm the service was registered or can be resolved.
CI runs on ubuntu-latest without libdns_sd, so even a working version would be skipped there. I’d suggest removing it and describing the manual macOS check (dns-sd -B / -L) in the PR description instead.
| public class HomekitAdvertisersTest { | ||
|
|
||
| @Test | ||
| public void isNativeDnssdSupportedMatchesAdvertiserDetection() { |
There was a problem hiding this comment.
isNativeDnssdSupported() just returns DnssdHomekitAdvertiser.isSupported(), so this test compares a value with itself. If HomekitAdvertisers is trimmed down, this test can go too.
| String setupId, | ||
| int configurationIndex, | ||
| int category) { | ||
| Map<String, String> props = new LinkedHashMap<>(); |
There was a problem hiding this comment.
Moving the JmDNS properties from HashMap to LinkedHashMap changes the TXT record order that existing users publish. On master dns-sd -L shows c#=1 s#=1 ff=0 sf=1 sh=… ci=2 pv=1.1 md=… id=…, and with this PR it’s insertion order. RFC 6763 says order doesn’t matter, so this is probably harmless. But it changes the existing JmDNS path, and JmdnsHomekitAdvertiserTest only checks sf and c#. Could you add a test that checks the full JmDNS property set is unchanged?
|
@yfre Thanks — useful that it works on your Mini. That difference is real, and it comes down to how UDP 5353 is bound on macOS, not to HAP-Java “always” failing. Same port, two processes — how?For normal unicast TCP/UDP, yes: one listener per So when you write that Java HAP and What Apple does in practice varies by boot/bind order:
How we reproduced #212On a native macOS openHAB host (Mac mini M2), not a setup where 5353 was already shared:
So we always took the dedicated JmDNS path against an exclusive Why this PR still helpsNative Happy to adjust the PR for your other review notes (optional JNA, |
Make JNA optional, catch LinkageError when it is absent, let callers force JmDNS or pass InetAddress to scope DNS-SD interface/host, deprecate auto(), and replace the non-running live IT with fuller TXT/JmDNS unit coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Pushed a follow-up commit addressing the review comments:
PR description updated accordingly. |
|
thank you @kgoderis for making adjustments. I tested locally, just hap-java without openhab, mac mini m4 pro (golden gate). without this PR I can pair the bridge in the home app no problem. with this PR using I think it's the host. shared port works fine here too btw. mDNSResponder always sets SO_REUSEPORT on 5353 (SetupSocket in mDNSMacOSX.c), and java does as well, tried 11, 21 and 27. you can see it in you also said openhab's own jmdns couldn't get 5353 either, so I'd rather think something's off on that machine. can you post that netstat output, the full stacktrace (openhab only logs the message) and your macos + jdk version? please test on your side and let me know. |
Keep DNS-SD host null so mDNSResponder publishes the system hostname with A/AAAA; only scope registration by interface index. A literal IP as host became an SRV target with no address record. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Fixed the SRV-host pairing hang you reported. Cause: Fix: Verified locally: |
|
Thanks @yfre — you were right about the host. I reproduced it here: with Fixed in the latest commit: On the 5353 / openHAB side, from Karel’s Mac mini M2:
Same Where it still fails for us is JmDNS as used by openHAB. Inside the live OH JVM (and standalone with OH’s jar):
Same machine, JmDNS 3.5.6 Live openHAB on this Mini with the native advertiser ( Happy to tweak further if you still see issues after the host=null fix. |
|
I tested the JmDNS versions and can confirm, the issue exists with JmDNS 3.6.4 only, which was in the openHAB snapshot for a few weeks in October. With the current snapshot (JmDNS 3.6.5) everything works without this PR on my mac. so the remaining question is more general: do we want native mDNS support? and if yes, wouldn't openHAB be a better place for it, since openHAB core has its own mdns service used by all bindings? @ccutrer what do you think? @kgoderis thanks a lot for your time on this and for the quick fixes. with the latest commit the native path works on my mac. there are still a few smaller things, but let's first agree on the main question before going into those. |
|
My take: 1. openHAB platform mDNS → openHAB core 2. HAP-Java → advertiser SPI (+ default JmDNS), not OH’s mDNS policy 3. Practical split
So: yes to native as a platform capability in openHAB if we still want it after 3.6.5; no as the primary fix living only in HAP-Java. I’m fine reshaping this PR toward a thin SPI (and keeping or dropping the native impl per what you and @ccutrer prefer), and putting any OH-wide Bonjour work on the core side instead. Happy to wait on that call before the smaller leftover nits. |
What
Adds a pluggable
HomekitAdvertiserand a native DNS-SD / Bonjour implementation (DnssdHomekitAdvertiser) that registers_hap._tcpviaDNSServiceRegister(JNA) without binding UDP 5353. Existing JmDNS constructors keep working.Factories on
HomekitAdvertisers:jmdns(JmDNS)/jmdns(InetAddress)— force JmDNS (e.g. share openHAB’s instance)nativeDnssd()— native on all interfacesnativeDnssd(InetAddress)— native scoped to that LAN address’s interface index + SRV hostisNativeDnssdSupported()— library load probe onlyauto(...)— deprecated; policy belongs in the application (openHAB)See
docs/NATIVE_DNSSD.md.Why
Fixes #212. On some macOS setups (exclusive
mDNSResponderon UDP 5353), a dedicated JmDNS advertiser fails with “Address already in use” / “no suitable mDNS” and Home never discovers the accessory. Other Macs share 5353 viaSO_REUSEPORT, so JmDNS can still work there — integrators should choose explicitly.Compatibility
HomekitServer/HomekitRootoverloads that takeHomekitAdvertiser.InetAddress/JmDNSconstructors unchanged.<optional>true</optional>). JmDNS-only consumers do not pull it transitively. Callers ofnativeDnssd()must declarenet.java.dev.jna:jna(openHAB already embeds it).Review follow-ups
LinkageErrorhandled inisSupported()interfaceIndex/hostvianativeDnssd(InetAddress)auto()deprecatedDnssdLiveAdvertiserITand tautologicalHomekitAdvertisersTestLinkedHashMapkey order; RFC 6763 order-independent)Testing
mvn teston JDK (local): unit suite greendns-sd -B/-LafternativeDnssd(bind); no Java UDP 5353 socketChecklist
CHANGES.mdmvn compile/ Google Java Format (fmt-maven-plugin)