[mielecloud] Fix i18n in addon.xml

[mhilbush]

Fix i18n tags in addon.xml so binding name and description displays correctly in binding list.

Signed-off-by: Mark Hilbush [email protected]

[mhilbush]

BTW, this problem may exist in other bindings. If this is the correct fix, I can submit a another PR for the others.

./bundles/org.openhab.binding.sagercaster/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/bindingName</name>
./bundles/org.openhab.binding.sagercaster/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/bindingDescription</description>
./bundles/org.openhab.binding.nanoleaf/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/binding.nanoleaf.name</name>
./bundles/org.openhab.binding.nanoleaf/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/binding.nanoleaf.description</description>
./bundles/org.openhab.binding.digitalstrom/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/binding_name</name>
./bundles/org.openhab.binding.digitalstrom/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/binding_desc</description>
./bundles/org.openhab.binding.nikohomecontrol/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/bindingName</name>
./bundles/org.openhab.binding.nikohomecontrol/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/bindingDescription</description>
./bundles/org.openhab.binding.jeelink/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/binding.jeelink.name</name>
./bundles/org.openhab.binding.jeelink/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/binding.jeelink.description</description>
./bundles/org.openhab.binding.velux/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/binding.velux.name</name>
./bundles/org.openhab.binding.velux/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/binding.velux.description</description>
./bundles/org.openhab.binding.amazondashbutton/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/bindingName</name>
./bundles/org.openhab.binding.amazondashbutton/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/bindingDescription</description>
./bundles/org.openhab.binding.pushbullet/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/binding.pushbullet.name</name>
./bundles/org.openhab.binding.pushbullet/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/binding.pushbullet.description</description>
./bundles/org.openhab.binding.urtsi/src/main/resources/OH-INF/addon/addon.xml:7:	<name>@text/bindingLabel</name>
./bundles/org.openhab.binding.urtsi/src/main/resources/OH-INF/addon/addon.xml:8:	<description>@text/bindingDescription</description>```

[jlaur]

Thanks! This is correct, nice catch. It would be much appreciated if you want to fix the other bindings as well.

[mhilbush]

It would be much appreciated if you want to fix the other bindings as well.

Certainly. I'll submit a PR for the rest of them.

[jimtng]

@mhilbush @jlaur could you please help me understand / point me to what addon.xml is used for / by, specifically the actual string that we entered inside addon.xml <name> and <description> tags.

<name>@text/addon.mielecloud.name</name>

I understand that there's i18n/mielecloud.properties which contains addon.mielecloud.name=Miele@home Cloud Binding

My current understanding is that as long as mielecloud.properties file contains the actual text that you want (as above), it doesn't matter if you entered any gibberish inside addon.xml.

The only time it matters what is entered inside addon.xml is the fact that the content of addon.xml is collected for all the addons, and combined into /runtime/etc/addons.xml which is used by AddonInfoAddonsXmlProvider

Oh, and also when you run the i18n tools, it would read what's in the addon.xml to generate the .properties file.

@merwege I understand you encountered problems with this in openhab/openhab-core#3908
However, there's this openhab/openhab-core#3865 (AddonInfoAddonsXmlProvider that I linked above).

So:

• We need to clean up addon.xml bindings, so that the <name> and <description> containing @text/xxxxx which AFAIK is useless (i.e. not actually used for what we might think it was intended for), are replaced with the actual text back from the i18n properties (English).
• Modify core to start picking up the info, especially description

All this is so we can have addons description even for uninstalled addons in the UI -> So that we can search in their description, see openhab/openhab-webui#3028

Once again, I still don't understand all this, so I'd appreciate any pointers and corrections.

[mherwege]

Thanks! This is correct, nice catch. It would be much appreciated if you want to fix the other bindings as well.

@jlaur @mhilbush I don't think you can make this assumption by just looking at the addon.xml file. The key there just needs to match the key in the properties file. You have the nikohomecontrol binding listed there as a problem, but it is correct in that case.

[mherwege]

• We need to clean up addon.xml bindings, so that the <name> and <description> containing @text/xxxxx which AFAIK is useless (i.e. not actually used for what we might think it was intended for), are replaced with the actual text back from the i18n properties (English).
• Modify core to start picking up the info, especially description

@jimtng openhab/openhab-core#3908 fixes an issue that was discovered after openhab/openhab-core#3865.

The fundamental problem is that i8n info for binding is only available after loading the binding jar or kar, as it is part of the jar. Just reading the addon.xml file will never give that information. But there is another source for binding name and description from the bundle list. So instead of trying to read it from the addon.xml, it just uses it from the bundle list. This will of course never be translated. That mechanism then also copes with any string reference you use in the addon.xml file.
If you would want to be able to translate in the addon suggestion phase, you would need to build a mechanism that extracts all these binding name and binding string translations, stores them separately and makes them available as an extra service. All of this extraction would need to happen in the build phase. I am not sure that's worth it.

So I don't think there is much to do here, except making sure the string keys are the same in the addon.xml and properties files. But the impact is only when the binding is already installed, not in the addon discovery phase.

[jimtng]

@mherwege, thanks for the extra info.

bundle list

What is this? I'm guessing this is the Karaf addons info which gets its data from the feature list. If this is the case, it does not have description, only label/name (which is called "description" but it's the same as "name" in addon.xml) - I know you already knew this... I'm just restating it here for my own clarity.

That mechanism then also copes with any string reference you use in the addon.xml file.

Could you elaborate this for me please

If you would want to be able to translate in the addon suggestion phase, you would need to build a mechanism that extracts all these binding name and binding string translations, stores them separately and makes them available as an extra service. All of this extraction would need to happen in the build phase.

Yes, I figured this out too, and I am contemplating this, or some (limited) variations of it.

With all this, could you please confirm whether this is correct (inside addon.xml), and who would actually perform the translation for it:

	<name>@text/addon.mielecloud.name</name>
	<description>@text/addon.mielecloud.description</description>

From my tests, it really doesn't matter if I entered any rubbish in it, e.g.

	<name>asdfasdf</name>
	<description>asdfasdf</description>

Once the binding is built into a jar, everything I know of would report the name/description from what's inside the .properties file, and not the gibberish I entered in addons.xml

[mherwege]

What is this? I'm guessing this is the Karaf addons info which gets its data from the feature list. If this is the case, it does not have description, only label/name (which is called "description" but it's the same as "name" in addon.xml) - I know you already knew this... I'm just restating it here for my own clarity.

Yes, indeed, the feature list. And you are right. That does not include the addon descriptions.

Could you elaborate this for me please

I am not an expert on i8n, but my understanding is this uses a standard Java mechanism. In OH we have the TranslationProvider service that makes this available. The implementation of it is here I believe: https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core/src/main/java/org/openhab/core/internal/i18n/I18nProviderImpl.java

Once the binding is built into a jar, everything I know of would report the name/description from what's inside the .properties file, and not the gibberish I entered in addons.xml

Even after installing the binding? I am surprised by that. I would expect the code to use the TranslationProvider to pick up the translation from the key in addon.xml to the properties file.
Did you try changing the language of your environment?
Here is where this should get resolved from the addon bundle: https://github.com/openhab/openhab-core/blob/main/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/AddonI18nUtil.java
And indeed, when it doesn't find @text in the value, it probably just uses addon.addonID.name as the key (see https://github.com/openhab/openhab-core/blob/ce374252fa2c821103da888302f930c1c127f73c/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/AddonI18nUtil.java#L51).

[jimtng]

Thanks for the pointers!

Even after installing the binding?

Yes, I believe so. I checked this by writing code (in jruby) that queries org.openhab.core.addon.AddonService and org.openhab.core.addon.AddonInfoRegistry OSGi services.

Did you try changing the language of your environment?

I don't think that's necessary? My code requests specific locales in the call to getAddons(locale) and getAddonInfo(id, locale), and I can confirm I am seeing the different languages being returned in the description.

And indeed, when it doesn't find @text in the value, it probably just uses addon.addonID.name as the key (see https://github.com/openhab/openhab-core/blob/ce374252fa2c821103da888302f930c1c127f73c/bundles/org.openhab.core.addon/src/main/java/org/openhab/core/addon/internal/AddonI18nUtil.java#L51).

Thank you! That really says that if you entered anything but @text/, it will be ignored and the text will then be inferred from the addon.addonID.name. So the only time you'd need to use @text/ is when you need to use a non-standard id.

So given this fact, why don't we just populate all addon.xml with the actual text, which we can then use in AddonInfoAddonsXmlProvider for the purpose of providing descriptions to webui? Even if we just have English, it is still going to be a big improvement to allow people to search inside the description.

Of course, in the conversion process of addon.xml, we'll need to ensure any replacements will still result in the same text for translation purposes.

WDYT?

[jlaur]

So given this fact, why don't we just populate all addon.xml with the actual text

IMHO that would be great. For completeness, I'll just add that in theory you have no redundancy when having:

openhab-addons/bundles/org.openhab.binding.mielecloud/src/main/resources/OH-INF/addon/addon.xml

Lines 7 to 8 in d0b918e

	<name>@text/addon.mielecloud.name</name>
	<description>@text/addon.mielecloud.description</description>

pointing towards:

openhab-addons/bundles/org.openhab.binding.mielecloud/src/main/resources/OH-INF/i18n/mielecloud.properties

Lines 2 to 3 in d0b918e

	addon.mielecloud.name=Miele@home Cloud Binding
	addon.mielecloud.description=This is the cloud-based Miele@home binding.

With:

	<name>Miele@home Cloud Binding</name>
	<description>This is the cloud-based Miele@home binding.</description>

you have the same strings in two places.

OTOH, with the i18n:generate-default-translations approach we can consider the properties file as (partially) auto-generated rather than manually created, so the redundancy shouldn't matter as long as we remember to run the tool after changing texts. This also dramatically reduces the risk of human mistakes of providing invalid @text/key references.

[mherwege]

So this would mean:

1. For all addons, in the addon.xml file, replace the name and description field content with the actual strings, without @text/ at the start.
2. Generate the default translations (and remove the old keys from the property files).

From reading through the code, I believe the name and description should then be available in English when the addon is not installed, and in local language when it gets installed.

We might want a script to run to make these changes to all addons in the repository in one go.

[jimtng]

We might want a script to run to make these changes to all addons in the repository in one go.

There aren't that many. I can do it manually. It's just a one-off operation.

[jimtng]

OTOH, with the i18n:generate-default-translations approach we can consider the properties file as (partially) auto-generated rather than manually created, so the redundancy shouldn't matter as long as we remember to run the tool after changing texts. This also dramatically reduces the risk of human mistakes of providing invalid @text/key references.

Indeed. This is why I was confused when I discovered this @text/key inside addon.xml. But the i18n:generate-default-translations is smart enough not to touch the @text/ stuff.

A case when this @text/ may be handy is to remove duplications (e.g. multiple configuration parameters with the same description perhaps?) but that doesn't apply here for name/description.

[mherwege]

A case when this @text/ may be handy is to remove duplications (e.g. multiple configuration parameters with the same description perhaps?) but that doesn't apply here for name/description.

And it is also handy when you need string replacements inside the string to be translated. But again, this does not apply for the binding name and description.

There aren't that many. I can do it manually. It's just a one-off operation.

When you do that, can you also try to check if there are already translations to keep these? Or would they have to be redone in Crowdin anyway to stay in sync?

[jimtng]

There aren't that many. I can do it manually. It's just a one-off operation.

When you do that, can you also try to check if there are already translations to keep these? Or would they have to be redone in Crowdin anyway to stay in sync?

Already started doing this. Found only 11 bindings.

I was told not to touch the other languages, so I'll only change the main .properties (if needed)

[jimtng]

Found more bugs in core while doing this, but it's not immediately related... so I'll deal with this later:

• built a bundle, e.g. icloud with custom name/description (e.g. XXXiCloud Binding). Then dropped that jar in my addons/ folder, then this happens:

The "official" binding shouldn't be copying the jar binding's name/description. But this is probably debatable, since they both share the same addon uid, just from different addonprovider

There are also other issues, not directly related to the topic at hand... for another time to deal with.

Just making a note here to remind myself

[mherwege]

The "official" binding shouldn't be copying the jar binding's name/description. But this is probably debatable, since they both share the same addon uid, just from different addonprovider

The addon info merge treats what is coming from addon.xml as non-master and something coming from a bundle as master. This is to make sure the translations are applied. If it comes from addon.xml, it cannot be applied. If it comes from a jar, translations could be there. So as this is the same addon id, I think this is acceptable here.

[mherwege]

There are also other issues, not directly related to the topic at hand... for another time to deal with.

What are these issues? Maybe create an issue for them.

[jimtng]

There are also other issues, not directly related to the topic at hand... for another time to deal with.

What are these issues? Maybe create an issue for them.

I noticed that the JarFileAddonService doesn't utilise localization. I haven't fully delved into it though as I didn't want to go on a tangent every time I found something odd (at least to me).

[jimtng]

The "official" binding shouldn't be copying the jar binding's name/description. But this is probably debatable, since they both share the same addon uid, just from different addonprovider

The addon info merge treats what is coming from addon.xml as non-master and something coming from a bundle as master. This is to make sure the translations are applied. If it comes from addon.xml, it cannot be applied. If it comes from a jar, translations could be there. So as this is the same addon id, I think this is acceptable here.

I guess the issue here is that the karaf / official addons should not be mixed up with jar addons. But it's not a big issue, so I'm happy to let it be for now.

Theoretically someone can build a bunch of bogus jar addons that would override official bindings' name / descriptions.