Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for #28 - Implement poor man's state machine for logging and disa… #31

Merged
merged 1 commit into from
Aug 26, 2023
Merged

Fix for #28 - Implement poor man's state machine for logging and disa… #31

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 24 additions & 7 deletions src/main/java/org/openhab/binding/solarman/internal/SolarmanLoggerHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.openhab.binding.solarman.internal.defmodel.Validation;
import org.openhab.binding.solarman.internal.modbus.SolarmanLoggerConnector;
import org.openhab.binding.solarman.internal.modbus.SolarmanV5Protocol;
import org.openhab.binding.solarman.internal.state.LoggerState;
import org.openhab.binding.solarman.internal.updater.SolarmanChannelUpdater;
import org.openhab.core.thing.*;
import org.openhab.core.thing.binding.BaseThingHandler;
Expand All @@ -54,13 +55,15 @@ public class SolarmanLoggerHandler extends BaseThingHandler {

private final DefinitionParser definitionParser;
private final SolarmanChannelManager solarmanChannelManager;
private final LoggerState loggerState;
@Nullable
private volatile ScheduledFuture<?> scheduledFuture;

public SolarmanLoggerHandler(Thing thing) {
super(thing);
this.definitionParser = new DefinitionParser();
this.solarmanChannelManager = new SolarmanChannelManager();
this.loggerState = new LoggerState();
}

@Override
Expand Down Expand Up @@ -113,16 +116,30 @@ public void initialize() {
);

SolarmanChannelUpdater solarmanChannelUpdater = new SolarmanChannelUpdater(
this::updateStatus,
this::updateState
);

scheduledFuture = scheduler.scheduleAtFixedRate(
() -> solarmanChannelUpdater.fetchDataFromLogger(
mergedRequests,
solarmanLoggerConnector,
solarmanV5Protocol,
paramToChannelMapping),
scheduledFuture = scheduler.scheduleAtFixedRate(() -> {
boolean fetchSuccessful = solarmanChannelUpdater.fetchDataFromLogger(
mergedRequests,
solarmanLoggerConnector,
solarmanV5Protocol,
paramToChannelMapping,
loggerState);

if (fetchSuccessful) {
updateStatus(ThingStatus.ONLINE);
loggerState.setOnline();
} else {
updateStatus(ThingStatus.OFFLINE);
loggerState.setPossiblyOffline();
}

if (loggerState.isJustBecameOffline()) {
logger.info("Assuming logger is OFFLINE after {} failed requests. Disabling connection error logging until it becomes available again",
LoggerState.NO_FAILED_REQUESTS);
}
},
0, config.refreshInterval, TimeUnit.SECONDS
);
}
Expand Down
23 changes: 14 additions & 9 deletions src/main/java/org/openhab/binding/solarman/internal/modbus/SolarmanLoggerConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ public SolarmanLoggerConnection(String hostName, int port) {
sockaddr = new InetSocketAddress(hostName, port);
}

public byte[] sendRequest(byte[] reqFrame) {
public byte[] sendRequest(byte[] reqFrame, Boolean allowLogging) {
// Will not be used by multiple threads, so not bothering making it thread safe for now
if (socket == null) {
if ((socket = connectSocket()) == null) {
LOGGER.info("Error creating socket");
if ((socket = connectSocket(allowLogging)) == null) {
if (allowLogging)
LOGGER.info("Error creating socket");
return new byte[0];
}
}
Expand All @@ -32,7 +33,8 @@ public byte[] sendRequest(byte[] reqFrame) {
LOGGER.debug("Request frame: " + bytesToHex(reqFrame));
socket.getOutputStream().write(reqFrame);
} catch (IOException e) {
LOGGER.info("Unable to send frame to logger", e);
if (allowLogging)
LOGGER.info("Unable to send frame to logger", e);
return new byte[0];
}

Expand All @@ -45,7 +47,8 @@ public byte[] sendRequest(byte[] reqFrame) {
try {
int bytesRead = socket.getInputStream().read(buffer);
if (bytesRead < 0) {
LOGGER.info("No data received");
if (allowLogging)
LOGGER.info("No data received");
} else {
byte[] data = Arrays.copyOfRange(buffer, 0, bytesRead);
if (LOGGER.isDebugEnabled())
Expand All @@ -54,11 +57,12 @@ public byte[] sendRequest(byte[] reqFrame) {
}
} catch (SocketTimeoutException e) {
LOGGER.debug("Connection timeout", e);
if (attempts == 0) {
if (attempts == 0 && allowLogging) {
LOGGER.info("Too many connection timeouts");
}
} catch (IOException e) {
LOGGER.info("Connection error", e);
if (allowLogging)
LOGGER.info("Connection error", e);
}
}

Expand All @@ -70,7 +74,7 @@ private static String bytesToHex(byte[] bytes) {
.collect(Collectors.joining());
}

private Socket connectSocket() {
private Socket connectSocket(Boolean allowLogging) {
try {
Socket clientSocket = new Socket();

Expand All @@ -79,7 +83,8 @@ private Socket connectSocket() {

return clientSocket;
} catch (IOException e) {
LOGGER.error("Could not open socket on IP " + sockaddr.toString(), e);
if (allowLogging)
LOGGER.error("Could not open socket on IP " + sockaddr.toString(), e);
return null;
}
}
Expand Down
33 changes: 20 additions & 13 deletions src/main/java/org/openhab/binding/solarman/internal/modbus/SolarmanV5Protocol.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ public SolarmanV5Protocol(SolarmanLoggerConfiguration solarmanLoggerConfiguratio
this.solarmanLoggerConfiguration = solarmanLoggerConfiguration;
}

public Map<Integer, byte[]> readRegisters(SolarmanLoggerConnection solarmanLoggerConnection, byte mbFunctionCode, int firstReg, int lastReg) {
public Map<Integer, byte[]> readRegisters(SolarmanLoggerConnection solarmanLoggerConnection, byte mbFunctionCode, int firstReg, int lastReg, Boolean allowLogging) {
byte[] solarmanV5Frame = buildSolarmanV5Frame(mbFunctionCode, firstReg, lastReg);
byte[] respFrame = solarmanLoggerConnection.sendRequest(solarmanV5Frame);
byte[] respFrame = solarmanLoggerConnection.sendRequest(solarmanV5Frame, allowLogging);
if (respFrame.length > 0) {
byte[] modbusRespFrame = extractModbusResponseFrame(respFrame, solarmanV5Frame);
return parseModbusReadHoldingRegistersResponse(modbusRespFrame, firstReg, lastReg);
byte[] modbusRespFrame = extractModbusResponseFrame(respFrame, solarmanV5Frame, allowLogging);
return parseModbusReadHoldingRegistersResponse(modbusRespFrame, firstReg, lastReg, allowLogging);
} else {
return Collections.emptyMap();
}
Expand Down Expand Up @@ -153,12 +153,13 @@ protected byte[] buildModbusReadHoldingRegistersRequestFrame(byte slaveId, byte
return ByteBuffer.allocate(req.length + crc.length).put(req).put(crc).array();
}

protected Map<Integer, byte[]> parseModbusReadHoldingRegistersResponse(byte[] frame, int firstReg, int lastReg) {
protected Map<Integer, byte[]> parseModbusReadHoldingRegistersResponse(byte[] frame, int firstReg, int lastReg, Boolean allowLogging) {
int regCount = lastReg - firstReg + 1;
Map<Integer, byte[]> registers = new HashMap<>();
int expectedFrameDataLen = 2 + 1 + regCount * 2;
if (frame == null || frame.length < expectedFrameDataLen + 2) {
LOGGER.error("Modbus frame is too short or empty");
if (allowLogging)
LOGGER.error("Modbus frame is too short or empty");
return registers;
}

Expand All @@ -167,7 +168,8 @@ protected Map<Integer, byte[]> parseModbusReadHoldingRegistersResponse(byte[] fr
int expectedCrc = CRC16Modbus.calculate(Arrays.copyOfRange(frame, 0, expectedFrameDataLen));

if (actualCrc != expectedCrc) {
LOGGER.error(String.format("Modbus frame crc is not valid. Expected %04x, got %04x", expectedCrc, actualCrc));
if (allowLogging)
LOGGER.error(String.format("Modbus frame crc is not valid. Expected %04x, got %04x", expectedCrc, actualCrc));
return registers;
}

Expand All @@ -181,21 +183,26 @@ protected Map<Integer, byte[]> parseModbusReadHoldingRegistersResponse(byte[] fr
return registers;
}

protected byte[] extractModbusResponseFrame(byte[] responseFrame, byte[] requestFrame) {
if (responseFrame == null) {
LOGGER.error("No response frame");
protected byte[] extractModbusResponseFrame(byte[] responseFrame, byte[] requestFrame, Boolean allowLogging) {
if (responseFrame == null || responseFrame.length == 0) {
if (allowLogging)
LOGGER.error("No response frame");
return null;
} else if (responseFrame.length == 29) {

parseResponseErrorCode(responseFrame, requestFrame);
return null;
} else if (responseFrame.length < (29 + 4)) {
LOGGER.error("Response frame is too short");
if (allowLogging)
LOGGER.error("Response frame is too short");
return null;
} else if (responseFrame[0] != (byte) 0xA5) {
LOGGER.error("Response frame has invalid starting byte");
if (allowLogging)
LOGGER.error("Response frame has invalid starting byte");
return null;
} else if (responseFrame[responseFrame.length - 1] != (byte) 0x15) {
LOGGER.error("Response frame has invalid ending byte");
if (allowLogging)
LOGGER.error("Response frame has invalid ending byte");
return null;
}

Expand Down
30 changes: 30 additions & 0 deletions src/main/java/org/openhab/binding/solarman/internal/state/LoggerState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package org.openhab.binding.solarman.internal.state;

public class LoggerState {
public final static Integer NO_FAILED_REQUESTS = 3;
private State state = State.ONLINE; // Let's assume we're online initially
private int offlineTryCount = 0;

public void setOnline() {
state = State.ONLINE;
offlineTryCount = 0;
}

public void setPossiblyOffline() {
state = ++offlineTryCount < NO_FAILED_REQUESTS ? State.LIMBO : State.OFFLINE;
}

public boolean isOffline() {
return state == State.OFFLINE;
}

public boolean isJustBecameOffline() {
return state == State.OFFLINE && offlineTryCount == NO_FAILED_REQUESTS;
}

public enum State {
ONLINE,
LIMBO,
OFFLINE,
}
}
29 changes: 13 additions & 16 deletions src/main/java/org/openhab/binding/solarman/internal/updater/SolarmanChannelUpdater.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@
import org.openhab.binding.solarman.internal.modbus.SolarmanLoggerConnection;
import org.openhab.binding.solarman.internal.modbus.SolarmanLoggerConnector;
import org.openhab.binding.solarman.internal.modbus.SolarmanV5Protocol;
import org.openhab.binding.solarman.internal.state.LoggerState;
import org.openhab.binding.solarman.internal.typeprovider.ChannelUtils;
import org.openhab.binding.solarman.internal.util.StreamUtils;
import org.openhab.core.library.types.DateTimeType;
import org.openhab.core.library.types.DecimalType;
import org.openhab.core.library.types.QuantityType;
import org.openhab.core.library.types.StringType;
import org.openhab.core.thing.ChannelUID;
import org.openhab.core.thing.ThingStatus;
import org.openhab.core.types.State;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -41,18 +41,17 @@

public class SolarmanChannelUpdater {
private final Logger LOGGER = LoggerFactory.getLogger(SolarmanChannelUpdater.class);
private final StatusUpdater statusUpdater;
private final StateUpdater stateUpdater;

public SolarmanChannelUpdater(StatusUpdater statusUpdater, StateUpdater stateUpdater) {
this.statusUpdater = statusUpdater;
public SolarmanChannelUpdater(StateUpdater stateUpdater) {
this.stateUpdater = stateUpdater;
}

public void fetchDataFromLogger(List<Request> requests,
SolarmanLoggerConnector solarmanLoggerConnector,
SolarmanV5Protocol solarmanV5Protocol,
Map<ParameterItem, ChannelUID> paramToChannelMapping) {
public boolean fetchDataFromLogger(List<Request> requests,
SolarmanLoggerConnector solarmanLoggerConnector,
SolarmanV5Protocol solarmanV5Protocol,
Map<ParameterItem, ChannelUID> paramToChannelMapping,
LoggerState loggerState) {

try (SolarmanLoggerConnection solarmanLoggerConnection = solarmanLoggerConnector.createConnection()) {
LOGGER.debug("Fetching data from logger");
Expand All @@ -61,15 +60,18 @@ public void fetchDataFromLogger(List<Request> requests,
.map(request -> solarmanV5Protocol.readRegisters(solarmanLoggerConnection,
(byte) request.getMbFunctioncode().intValue(),
request.getStart(),
request.getEnd())
request.getEnd(),
!loggerState.isOffline())
)
.reduce(new HashMap<>(), this::mergeMaps);

updateChannelsForReadRegisters(paramToChannelMapping, readRegistersMap);
if (!readRegistersMap.isEmpty())
updateChannelsForReadRegisters(paramToChannelMapping, readRegistersMap);

statusUpdater.updateStatus(readRegistersMap.isEmpty() ? ThingStatus.OFFLINE : ThingStatus.ONLINE);
return !readRegistersMap.isEmpty();
} catch (Exception e) {
LOGGER.error("Error invoking handler", e);
return false;
}
}

Expand Down Expand Up @@ -219,11 +221,6 @@ private enum ValueType {
UNSIGNED, SIGNED
}

@FunctionalInterface
public interface StatusUpdater {
void updateStatus(ThingStatus thingStatus);
}

@FunctionalInterface
public interface StateUpdater {
void updateState(ChannelUID channelUID, State state);
Expand Down
17 changes: 9 additions & 8 deletions src/test/java/org/openhab/binding/solarman/internal/modbus/SolarmanV5ProtocolTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.openhab.binding.solarman.internal.modbus;

import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.when;

Expand Down Expand Up @@ -47,11 +48,11 @@ void testbuildSolarmanV5Frame() {
@Test
void testReadRegister0x01() {
// given
when(solarmanLoggerConnection.sendRequest(any())).thenReturn(
when(solarmanLoggerConnection.sendRequest(any(), eq(true))).thenReturn(
hexStringToByteArray("a5000000000000000000000000000000000000000000000000010301000ac84300000015"));

// when
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 1, 1);
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 1, 1, true);

// then
assertEquals(1, regValues.size());
Expand All @@ -62,11 +63,11 @@ void testReadRegister0x01() {
@Test
void testReadRegisters0x02_0x03() {
// given
when(solarmanLoggerConnection.sendRequest(any())).thenReturn(
when(solarmanLoggerConnection.sendRequest(any(), eq(true))).thenReturn(
hexStringToByteArray("a5000000000000000000000000000000000000000000000000010302000a000b13f600000015"));

// when
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 2, 3);
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 2, 3, true);

// then
assertEquals(2, regValues.size());
Expand All @@ -79,11 +80,11 @@ void testReadRegisters0x02_0x03() {
@Test
void testReadRegisterSUN_10K_SG04LP3_EU_part1() {
// given
when(solarmanLoggerConnection.sendRequest(any())).thenReturn(hexStringToByteArray(
when(solarmanLoggerConnection.sendRequest(any(), eq(true))).thenReturn(hexStringToByteArray(
"a53b0010150007482ee38d020121d0060091010000403e486301032800ffffff160a12162420ffffffffffffffffffffffffffffffffffff0001ffff0001ffff000003e81fa45115"));

// when
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 0x3c, 0x4f);
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 0x3c, 0x4f, true);

// then
assertEquals(20, regValues.size());
Expand All @@ -96,11 +97,11 @@ void testReadRegisterSUN_10K_SG04LP3_EU_part1() {
@Test
void testReadRegisterSUN_10K_SG04LP3_EU_part2() {
// given
when(solarmanLoggerConnection.sendRequest(any())).thenReturn(hexStringToByteArray(
when(solarmanLoggerConnection.sendRequest(any(), eq(true))).thenReturn(hexStringToByteArray(
"a5330010150008482ee38d020122d0060091010000403e486301032000010000ffffffffffff0001ffffffffffffffffffff0000ffff0011ffffffff3a005715"));

// when
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 0x50, 0x5f);
Map<Integer, byte[]> regValues = solarmanV5Protocol.readRegisters(solarmanLoggerConnection, (byte) 0x03, 0x50, 0x5f, true);

// then
assertEquals(16, regValues.size());
Expand Down