[ankersolix] Detect illegal data address precisely

Distinguish a genuine protocol-level rejection from a general
communication failure using openHAB Core's public
ModbusSlaveErrorResponseException/ILLEGAL_DATA_ACCESS instead of the
previous ONLINE-status heuristic: a device can only respond with this
exception if it is actually reachable and explicitly rejected the
register, whereas connection/IO failures surface as different
exception types.

Also use maxTries=1 for optional registers, since retrying a
permanently absent register cannot succeed. This reduces the one-time
log footprint from openHAB Core from three lines (two WARN, one ERROR)
to a single ERROR line, directly followed by the binding's own INFO
note explaining the situation.

Update tests and README accordingly.

Signed-off-by: Thorben Grove <moin@thorbengrove.de>

Author: evorg

bundles/org.openhab.binding.modbus.ankersolix/README.md

@@ -254,4 +254,4 @@ Number:Power Solarbank_Setpoint "Battery Setpoint [%.0f %unit%]" { channel="modb
 1. Setpoint direction seems wrong: Set `battery-power-direction` first, then write `battery-power-setpoint`.
 1. Setpoint or direction writes are ignored: Solarbank requires `third_party_control` operating mode. Keep `autoThirdPartyControl` enabled, or set `operating-mode` to `third_party_control` manually before writing.
 1. Temporary UI value jumps: Tune `writeProtectionDurationSeconds` to keep write shadow values long enough until the next stable readback.
-1. Log shows `ModbusManagerImpl` WARN/ERROR entries for the capability mask register once, then nothing: This is expected, not an error. The device (or its firmware) does not implement the optional capability mask register, openHAB Core logs the rejected read attempt, and the binding then stops polling that register regularly (all channels keep working normally with fail-open defaults). Polling resumes automatically if the device firmware version changes, or when another Anker SOLIX thing is added to the binding (the register is also used for parallel-machine capability negotiation, so it may only start answering once a second unit is paired). A general communication problem (e.g. network outage) does not trigger this, since the binding only stops polling the register while the Thing is otherwise ONLINE and reachable.
+1. Log shows a single `ModbusManagerImpl` ERROR entry for the capability mask register once, then nothing: This is expected, not an error. The device (or its firmware) does not implement the optional capability mask register (openHAB Core reports the rejected read as `Illegal Data Access`); the binding does not retry this specific register (so only one log entry appears instead of several) and then stops polling it regularly (all channels keep working normally with fail-open defaults). Polling resumes automatically if the device firmware version changes, or when another Anker SOLIX thing is added to the binding (the register is also used for parallel-machine capability negotiation, so it may only start answering once a second unit is paired). A general communication problem (e.g. network outage, timeout) does not trigger this, since only an explicit protocol-level rejection from the device is treated as "not supported".

bundles/org.openhab.binding.modbus.ankersolix/src/main/java/org/openhab/binding/modbus/ankersolix/internal/AbstractAnkerSolixHandler.java

@@ -31,6 +31,7 @@
 import org.openhab.core.io.transport.modbus.ModbusRegisterArray;
 import org.openhab.core.io.transport.modbus.ModbusWriteRegisterRequestBlueprint;
 import org.openhab.core.io.transport.modbus.PollTask;
+import org.openhab.core.io.transport.modbus.exception.ModbusSlaveErrorResponseException;
 import org.openhab.core.library.types.QuantityType;
 import org.openhab.core.library.types.StringType;
 import org.openhab.core.library.unit.Units;
@@ -151,8 +152,11 @@ public void dispose() {
     }
 
     private void registerPoll(PollRange range, AnkerSolixConfiguration localConfig) {
+        // optional registers are never worth retrying: a permanently absent register won't start existing on a
+        // retry, so maxTries=1 keeps the one-time log footprint (from openHAB Core) to a single ERROR line
+        int maxTries = range.optional() ? 1 : localConfig.maxTries;
         ModbusReadRequestBlueprint request = new ModbusReadRequestBlueprint(getSlaveId(), range.functionCode,
-                range.startAddress, range.length, localConfig.maxTries);
+                range.startAddress, range.length, maxTries);
         PollTask task = registerRegularPoll(request, localConfig.pollInterval, 0,
                 result -> handleReadSuccess(range, result), failure -> handleReadFailure(range, failure));
         activePollTasks.put(range, task);
@@ -204,15 +208,16 @@ private void handleReadSuccess(PollRange range, AsyncModbusReadResult result) {
     }
 
     private void handleReadFailure(PollRange range, AsyncModbusFailure<ModbusReadRequestBlueprint> failure) {
-        String message = String.valueOf(failure.getCause().getMessage());
-        logger.debug("Failed to read Anker SOLIX registers: {}", message);
+        Exception cause = failure.getCause();
+        logger.debug("Failed to read Anker SOLIX registers: {}", cause.getMessage());
         if (!range.optional()) {
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, message);
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, cause.getMessage());
             return;
         }
-        // only back off while ONLINE: a device that is reachable enough to reject this one register (rather than
-        // failing to respond at all) has genuinely rejected it, whereas a network/bridge outage fails every range
-        if (getThing().getStatus() != ThingStatus.ONLINE) {
+        // only back off on an explicit "illegal data address" protocol response: the device answered and rejected
+        // this exact register, as opposed to a connection/IO failure where no response was received at all
+        if (!(cause instanceof ModbusSlaveErrorResponseException responseException)
+                || responseException.getExceptionCode() != ModbusSlaveErrorResponseException.ILLEGAL_DATA_ACCESS) {
             return;
         }
         if (backedOffRanges.add(range)) {

bundles/org.openhab.binding.modbus.ankersolix/src/test/java/org/openhab/binding/modbus/ankersolix/internal/AnkerSolixHandlerInternalsTest.java

@@ -33,6 +33,7 @@
 import org.junit.jupiter.api.Test;
 import org.openhab.core.io.transport.modbus.AsyncModbusFailure;
 import org.openhab.core.io.transport.modbus.ModbusReadRequestBlueprint;
+import org.openhab.core.io.transport.modbus.exception.ModbusSlaveErrorResponseException;
 import org.openhab.core.library.types.DecimalType;
 import org.openhab.core.library.types.OnOffType;
 import org.openhab.core.library.types.QuantityType;
@@ -304,7 +305,7 @@ void capabilityMaskReadFailureShouldBackOffRegularPolling() throws Exception {
         List<?> pollRanges = invoke(handler, "getPollRanges");
         Object capabilityRange = nonNull(pollRanges.get(pollRanges.size() - 1));
         AsyncModbusFailure<ModbusReadRequestBlueprint> failure = new AsyncModbusFailure<>(
-                mock(ModbusReadRequestBlueprint.class), new RuntimeException("Illegal Data Address"));
+                mock(ModbusReadRequestBlueprint.class), illegalDataAccessException());
 
         invokeVoid(handler, "handleReadFailure", capabilityRange, failure);
 
@@ -317,20 +318,17 @@ void capabilityMaskReadFailureShouldBackOffRegularPolling() throws Exception {
     }
 
     @Test
-    void capabilityMaskReadFailureShouldNotBackOffWhileThingIsNotOnline() throws Exception {
-        Thing offlineThing = mock(Thing.class);
-        when(offlineThing.getStatus()).thenReturn(ThingStatus.OFFLINE);
-        AbstractAnkerSolixHandler offlineHandler = new AnkerSolixSolarbankHandler(offlineThing);
-
-        List<?> pollRanges = invoke(offlineHandler, "getPollRanges");
+    void capabilityMaskReadFailureShouldNotBackOffOnGeneralCommunicationFailure() throws Exception {
+        List<?> pollRanges = invoke(handler, "getPollRanges");
         Object capabilityRange = nonNull(pollRanges.get(pollRanges.size() - 1));
         AsyncModbusFailure<ModbusReadRequestBlueprint> failure = new AsyncModbusFailure<>(
                 mock(ModbusReadRequestBlueprint.class), new RuntimeException("connection timed out"));
 
-        // a general communication failure (bridge/network down) must not be mistaken for register rejection
-        invokeVoid(offlineHandler, "handleReadFailure", capabilityRange, failure);
+        // a general communication failure (bridge/network down, timeout, ...) must not be mistaken for an
+        // explicit register rejection, since no response was ever received from the device
+        invokeVoid(handler, "handleReadFailure", capabilityRange, failure);
 
-        Set<Object> backedOffRanges = getField(offlineHandler, "backedOffRanges");
+        Set<Object> backedOffRanges = getField(handler, "backedOffRanges");
         assertTrue(backedOffRanges.isEmpty());
     }
 
@@ -449,4 +447,13 @@ private static AbstractAnkerSolixHandler newHandler() {
         when(thing.getStatus()).thenReturn(ThingStatus.ONLINE);
         return new AnkerSolixSolarbankHandler(thing);
     }
+
+    private static ModbusSlaveErrorResponseException illegalDataAccessException() {
+        return new ModbusSlaveErrorResponseException() {
+            @Override
+            public int getExceptionCode() {
+                return ModbusSlaveErrorResponseException.ILLEGAL_DATA_ACCESS;
+            }
+        };
+    }
 }