[argoclima] Review remarks Aug'23<>May'24-squashed

Contents:
[argoclima] 1st round of review remarks (editorials)
[argoclima] 2nd round of review remarks (config)
            showCleartextPasswords --> includeDeviceSidePasswordsInProperties
            (default: NEVER)
[argoclima] i18n: Missing localization added
            Adds i18n for dynamic Channels and Thing statuses (incl. exceptions)
[argoclima] 3rd round of review remarks
[argoclima] 4th round of review remarks
            Renamed localDevicePort --> hvacListenPort
            Used instanceof pattern matching (where applicable)
            Demoted device communication logs to TRACE
            (non-review feedback) Added missing indirect command intercept in STUB
            mode
[argoclima] Fixed typos in docs (README, diagrams)
[argoclima] Post-review corrections: javadoc, etc.
[argoclima] Bump version to 4.2.0-SNAPSHOT
[argoclima] README.md minutiae
[argoclima] Channels rename to lower-case-hyphen
[argoclima] CP header update to 2024
[argoclima] Review remarks (password hashing extracted + minutiae)

Signed-off-by: Mateusz Bronk <[email protected]>

bundles/org.openhab.binding.argoclima/pom.xml

@@ -7,7 +7,7 @@
   <parent>
     <groupId>org.openhab.addons.bundles</groupId>
     <artifactId>org.openhab.addons.reactor.bundles</artifactId>
-    <version>4.1.0-SNAPSHOT</version>
+    <version>4.2.0-SNAPSHOT</version>
   </parent>
 
   <artifactId>org.openhab.binding.argoclima</artifactId>

bundles/org.openhab.binding.argoclima/scripts/download_ota_fw_files.py

@@ -4,7 +4,7 @@
 """
 
 __author__ = "Mateusz Bronk"
-__copyright__ = """Copyright (c) 2010-2022 Contributors to the openHAB project
+__copyright__ = """Copyright (c) 2010-2024 Contributors to the openHAB project
 
 See the NOTICE file(s) distributed with this work for additional
 information.

bundles/org.openhab.binding.argoclima/src/main/java/org/openhab/binding/argoclima/internal/ArgoClimaBindingConstants.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2010-2023 Contributors to the openHAB project
+ * Copyright (c) 2010-2024 Contributors to the openHAB project
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information.
@@ -37,9 +37,9 @@ public class ArgoClimaBindingConstants {
     /////////////
     // Thing configuration parameters
     /////////////
-    public static final String PARAMETER_HOSTNAME = "hostname"; // not used
+    public static final String PARAMETER_HOSTNAME = "hostname";
     public static final String PARAMETER_LOCAL_DEVICE_IP = "localDeviceIP";
-    public static final String PARAMETER_LOCAL_DEVICE_PORT = "localDevicePort"; // not used
+    public static final String PARAMETER_HVAC_LISTEN_PORT = "hvacListenPort";
     public static final String PARAMETER_DEVICE_CPU_ID = "deviceCpuId";
     public static final String PARAMETER_CONNECTION_MODE = "connectionMode"; // LOCAL_CONNECTION | REMOTE_API_STUB |
                                                                              // REMOTE_API_PROXY
@@ -49,7 +49,7 @@ public class ArgoClimaBindingConstants {
     public static final String PARAMETER_STUB_SERVER_LISTEN_ADDRESSES = "stubServerListenAddresses";
     public static final String PARAMETER_OEM_SERVER_PORT = "oemServerPort";
     public static final String PARAMETER_OEM_SERVER_ADDRESS = "oemServerAddress";
-    public static final String PARAMETER_SHOW_CLEARTEXT_PASSWORDS = "showCleartextPasswords";
+    public static final String PARAMETER_INCLUDE_DEVICE_SIDE_PASSWORDS_IN_PROPERTIES = "includeDeviceSidePasswordsInProperties";
     public static final String PARAMETER_MATCH_ANY_INCOMING_DEVICE_IP = "matchAnyIncomingDeviceIp";
 
     public static final String PARAMETER_USERNAME = "username";
@@ -80,26 +80,26 @@ public class ArgoClimaBindingConstants {
     /////////////
     // List of all Channel IDs
     /////////////
-    public static final String CHANNEL_POWER = "acControls#power";
-    public static final String CHANNEL_MODE = "acControls#mode";
-    public static final String CHANNEL_SET_TEMPERATURE = "acControls#setTemperature";
-    public static final String CHANNEL_CURRENT_TEMPERATURE = "acControls#currentTemperature";
-    public static final String CHANNEL_FAN_SPEED = "acControls#fanSpeed";
-    public static final String CHANNEL_ECO_MODE = "modes#ecoMode";
-    public static final String CHANNEL_TURBO_MODE = "modes#turboMode";
-    public static final String CHANNEL_NIGHT_MODE = "modes#nightMode";
-    public static final String CHANNEL_ACTIVE_TIMER = "timers#activeTimer";
-    public static final String CHANNEL_DELAY_TIMER = "timers#delayTimer";
+    public static final String CHANNEL_POWER = "ac-controls#power";
+    public static final String CHANNEL_MODE = "ac-controls#mode";
+    public static final String CHANNEL_SET_TEMPERATURE = "ac-controls#set-temperature";
+    public static final String CHANNEL_CURRENT_TEMPERATURE = "ac-controls#current-temperature";
+    public static final String CHANNEL_FAN_SPEED = "ac-controls#fan-speed";
+    public static final String CHANNEL_ECO_MODE = "modes#eco-mode";
+    public static final String CHANNEL_TURBO_MODE = "modes#turbo-mode";
+    public static final String CHANNEL_NIGHT_MODE = "modes#night-mode";
+    public static final String CHANNEL_ACTIVE_TIMER = "timers#active-timer";
+    public static final String CHANNEL_DELAY_TIMER = "timers#delay-timer";
     // Note: schedule timers day of week/time setting not currently supported as channels (YAGNI), and moved to config
-    public static final String CHANNEL_MODE_EX = "unsupported#modeEx";
-    public static final String CHANNEL_SWING_MODE = "unsupported#swingMode";
-    public static final String CHANNEL_FILTER_MODE = "unsupported#filterMode";
+    public static final String CHANNEL_MODE_EX = "unsupported#mode-ex";
+    public static final String CHANNEL_SWING_MODE = "unsupported#swing-mode";
+    public static final String CHANNEL_FILTER_MODE = "unsupported#filter-mode";
 
-    public static final String CHANNEL_I_FEEL_ENABLED = "settings#iFeelEnabled";
-    public static final String CHANNEL_DEVICE_LIGHTS = "settings#deviceLights";
+    public static final String CHANNEL_I_FEEL_ENABLED = "settings#ifeel-enabled";
+    public static final String CHANNEL_DEVICE_LIGHTS = "settings#device-lights";
 
-    public static final String CHANNEL_TEMPERATURE_DISPLAY_UNIT = "settings#temperatureDisplayUnit";
-    public static final String CHANNEL_ECO_POWER_LIMIT = "settings#ecoPowerLimit";
+    public static final String CHANNEL_TEMPERATURE_DISPLAY_UNIT = "settings#temperature-display-unit";
+    public static final String CHANNEL_ECO_POWER_LIMIT = "settings#eco-power-limit";
 
     /////////////
     // Binding's hard-coded configuration (not parameterized)
@@ -123,16 +123,6 @@ public class ArgoClimaBindingConstants {
      */
     public static final Duration SEND_COMMAND_DUTY_CYCLE = Duration.ofSeconds(1);
 
-    /**
-     * Whether the binding shall wait for the device confirming commands have been received (by flipping to the desired
-     * state) or work in a fire&forget mode and stop tracking upon first send.
-     * <p>
-     * This applies only to confirmable commands (read-write) and is a default behavior of Argo's own web implementation
-     *
-     * @implNote This is a debug-only switch (makes little to no sense to disable it in real-world usage)
-     */
-    public static final boolean AWAIT_DEVICE_CONFIRMATIONS_AFTER_COMMANDS = true;
-
     /**
      * The frequency to poll the device with, waiting for the command confirmation
      */
@@ -148,8 +138,8 @@ public class ArgoClimaBindingConstants {
      * Aka. the optimistic time when the device "should acknowledge. Should be greater than
      * {@link #POLL_FREQUENCY_AFTER_COMMAND_SENT_LOCAL}
      *
-     * @see {@link #SEND_COMMAND_MAX_WAIT_TIME_LOCAL_DIRECT}
-     * @see {@link #SEND_COMMAND_MAX_WAIT_TIME_LOCAL_INDIRECT}
+     * @see #SEND_COMMAND_MAX_WAIT_TIME_LOCAL_DIRECT
+     * @see #SEND_COMMAND_MAX_WAIT_TIME_LOCAL_INDIRECT
      */
     public static final Duration SEND_COMMAND_RETRY_FREQUENCY_LOCAL = Duration.ofSeconds(10);
 
@@ -158,16 +148,17 @@ public class ArgoClimaBindingConstants {
      * Aka. the optimistic time when the server "should acknowledge. Should be greater than
      * {@link #POLL_FREQUENCY_AFTER_COMMAND_SENT_REMOTE}
      *
-     * @see {@link #SEND_COMMAND_MAX_WAIT_TIME_REMOTE}
+     * @see #SEND_COMMAND_MAX_WAIT_TIME_REMOTE
      */
     public static final Duration SEND_COMMAND_RETRY_FREQUENCY_REMOTE = Duration.ofSeconds(20);
 
     /**
      * Max time to wait for a pending command to be confirmed by the device in a local-direct mode (when we are issuing
      * communications to a device in local LAN).
      * <p>
-     * During this time, the commands may get {@link SEND_COMMAND_RETRY_FREQUENCY retried} and the device status may be
-     * {@link POLL_FREQUENCY_AFTER_COMMAND_SENT re-fetched}
+     * During this time, the commands may get {@link #SEND_COMMAND_RETRY_FREQUENCY_LOCAL retried} and the device status
+     * may be
+     * {@link #POLL_FREQUENCY_AFTER_COMMAND_SENT_LOCAL re-fetched}
      */
     public static final Duration SEND_COMMAND_MAX_WAIT_TIME_LOCAL_DIRECT = Duration.ofSeconds(20); // 60-remote
 
@@ -200,4 +191,22 @@ public class ArgoClimaBindingConstants {
      */
     public static final Duration PENDING_COMMAND_EXPIRE_TIME = SEND_COMMAND_MAX_WAIT_TIME_LOCAL_INDIRECT
             .plus(Duration.ofSeconds(1));
+
+    /**
+     * Timeout for getting the HTTP response from Argo servers in pass-through(proxy) mode
+     */
+    public static final Duration UPSTREAM_PROXY_HTTP_REQUEST_TIMEOUT = Duration.ofSeconds(30);
+
+    /////////////
+    // R&D-only switches
+    /////////////
+    /**
+     * Whether the binding shall wait for the device confirming commands have been received (by flipping to the desired
+     * state) or work in a fire and forget mode and stop tracking upon first send.
+     * <p>
+     * This applies only to confirmable commands (read-write) and is a default behavior of Argo's own web implementation
+     *
+     * @implNote This is a debug-only switch (makes little to no sense to disable it in real-world usage)
+     */
+    public static final boolean AWAIT_DEVICE_CONFIRMATIONS_AFTER_COMMANDS = true;
 }

bundles/org.openhab.binding.argoclima/src/main/java/org/openhab/binding/argoclima/internal/ArgoClimaConfigProvider.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2010-2023 Contributors to the openHAB project
+ * Copyright (c) 2010-2024 Contributors to the openHAB project
  *
  * See the NOTICE file(s) distributed with this work for additional
  * information.
@@ -55,6 +55,7 @@
 public class ArgoClimaConfigProvider implements ConfigDescriptionProvider {
     private final Logger logger = LoggerFactory.getLogger(getClass());
     private final ThingRegistry thingRegistry;
+    private final ArgoClimaTranslationProvider i18nProvider;
     private static final int SCHEDULE_TIMERS_COUNT = 3;
 
     public record ScheduleDefaults(String startTime, String endTime, EnumSet<Weekday> weekdays) {
@@ -87,8 +88,10 @@ public static final ScheduleDefaults getScheduleDefaults(ScheduleTimerType sched
     }
 
     @Activate
-    public ArgoClimaConfigProvider(final @Reference ThingRegistry thingRegistry) {
+    public ArgoClimaConfigProvider(final @Reference ThingRegistry thingRegistry,
+            final @Reference ArgoClimaTranslationProvider i18nProvider) {
         this.thingRegistry = thingRegistry;
+        this.i18nProvider = i18nProvider;
     }
 
     /**
@@ -107,7 +110,7 @@ public Collection<ConfigDescription> getConfigDescriptions(@Nullable Locale loca
      * Provides a {@link ConfigDescription} for the given URI.
      *
      * @param uri URI of the config description (may be either thing or thing-type URI)
-     * @param locale locale
+     * @param locale locale (not using this value, as our i18n provider comes with it pre-populated!)
      * @return config description or null if no config description could be found
      *
      * @implNote {@code ConfigDescriptionParameterBuilder} doesn't have non-null-defaults, while
@@ -117,7 +120,7 @@ public Collection<ConfigDescription> getConfigDescriptions(@Nullable Locale loca
     @Override
     @Nullable
     public ConfigDescription getConfigDescription(URI uri, @Nullable Locale locale) {
-        if (!uri.getScheme().equalsIgnoreCase("thing")) {
+        if (!"thing".equalsIgnoreCase(uri.getScheme())) {
             return null; // Deliberately not supporting "thing-type" (no dynamic parameters there)
         }
         ThingUID thingUID = new ThingUID(Objects.requireNonNull(uri.getSchemeSpecificPart()));
@@ -135,32 +138,46 @@ public ConfigDescription getConfigDescription(URI uri, @Nullable Locale locale)
         for (int i = 1; i <= SCHEDULE_TIMERS_COUNT; ++i) {
             paramGroups.add(ConfigDescriptionParameterGroupBuilder
                     .create(String.format(ArgoClimaBindingConstants.PARAMETER_SCHEDULE_GROUP_NAME, i))
-                    .withLabel(String.format("Schedule %d", i))
-                    .withDescription(String.format("Schedule timer - profile %d.", i)).build());
+                    .withLabel(
+                            i18nProvider.getText("dynamic-config.argoclima.group.schedule.label", "Schedule {0} ", i))
+                    .withDescription(i18nProvider.getText("dynamic-config.argoclima.group.schedule.description",
+                            "Schedule timer - profile {0}.", i))
+                    .build());
         }
         if (thing.isEnabled()) {
-            // Note: Do not localize the label & description (ref: https://github.com/openhab/openhab-webui/issues/1491)
-            paramGroups.add(ConfigDescriptionParameterGroupBuilder.create("actions").withLabel("Actions")
-                    .withDescription("Actions").build());
+            paramGroups.add(ConfigDescriptionParameterGroupBuilder.create("actions").withContext("actions")
+                    .withLabel(i18nProvider.getText("dynamic-config.argoclima.group.actions.label", "Actions"))
+                    .build());
         }
 
         var parameters = new ArrayList<ConfigDescriptionParameter>();
 
-        var daysOfWeek = List.<@Nullable ParameterOption> of(new ParameterOption(Weekday.MON.toString(), "Monday"),
-                new ParameterOption(Weekday.TUE.toString(), "Tuesday"),
-                new ParameterOption(Weekday.WED.toString(), "Wednesday"),
-                new ParameterOption(Weekday.THU.toString(), "Thursday"),
-                new ParameterOption(Weekday.FRI.toString(), "Friday"),
-                new ParameterOption(Weekday.SAT.toString(), "Saturday"),
-                new ParameterOption(Weekday.SUN.toString(), "Sunday"));
+        var daysOfWeek = List.<@Nullable ParameterOption> of(
+                new ParameterOption(Weekday.MON.toString(),
+                        i18nProvider.getText("dynamic-config.argoclima.schedule.days.monday", "Monday")),
+                new ParameterOption(Weekday.TUE.toString(),
+                        i18nProvider.getText("dynamic-config.argoclima.schedule.days.tuesday", "Tuesday")),
+                new ParameterOption(Weekday.WED.toString(),
+                        i18nProvider.getText("dynamic-config.argoclima.schedule.days.wednesday", "Wednesday")),
+                new ParameterOption(Weekday.THU.toString(),
+                        i18nProvider.getText("dynamic-config.argoclima.schedule.days.thursday", "Thursday")),
+                new ParameterOption(Weekday.FRI.toString(),
+                        i18nProvider.getText("dynamic-config.argoclima.schedule.days.friday", "Friday")),
+                new ParameterOption(Weekday.SAT.toString(),
+                        i18nProvider.getText("dynamic-config.argoclima.schedule.days.saturday", "Saturday")),
+                new ParameterOption(Weekday.SUN.toString(),
+                        i18nProvider.getText("dynamic-config.argoclima.schedule.days.sunday", "Sunday")));
 
         for (int i = 1; i <= SCHEDULE_TIMERS_COUNT; ++i) {
             // NOTE: Deliberately *not* using .withContext("dayOfWeek") - doesn't seem to work correctly :(
             parameters.add(Objects.requireNonNull(ConfigDescriptionParameterBuilder
                     .create(String.format(ArgoClimaBindingConstants.PARAMETER_SCHEDULE_X_DAYS, i), Type.TEXT)
                     .withRequired(true)
                     .withGroupName(String.format(ArgoClimaBindingConstants.PARAMETER_SCHEDULE_GROUP_NAME, i))//
-                    .withLabel("Days").withDescription("Days when the schedule is run").withOptions(daysOfWeek)
+                    .withLabel(i18nProvider.getText("dynamic-config.argoclima.schedule.days.label", "Days"))
+                    .withDescription(i18nProvider.getText("dynamic-config.argoclima.schedule.days.description",
+                            "Days when the schedule is run"))
+                    .withOptions(daysOfWeek)
                     .withDefault(getScheduleDefaults(ScheduleTimerType.fromInt(i)).weekdays().toString())
                     .withMultiple(true).withMultipleLimit(7).build()));
 
@@ -170,25 +187,30 @@ public ConfigDescription getConfigDescription(URI uri, @Nullable Locale locale)
                     .create(String.format(ArgoClimaBindingConstants.PARAMETER_SCHEDULE_X_ON_TIME, i), Type.TEXT)
                     .withRequired(true)
                     .withGroupName(String.format(ArgoClimaBindingConstants.PARAMETER_SCHEDULE_GROUP_NAME, i))
-                    .withPattern("\\d{1-2}:\\d{1-2}").withLabel("On time").withDescription("Time when the A/C turns on")
+                    .withPattern("\\d{1-2}:\\d{1-2}")
+                    .withLabel(i18nProvider.getText("dynamic-config.argoclima.schedule.on-time.label", "On Time"))
+                    .withDescription(i18nProvider.getText("dynamic-config.argoclima.schedule.on-time.description",
+                            "Time when the A/C turns on"))
                     .withDefault(getScheduleDefaults(ScheduleTimerType.fromInt(i)).startTime()).build()));
             parameters.add(Objects.requireNonNull(ConfigDescriptionParameterBuilder
                     .create(String.format(ArgoClimaBindingConstants.PARAMETER_SCHEDULE_X_OFF_TIME, i), Type.TEXT)
                     .withRequired(true)
                     .withGroupName(String.format(ArgoClimaBindingConstants.PARAMETER_SCHEDULE_GROUP_NAME, i))
-                    .withLabel("Off time").withDescription("Time when the A/C turns off")
+                    .withLabel(i18nProvider.getText("dynamic-config.argoclima.schedule.off-time.label", "Off Time"))
+                    .withDescription(i18nProvider.getText("dynamic-config.argoclima.schedule.off-time.description",
+                            "Time when the A/C turns off"))
                     .withDefault(getScheduleDefaults(ScheduleTimerType.fromInt(i)).endTime()).build()));
         }
         if (thing.isEnabled()) {
             parameters.add(Objects.requireNonNull(ConfigDescriptionParameterBuilder
                     .create(ArgoClimaBindingConstants.PARAMETER_RESET_TO_FACTORY_DEFAULTS, Type.BOOLEAN)
                     .withRequired(false).withGroupName(ArgoClimaBindingConstants.PARAMETER_ACTIONS_GROUP_NAME)
-                    .withLabel("Reset settings").withDescription("Reset device settings to factory defaults")
+                    .withLabel(i18nProvider.getText("dynamic-config.argoclima.schedule.reset.label", "Reset Settings"))
+                    .withDescription(i18nProvider.getText("dynamic-config.argoclima.schedule.reset.description",
+                            "Reset device settings to factory defaults"))
                     .withDefault("false").withVerify(true).build()));
         }
 
-        var config = ConfigDescriptionBuilder.create(uri).withParameterGroups(paramGroups).withParameters(parameters)
-                .build();
-        return config;
+        return ConfigDescriptionBuilder.create(uri).withParameterGroups(paramGroups).withParameters(parameters).build();
     }
 }