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

[govee] Addon info for suggestions #16109

Merged
merged 4 commits into from
Feb 16, 2024
Merged

Conversation

holgerfriedrich
Copy link
Member

Allow addon suggestion finders to send a discovery message to multicast address.
Requires openhab/openhab-core#3943

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
@holgerfriedrich holgerfriedrich added work in progress A PR that is not yet ready to be merged awaiting other PR Depends on another PR labels Dec 24, 2023
@holgerfriedrich
Copy link
Member Author

@stefan-hoehn it could look like this....

@stefan-hoehn
Copy link
Contributor

Is this meant to be work already?
Do you like me to try out?
Do you know why the build failed?

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
@holgerfriedrich
Copy link
Member Author

Hi @stefan-hoehn, sorry for the stupid copy-paste-error introduced when copying back from my test xml file.

If you have time to give it a try, yes, please!

Please install openhab/openhab-core#3943 first (I needed to delete the cache directory as well).
log:set TRACE org.openhab.core.config.discovery.addon.ip.IpAddonFinder will be helpful to debug.
Scan will run 20s after being scheduled.

For me it looks like this:

00:53:24.502 [TRACE] [nfig.discovery.addon.ip.IpAddonFinder] - Checking candidate: binding-govee
00:53:24.596 [TRACE] [nfig.discovery.addon.ip.IpAddonFinder] - binding-govee: probing 192.168.xxx.xxx -> 239.255.255.250:4001
00:53:24.597 [TRACE] [nfig.discovery.addon.ip.IpAddonFinder] - binding-govee: '{"msg": {"cmd": "scan", "data": {"account_topic": "reserve"}}}'
00:53:24.597 [TRACE] [nfig.discovery.addon.ip.IpAddonFinder] - binding-govee: 7b 22 6d 73 67 22 3a 20 7b 22 63 6d 64 22 3a 20 22 73 63 61 6e 22 2c 20 22 64 61 74 61 22 3a 20 7b 22 61 63 63 6f 75 6e 74 5f 74 6f 70 69 63 22 3a 20 22 72 65 73 65 72 76 65 22 7d 7d 7d
00:53:24.597 [TRACE] [nfig.discovery.addon.ip.IpAddonFinder] - binding-govee: listening on 192.168.xxx.xxx:4002 for 5000 ms
00:53:29.599 [TRACE] [nfig.discovery.addon.ip.IpAddonFinder] - binding-govee: no response received on 192.168.xxx.xxx

@stefan-hoehn
Copy link
Contributor

As discussed together (in the background to avoid noise here) the results are pretty promising:

After compiling and deploying the pending changes of openhab/openhab-core#3943 and applying the hereby provided config-xml for Govee the result is as follows

The finder receives a response of a Govee devices and marks the binding as a proposal:

2023-12-27 14:08:49.835 [DEBUG] [fig.discovery.addon.ip.IpAddonFinder] - IpAddonFinder::setAddonCandidates(414)
2023-12-27 14:08:49.836 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - Scheduling new IP scan
2023-12-27 14:09:09.838 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - IpAddonFinder::scan started
2023-12-27 14:09:09.841 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - Now Checking candidate: binding-knx
2023-12-27 14:09:09.843 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - Type: ipMulticast
2023-12-27 14:09:09.845 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - ip addresses: [192.168.178.153]
2023-12-27 14:09:09.848 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-knx: probing 192.168.178.153 -> 224.0.23.12:3671
2023-12-27 14:09:09.849 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-knx: 06 10 02 01 00 0e 08 01 c0 a8 b2 99 e9 c4
2023-12-27 14:09:09.851 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-knx: listening on 192.168.178.153:59844 for 5000 ms
2023-12-27 14:09:14.857 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-knx: no response received on 192.168.178.153
2023-12-27 14:09:14.859 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - Now Checking candidate: binding-govee
2023-12-27 14:09:14.860 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - Type: ipMulticast
2023-12-27 14:09:14.862 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - ip addresses: [192.168.178.153]
2023-12-27 14:09:14.863 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-govee: probing 192.168.178.153 -> 239.255.255.250:4001
2023-12-27 14:09:14.864 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-govee: '{"msg": {"cmd": "scan", "data": {"account_topic": "reserve"}}}'
2023-12-27 14:09:14.865 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-govee: 7b 22 6d 73 67 22 3a 20 7b 22 63 6d 64 22 3a 20 22 73 63 61 6e 22 2c 20 22 64 61 74 61 22 3a 20 7b 22 61 63 63 6f 75 6e 74 5f 74 6f 70 69 63 22 3a 20 22 72 65 73 65 72 76 65 22 7d 7d 7d
2023-12-27 14:09:14.866 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - binding-govee: listening on 192.168.178.153:4002 for 5000 ms
2023-12-27 14:09:14.883 [DEBUG] [fig.discovery.addon.ip.IpAddonFinder] - Received return frame from 192.168.178.134
2023-12-27 14:09:14.883 [DEBUG] [fig.discovery.addon.ip.IpAddonFinder] - Suggested add-on found: binding-govee
2023-12-27 14:09:14.884 [TRACE] [fig.discovery.addon.ip.IpAddonFinder] - IpAddonFinder::scan completed
image

Some thoughts and things worth mentioning: (@andrewfg @mherwege)

  • The binding is provided as "Suggested" even if the binding is already installed. This may be an edge case because I had to add my binding through the addon-folder to test the new config xml, so I can't avoid it to be active by default.
  • In the beginning I though there might be a battle between the finder receiving the discovery result on port 4002 and the binding itself but thinking about it more in depth, the chances are pretty low because this would only happen if the binding was scanning for things while the finder is active but that is unlikely as it means the binding IS already installed. And even IF that was the case (like mentioned above) the likelihood is pretty low that scanning a Thing would happen exactly at the same time will openHAB is starting up, triggering the IP finder while someone was scanning for new devices. So, to sum up: very likely we can neglect this risk.

Copy link
Contributor

@stefan-hoehn stefan-hoehn left a comment

Choose a reason for hiding this comment

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

Works like a charm, thanks @holgerfriedrich

@holgerfriedrich holgerfriedrich removed the work in progress A PR that is not yet ready to be merged label Dec 27, 2023
@holgerfriedrich holgerfriedrich added the enhancement An enhancement or new feature for an existing add-on label Dec 27, 2023
@holgerfriedrich holgerfriedrich removed the awaiting other PR Depends on another PR label Feb 14, 2024
@holgerfriedrich
Copy link
Member Author

openhab/openhab-core#3943 is merged, we could continue with this one

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!

@lsiepel lsiepel merged commit 2db9fb0 into openhab:main Feb 16, 2024
3 checks passed
@lsiepel lsiepel added this to the 4.2 milestone Feb 16, 2024
@stefan-hoehn
Copy link
Contributor

yeah 🎉

austvik pushed a commit to austvik/openhab-addons that referenced this pull request Mar 27, 2024
* [govee] Addon info for suggestions

Signed-off-by: Holger Friedrich <mail@holger-friedrich.de>
Signed-off-by: Jørgen Austvik <jaustvik@acm.org>
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.

None yet

4 participants