Skip to content

Commit

Permalink
[comfoair] improve data handling (openhab#9401)
Browse files Browse the repository at this point in the history 
* improve data handling and suspend
* fix command position
* add ACK to subResponse
* fix data position
* fix reactivation, trim too long responses
* ensure correct setting of activation channel
* Remove @SuppressWarnings(null), which is not needed anymore
* Remove updateState on dispose

Signed-off-by: Hans Böhm <[email protected]>
  • Loading branch information
@boehan @seaside1
boehan authored and seaside1 committed Dec 28, 2020
1 parent eaf77c0 commit 32c68c5
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 39 deletions.
5 changes: 2 additions & 3 deletions ...ing.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirCommandType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ public enum ComfoAirCommandType {
*/
ACTIVATE(ComfoAirBindingConstants.CG_CONTROL_PREFIX + ComfoAirBindingConstants.CHANNEL_ACTIVATE,
DataTypeBoolean.getInstance(), new int[] { 0x03 }, Constants.REQUEST_SET_RS232, 1, 0,
Constants.EMPTY_TYPE_ARRAY, Constants.REPLY_SET_RS232, Constants.REPLY_SET_RS232, new int[] { 0 }, 0x03),
Constants.EMPTY_TYPE_ARRAY),
MENU20_MODE(ComfoAirBindingConstants.CG_MENUP1_PREFIX + ComfoAirBindingConstants.CHANNEL_MENU20_MODE,
DataTypeBoolean.getInstance(), Constants.REQUEST_GET_STATES, Constants.REPLY_GET_STATES, new int[] { 6 },
0x01),
Expand Down Expand Up @@ -771,7 +771,7 @@ public int getReadReplyDataBits() {
public static @Nullable ComfoAirCommand getReadCommand(String key) {
ComfoAirCommandType commandType = getCommandTypeByKey(key);

if (commandType != null) {
if (commandType != null && commandType.readCommand > 0) {
return new ComfoAirCommand(key);
}
return null;
Expand Down Expand Up @@ -891,7 +891,6 @@ public static List<ComfoAirCommandType> getCommandTypesByReplyCmd(int replyCmd)
return null;
}

@SuppressWarnings("null")
private static void uniteCommandsMap(Map<Integer, ComfoAirCommand> commands, ComfoAirCommandType commandType) {
if (commandType.readReplyCommand != 0) {
int replyCmd = commandType.readReplyCommand;
Expand Down
82 changes: 59 additions & 23 deletions ...binding.comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.comfoair.internal.datatypes.ComfoAirDataType;
import org.openhab.core.io.transport.serial.SerialPortManager;
import org.openhab.core.library.types.OnOffType;
import org.openhab.core.thing.Channel;
import org.openhab.core.thing.ChannelUID;
import org.openhab.core.thing.Thing;
Expand Down Expand Up @@ -55,6 +56,8 @@ public class ComfoAirHandler extends BaseThingHandler {
private @Nullable ComfoAirSerialConnector comfoAirConnector;

public static final int BAUDRATE = 9600;
public static final String ACTIVATE_CHANNEL_ID = ComfoAirBindingConstants.CG_CONTROL_PREFIX
+ ComfoAirBindingConstants.CHANNEL_ACTIVATE;

public ComfoAirHandler(Thing thing, final SerialPortManager serialPortManager) {
super(thing);
Expand All @@ -64,29 +67,36 @@ public ComfoAirHandler(Thing thing, final SerialPortManager serialPortManager) {
@Override
public void handleCommand(ChannelUID channelUID, Command command) {
String channelId = channelUID.getId();
if (comfoAirConnector != null) {
boolean isActive = !comfoAirConnector.getIsSuspended();

if (command instanceof RefreshType) {
Channel channel = this.thing.getChannel(channelUID);
if (channel != null) {
updateChannelState(channel);
}
} else {
ComfoAirCommand changeCommand = ComfoAirCommandType.getChangeCommand(channelId, command);
if (isActive || channelId.equals(ACTIVATE_CHANNEL_ID)) {
if (command instanceof RefreshType) {
Channel channel = this.thing.getChannel(channelUID);
if (channel != null) {
updateChannelState(channel);
}
} else {
ComfoAirCommand changeCommand = ComfoAirCommandType.getChangeCommand(channelId, command);

if (changeCommand != null) {
Set<String> keysToUpdate = getThing().getChannels().stream().map(Channel::getUID).filter(this::isLinked)
.map(ChannelUID::getId).collect(Collectors.toSet());
sendCommand(changeCommand, channelId);
if (changeCommand != null) {
Set<String> keysToUpdate = getThing().getChannels().stream().map(Channel::getUID)
.filter(this::isLinked).map(ChannelUID::getId).collect(Collectors.toSet());
sendCommand(changeCommand, channelId);

Collection<ComfoAirCommand> affectedReadCommands = ComfoAirCommandType
.getAffectedReadCommands(channelId, keysToUpdate);
Collection<ComfoAirCommand> affectedReadCommands = ComfoAirCommandType
.getAffectedReadCommands(channelId, keysToUpdate);

if (affectedReadCommands.size() > 0) {
Runnable updateThread = new AffectedItemsUpdateThread(affectedReadCommands);
affectedItemsPoller = scheduler.schedule(updateThread, 3, TimeUnit.SECONDS);
if (affectedReadCommands.size() > 0) {
Runnable updateThread = new AffectedItemsUpdateThread(affectedReadCommands);
affectedItemsPoller = scheduler.schedule(updateThread, 3, TimeUnit.SECONDS);
}
} else {
logger.warn("Unhandled command type: {}, channelId: {}", command.toString(), channelId);
}
}
} else {
logger.warn("Unhandled command type: {}, channelId: {}", command.toString(), channelId);
logger.debug("Binding control is currently not active.");
}
}
}
Expand Down Expand Up @@ -115,6 +125,8 @@ private void connect() {
updateStatus(ThingStatus.ONLINE);
pullDeviceProperties();

updateState(ACTIVATE_CHANNEL_ID, OnOffType.ON);

List<Channel> channels = this.thing.getChannels();

poller = scheduler.scheduleWithFixedDelay(() -> {
Expand Down Expand Up @@ -159,14 +171,38 @@ private void updateChannelState(Channel channel) {
if (!isLinked(channel.getUID())) {
return;
}
String commandKey = channel.getUID().getId();

ComfoAirCommand readCommand = ComfoAirCommandType.getReadCommand(commandKey);
if (readCommand != null) {
scheduler.submit(() -> {
State state = sendCommand(readCommand, commandKey);
if (comfoAirConnector != null) {
boolean isActive = !comfoAirConnector.getIsSuspended();

String commandKey = channel.getUID().getId();
if (commandKey.equals(ACTIVATE_CHANNEL_ID)) {
State state = OnOffType.from(isActive);
updateState(channel.getUID(), state);
});
return;
}

if (!isActive) {
logger.debug("Binding control is currently not active.");
return;
}

ComfoAirCommand readCommand = ComfoAirCommandType.getReadCommand(commandKey);
if (readCommand != null && readCommand.getRequestCmd() != null) {
scheduler.submit(() -> {
State state = sendCommand(readCommand, commandKey);
updateState(channel.getUID(), state);
});
}
}
}

@Override
public void channelLinked(ChannelUID channelUID) {
super.channelLinked(channelUID);
Channel channel = this.thing.getChannel(channelUID);
if (channel != null) {
updateChannelState(channel);
}
}

Expand Down
61 changes: 51 additions & 10 deletions ...comfoair/src/main/java/org/openhab/binding/comfoair/internal/ComfoAirSerialConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ public class ComfoAirSerialConnector {
private static final byte[] END = { CTRL, (byte) 0x0f };
private static final byte[] ACK = { CTRL, (byte) 0xf3 };

private static final int RS232_ENABLED_VALUE = 0x03;
private static final int RS232_DISABLED_VALUE = 0x00;
private static final int MAX_RETRIES = 5;

private boolean isSuspended = true;

Expand Down Expand Up @@ -150,14 +149,17 @@ public void close() {
*/
public synchronized int[] sendCommand(ComfoAirCommand command, int[] preRequestData) {
Integer requestCmd = command.getRequestCmd();
Integer requestValue = command.getRequestValue();
int retry = 0;

if (requestCmd != null) {
// Switch support for app or ccease control
if (requestCmd == ComfoAirCommandType.Constants.REQUEST_SET_RS232) {
isSuspended = !isSuspended;
} else if (requestCmd == ComfoAirCommandType.Constants.REPLY_SET_RS232) {
return new int[] { isSuspended ? RS232_DISABLED_VALUE : RS232_ENABLED_VALUE };
if (requestCmd == ComfoAirCommandType.Constants.REQUEST_SET_RS232 && requestValue != null) {
if (requestValue == 1) {
isSuspended = false;
} else if (requestValue == 0) {
isSuspended = true;
}
} else if (isSuspended) {
logger.trace("Ignore cmd. Service is currently suspended");
return ComfoAirCommandType.Constants.EMPTY_INT_ARRAY;
Expand Down Expand Up @@ -226,13 +228,44 @@ public synchronized int[] sendCommand(ComfoAirCommand command, int[] preRequestD
return ComfoAirCommandType.Constants.EMPTY_INT_ARRAY;
}

boolean isValidData = false;

// check for start and end sequence and if the response cmd
// matches
// 11 is the minimum response length with one data byte
if (responseBlock.length >= 11 && responseBlock[2] == START[0] && responseBlock[3] == START[1]
&& responseBlock[responseBlock.length - 2] == END[0]
&& responseBlock[responseBlock.length - 1] == END[1]
&& (responseBlock[5] & 0xff) == command.getReplyCmd()) {
&& responseBlock[responseBlock.length - 1] == END[1]) {
if ((responseBlock[5] & 0xff) == command.getReplyCmd()) {
isValidData = true;
} else {
int startIndex = -1;
int endIndex = -1;

for (int i = 4; i < (responseBlock.length - 11) && endIndex < 0; i++) {
if (responseBlock[i] == START[0] && responseBlock[i + 1] == START[1]
&& ((responseBlock[i + 3] & 0xff) == command.getReplyCmd())) {
startIndex = i;
for (int j = startIndex; j < responseBlock.length; j++) {
if (responseBlock[j] == END[0] && responseBlock[j + 1] == END[1]) {
endIndex = j + 1;
break;
}
}
}
}

if (startIndex > -1 && endIndex > -1) {
byte[] subResponse = new byte[endIndex - startIndex + 3];
System.arraycopy(responseBlock, 0, subResponse, 0, 2);
System.arraycopy(responseBlock, startIndex, subResponse, 2, subResponse.length - 2);
responseBlock = subResponse;
isValidData = true;
}
}
}

if (isValidData) {
if (logger.isTraceEnabled()) {
logger.trace("receive RAW DATA: {}", dumpData(responseBlock));
}
Expand Down Expand Up @@ -294,9 +327,9 @@ public synchronized int[] sendCommand(ComfoAirCommand command, int[] preRequestD
logger.warn("Transmission was interrupted: {}", e.getMessage());
throw new RuntimeException(e);
}
} while (retry++ < 5);
} while (retry++ < MAX_RETRIES);

if (retry == 5) {
if (retry >= MAX_RETRIES) {
logger.debug("Unable to send command. {} retries failed.", retry);
}
}
Expand Down Expand Up @@ -375,6 +408,10 @@ private byte[] cleanupBlock(byte[] processBuffer) {
}
cleanedBuffer[pos] = processBuffer[i];
pos++;
// Trim unrequested data in response
if (END[0] == processBuffer[i + 1] && END[1] == processBuffer[i + 2]) {
break;
}
}
return Arrays.copyOf(cleanedBuffer, pos);
}
Expand Down Expand Up @@ -560,4 +597,8 @@ private int checkByteAndCalculateValue(ComfoAirCommand command, int requestValue
}
return 0;
}

public boolean getIsSuspended() {
return isSuspended;
}
}
3 changes: 0 additions & 3 deletions ...mfoair/src/main/java/org/openhab/binding/comfoair/internal/datatypes/DataTypeBoolean.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public State convertToState(int @Nullable [] data, ComfoAirCommandType commandTy
} else {
int[] readReplyDataPos = commandType.getReadReplyDataPos();
int readReplyDataBits = commandType.getReadReplyDataBits();
int readCommand = commandType.getReadCommand();
boolean result;

if (readReplyDataPos != null && readReplyDataPos[0] < data.length) {
Expand All @@ -59,8 +58,6 @@ public State convertToState(int @Nullable [] data, ComfoAirCommandType commandTy
result = (data[readReplyDataPos[0]] & readReplyDataBits) == readReplyDataBits;
}
return OnOffType.from(result);
} else if (readCommand == 0) {
return OnOffType.OFF; // handle write-only commands (resets)
} else {
return UnDefType.NULL;
}
Expand Down

0 comments on commit 32c68c5

Please sign in to comment.