openhab-core
openhab-core copied to clipboard
Develop additional AddonSuggestionFinder components
@andrewfg It is maybe worth adding some futures to your architecture document as well (maybe make that an issue here as well):
- MQTT broker discovery
- discovery of JSON storage provider addons
- discovery of marketplace addons
- serial discovery (could be useful to discover and suggest ZWave and Zigbee dongles and suggest the binding)
- bluetooth discovery (may need moving some of the core bluetooth functionality from addons to core similar to mDNS and uPnP)
Note that Homeassistant does serial and bluetooth discovery on top of mDNS and SSDP (uPnP).
I don't know how feasible each of these is, but that would build upon what you are doing here.
Originally posted by @mherwege in https://github.com/openhab/openhab-core/issues/3806#issuecomment-1794586738
Other ideas:
- Use
ProcessHandle.allProcesses()
to get a list of process running on the server. We could first try to cover the optional components installable via openHABian:Thinks like influxdb, knxd, etc should be easy to detect.
- Multicast/Broadcast discovery: construct a special IP frame and check if a device responds. Not sure how to generalize the construction of the special IP frame. Would need either binding-specific finders in core (ugly but effective) or a way to blueprint IP frames with patterns for IP address and ports etc. See https://github.com/openhab/openhab-core/pull/3806#issuecomment-1795804713 Not sure which bindings besides KNX could be detected this way.
For 3rd Party JSON add-on service the discovery information could be made available from the addons.json
, but it would require the add-on service to be installed when the assistant runs. That would require the assistant to run constantly (and maybe showing suggestions in the add-on store instead of the suggestions we have now).
For 3rd Party JSON add-on service the discovery information could be made available from the
addons.json
, but it would require the add-on service to be installed when the assistant runs. That would require the assistant to run constantly (and maybe showing suggestions in the add-on store instead of the suggestions we have now).
In the proposed UI PR, the suggestions are shown both in the setup wizard and in the addon store (at the top). Of course that only works if the respective finders are running. I think switching specific finders off after initial setup should be optional (and granular). E.g. it could makes sense to keep mdns discovery running, but not upnp if no bindings relying on upnp are already installed (as upnp could have a strong performance impact). That’s why I was driving to create configuration options to enable/disable finders. In summary, it does already what you suggest.
A few things..
- I think the marketplace finder service may be quite generic.
- However I fear that the others, (e.g. udp multicast scan, http scan, serial port scan, usb device scan, or running applications scan) will need a specific custom finder for each addon. It's not a problem, but we would have to ask developers to create their own finder based on the standard interface.
- Apropos zwave and zigbee, it is not evident from those bindings' code whether the respective sticks can be auto detected at all. So I am not sure if we could detect such..
as upnp could have a strong performance impact
@mherwege do you have any actual reasons for making this assertion? Did anyone actually measure it? Upnp has two mechanisms for device discovery -- namely active polling with M-SEARCH requests, and then listening for two types of NOTIFY messages -- namely subscription renewal monocast notifications and general multicast ALIVE notifications. I don't know the code of Jupnp but IMHO it should be perfectly possible to configure it to run the M-SEARCH phase only when needed during OH startup and when the user looking in the addon store (say) but otherwise let it drop back to passive listening.
Upnp has two mechanisms for device discovery -- namely active polling with M-SEARCH requests, and then listening for two types of NOTIFY messages -- namely subscription renewal monocast notifications and general multicast ALIVE notifications. I don't know the code of Jupnp but IMHO it should be perfectly possible to configure it to run the M-SEARCH phase only when needed during OH startup and when the user looking in the addon store (say) but otherwise let it drop back to passive listening.
My understanding is that this is exactly what jupnp does. It does an M-SEARCH when it is started, and then keeps listening for NOTIFY messages. But there have been reports of this being a resource hog when the the network has a lot of upnp devices. I don't have the issue myself, just going on comments from other people, like here: https://community.openhab.org/t/high-cpu-usage-after-migration-to-oh4/148200/13
From reading that thread, it seems that https://bugs.java.com/bugdatabase/view_bug?bug_id=8301341 was the cause of the issue, (not actually a Jupnp issue at all) and so it will have been resolved by OH's respective work around.
.. and as a general rule, if there would be an error in Jupnp then we should resolve that error at the source, rather than making absurd contortions in other code to try stick band-aids over it .. (but that is just my opinion)
Unfortunately, the addon suggestion does not work for persistence. Just figured out that the process finder can detect InfluxDB (when added to addon.xml), but it will not be suggested, as only bindings are suggested. Persistence jdbc does not use addon.xml, but several addon-xxx.xml files and is not properly collected by the feature openhab-addons-external (and if you extend the pattern, the id in the xml is jdbc-xxx instead if jdbc). So openhab/openhab-addons#16031 is now only adding info for the bindings :-(
^ A few comments..
- The architecture comprises an AddonInfoProvider that provides candidates, and a SuggestionFinder that looks for a subset of the candidates that actually exist on the system.
- The current AddonInfoProvider indeed suggests binding candidates, it does so dynamically based on the bindings that will be packed in the kar
- However it is perfectly possible to create other AddonInfoProviders to suggest other addon candidates such as persistence; its data source could be static hard wired in code.
- We currently have two SuggestionFinders (mDNS and UPNP)
- However it is perfectly possible to create other SuggestionFinders
- The AddonInfoProviders and SuggestionFinders are OSGI components, so any or all SuggestionFinders can consume the candidate list from any or all AddonInfoProviders
So apropos the process suggestion finder issue, it seems to me that someone needs simply to create a matching candidate provider for it. In addition to the current candidate provider for addons in the kar.
—-
EDIT: or instead of writing a new AddonInfoProvider for persistence candidates, you could harness the existing AddonInfoProvider .. this loads candidates from xml files the userdata/addons folder. The binding candidates are in a file called I think openhab-addons.xml. So if you dropped in a file say openhab-persistence.xml in the same folder, the existing candidate provider would add its contents to the list of candidates (so long as the extra file follows the same syntax of the existing one)
Hi @andrewfg, thanks for the explanation and sorry for my comment above. I think I have to re-formulate my comment.
- AddonInfoProvider already provides the list of candidates also also for all add-on types.
- There is only one persistence add-on which does not use the standard addons.xml (persistence.jdbc), but several files. Those are currently not picked up by feature openhab-addons-external. This would be easy to change in pom.xml.
- SuggestionFinders seem to make not difference between addon-types (please correct me if I am wrong) and will report the result. I have seen then new process suggestion finder reporting that it found for example InfluxDB.
- In the wizard it works also for other addon-types! No need to adapt IDs for jdbc as mentioned above. The documentation button for jdbc-mariadb does not work, but I assume this is to the special format of the IDs in this add-on.
- When opening the Add-On Store, the GUI shows bindings and suggestions for bindings. Not sure what would be a good way to extend this to persistence etc. The big advantage for bindings is that this tab is selected by default, and the user sees the suggested bindings on the first screen. Other add-on types are less visible, even if we would extend the UI to show suggestions for other types, since users have to change the tab.
So basically the summary of the current state is: UI (Add-on store) may be extended for add-on types != binding.
@andrewfg I don’t think there is anything specific in the code that would restrict the finders to binding add-ons. In fact, it correct discovers the Neeo transport in my network. As long as the add-on has an addon.xml, it should be picked up. If that’s not the case (as @holgerfriedrich says) I need to look into why and it should be fixed.
The UI already show suggestions for transports. I may not have included persistence (need to check) as I assumed they were not discoverable with the logic so far. I can extend.
The UI already show suggestions for transports. I may not have included persistence (need to check) as I assumed they were not discoverable with the logic so far. I can extend.
@holgerfriedrich @andrewfg
I just checked. There is no specific tab in the add-on store for persistence. They are under ‘other’. I do expose a ‘Suggestions’ section under the ‘other’ tab already. That’s where my discovered transport appears, and I think suggested persistence add-ons should appear there as well. I did not implement suggestions in the ui and automation tabs.
An idea could be to also put suggestions in a separate tab that shows by default. Please create an enhancement request for it so it can get discussed.
@mherwege FYI, this is how it looks like for me; maybe my test setup is not completely up to date....
I will create a new issue later where we can discuss if it makes sense to increase visibility of the suggested add-ons.
@holgerfriedrich Here is how it looks for me:
@holgerfriedrich I found out what the issue in the UI is. I will provide a fix in the 'other' tab. As indicated, I would keep making them more prominent in a separate tab out of this quick fix.
PR created: https://github.com/openhab/openhab-webui/pull/2213
https://github.com/openhab/openhab-core/issues/3912#issuecomment-1851474817
Continuing from the above..
Can we ask @openhab/core-maintainers what is the official opinion about whether to suggest marketplace addons? IMHO if both OH and MP contain an addon of the same name, we should only suggest the OH one. But if OH does not (yet) contain such addon, then I see little problem to suggest an MP one (even with a warning message).
@mherwege I am a bit confused; can you please clarify what you mean by "3rd party add-ons" versus "market place add-ons"?
@mherwege I am a bit confused; can you please clarify what you mean by "3rd party add-ons" versus "market place add-ons"?
If you look in OH under settings, in the system settings columns, you see 2 sets of configuration options:
- Community Marketplace
- Json 3rd Party Add-on service
These are 2 different external add-on sources and have separate code in OH.
I believe @J-N-K maintains a 3rd-party add-on service with some bindings not available in the distribution and not in the marketplace. And there are others.
In case the add-ons are marked stable I don't see why we should not suggest them. For JSON we can easily enhance the JSON to provide the necessary information, but I have no idea how to do that with the community marketplace. We don't have a good way of providing metadata there and rely on the subject only.
I'm currently investigating if we can have different versions in one post, but that is also quite difficult, so just adding a file with metadata is not so easy to achieve.
After the release I'll try to refactor the KarafAddonService
so it better provides all necessary information. IMO all add-on information should be supplied by the AddonService
that provides the add-on, not by a different service. This would make a lot of things MUCH easier. However, it creates issues with add-ons on the addons folder.
We don't have a good way of providing metadata there and rely on the subject only.
I thought that would be the issue. Could it be an option to have the developer also provide the addon.xml file in the marketplace post? A file with that name could then be downloaded and interpreted. If the file is not there, we default to the current info and no add-on discovery.
I'll write a proposal for refactoring of all add-on services. As I already said: there are also issues with add-ons for different OH versions in one community marketplace post. Also the show potentially incompatible add-ons
switch should be removed, it makes it nearly impossible to achive the different-versions goal because the uid of the add-on is determined by the topic-id in the community forum.
In the same context, when developing the finder service, we struggled with the separation between the Addon class and AddonInfo class. It felt a bit complex, ad often the info is propagated from AddonInfo to Addon. So it is probably another thing to consider when refactoring.
Yes exactly. This has historical reasons but we can overcome that with the new aggregated addon.xml for all distribution add-ons. As I already said: this is also easy to achieve for the JSON add-on service. It's only difficult for the community marketplace and the addons-direcory (and probably Eclipse, I can't comment on that, I don't use Eclipse).
With all the finders in place, just one question regarding manually installed add-ons.... Would it make sense to suppress suggestions for add-ons installed manually? Just got KNX suggested - though my local build was running.
Yes, I'd think this would make a lot of sense!
With all the finders in place, just one question regarding manually installed add-ons.... Would it make sense to suppress suggestions for add-ons installed manually? Just got KNX suggested - though my local build was running.
I am not so sure. Often these are installed because there is no 'official' binding yet, and this is the development version. Seeing it appear in the suggestions may be a hint that it is now in the distribution. I am also not so sure it is easy to do, as the id is different for a manually installed add-on (through the add-ons folder) and the add-on from the distribution (or the marketplace). I don't think it hurts having it appear. As this is a 'novice' feature, manual binding installation is more advance and users would be able to interpret what they see. So far, only add-ons from the distribution are suggested. Marketplace and third-party add-ons would require work to be able to provide the meta-data to suggest them. In that case, the UI should probably be enhanced to clearly differentiate between these suggestions.
Ah, then I misunderstood the question - I assumed "manually installed add-ons" referred to distro add-ons that are already installed by the user. If it is about add-ons that are sourced from somewhere else and put into the addons folder, I agree with your arguments. I just wanted to make sure that users do not get suggestions for stuff that they already have.
I just wanted to make sure that users do not get suggestions for stuff that they already have.
And right now, it shows them in the add-on store suggestions, but with the label installed. So I would need to change that in the UI.
Extension of generic IP finder (towards OH 4.2): #3936