[3.2.x] [ocpp] Retry MeterValues ChangeConfiguration with stripped measurand on Reject

bundles/org.connectorio.addons.binding.ocpp/src/main/java/org/connectorio/addons/binding/ocpp/internal/server/adapter/MeterValuesConfigAdapter.java

@@ -20,7 +20,11 @@
 import eu.chargetime.ocpp.model.core.BootNotificationConfirmation;
 import eu.chargetime.ocpp.model.core.BootNotificationRequest;
 import eu.chargetime.ocpp.model.core.ChangeConfigurationRequest;
+import eu.chargetime.ocpp.model.core.ChangeConfigurationConfirmation;
+import eu.chargetime.ocpp.model.core.ConfigurationStatus;
+import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.ConcurrentHashMap;
 import org.connectorio.addons.binding.ocpp.internal.OcppSender;
 import org.connectorio.addons.binding.ocpp.internal.server.ChargerReference;
 import org.connectorio.addons.binding.ocpp.internal.server.OcppChargerSessionRegistry;
@@ -36,6 +40,15 @@ public class MeterValuesConfigAdapter extends CoreEventHandlerAdapter {
   private final String sampledData;
   private final int clockAlignedInterval;
 
+  /**
+   * Package-scoped for testability. Keyed by charger serial: the measurand list this charger has
+   * last accepted. OCPP 1.6 has no way to enumerate a charger's supported measurands up front — the
+   * only signal that one is unsupported is the ChangeConfiguration Reject itself — so the accepted
+   * set is discovered once by elimination and reused on every later (re)connect instead of
+   * re-discovering it from scratch each time.
+   */
+  final Map<String, String> acceptedMeasurands = new ConcurrentHashMap<>();
+
   public MeterValuesConfigAdapter(OcppChargerSessionRegistry sessionRegistry, OcppSender sender,
       int sampleInterval, String sampledData, int clockAlignedInterval) {
     this.sessionRegistry = sessionRegistry;
@@ -51,20 +64,63 @@ public BootNotificationConfirmation handleBootNotificationRequest(UUID sessionIn
     if (reference == null) {
       return null;
     }
+    String measurands = acceptedMeasurands.getOrDefault(reference.getSerial(), sampledData);
     apply(reference, "MeterValueSampleInterval", Integer.toString(sampleInterval));
-    apply(reference, "MeterValuesSampledData", sampledData);
-    apply(reference, "MeterValuesAlignedData", sampledData);
+    apply(reference, "MeterValuesSampledData", measurands);
+    apply(reference, "MeterValuesAlignedData", measurands);
     apply(reference, "ClockAlignedDataInterval", Integer.toString(clockAlignedInterval));
     return null;
   }
 
   private void apply(ChargerReference reference, String key, String value) {
+    boolean isMeterMeasurandKey = "MeterValuesSampledData".equals(key) || "MeterValuesAlignedData".equals(key);
     sender.send(reference, new ChangeConfigurationRequest(key, value)).whenComplete((confirmation, ex) -> {
       if (ex != null) {
         logger.warn("ChangeConfiguration[{}] for {} failed: {}", key, reference, ex.getMessage());
-      } else {
-        logger.debug("ChangeConfiguration[{}={}] for {}: {}", key, value, reference, confirmation);
+        return;
+      }
+      if (isMeterMeasurandKey && confirmation instanceof ChangeConfigurationConfirmation
+          && ((ChangeConfigurationConfirmation) confirmation).getStatus() == ConfigurationStatus.Rejected) {
+        String stripped = stripLast(value);
+        if (!stripped.equals(value) && !stripped.isEmpty()) {
+          logger.info("Charger {} rejected ChangeConfiguration[{}={}] — retrying with one fewer measurand: {}",
+              reference, key, value, stripped);
+          apply(reference, key, stripped);
+          return;
+        }
+        logger.warn("Charger {} rejected ChangeConfiguration[{}] down to a single measurand; giving up",
+            reference, key);
+        return;
+      }
+      if (isMeterMeasurandKey) {
+        acceptedMeasurands.put(reference.getSerial(), value);
       }
+      logger.debug("ChangeConfiguration[{}={}] for {}: {}", key, value, reference, confirmation);
     });
   }
+
+  /**
+   * Return {@code value} with its last comma-separated measurand removed (remaining tokens
+   * trimmed), or {@code value} unchanged if it is null/empty. Trial-based elimination: OCPP 1.6
+   * gives no signal on which measurand a Reject was actually about, so candidates are tried by
+   * dropping one at a time rather than by any hardcoded fragile-measurand list — no measurand name
+   * is baked into the binding.
+   */
+  static String stripLast(String value) {
+    if (value == null || value.isEmpty()) {
+      return value;
+    }
+    String[] tokens = value.split(",");
+    if (tokens.length <= 1) {
+      return "";
+    }
+    StringBuilder sb = new StringBuilder();
+    for (int i = 0; i < tokens.length - 1; i++) {
+      if (sb.length() > 0) {
+        sb.append(',');
+      }
+      sb.append(tokens[i].trim());
+    }
+    return sb.toString();
+  }
 }

bundles/org.connectorio.addons.binding.ocpp/src/test/java/org/connectorio/addons/binding/ocpp/internal/server/adapter/MeterValuesConfigAdapterTest.java

@@ -0,0 +1,144 @@
+/*
+ * Copyright (C) 2022-2022 ConnectorIO Sp. z o.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+package org.connectorio.addons.binding.ocpp.internal.server.adapter;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import eu.chargetime.ocpp.model.Confirmation;
+import eu.chargetime.ocpp.model.Request;
+import eu.chargetime.ocpp.model.core.BootNotificationRequest;
+import eu.chargetime.ocpp.model.core.ChangeConfigurationConfirmation;
+import eu.chargetime.ocpp.model.core.ChangeConfigurationRequest;
+import eu.chargetime.ocpp.model.core.ConfigurationStatus;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.CompletionStage;
+import org.connectorio.addons.binding.ocpp.internal.OcppSender;
+import org.connectorio.addons.binding.ocpp.internal.server.ChargerReference;
+import org.connectorio.addons.binding.ocpp.internal.server.OcppChargerSessionRegistry;
+import org.junit.jupiter.api.Test;
+
+class MeterValuesConfigAdapterTest {
+
+  @Test
+  void stripsTheLastMeasurand() {
+    assertThat(MeterValuesConfigAdapter.stripLast(
+        "Energy.Active.Import.Register,Power.Active.Import,Current.Import,Voltage"))
+        .isEqualTo("Energy.Active.Import.Register,Power.Active.Import,Current.Import");
+  }
+
+  @Test
+  void stripsDownToEmptyOnceOnlyOneMeasurandIsLeft() {
+    assertThat(MeterValuesConfigAdapter.stripLast("Voltage")).isEmpty();
+  }
+
+  @Test
+  void handlesWhitespaceAroundCommas() {
+    assertThat(MeterValuesConfigAdapter.stripLast("Voltage, Temperature, Current.Import"))
+        .isEqualTo("Voltage,Temperature");
+  }
+
+  @Test
+  void handlesNullAndEmpty() {
+    assertThat(MeterValuesConfigAdapter.stripLast(null)).isNull();
+    assertThat(MeterValuesConfigAdapter.stripLast("")).isEmpty();
+  }
+
+  /**
+   * Fake sender: Rejects a MeterValuesSampledData/AlignedData ChangeConfiguration while its value
+   * still contains {@code rejectedMeasurand}, Accepts everything else. Stands in for a charger that
+   * only supports a subset of the configured measurands, without hardcoding which subset.
+   */
+  private static OcppSender rejectingSenderFor(String rejectedMeasurand, List<String> sentMeasurandValues) {
+    return new OcppSender() {
+      @Override
+      @SuppressWarnings("unchecked")
+      public <T extends Confirmation> CompletionStage<T> send(ChargerReference reference, Request request) {
+        ChangeConfigurationRequest change = (ChangeConfigurationRequest) request;
+        if ("MeterValuesSampledData".equals(change.getKey()) || "MeterValuesAlignedData".equals(change.getKey())) {
+          sentMeasurandValues.add(change.getValue());
+        }
+        ConfigurationStatus status = change.getValue() != null && change.getValue().contains(rejectedMeasurand)
+            ? ConfigurationStatus.Rejected : ConfigurationStatus.Accepted;
+        return (CompletionStage<T>) CompletableFuture.completedFuture(new ChangeConfigurationConfirmation(status));
+      }
+    };
+  }
+
+  @Test
+  void negotiatesDownOnRejectAndCachesTheAcceptedSetPerCharger() {
+    ChargerReference charx = new ChargerReference("charx");
+    UUID session = UUID.randomUUID();
+    OcppChargerSessionRegistry registry = mock(OcppChargerSessionRegistry.class);
+    when(registry.getCharger(session)).thenReturn(charx);
+
+    List<String> sentMeasurandValues = new ArrayList<>();
+    MeterValuesConfigAdapter adapter = new MeterValuesConfigAdapter(registry,
+        rejectingSenderFor("Temperature", sentMeasurandValues), 30,
+        "Energy.Active.Import.Register,Temperature", 30);
+
+    adapter.handleBootNotificationRequest(session, new BootNotificationRequest());
+
+    assertThat(adapter.acceptedMeasurands.get("charx")).isEqualTo("Energy.Active.Import.Register");
+  }
+
+  @Test
+  void aReconnectStartsFromTheCachedAcceptedSetInsteadOfRenegotiating() {
+    ChargerReference charx = new ChargerReference("charx");
+    UUID session = UUID.randomUUID();
+    OcppChargerSessionRegistry registry = mock(OcppChargerSessionRegistry.class);
+    when(registry.getCharger(session)).thenReturn(charx);
+
+    List<String> sentMeasurandValues = new ArrayList<>();
+    MeterValuesConfigAdapter adapter = new MeterValuesConfigAdapter(registry,
+        rejectingSenderFor("Temperature", sentMeasurandValues), 30,
+        "Energy.Active.Import.Register,Temperature", 30);
+
+    adapter.handleBootNotificationRequest(session, new BootNotificationRequest()); // first boot: negotiates down
+    adapter.handleBootNotificationRequest(session, new BootNotificationRequest()); // reconnect: reuses the cache
+
+    // First boot tries the full configured list for both measurand keys, rejected both times, then
+    // retries each down to the already-accepted set. The reconnect goes straight to that set for
+    // both keys, first try, no further negotiation.
+    assertThat(sentMeasurandValues).containsExactly(
+        "Energy.Active.Import.Register,Temperature", "Energy.Active.Import.Register",
+        "Energy.Active.Import.Register,Temperature", "Energy.Active.Import.Register",
+        "Energy.Active.Import.Register", "Energy.Active.Import.Register");
+  }
+
+  @Test
+  void aChargerThatAcceptsTheFullListIsCachedTooSoLaterBootsSkipRenegotiation() {
+    ChargerReference wallbox = new ChargerReference("wallbox");
+    UUID session = UUID.randomUUID();
+    OcppChargerSessionRegistry registry = mock(OcppChargerSessionRegistry.class);
+    when(registry.getCharger(session)).thenReturn(wallbox);
+
+    List<String> sentMeasurandValues = new ArrayList<>();
+    MeterValuesConfigAdapter adapter = new MeterValuesConfigAdapter(registry,
+        rejectingSenderFor("Temperature", sentMeasurandValues), 30,
+        "Energy.Active.Import.Register,Voltage", 30);
+
+    adapter.handleBootNotificationRequest(session, new BootNotificationRequest());
+
+    assertThat(adapter.acceptedMeasurands.get("wallbox")).isEqualTo("Energy.Active.Import.Register,Voltage");
+  }
+}