search

openhab / openhab-core

Core framework of openHAB
https://www.openhab.org/
Eclipse Public License 2.0
925 stars 425 forks source link

[Rfc] Addon finder for 'Always Suggest' #4103

Closed andrewfg closed 8 months ago

andrewfg commented 8 months ago

There is talk on the forum about some addons that should always be suggested e.g. Astro, or Weather.

Two questions..

  1. WDYT?
  2. If we do want it, then how to do it?

Concerning 2. we could either create a special new finder service, or simply misuse one of the existing ones with a wildcard match condition.

mherwege commented 8 months ago

This could potentially also be solved in the webui. E.g. for weather you could insert a step to list weather services and suggest one. Or it could be region based (there is already country info in addon metadata).

andrewfg commented 8 months ago

could potentially also be solved in the webui

That is true. If it is done in the UI then OH core maintainers must decide the list of addons to include. Whereas if it is done via addon.xml the addon maintainer can decide.

mherwege commented 8 months ago

Just linking back to some of the discussion on the forum: https://community.openhab.org/t/ideas-for-new-openhab-installation-experience/154042/43?u=mherwege

That is true. If it is done in the UI then OH core maintainers must decide the list of addons to include. Whereas if it is done via addon.xml the addon maintainer can decide.

I don't think it is one or the other. I think adding a suggestion finder that is returning suggestions for countries could be valuable (e.g. propose the binding for the local smart meter, a local disaster service...). This could indeed come from the binding addon.xml info. At the same time, proposing addons that are generically valuable should not be left to the addon developers. After all, everyone may think his addon is the one that cannot be missed. That should be a small list and better managed directly in the UI.

I also think persistence should be treated separately, as @florian-h05 suggests in the linked community post.

andrewfg commented 8 months ago

We could write two new addon finder services..

  1. One that selects countries in addon.xml
  2. One that delivers a hardwired list.

The advantage of that would be that all the code would be in OH core, rather than having to curate something separately in OH webui..

mherwege commented 8 months ago

2. One that delivers a hardwired list.

I don't like this being part of core. Core is agnostic of addons, apart from having mechanisms to find, install and use them. It does not keep any hardcoded list at all. I think we should keep it that way. There may be distributions that do not even include certain addons. That is not true for the webui. So maintaining it there makes more sense. The other logical place would be distro, but that would be a more significant change (and would also require changes in core to read and interpret that list from distro).

andrewfg commented 8 months ago

I don't like this being part of core.

Maybe in the addons.xml after all? The addon xml could contain a 'discovery-method' of type 'alwaysSuggest', and the finder can scan addons.xml (as for other finders) and return the respective addon id if it has such a disco method field.

andrewfg commented 8 months ago

Thinking about this further, the alwaysSuggest finder and the country finder are really the same thing. This finder would behave as follows..

  1. service type == alwaysSuggest && addon country == null => global always suggest
  2. service type == alwaysSuggest && addon country == [DE,FR,PL] => suggest if system country is in addon country list
mherwege commented 8 months ago

Thinking about this further, the alwaysSuggest finder and the country finder are really the same thing. This finder would behave as follows..

Yes, makes sense. But I have 2 concerns:

  1. I am not yet convinced we should leave this alwaysSuggest in the hands of the individual add-on developer. Doing it in mainUI avoids this.
  2. Extending the addon.xml again will have an extra backward compatibility impact. As long as we do not solve https://github.com/openhab/openhab-core/issues/4062 (and I don't see an easy way), this will keep hunting us.
andrewfg commented 8 months ago

To your point 1. => the PRs would be controlled by OH maintainers before they can be merged. To your point 2. => this would not be an extension of the addon.xml schema

florian-h05 commented 8 months ago

Nice proposal Andrew.

However, depending on what exactly the setup wizard should do, we still need to hard-code addon-specific stuff to the UI: For a weather service, we would need to ask the user to create an API token and give it to openHAB.

I have just checked which bindings provide weather forecasts and have requirements everyone should be able to satisify (e.g. no need to send personal weather station data to the service):

Looking at this list, I would say OpenWeatherMap should be "the" weather service promoted by the setup wizard and therefore 1. For northern Europe, FMI Weather could be provided as an alternative not requiring an API key.

DWD Unwetter and Meteo Alerte would however stil be good cancidates for a 2.

andrewfg commented 8 months ago

Note: the setting of an apiKey for a weather binding is no different from the setting of an apiKey for (say) the Hue binding. So IMHO it doesn't make any sense to have a UI magic wizard for the former and not the latter.

I think all config param settings should be done by the user in the (old) normal way.

And we should definitely exclude the setting of any config param values from the suggestion finder services.

mherwege commented 8 months ago

Note: the setting of an apiKey for a weather binding is no different from the setting of an apiKey for (say) the Hue binding. So IMHO it doesn't make any sense to have a UI magic wizard for the former and not the latter.

While in principle I agree with you I do think there is a difference here. If we all agree a few bindings make sense in (almost) all OH initial setup, we should make it as easy as possible for these. But we need an agreed list of these.

I would think the following would qualify:

I do think this part of the configuration flow should be mostly done in the UI, and therefore the addons to show as recommended with the extra flow should be different from suggested.

rkoshak commented 8 months ago

It's probably irrelevant for this discussion but IIRC Wunderground only allows free API keys if you have a contributing weather station.

Astro (does not require further configuration): can be offered as a suggestion easily

It does require the location to be set in the Regional settings though if the "local" Things are to be discovered.

Local alarm services: request the required parameters if needed

Which add-ons would be these? I'm just curious more than anything but based on my limited experience integrating with these are not always so easy to achieve.

I'd add to the list JS Scripting to be offered to enable Blockly.

I agree that the list of suggestions should be separately curated (whether it's in core or MainUI I don't have an opinion) and not left up to the add-on developers and adding one more thing for the maintainers to check in a review.

mherwege commented 8 months ago

It does require the location to be set in the Regional settings though if the "local" Things are to be discovered.

These are already set in the setup wizard (if not skipped).

I'd add to the list JS Scripting to be offered to enable Blockly.

Indeed (already there now). Another potential one would be the cloud connector. However, the configuration would need to be improved (not requiring to go and hunt for uid and secret on the file system).

Which add-ons would be these?

I was thinking about examples like DWD Unwetter and Meteo Alerte for Germany and France. These are not full weather services, only provide alarms.

rkoshak commented 8 months ago

I was thinking about examples like DWD Unwetter and Meteo Alerte for Germany and France. These are not full weather services, only provide alarms.

Oh, I was thing like home burglar alarms, not weather alerts. No wonder I was confused.

florian-h05 commented 8 months ago

It's probably irrelevant for this discussion but IIRC Wunderground only allows free API keys if you have a contributing weather station.

The README does not mention it, but still might be true. Technically seen Open Weather Map is the „better“ binding because it has forecast support. Optimally we could even deliver a weather widget with the setup wizard …

andrewfg commented 8 months ago

An alternate solution was implemented in webui so closing this.