[i18n] Added translation feature for label and description of channels inside a channel-group-type #5751

cweitkamp · 2018-06-18T20:55:14Z

Added translation feature for label and description of channels inside a channel-group-type
Refactoring for centralized handling of translations in ThingTypeI18nLocalizationService
Switched to ChannelTypeBuilder API
Introduced a ChannelGroupTypeBuilder
Updated documentation

See https://github.com/openhab/openhab2-addons/pull/3678#issuecomment-397965219

Signed-off-by: Christoph Weitkamp github@christophweitkamp.de

cweitkamp · 2018-06-18T21:01:40Z

...ng/src/main/java/org/eclipse/smarthome/core/thing/i18n/ThingTypeI18nLocalizationService.java

+ String description = thingTypeI18nUtil.getChannelGroupDescription(bundle, channelGroupTypeUID,
+ channelGroupType.getDescription(), locale);
+
+ List<ChannelDefinition> localizedChannelDefinitions = new ArrayList<>(


The lines 119-142 are almost identical to 154-176 but I could not find a way to extract a method for them. Any thoughts?

Difficult to identify the problem in the Github diff view only. What's the problem?

@maggu2810 Thanks for your review.

This is the duplicate code which I want to extract into a new method. The problem is that I want to determine the channelLabel and channelDescription inside the foreach loop derived from either the thingTypeUID or the channelGroupTypeUID - I marked the code with **.

private List<ChannelDefinition> createLocalizedChannelDefinitions(Bundle bundle, List<ChannelDefinition> channelDefinitions, Locale locale) { // start List<ChannelDefinition> localizedChannelDefinitions = new ArrayList<>(channelDefinitions.size()); for (ChannelDefinition channelDefinition : channelDefinitions) { String channelLabel = thingTypeI18nUtil.getChannelLabel(bundle, **<thingTypeUID|channelGroupTypeUID>**, channelDefinition, channelDefinition.getLabel(), locale); String channelDescription = thingTypeI18nUtil.getChannelDescription(bundle, **<thingTypeUID|channelGroupTypeUID>**, channelDefinition, channelDefinition.getDescription(), locale); if (channelLabel == null || channelDescription == null) { ChannelTypeUID channelTypeUID = channelDefinition.getChannelTypeUID(); ChannelType channelType = channelTypeRegistry.getChannelType(channelTypeUID, locale); if (channelType != null) { if (channelLabel == null) { channelLabel = thingTypeI18nUtil.getChannelLabel(bundle, channelTypeUID, channelType.getLabel(), locale); } if (channelDescription == null) { channelDescription = thingTypeI18nUtil.getChannelDescription(bundle, channelTypeUID, channelType.getDescription(), locale); } } } localizedChannelDefinitions .add(new ChannelDefinition(channelDefinition.getId(), channelDefinition.getChannelTypeUID(), channelDefinition.getProperties(), channelLabel, channelDescription)); } // end return localizedChannelDefinitions; }

diff --git a/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/i18n/ThingTypeI18nLocalizationService.java b/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/i18n/ThingTypeI18nLocalizationService.java index df5c1c14fb..2a2b50c766 100644 --- a/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/i18n/ThingTypeI18nLocalizationService.java +++ b/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/i18n/ThingTypeI18nLocalizationService.java @@ -15,6 +15,7 @@ package org.eclipse.smarthome.core.thing.i18n; import java.util.ArrayList; import java.util.List; import java.util.Locale; +import java.util.function.Function; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -116,31 +117,12 @@ public class ThingTypeI18nLocalizationService { String description = thingTypeI18nUtil.getChannelGroupDescription(bundle, channelGroupTypeUID, channelGroupType.getDescription(), locale); - List<ChannelDefinition> localizedChannelDefinitions = new ArrayList<>( - channelGroupType.getChannelDefinitions().size()); - for (ChannelDefinition channelDefinition : channelGroupType.getChannelDefinitions()) { - String channelLabel = thingTypeI18nUtil.getChannelLabel(bundle, channelGroupTypeUID, channelDefinition, - channelDefinition.getLabel(), locale); - String channelDescription = thingTypeI18nUtil.getChannelDescription(bundle, channelGroupTypeUID, - channelDefinition, channelDefinition.getDescription(), locale); - if (channelLabel == null || channelDescription == null) { - ChannelTypeUID channelTypeUID = channelDefinition.getChannelTypeUID(); - ChannelType channelType = channelTypeRegistry.getChannelType(channelTypeUID, locale); - if (channelType != null) { - if (channelLabel == null) { - channelLabel = thingTypeI18nUtil.getChannelLabel(bundle, channelTypeUID, channelType.getLabel(), - locale); - } - if (channelDescription == null) { - channelDescription = thingTypeI18nUtil.getChannelDescription(bundle, channelTypeUID, - channelType.getDescription(), locale); - } - } - } - localizedChannelDefinitions - .add(new ChannelDefinition(channelDefinition.getId(), channelDefinition.getChannelTypeUID(), - channelDefinition.getProperties(), channelLabel, channelDescription)); - } + List<ChannelDefinition> localizedChannelDefinitions = localizeChannelDefinitions(bundle, channelGroupType.getChannelDefinitions(), + channelDefinition -> thingTypeI18nUtil.getChannelLabel(bundle, channelGroupTypeUID, channelDefinition, + channelDefinition.getLabel(), locale), + channelDefinition -> thingTypeI18nUtil.getChannelDescription(bundle, channelGroupTypeUID, + channelDefinition, channelDefinition.getDescription(), locale), + locale); return new ChannelGroupType(channelGroupTypeUID, channelGroupType.isAdvanced(), label, description, channelGroupType.getCategory(), localizedChannelDefinitions); @@ -151,30 +133,12 @@ public class ThingTypeI18nLocalizationService { String label = thingTypeI18nUtil.getLabel(bundle, thingTypeUID, thingType.getLabel(), locale); String description = thingTypeI18nUtil.getDescription(bundle, thingTypeUID, thingType.getDescription(), locale); - List<ChannelDefinition> localizedChannelDefinitions = new ArrayList<>(thingType.getChannelDefinitions().size()); - for (ChannelDefinition channelDefinition : thingType.getChannelDefinitions()) { - String channelLabel = thingTypeI18nUtil.getChannelLabel(bundle, thingTypeUID, channelDefinition, - channelDefinition.getLabel(), locale); - String channelDescription = thingTypeI18nUtil.getChannelDescription(bundle, thingTypeUID, channelDefinition, - channelDefinition.getDescription(), locale); - if (channelLabel == null || channelDescription == null) { - ChannelTypeUID channelTypeUID = channelDefinition.getChannelTypeUID(); - ChannelType channelType = channelTypeRegistry.getChannelType(channelTypeUID, locale); - if (channelType != null) { - if (channelLabel == null) { - channelLabel = thingTypeI18nUtil.getChannelLabel(bundle, channelTypeUID, channelType.getLabel(), - locale); - } - if (channelDescription == null) { - channelDescription = thingTypeI18nUtil.getChannelDescription(bundle, channelTypeUID, - channelType.getDescription(), locale); - } - } - } - localizedChannelDefinitions - .add(new ChannelDefinition(channelDefinition.getId(), channelDefinition.getChannelTypeUID(), - channelDefinition.getProperties(), channelLabel, channelDescription)); - } + List<ChannelDefinition> localizedChannelDefinitions = localizeChannelDefinitions(bundle, thingType.getChannelDefinitions(), + channelDefinition -> thingTypeI18nUtil.getChannelLabel(bundle, thingTypeUID, channelDefinition, + channelDefinition.getLabel(), locale), + channelDefinition -> thingTypeI18nUtil.getChannelDescription(bundle, thingTypeUID, channelDefinition, + channelDefinition.getDescription(), locale), + locale); List<ChannelGroupDefinition> localizedChannelGroupDefinitions = new ArrayList<>( thingType.getChannelGroupDefinitions().size()); @@ -213,4 +177,32 @@ public class ThingTypeI18nLocalizationService { return builder.build(); } + public <T> List<ChannelDefinition> localizeChannelDefinitions(final Bundle bundle, final List<ChannelDefinition> channelDefinitions, + final Function<ChannelDefinition, @Nullable String> channelLabelResolver, + final Function<ChannelDefinition, @Nullable String> channelDescriptionResolver, final Locale locale) { + List<ChannelDefinition> localizedChannelDefinitions = new ArrayList<>(channelDefinitions.size()); + for (ChannelDefinition channelDefinition : channelDefinitions) { + String channelLabel = channelLabelResolver.apply(channelDefinition); + String channelDescription = channelDescriptionResolver.apply(channelDefinition); + if (channelLabel == null || channelDescription == null) { + ChannelTypeUID channelTypeUID = channelDefinition.getChannelTypeUID(); + ChannelType channelType = channelTypeRegistry.getChannelType(channelTypeUID, locale); + if (channelType != null) { + if (channelLabel == null) { + channelLabel = thingTypeI18nUtil.getChannelLabel(bundle, channelTypeUID, channelType.getLabel(), + locale); + } + if (channelDescription == null) { + channelDescription = thingTypeI18nUtil.getChannelDescription(bundle, channelTypeUID, + channelType.getDescription(), locale); + } + } + } + localizedChannelDefinitions + .add(new ChannelDefinition(channelDefinition.getId(), channelDefinition.getChannelTypeUID(), + channelDefinition.getProperties(), channelLabel, channelDescription)); + } + return localizedChannelDefinitions; + } + }

Very nice. Works like a charm. Thank you very much.

Now I will leave it to another committer to press the merge button 😉

lolodomo · 2018-06-19T05:38:55Z

Just for my understanding: what is wrong exactly ? I am running the openHAB snapshot 1302 and I get the correct translations in French in Paper UI with the WU binding.

cweitkamp · 2018-06-19T06:11:20Z

@lolodomo The weather binding does not contain an example for this. Try to translate the label and description in

https://github.com/cweitkamp/smarthome/blob/edec55aa1f0833db84e2e056b18ca862fb49c529/bundles/core/org.eclipse.smarthome.core.thing.xml.test/src/test/resources/test-bundle-pool/yahooweather.bundle/ESH-INF/thing/thing-types.xml#L43-46

lolodomo · 2018-06-19T07:23:46Z

Ok I see the difference now.

maggu2810

LGTM

maggu2810 · 2018-06-25T16:30:49Z

...ng/src/main/java/org/eclipse/smarthome/core/thing/i18n/ThingTypeI18nLocalizationService.java

+ String description = thingTypeI18nUtil.getChannelGroupDescription(bundle, channelGroupTypeUID,
+ channelGroupType.getDescription(), locale);
+
+ List<ChannelDefinition> localizedChannelDefinitions = new ArrayList<>(


Difficult to identify the problem in the Github diff view only. What's the problem?

…nel-group-type Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

cweitkamp · 2018-07-08T13:52:32Z

@maggu2810 Sry to break your approval. I had to rebase because of #5864. Thus I used the opportunity to switch to the new ChannelTypeBuilder API. I additionally introduced a new ChannelGroupTypeBuilder API on my own.

maggu2810 · 2018-07-09T11:40:41Z

@cweitkamp I agree with the ChannelGroupTypeBuilder. If the code of the i18n stuff has been changed only (because of the nullness annotations), I assume my review is still valid.

maggu2810 · 2018-07-09T11:40:59Z

@triller-telekom WRT to the builder, would you like to have a look at, too?

cweitkamp · 2018-07-09T12:03:44Z

If the code of the i18n stuff has been changed only (because of the nullness annotations)

I have added annotations in my i18n part and switched to the new I18nUtil.stripConstantOr() method.

triller-telekom

I just had a look at the builder and except for one typo it looks good to me.

triller-telekom · 2018-07-09T13:02:17Z

....core.thing/src/main/java/org/eclipse/smarthome/core/thing/type/ChannelGroupTypeBuilder.java

+ /**
+ * Specify whether this is an advanced {@link ChannelGroupType}, default is false
+ *
+ * @param advanced true is this is an advanced {@link ChannelGroupType}


typo: "if" this is an...

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

triller-telekom · 2018-07-09T15:21:20Z

@maggu2810 The typo I found is fixed now, feel free to merge if travis agrees :)

cweitkamp · 2018-07-09T15:27:43Z

@triller-telekom I copied & pasted the typo from your code. 😉

https://github.com/eclipse/smarthome/blob/8d107bd6eae5792d1ec1089a4ee3d684dc100828/bundles/core/org.eclipse.smarthome.core.thing/src/main/java/org/eclipse/smarthome/core/thing/internal/type/IChannelTypeBuilder.java#L32

triller-telekom · 2018-07-09T15:31:56Z

Upps... Thanks, fixed now with #5883 :)

cweitkamp · 2018-07-09T15:39:44Z

Thank you, too.

…s inside a channel-group-type (#5751) Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

cweitkamp · 2018-07-11T07:02:44Z

@maggu2810 Thanks

cweitkamp commented Jun 18, 2018

View reviewed changes

cweitkamp mentioned this pull request Jun 21, 2018

[OpenWeatherMap] Initial contribution of OpenWeatherMap binding #5694

Merged

9 tasks

maggu2810 reviewed Jun 25, 2018

View reviewed changes

cweitkamp added 5 commits July 8, 2018 13:23

Added translation of label ans description for channels inside a chan…

40c3fa3

…nel-group-type Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

Factored out methods

85b93cb

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

Rebase and improve nullness handling

e0d6260

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

Switched to new ChannelTypeBuilder API

a2dbcf5

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

Added a ChannelGroupTypeBuilder

b280b5d

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

triller-telekom reviewed Jul 9, 2018

View reviewed changes

Fixed typo in Javadoc

f9d37ad

Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

maggu2810 merged commit ee847b5 into eclipse-archived:master Jul 11, 2018

maggu2810 pushed a commit that referenced this pull request Jul 11, 2018

[i18n] Added translation feature for label and description of channel…

a5cd9d1

…s inside a channel-group-type (#5751) Signed-off-by: Christoph Weitkamp <github@christophweitkamp.de>

cweitkamp deleted the bugfix-channel-i18n branch July 11, 2018 07:02

cweitkamp mentioned this pull request Aug 16, 2018

Circular reference in XmlChannelTypeProvider #6054

Closed

htreu added this to the 0.10.0 milestone Oct 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[i18n] Added translation feature for label and description of channels inside a channel-group-type #5751

[i18n] Added translation feature for label and description of channels inside a channel-group-type #5751

cweitkamp commented Jun 18, 2018 •

edited

Loading

cweitkamp Jun 18, 2018

maggu2810 Jun 25, 2018

cweitkamp Jun 28, 2018

maggu2810 Jun 29, 2018 •

edited

Loading

cweitkamp Jun 29, 2018

maggu2810 Jun 30, 2018

lolodomo commented Jun 19, 2018

cweitkamp commented Jun 19, 2018

lolodomo commented Jun 19, 2018

maggu2810 left a comment

maggu2810 Jun 25, 2018

cweitkamp commented Jul 8, 2018 •

edited

Loading

maggu2810 commented Jul 9, 2018

maggu2810 commented Jul 9, 2018

cweitkamp commented Jul 9, 2018

triller-telekom left a comment

triller-telekom Jul 9, 2018

triller-telekom commented Jul 9, 2018

cweitkamp commented Jul 9, 2018

triller-telekom commented Jul 9, 2018

cweitkamp commented Jul 9, 2018

cweitkamp commented Jul 11, 2018

[i18n] Added translation feature for label and description of channels inside a channel-group-type #5751

[i18n] Added translation feature for label and description of channels inside a channel-group-type #5751

Conversation

cweitkamp commented Jun 18, 2018 • edited Loading

cweitkamp Jun 18, 2018

Choose a reason for hiding this comment

maggu2810 Jun 25, 2018

Choose a reason for hiding this comment

cweitkamp Jun 28, 2018

Choose a reason for hiding this comment

maggu2810 Jun 29, 2018 • edited Loading

Choose a reason for hiding this comment

cweitkamp Jun 29, 2018

Choose a reason for hiding this comment

maggu2810 Jun 30, 2018

Choose a reason for hiding this comment

lolodomo commented Jun 19, 2018

cweitkamp commented Jun 19, 2018

lolodomo commented Jun 19, 2018

maggu2810 left a comment

Choose a reason for hiding this comment

maggu2810 Jun 25, 2018

Choose a reason for hiding this comment

cweitkamp commented Jul 8, 2018 • edited Loading

maggu2810 commented Jul 9, 2018

maggu2810 commented Jul 9, 2018

cweitkamp commented Jul 9, 2018

triller-telekom left a comment

Choose a reason for hiding this comment

triller-telekom Jul 9, 2018

Choose a reason for hiding this comment

triller-telekom commented Jul 9, 2018

cweitkamp commented Jul 9, 2018

triller-telekom commented Jul 9, 2018

cweitkamp commented Jul 9, 2018

cweitkamp commented Jul 11, 2018

cweitkamp commented Jun 18, 2018 •

edited

Loading

maggu2810 Jun 29, 2018 •

edited

Loading

cweitkamp commented Jul 8, 2018 •

edited

Loading