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

[hdpowerview] Improve Gen 1/2 discovery label #16865

Merged
merged 3 commits into from
Jun 15, 2024

Conversation

jlaur
Copy link
Contributor

@jlaur jlaur commented Jun 12, 2024

This is a minor improvement to show the specific Hub discovered, e.g. "PowerView Gen 2 Hub" rather than "PowerView Gen 1/2 Hub" (introduced with #13355).

For reference:

Signed-off-by: Jacob Laursen <[email protected]>
@jlaur jlaur added the enhancement An enhancement or new feature for an existing add-on label Jun 12, 2024
@andrewfg
Copy link
Contributor

@jlaur thank you for this. But..

An extra HTTP GET request is not required to make the distinction between Gen 1 and 2 hubs. The SDDP and MDNS response telegrams already contain distinguishing markers..

  • SDDP the type header ends with 'v2'
  • MDNS the 'md' property contains '2.0'

@jlaur
Copy link
Contributor Author

jlaur commented Jun 13, 2024

@jlaur thank you for this. But..

An extra HTTP GET request is not required to make the distinction between Gen 1 and 2 hubs. The SDDP and MDNS response telegrams already contain distinguishing markers..

  • SDDP the type header ends with 'v2'
  • MDNS the 'md' property contains '2.0'

Thanks, I will adapt and test this. Since we don't know what Gen 1 Hub provides, I guess we can consider "v2"/"2.0" as Gen 2 and everything else as Gen 1.

@jlaur
Copy link
Contributor Author

jlaur commented Jun 13, 2024

  • SDDP the type header ends with 'v2'
  • MDNS the 'md' property contains '2.0'

I can confirm that type in SddpDevice is "hunterdouglas:hub:powerviewv2". I can't figure out how to get the md property for mDNS, can you advise? I've tried service.getPropertyNames() which doesn't give any results, and service.getPropertyString("md") returns null. In the Android app mDNS Discovery I only see IP Address, Port and Service Type.

@andrewfg
Copy link
Contributor

Ah. The v2 hub also responds on '_hap._tcp.local' and this is where it reports an 'md' property. But, (not having a v1 hub), I don't know if that one also answers on '_hap._tcp.local'..

@jlaur jlaur force-pushed the hdpowerview-discovery-label branch from 3fd3c9f to e8ddd15 Compare June 13, 2024 21:21
Signed-off-by: Jacob Laursen <[email protected]>
@jlaur jlaur force-pushed the hdpowerview-discovery-label branch from bfca0f1 to 27e2004 Compare June 13, 2024 21:35
@jlaur
Copy link
Contributor Author

jlaur commented Jun 13, 2024

Ah. The v2 hub also responds on '_hap._tcp.local' and this is where it reports an 'md' property. But, (not having a v1 hub), I don't know if that one also answers on '_hap._tcp.local'..

Thanks, that worked better for finding the md property. I guess it's tricky then.

Assuming that Gen 1 doesn't respond on _hap._tcp.local, we would need two different MDNSDiscoveryParticipant implementations: _powerview._tcp.local for Gen 1 and _hap._tcp.local for Gen 2. But the Gen 2 Hub would respond to both, so this would cause duplicate discovery with different labels. I don't see a good solution for this, and the only benefit would be one HTTP call avoided when finding a Gen 2 Hub through mDNS.

@andrewfg
Copy link
Contributor

^
Yeah. I think that for MDNS an extra GET is indeed the only viable solution.

@jlaur
Copy link
Contributor Author

jlaur commented Jun 14, 2024

Yeah. I think that for MDNS an extra GET is indeed the only viable solution.

So it's ready for review. 🙂

Copy link
Contributor

@andrewfg andrewfg left a comment

Choose a reason for hiding this comment

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

LGTM

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 5e15726 into openhab:main Jun 15, 2024
5 checks passed
@lsiepel lsiepel added this to the 4.2 milestone Jun 15, 2024
@jlaur jlaur deleted the hdpowerview-discovery-label branch June 15, 2024 19:50
psmedley pushed a commit to psmedley/openhab-addons that referenced this pull request Jun 29, 2024
* Improve Gen 1/2 discovery label

Signed-off-by: Jacob Laursen <[email protected]>
pgfeller pushed a commit to pgfeller/openhab-addons that referenced this pull request Sep 29, 2024
* Improve Gen 1/2 discovery label

Signed-off-by: Jacob Laursen <[email protected]>
Signed-off-by: Patrik Gfeller <[email protected]>
joni1993 pushed a commit to joni1993/openhab-addons that referenced this pull request Oct 15, 2024
* Improve Gen 1/2 discovery label

Signed-off-by: Jacob Laursen <[email protected]>
matchews pushed a commit to matchews/openhab-addons that referenced this pull request Oct 18, 2024
* Improve Gen 1/2 discovery label

Signed-off-by: Jacob Laursen <[email protected]>
cipianpascu pushed a commit to cipianpascu/openhab-addons that referenced this pull request Jan 2, 2025
* Improve Gen 1/2 discovery label

Signed-off-by: Jacob Laursen <[email protected]>
Signed-off-by: Ciprian Pascu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature for an existing add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants