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

New SDDP service for addon discovery and thing discovery #4237

Merged
merged 28 commits into from
May 30, 2024

Conversation

andrewfg
Copy link
Contributor

Resolves #4234

This PR provides an SDDP service for discovering equipment on the LAN. It has twofold application:

  1. Bindings can implement an SddpDiscoveryParticipant that allows discovering Things. It operates similarly to the MDNS and UPNP discovery services.
  2. The core uses the same service and implements an Addon Finder. It operates similarly to the MDNS and UPNP addon finders.

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg requested a review from a team as a code owner May 21, 2024 09:46
@andrewfg andrewfg marked this pull request as draft May 21, 2024 09:46
@andrewfg
Copy link
Contributor Author

Pinging @mherwege and @mlobstein for info..

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@wborn wborn added the enhancement An enhancement or new feature of the Core label May 21, 2024
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

Note to Reviewers: SDDP has a very low LAN footprint. In background (passive scan) mode it simply subscribes to, and listens on, the designated SDDP multicast address/port for UDP NOTIFY ALIVE SDDP datagrams. And (unlike UPNP) there is no further LAN traffic once a device is observed. And if an active scan is started, if makes just one single UDP SEARCH * SDDP ping on the LAN. Therefore I don't think there is any risk of LAN storms. So I think it is NOT necessary for this service to be un-installable. => WDYT?

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg marked this pull request as ready for review May 22, 2024 19:29
@andrewfg andrewfg marked this pull request as draft May 22, 2024 19:30
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

andrewfg commented May 25, 2024

Status Update

  1. @mherwege / @holgerfriedrich as usual I am struggling on how to get the features / components loaded by OH. There are basically two components -- an SddpDiscoveryService (which works similar to MdnsDiscoveryService to discover devices on the network), and an SddpAddonFinder (which works similar to MdnsAddonFinder). The latter component has an OSGI dependency of the former. I have got it working so far as the SddpDiscoveryService gets loaded, but unfortunately I cannot get the SddpAddonFinder to load. => So can you please help me with this?

  2. @mlobstein via the logging, I can see that the discovery service is passively finding stuff that sends SDDP NOTIFY on the LAN. However due to the above-mentioned lack of SddpAddonFinder it can not yet do a) active SEARCH * SSDP requests, nor b) match the actual found devices against the candidate bindings addon.xml. Currently I have two 'test' SDDP devices -- namely the HDPowerView Gen2 hub (which I think has a faulty implementation of SDDP), and a 'fake' device that I wrote myself for testing purposes; where the problem is that I wrote it myself as the mirror of my own code for the discovery service, so it is more or less guaranteed to 'work'. => So I will definitely need your testing @mlobstein to confirm that in particular the active SEARCH * SSDP process, and suggestion matching process do both work.


2024-05-25 18:20:54.825 [TRACE] [y.sddp.internal.SddpDiscoveryService] - SddpDiscoveryService() isBackgroundDiscoveryEnabled=true
2024-05-25 18:20:54.830 [TRACE] [y.sddp.internal.SddpDiscoveryService] - startBackgroundDiscovery()
2024-05-25 18:20:54.903 [DEBUG] [y.sddp.internal.SddpDiscoveryService] - listenMulticast() starting on interface 'Realtek PCIe GbE Family Controller'
2024-05-25 18:21:56.928 [TRACE] [y.sddp.internal.SddpDiscoveryService] - processPacket()
2024-05-25 18:21:56.928 [DEBUG] [y.sddp.internal.SddpDiscoveryService] - Packet received from '192.168.1.168:51009' content:
NOTIFY ALIVE SDDP/1.0
From: "192.168.1.168:1902"
Host: "Living-Room-Hub-0026740cf2b1"
Max-Age: 1800
Type: "hunterdouglas:hub:powerviewv2"
Primary-Proxy: "hunterdouglas_powerview_hub_cin"
Proxies: "hunterdouglas_powerview_hub_cin"
Manufacturer: "HunterDouglas"
Model: "PowerView"
Driver: "hunterdouglas_powerview_hub_cin.c4z"

2024-05-25 18:21:56.929 [DEBUG] [y.sddp.internal.SddpDiscoveryService] - processPacket() foundDevices=1, deviceParticipants=0, discoveryParticipants=0

image

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

andrewfg commented May 28, 2024

@mherwege my latest commit is now working correctly :) -- please have a look at it; and I will look at yours too.


2024-05-28 16:55:47.297 [DEBUG] [.discovery.sddp.SddpDiscoveryService] - listenActiveScanUnicast() starting on '192.168.1.65:65223' on interface 'Realtek PCIe GbE Family Controller'
2024-05-28 16:55:47.298 [DEBUG] [.discovery.sddp.SddpDiscoveryService] - Packet sent to '239.255.255.250:1902' content:
SEARCH * SDDP/1.0
Host: "192.168.1.65:65223"

2024-05-28 16:55:47.298 [TRACE] [.discovery.sddp.SddpDiscoveryService] - processPacket()
2024-05-28 16:55:47.299 [DEBUG] [.discovery.sddp.SddpDiscoveryService] - Packet received from '192.168.1.65:65223' content:
SEARCH * SDDP/1.0
Host: "192.168.1.65:65223"

2024-05-28 16:55:47.299 [TRACE] [.discovery.sddp.SddpDiscoveryService] - createSddpDevice()
2024-05-28 16:55:47.652 [TRACE] [.discovery.sddp.SddpDiscoveryService] - processPacket()
2024-05-28 16:55:47.652 [DEBUG] [.discovery.sddp.SddpDiscoveryService] - Packet received from '192.168.1.168:48983' content:
SDDP/1.0 200 OK
From: "192.168.1.168:1902"
Host: "Living-Room-Hub-0026740cf2b1"
Max-Age: 1800
Type: "hunterdouglas:hub:powerviewv2"
Primary-Proxy: "hunterdouglas_powerview_hub_cin"
Proxies: "hunterdouglas_powerview_hub_cin"
Manufacturer: "HunterDouglas"
Model: "PowerView"
Driver: "hunterdouglas_powerview_hub_cin.c4z"

2024-05-28 16:55:47.653 [TRACE] [.discovery.sddp.SddpDiscoveryService] - createSddpDevice()
2024-05-28 16:55:47.654 [TRACE] [discovery.addon.sddp.SddpAddonFinder] - deviceAdded()
2024-05-28 16:55:47.655 [DEBUG] [.discovery.sddp.SddpDiscoveryService] - processPacket() foundDevices=1, deviceParticipants=1, discoveryParticipants=0
2024-05-28 16:55:51.851 [INFO ] [e.automation.internal.RuleEngineImpl] - Rule engine started.
2024-05-28 16:55:59.386 [TRACE] [.discovery.sddp.SddpDiscoveryService] - processPacket()
2024-05-28 16:55:59.386 [DEBUG] [.discovery.sddp.SddpDiscoveryService] - Packet received from '192.168.1.65:4444' content:
NOTIFY ALIVE SDDP/1.0
From: "192.168.1.65:4444"
Host: "JVC_PROJECTOR-E0DADC152802"
Max-Age: 1800
Type: "JVCKENWOOD:Projector"
Primary-Proxy: "projector"
Proxies: "projector"
Manufacturer: "JVCKENWOOD"
Model: "DLA-RS3100_NZ8"
Driver: "projector_JVCKENWOOD_DLA-RS3100_NZ8.c4i"

2024-05-28 16:55:59.386 [TRACE] [.discovery.sddp.SddpDiscoveryService] - createSddpDevice()
2024-05-28 16:55:59.387 [TRACE] [discovery.addon.sddp.SddpAddonFinder] - deviceAdded()
2024-05-28 16:55:59.387 [DEBUG] [.discovery.sddp.SddpDiscoveryService] - processPacket() foundDevices=2, deviceParticipants=1, discoveryParticipants=0

@mherwege
Copy link
Contributor

My changes did indeed make it pass feature verification, but it will not work. You do indeed need to set the SddpDiscoveryService and register the participant, which was still missing in my version.
Very good you are getting much closer now!

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg
Copy link
Contributor Author

andrewfg commented May 29, 2024

@mlobstein I am currently running a full build of this PR together with your openhab/openhab-addons#16802 and openhab/openhab-addons#16794 to see if it all works as expected. I assume that you are doing the same thing? If so, then please provide feedback. Note that I don't own a physical Epson Projector so in my tests I am using a test app to simulate the projector. Therefore your own tests would ultimately be more definitive than mine.

@mlobstein
Copy link
Contributor

@mlobstein I am currently running a full build if this PR together with your openhab/openhab-addons#16802 and openhab/openhab-addons#16794 to see if it all works as expected. I assume that you are doing the same thing? If so, then please provide feedback. Note that I don't own a physical Epson Projector so in my tests I am using a test app to simulate the projector. Therefore your own tests would ultimately be more definitive than mine.

I just pushed a small fix to my branch. The SDDP discovery is working as expected. I also do not have a projector that supports SDDP. I am testing with my Pana UB-9000 by changing the strings in the DiscoveryParticipant. The device is discovered automatically and from an active scan. The discovered thing can be added successfully.
image

@andrewfg
Copy link
Contributor Author

andrewfg commented May 29, 2024

@mlobstein just to confirm that the core finder service does discover an addon for my 'fake' Epson device, and the discovery service in your binding does indeed find the respective thing -- see below

image


image

Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
Signed-off-by: Andrew Fiddian-Green <software@whitebear.ch>
@andrewfg andrewfg changed the title Add SDDP service New SDDP service for addon discovery and thing discovery May 30, 2024
@andrewfg
Copy link
Contributor Author

Note Java 17 CI build failed for some reason probably unrelated to this PR?

@andrewfg
Copy link
Contributor Author

@mherwege I would like to give you particular thanks for your help in general concerning OSGI component instantiation and referencing, and in specific concerning the respective karaf features construction. :)

I also want to thank you for your work on OH WebUI concerning the addon finders (enablement UI, display UI), as well as the primary IP selector page. The workflow on new installs is now very smooth and intuitive. Many thanks.

Copy link
Member

@holgerfriedrich holgerfriedrich left a comment

Choose a reason for hiding this comment

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

LGTM.
Thanks @andrewfg, and thank you all for this team effort!
Lets go for a try.

@holgerfriedrich holgerfriedrich merged commit c5336c5 into openhab:main May 30, 2024
5 checks passed
@holgerfriedrich holgerfriedrich added this to the 4.2 milestone May 30, 2024
@wborn
Copy link
Member

wborn commented Jun 1, 2024

This PR has introduced JavaDoc warnings causing the build to fail. Can you fix them @andrewfg?

See: https://ci.openhab.org/view/Documentation/job/openHAB-JavaDoc/779/console

[WARNING] Javadoc Warnings
[WARNING] /var/jenkins_home/workspace/openHAB-JavaDoc/bundles/org.openhab.core.config.discovery.sddp/src/main/java/org/openhab/core/config/discovery/sddp/SddpDiscoveryService.java:155: warning: invalid usage of tag {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] * Optionally create an {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] ^
[WARNING] /var/jenkins_home/workspace/openHAB-JavaDoc/bundles/org.openhab.core.config.discovery.sddp/src/main/java/org/openhab/core/config/discovery/sddp/SddpDiscoveryService.java:155: warning: invalid usage of tag {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] * Optionally create an {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] ^
[WARNING] /var/jenkins_home/workspace/openHAB-JavaDoc/bundles/org.openhab.core.config.discovery.sddp/src/main/java/org/openhab/core/config/discovery/sddp/SddpDiscoveryService.java:155: warning: invalid usage of tag {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] * Optionally create an {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] ^
[WARNING] /var/jenkins_home/workspace/openHAB-JavaDoc/bundles/org.openhab.core.config.discovery.sddp/src/main/java/org/openhab/core/config/discovery/sddp/SddpDiscoveryService.java:155: warning: invalid usage of tag {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] * Optionally create an {@link SddpDevice) object from UDP packet data if the data is good.
[WARNING] ^
[WARNING] Note: Custom tags that could override future standard tags:  @apiNote, @implNote. To avoid potential overrides, use at least one period character (.) in custom tag names.
[WARNING] Note: Custom tags that were not seen:  @apiNote, @implNote
[WARNING] 4 warnings

@andrewfg
Copy link
Contributor Author

andrewfg commented Jun 1, 2024

Can you fix them @andrewfg?

@wborn Umm. I would be happy to do that. But I am not sure actually what I have to do.

@wborn
Copy link
Member

wborn commented Jun 1, 2024

How about changing the ) in a } ?

@andrewfg
Copy link
Contributor Author

andrewfg commented Jun 1, 2024

@wborn #4265

@andrewfg andrewfg deleted the oh-core-sddp branch August 25, 2024 16:35
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 of the Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SDDP service to core
6 participants