[Linky] Some enhancements and corrections on the linky binding (#8871)

* Some enhancements and corrections on the linky binding
spotless apply
Adressing code review comments
* Adressing potential NPR.
* Code review findings correction
* Pleasing SAT checks

Signed-off-by: clinique <gael@lhopital.org>

bundles/org.openhab.binding.linky/README.md

@@ -40,7 +40,8 @@ Instructions given for Firefox :
 4. Clic on "Suivant".
 5. In the login page, prefilled with your mail address, enter your Enedis account password and click on "Connexion à Espace Client Enedis".
 6. You will be directed to your Enedis account environment. Get back to previous page in you browser.
-7. Open the developper tool window (F12) and select "Stockage" tab. In the "Cookies" entry, select "https://mon-compte-enedis.fr". You should see an entry named "internalAuthId", copy this value in your Openhab configuration.
+7. Disconnect from your Enedis account
+8. Repeat steps 1, 2. You should arrive directly on step 5, then open the developer tool window (F12) and select "Stockage" tab. In the "Cookies" entry, select "https://mon-compte-enedis.fr". You'll find an entry named "internalAuthId", copy this value in your Openhab configuration.
 
 ## Channels
 

bundles/org.openhab.binding.linky/src/main/java/org/openhab/binding/linky/internal/LinkyHandlerFactory.java

@@ -28,9 +28,12 @@ import org.openhab.core.thing.ThingTypeUID;
 import org.openhab.core.thing.binding.BaseThingHandlerFactory;
 import org.openhab.core.thing.binding.ThingHandler;
 import org.openhab.core.thing.binding.ThingHandlerFactory;
+import org.osgi.service.component.ComponentContext;
 import org.osgi.service.component.annotations.Activate;
 import org.osgi.service.component.annotations.Component;
 import org.osgi.service.component.annotations.Reference;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
@@ -44,6 +47,8 @@ import com.google.gson.JsonDeserializer;
 @NonNullByDefault
 @Component(service = ThingHandlerFactory.class, configurationPid = "binding.linky")
 public class LinkyHandlerFactory extends BaseThingHandlerFactory {
+    private final Logger logger = LoggerFactory.getLogger(LinkyHandlerFactory.class);
+
     private static final DateTimeFormatter formatter = DateTimeFormatter.ofPattern("uuuu-MM-dd'T'HH:mm:ss.SSSX");
     private final LocaleProvider localeProvider;
     private final Gson gson;
@@ -53,11 +58,33 @@ public class LinkyHandlerFactory extends BaseThingHandlerFactory {
     public LinkyHandlerFactory(final @Reference LocaleProvider localeProvider,
             final @Reference HttpClientFactory httpClientFactory) {
         this.localeProvider = localeProvider;
-        this.httpClient = httpClientFactory.createHttpClient(LinkyBindingConstants.BINDING_ID);
         this.gson = new GsonBuilder().registerTypeAdapter(ZonedDateTime.class,
                 (JsonDeserializer<ZonedDateTime>) (json, type, jsonDeserializationContext) -> ZonedDateTime
                         .parse(json.getAsJsonPrimitive().getAsString(), formatter))
                 .create();
+        this.httpClient = httpClientFactory.createHttpClient(LinkyBindingConstants.BINDING_ID);
+    }
+
+    @Override
+    protected void activate(ComponentContext componentContext) {
+        super.activate(componentContext);
+        httpClient.getSslContextFactory().setExcludeCipherSuites(new String[0]);
+        httpClient.setFollowRedirects(false);
+        try {
+            httpClient.start();
+        } catch (Exception e) {
+            logger.warn("Unable to start Jetty HttpClient {}", e.getMessage());
+        }
+    }
+
+    @Override
+    protected void deactivate(ComponentContext componentContext) {
+        super.deactivate(componentContext);
+        try {
+            httpClient.stop();
+        } catch (Exception e) {
+            logger.warn("Unable to stop Jetty HttpClient {}", e.getMessage());
+        }
     }
 
     @Override
@@ -69,10 +96,6 @@ public class LinkyHandlerFactory extends BaseThingHandlerFactory {
     protected @Nullable ThingHandler createHandler(Thing thing) {
         ThingTypeUID thingTypeUID = thing.getThingTypeUID();
 
-        if (supportsThingType(thingTypeUID)) {
+        return supportsThingType(thingTypeUID) ? new LinkyHandler(thing, localeProvider, gson, httpClient) : null;
-            return new LinkyHandler(thing, localeProvider, gson, httpClient);
-        }
-
-        return null;
     }
 }

bundles/org.openhab.binding.linky/src/main/java/org/openhab/binding/linky/internal/api/EnedisHttpApi.java

@@ -73,17 +73,6 @@ public class EnedisHttpApi {
     }
 
     public void initialize() throws LinkyException {
-        httpClient.getSslContextFactory().setExcludeCipherSuites(new String[0]);
-        httpClient.setFollowRedirects(false);
-        try {
-            httpClient.start();
-        } catch (Exception e) {
-            throw new LinkyException("Unable to start Jetty HttpClient", e);
-        }
-        connect();
-    }
-
-    private void connect() throws LinkyException {
         addCookie(LinkyConfiguration.INTERNAL_AUTH_ID, config.internalAuthId);
 
         logger.debug("Starting login process for user : {}", config.username);
@@ -185,12 +174,7 @@ public class EnedisHttpApi {
     }
 
     public void dispose() throws LinkyException {
-        try {
+        disconnect();
-            disconnect();
-            httpClient.stop();
-        } catch (Exception e) {
-            throw new LinkyException("Error stopping Jetty client", e);
-        }
     }
 
     private void addCookie(String key, String value) {

bundles/org.openhab.binding.linky/src/main/java/org/openhab/binding/linky/internal/api/ExpiringDayCache.java

@@ -15,6 +15,7 @@ package org.openhab.binding.linky.internal.api;
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
 import java.time.temporal.ChronoUnit;
+import java.util.Optional;
 import java.util.function.Supplier;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -64,7 +65,7 @@ public class ExpiringDayCache<V> {
     /**
      * Returns the value - possibly from the cache, if it is still valid.
      */
-    public synchronized @Nullable V getValue() {
+    public synchronized Optional<V> getValue() {
         @Nullable
         V cachedValue = value;
         if (cachedValue == null || isExpired()) {
@@ -73,7 +74,7 @@ public class ExpiringDayCache<V> {
         } else {
             logger.debug("getValue from cache \"{}\" is returning a cached value", name);
         }
-        return cachedValue;
+        return Optional.ofNullable(cachedValue);
     }
 
     /**

bundles/org.openhab.binding.linky/src/main/java/org/openhab/binding/linky/internal/dto/AuthData.java

@@ -15,6 +15,7 @@ package org.openhab.binding.linky.internal.dto;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
 
 /**
@@ -22,12 +23,12 @@ import org.eclipse.jdt.annotation.Nullable;
  *
  * @author Gaël L'hopital - Initial contribution
  */
-
+@NonNullByDefault
 public class AuthData {
     public class AuthDataCallBack {
         public class NameValuePair {
-            public String name;
+            public @Nullable String name;
-            public Object value;
+            public @Nullable Object value;
 
             public @Nullable String valueAsString() {
                 if (value instanceof String) {
@@ -37,15 +38,15 @@ public class AuthData {
             }
         }
 
-        public String type;
+        public @Nullable String type;
 
         public List<NameValuePair> output = new ArrayList<>();
         public List<NameValuePair> input = new ArrayList<>();
     }
 
-    public String authId;
+    public @Nullable String authId;
-    public String template;
+    public @Nullable String template;
-    public String stage;
+    public @Nullable String stage;
-    public String header;
+    public @Nullable String header;
     public List<AuthDataCallBack> callbacks = new ArrayList<>();
 }

bundles/org.openhab.binding.linky/src/main/java/org/openhab/binding/linky/internal/handler/LinkyHandler.java

@@ -70,12 +70,12 @@ public class LinkyHandler extends BaseThingHandler {
 
     private final HttpClient httpClient;
     private final Gson gson;
+    private final WeekFields weekFields;
 
     private @Nullable ScheduledFuture<?> refreshJob;
     private @Nullable EnedisHttpApi enedisApi;
-    private final WeekFields weekFields;
 
-    private final ExpiringDayCache<Consumption> cachedDaylyData;
+    private final ExpiringDayCache<Consumption> cachedDailyData;
     private final ExpiringDayCache<Consumption> cachedPowerData;
     private final ExpiringDayCache<Consumption> cachedMonthlyData;
     private final ExpiringDayCache<Consumption> cachedYearlyData;
@@ -87,12 +87,11 @@ public class LinkyHandler extends BaseThingHandler {
         super(thing);
         this.gson = gson;
         this.httpClient = httpClient;
-
         this.weekFields = WeekFields.of(localeProvider.getLocale());
 
-        this.cachedDaylyData = new ExpiringDayCache<>("daily cache", REFRESH_FIRST_HOUR_OF_DAY, () -> {
+        this.cachedDailyData = new ExpiringDayCache<>("daily cache", REFRESH_FIRST_HOUR_OF_DAY, () -> {
             LocalDate today = LocalDate.now();
-            return getConsumptionData(today.minusDays(13), today);
+            return getConsumptionData(today.minusDays(15), today);
         });
 
         this.cachedPowerData = new ExpiringDayCache<>("power cache", REFRESH_FIRST_HOUR_OF_DAY, () -> {
@@ -120,31 +119,37 @@ public class LinkyHandler extends BaseThingHandler {
         LinkyConfiguration config = getConfigAs(LinkyConfiguration.class);
         enedisApi = new EnedisHttpApi(config, gson, httpClient);
 
-        try {
+        scheduler.submit(() -> {
-            enedisApi.initialize();
+            try {
-            updateStatus(ThingStatus.ONLINE);
+                EnedisHttpApi api = this.enedisApi;
+                if (api != null) {
+                    api.initialize();
+                    updateStatus(ThingStatus.ONLINE);
 
-            if (thing.getProperties().isEmpty()) {
+                    if (thing.getProperties().isEmpty()) {
-                Map<String, String> properties = discoverAttributes();
+                        Map<String, String> properties = discoverAttributes();
-                updateProperties(properties);
+                        updateProperties(properties);
+                    }
+
+                    prmId = thing.getProperties().get(PRM_ID);
+                    userId = thing.getProperties().get(USER_ID);
+
+                    final LocalDateTime now = LocalDateTime.now();
+                    final LocalDateTime nextDayFirstTimeUpdate = now.plusDays(1).withHour(REFRESH_FIRST_HOUR_OF_DAY)
+                            .truncatedTo(ChronoUnit.HOURS);
+
+                    updateData();
+
+                    refreshJob = scheduler.scheduleWithFixedDelay(this::updateData,
+                            ChronoUnit.MINUTES.between(now, nextDayFirstTimeUpdate) % REFRESH_INTERVAL_IN_MIN + 1,
+                            REFRESH_INTERVAL_IN_MIN, TimeUnit.MINUTES);
+                } else {
+                    throw new LinkyException("Enedis Api is not initialized");
+                }
+            } catch (LinkyException e) {
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
             }
-
+        });
-            prmId = thing.getProperties().get(PRM_ID);
-            userId = thing.getProperties().get(USER_ID);
-
-            final LocalDateTime now = LocalDateTime.now();
-            final LocalDateTime nextDayFirstTimeUpdate = now.plusDays(1).withHour(REFRESH_FIRST_HOUR_OF_DAY)
-                    .truncatedTo(ChronoUnit.HOURS);
-
-            updateData();
-
-            refreshJob = scheduler.scheduleWithFixedDelay(this::updateData,
-                    ChronoUnit.MINUTES.between(now, nextDayFirstTimeUpdate) % REFRESH_INTERVAL_IN_MIN + 1,
-                    REFRESH_INTERVAL_IN_MIN, TimeUnit.MINUTES);
-
-        } catch (LinkyException e) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
-        }
     }
 
     private Map<String, String> discoverAttributes() throws LinkyException {
@@ -167,17 +172,17 @@ public class LinkyHandler extends BaseThingHandler {
     private void updateData() {
         updatePowerData();
         updateDailyData();
+        updateWeeklyData();
         updateMonthlyData();
         updateYearlyData();
     }
 
     private synchronized void updatePowerData() {
         if (isLinked(PEAK_POWER) || isLinked(PEAK_TIMESTAMP)) {
-            Consumption result = cachedPowerData.getValue();
+            cachedPowerData.getValue().ifPresent(values -> {
-            if (result != null) {
+                updateVAChannel(PEAK_POWER, values.aggregats.days.datas.get(0));
-                updateVAChannel(PEAK_POWER, result.aggregats.days.datas.get(0));
+                updateState(PEAK_TIMESTAMP, new DateTimeType(values.aggregats.days.periodes.get(0).dateDebut));
-                updateState(PEAK_TIMESTAMP, new DateTimeType(result.aggregats.days.periodes.get(0).dateDebut));
+            });
-            }
         }
     }
 
@@ -185,23 +190,19 @@ public class LinkyHandler extends BaseThingHandler {
      * Request new dayly/weekly data and updates channels
      */
     private synchronized void updateDailyData() {
-        if (isLinked(YESTERDAY) || isLinked(LAST_WEEK) || isLinked(THIS_WEEK)) {
+        if (isLinked(YESTERDAY) || isLinked(THIS_WEEK)) {
-            Consumption result = cachedDaylyData.getValue();
+            cachedDailyData.getValue().ifPresent(values -> {
-            if (result != null) {
+                Aggregate days = values.aggregats.days;
-                Aggregate days = result.aggregats.days;
-
                 int maxValue = days.periodes.size() - 1;
                 int thisWeekNumber = days.periodes.get(maxValue).dateDebut.get(weekFields.weekOfWeekBasedYear());
                 double yesterday = days.datas.get(maxValue);
-                double lastWeek = 0.0;
+                double thisWeek = 0.00;
-                double thisWeek = 0.0;
 
                 for (int i = maxValue; i >= 0; i--) {
                     int weekNumber = days.periodes.get(i).dateDebut.get(weekFields.weekOfWeekBasedYear());
                     if (weekNumber == thisWeekNumber) {
-                        thisWeek += days.datas.get(i);
+                        Double value = days.datas.get(i);
-                    } else if (weekNumber == thisWeekNumber - 1) {
+                        thisWeek += !value.isNaN() ? value : 0;
-                        lastWeek += days.datas.get(i);
                     } else {
                         break;
                     }
@@ -209,8 +210,21 @@ public class LinkyHandler extends BaseThingHandler {
 
                 updateKwhChannel(YESTERDAY, yesterday);
                 updateKwhChannel(THIS_WEEK, thisWeek);
-                updateKwhChannel(LAST_WEEK, lastWeek);
+            });
-            }
+        }
+    }
+
+    /**
+     * Request new weekly data and updates channels
+     */
+    private synchronized void updateWeeklyData() {
+        if (isLinked(LAST_WEEK)) {
+            cachedDailyData.getValue().ifPresent(values -> {
+                Aggregate weeks = values.aggregats.weeks;
+                if (weeks.datas.size() > 1) {
+                    updateKwhChannel(LAST_WEEK, weeks.datas.get(1));
+                }
+            });
         }
     }
 
@@ -219,16 +233,15 @@ public class LinkyHandler extends BaseThingHandler {
      */
     private synchronized void updateMonthlyData() {
         if (isLinked(LAST_MONTH) || isLinked(THIS_MONTH)) {
-            Consumption result = cachedMonthlyData.getValue();
+            cachedMonthlyData.getValue().ifPresent(values -> {
-            if (result != null) {
+                Aggregate months = values.aggregats.months;
-                Aggregate months = result.aggregats.months;
-                if (months.datas.size() < 2) {
-                    logger.debug("Received data array too small (required size is 2): {}", months);
-                    return;
-                }
                 updateKwhChannel(LAST_MONTH, months.datas.get(0));
-                updateKwhChannel(THIS_MONTH, months.datas.get(1));
+                if (months.datas.size() > 1) {
-            }
+                    updateKwhChannel(THIS_MONTH, months.datas.get(1));
+                } else {
+                    updateKwhChannel(THIS_MONTH, Double.NaN);
+                }
+            });
         }
     }
 
@@ -237,26 +250,28 @@ public class LinkyHandler extends BaseThingHandler {
      */
     private synchronized void updateYearlyData() {
         if (isLinked(LAST_YEAR) || isLinked(THIS_YEAR)) {
-            Consumption result = cachedYearlyData.getValue();
+            cachedYearlyData.getValue().ifPresent(values -> {
-            if (result != null) {
+                Aggregate years = values.aggregats.years;
-                Aggregate years = result.aggregats.years;
                 updateKwhChannel(LAST_YEAR, years.datas.get(0));
-                updateKwhChannel(THIS_YEAR, years.datas.get(1));
+                if (years.datas.size() > 1) {
-            }
+                    updateKwhChannel(THIS_YEAR, years.datas.get(1));
+                } else {
+                    updateKwhChannel(THIS_YEAR, Double.NaN);
+                }
+            });
         }
     }
 
     private void updateKwhChannel(String channelId, double consumption) {
         logger.debug("Update channel {} with {}", channelId, consumption);
-        updateState(channelId,
+        updateState(channelId, Double.isNaN(consumption) ? UnDefType.UNDEF
-                !Double.isNaN(consumption) ? new QuantityType<>(consumption, SmartHomeUnits.KILOWATT_HOUR)
+                : new QuantityType<>(consumption, SmartHomeUnits.KILOWATT_HOUR));
-                        : UnDefType.UNDEF);
     }
 
     private void updateVAChannel(String channelId, double power) {
         logger.debug("Update channel {} with {}", channelId, power);
         updateState(channelId,
-                !Double.isNaN(power) ? new QuantityType<>(power, SmartHomeUnits.VOLT_AMPERE) : UnDefType.UNDEF);
+                Double.isNaN(power) ? UnDefType.UNDEF : new QuantityType<>(power, SmartHomeUnits.VOLT_AMPERE));
     }
 
     /**

bundles/org.openhab.binding.linky/src/main/resources/OH-INF/thing/thing-types.xml

@@ -43,8 +43,13 @@
 			<channel id="yesterday" typeId="consumption">
 				<label>Yesterday Consumption</label>
 			</channel>
-			<channel id="power" typeId="power"/>
+			<channel id="power" typeId="power">
-			<channel id="timestamp" typeId="timestamp"/>
+				<label>Maximum power usage yesterday</label>
+			</channel>
+			<channel id="timestamp" typeId="timestamp">
+				<label>Peak Timestamp</label>
+				<description>Maximum power usage timestamp</description>
+			</channel>
 		</channels>
 	</channel-group-type>
 
@@ -55,7 +60,7 @@
 				<label>This Week Consumption</label>
 			</channel>
 			<channel id="lastWeek" typeId="consumption">
-				<label>Maximum power usage yesterday</label>
+				<label>Last Week Consumption</label>
 			</channel>
 		</channels>
 	</channel-group-type>
@@ -88,21 +93,21 @@
 		<item-type>Number:Energy</item-type>
 		<label>Total Consumption</label>
 		<description>Consumption at given time interval</description>
-		<state readOnly="true" pattern="%.3f %unit%"></state>
+		<category>energy</category>
+		<state readOnly="true" pattern="%.3f %unit%"/>
 	</channel-type>
 
 	<channel-type id="power">
 		<item-type>Number:Power</item-type>
 		<label>Peak Power</label>
 		<description>Maximum power usage yesterday</description>
-		<state readOnly="true" pattern="%.3f %unit%"></state>
+		<state readOnly="true" pattern="%.3f %unit%"/>
 	</channel-type>
 
 	<channel-type id="timestamp">
 		<item-type>DateTime</item-type>
-		<label>Peak Timestamp</label>
+		<label>Timestamp</label>
-		<description>Maximum power usage timestamp</description>
+		<category>time</category>
-		<state readOnly="true">
+		<state readOnly="true"/>
-		</state>
 	</channel-type>
 </thing:thing-descriptions>