Skip to content

Commit

Permalink
[dsmr] Improved error handling to better handle corrupt messages.
Browse files Browse the repository at this point in the history 
Signed-off-by: Hilbrand Bouwkamp <[email protected]>
  • Loading branch information
@Hilbrand
Hilbrand committed Nov 20, 2022
1 parent a8f6719 commit 137dbba
Show file tree
Hide file tree
Showing 24 changed files with 326 additions and 215 deletions.
10 changes: 6 additions & 4 deletions ...nding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRDeviceRunnable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/
package org.openhab.binding.dsmr.internal.device;

import java.util.Optional;
import java.util.concurrent.Semaphore;

import org.eclipse.jdt.annotation.NonNullByDefault;
Expand Down Expand Up @@ -45,7 +46,7 @@ public class DSMRDeviceRunnable implements Runnable {
* @param device the device to control
* @param eventListener listener to used ot report errors.
*/
public DSMRDeviceRunnable(DSMRDevice device, DSMREventListener eventListener) {
public DSMRDeviceRunnable(final DSMRDevice device, final DSMREventListener eventListener) {
this.device = device;
this.portEventListener = eventListener;
}
Expand Down Expand Up @@ -83,10 +84,11 @@ public void run() {
}
}
logger.trace("Device shutdown");
} catch (RuntimeException e) {
} catch (final RuntimeException e) {
logger.warn("DSMRDeviceRunnable stopped with a RuntimeException", e);
portEventListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR);
} catch (InterruptedException e) {
portEventListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR,
Optional.ofNullable(e.getMessage()).orElse(""));
} catch (final InterruptedException e) {
Thread.currentThread().interrupt();
} finally {
device.stop();
Expand Down
3 changes: 2 additions & 1 deletion ...inding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMREventListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ public interface DSMREventListener {
* Callback for DSMRPortEvent events
*
* @param connectorErrorEvent {@link DSMRConnectorErrorEvent} that has occurred
* @param message Additional error message
*/
public void handleErrorEvent(DSMRConnectorErrorEvent connectorErrorEvent);
public void handleErrorEvent(DSMRConnectorErrorEvent connectorErrorEvent, String message);

/**
* Callback for received P1 telegrams
Expand Down
32 changes: 15 additions & 17 deletions ...ing.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRSerialAutoDevice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ enum DeviceState {
/**
* The listener of the class handling the connector events
*/
private DSMREventListener parentListener;
private final DSMREventListener parentListener;

/**
* Time in nanos the last time the baudrate was switched. This is used during discovery ignore errors retrieved
Expand All @@ -132,9 +132,9 @@ enum DeviceState {
* @param baudrateSwitchTimeoutSeconds timeout period for when to try other baudrate settings and end the discovery
* of the baudrate
*/
public DSMRSerialAutoDevice(SerialPortManager serialPortManager, String serialPortName, DSMREventListener listener,
DSMRTelegramListener telegramListener, ScheduledExecutorService scheduler,
int baudrateSwitchTimeoutSeconds) {
public DSMRSerialAutoDevice(final SerialPortManager serialPortManager, final String serialPortName,
final DSMREventListener listener, final DSMRTelegramListener telegramListener,
final ScheduledExecutorService scheduler, final int baudrateSwitchTimeoutSeconds) {
this.parentListener = listener;
this.scheduler = scheduler;
this.baudrateSwitchTimeoutSeconds = baudrateSwitchTimeoutSeconds;
Expand Down Expand Up @@ -178,13 +178,11 @@ public synchronized void stop() {
* @param telegram the details of the received telegram
*/
@Override
public void handleTelegramReceived(P1Telegram telegram) {
if (!telegram.getCosemObjects().isEmpty()) {
stopDiscover(DeviceState.NORMAL);
parentListener.handleTelegramReceived(telegram);
logger.info("Start receiving telegrams on port {} with settings: {}", dsmrConnector.getPortName(),
portSettings);
}
public void handleTelegramReceived(final P1Telegram telegram) {
stopDiscover(DeviceState.NORMAL);
parentListener.handleTelegramReceived(telegram);
logger.info("Start receiving telegrams on port {} with settings: {}", dsmrConnector.getPortName(),
portSettings);
}

/**
Expand All @@ -193,23 +191,23 @@ public void handleTelegramReceived(P1Telegram telegram) {
* @param portEvent {@link DSMRConnectorErrorEvent} to handle
*/
@Override
public void handleErrorEvent(DSMRConnectorErrorEvent portEvent) {
public void handleErrorEvent(final DSMRConnectorErrorEvent portEvent, final String message) {
logger.trace("Received portEvent {}", portEvent.getEventDetails());
if (portEvent == DSMRConnectorErrorEvent.READ_ERROR) {
switchBaudrate();
} else {
logger.debug("Error during discovery of port settings: {}, current state:{}.", portEvent.getEventDetails(),
state);
stopDiscover(DeviceState.ERROR);
parentListener.handleErrorEvent(portEvent);
parentListener.handleErrorEvent(portEvent, message);
}
}

/**
* @param lenientMode the lenientMode to set
*/
@Override
public void setLenientMode(boolean lenientMode) {
public void setLenientMode(final boolean lenientMode) {
telegramListener.setLenientMode(lenientMode);
}

Expand Down Expand Up @@ -248,7 +246,7 @@ private void switchBaudrate() {
private void endTimeScheduledCall() {
if (state == DeviceState.DISCOVER_SETTINGS) {
stopDiscover(DeviceState.ERROR);
parentListener.handleErrorEvent(DSMRConnectorErrorEvent.DONT_EXISTS);
parentListener.handleErrorEvent(DSMRConnectorErrorEvent.DONT_EXISTS, "");
}
}

Expand All @@ -257,7 +255,8 @@ private void endTimeScheduledCall() {
*
* @param state the state with which the process was stopped.
*/
private void stopDiscover(DeviceState state) {
private void stopDiscover(final DeviceState state) {
this.state = state;
telegramListener.setDsmrEventListener(parentListener);
logger.debug("Stop discovery of port settings.");
if (halfTimeTimer != null) {
Expand All @@ -268,7 +267,6 @@ private void stopDiscover(DeviceState state) {
endTimeTimer.cancel(true);
endTimeTimer = null;
}
this.state = state;
}

/**
Expand Down
19 changes: 8 additions & 11 deletions ...ing.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/DSMRTelegramListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
package org.openhab.binding.dsmr.internal.device;

import java.util.List;
import java.util.stream.Collectors;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.binding.dsmr.internal.device.connector.DSMRConnectorErrorEvent;
Expand Down Expand Up @@ -76,8 +75,8 @@ public void handleData(final byte[] data, final int length) {
}

@Override
public void handleErrorEvent(final DSMRConnectorErrorEvent portEvent) {
dsmrEventListener.handleErrorEvent(portEvent);
public void handleErrorEvent(final DSMRConnectorErrorEvent portEvent, final String message) {
dsmrEventListener.handleErrorEvent(portEvent, message);
parser.reset();
}

Expand All @@ -94,14 +93,12 @@ public void telegramReceived(final P1Telegram telegram) {
if (logger.isTraceEnabled()) {
logger.trace("Received {} Cosem Objects with state: '{}'", cosemObjects.size(), telegramState);
}
if (telegramState == TelegramState.OK || telegramState == TelegramState.INVALID_ENCRYPTION_KEY) {
dsmrEventListener.handleTelegramReceived(telegram);
} else {
if (logger.isDebugEnabled()) {
logger.debug("Telegram received with error state '{}': {}", telegramState,
cosemObjects.stream().map(CosemObject::toString).collect(Collectors.joining(",")));
}
}
dsmrEventListener.handleTelegramReceived(telegram);
}

@Override
public void onTelegramError(final TelegramState state, final String message) {
dsmrEventListener.handleErrorEvent(DSMRConnectorErrorEvent.PARSE_ERROR, message);
}

/**
Expand Down
45 changes: 27 additions & 18 deletions ....binding.dsmr/src/main/java/org/openhab/binding/dsmr/internal/device/SmartyDecrypter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.util.Collections;
import java.util.Arrays;
import java.util.Optional;

import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
Expand All @@ -28,7 +29,6 @@
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.dsmr.internal.DSMRBindingConstants;
import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram;
import org.openhab.binding.dsmr.internal.device.p1telegram.P1Telegram.TelegramState;
import org.openhab.binding.dsmr.internal.device.p1telegram.P1TelegramListener;
import org.openhab.binding.dsmr.internal.device.p1telegram.TelegramParser;
Expand All @@ -53,8 +53,7 @@ private enum State {
READ_SEPARATOR_30,
READ_FRAME_COUNTER,
READ_PAYLOAD,
READ_GCM_TAG,
DONE_READING_TELEGRAM
READ_GCM_TAG
}

private static final byte START_BYTE = (byte) 0xDB;
Expand Down Expand Up @@ -113,6 +112,11 @@ public void parse(final byte[] data, final int length) {
}

private boolean processStateActions(final byte rawInput) {
// Safeguard against buffer overrun in case corrupt data is received.
if (ivLength == IV_BUFFER_LENGTH) {
reset();
return false;
}
switch (state) {
case WAITING_FOR_START_BYTE:
if (rawInput == START_BYTE) {
Expand Down Expand Up @@ -179,26 +183,23 @@ private boolean processStateActions(final byte rawInput) {
// All input has been read.
cipherText.put(rawInput);
if (currentBytePosition >= changeToNextStateAt) {
state = State.DONE_READING_TELEGRAM;
state = State.WAITING_FOR_START_BYTE;
return true;
}
break;
}
if (state == State.DONE_READING_TELEGRAM) {
state = State.WAITING_FOR_START_BYTE;
return true;
}
return false;
}

private void processCompleted() {
final byte[] plainText = decrypt();

reset();
if (plainText == null) {
telegramListener
.telegramReceived(new P1Telegram(Collections.emptyList(), TelegramState.INVALID_ENCRYPTION_KEY));
} else {
parser.parse(plainText, plainText.length);
try {
final byte[] plainText = decrypt();

if (plainText != null) {
parser.parse(plainText, plainText.length);
}
} finally {
reset();
}
}

Expand All @@ -218,7 +219,15 @@ private void processCompleted() {
}
} catch (NoSuchAlgorithmException | NoSuchPaddingException | InvalidKeyException
| InvalidAlgorithmParameterException | IllegalBlockSizeException | BadPaddingException e) {
logger.warn("Decrypting smarty telegram failed: ", e);
if (lenientMode || logger.isDebugEnabled()) {
// log in lenient mode or when debug is enabled. But log to warn to also work when lenientMode is
// enabled.
logger.warn("Failed encrypted telegram: {}",
HexUtils.bytesToHex(Arrays.copyOf(cipherText.array(), cipherText.position())));
logger.warn("Exception of failed decryption of telegram: ", e);
}
telegramListener.onTelegramError(TelegramState.INVALID_ENCRYPTION_KEY,
Optional.ofNullable(e.getMessage()).orElse(""));
}
return null;
}
Expand Down
16 changes: 9 additions & 7 deletions ...r/src/main/java/org/openhab/binding/dsmr/internal/device/connector/DSMRBaseConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.Optional;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
Expand Down Expand Up @@ -53,7 +54,7 @@ class DSMRBaseConnector {
*/
private boolean open;

public DSMRBaseConnector(DSMRConnectorListener connectorListener) {
public DSMRBaseConnector(final DSMRConnectorListener connectorListener) {
this.dsmrConnectorListener = connectorListener;
}

Expand All @@ -68,7 +69,7 @@ public DSMRBaseConnector(DSMRConnectorListener connectorListener) {
* @param inputStream input stream to read data from
* @throws IOException throws exception in case input stream is null
*/
protected void open(@Nullable InputStream inputStream) throws IOException {
protected void open(@Nullable final InputStream inputStream) throws IOException {
if (inputStream == null) {
throw new IOException("Inputstream is null");
}
Expand All @@ -91,7 +92,7 @@ protected void close() {
if (inputStream != null) {
try {
inputStream.close();
} catch (IOException ioe) {
} catch (final IOException ioe) {
logger.debug("Failed to close reader", ioe);
}
}
Expand All @@ -104,12 +105,12 @@ protected void close() {
protected void handleDataAvailable() {
try {
synchronized (readLock) {
BufferedInputStream localInputStream = inputStream;
final BufferedInputStream localInputStream = inputStream;

if (localInputStream != null) {
int bytesAvailable = localInputStream.available();
while (bytesAvailable > 0) {
int bytesAvailableRead = localInputStream.read(buffer, 0,
final int bytesAvailableRead = localInputStream.read(buffer, 0,
Math.min(bytesAvailable, buffer.length));

if (open && bytesAvailableRead > 0) {
Expand All @@ -122,8 +123,9 @@ protected void handleDataAvailable() {
}
}
}
} catch (IOException e) {
dsmrConnectorListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR);
} catch (final IOException e) {
dsmrConnectorListener.handleErrorEvent(DSMRConnectorErrorEvent.READ_ERROR,
Optional.ofNullable(e.getMessage()).orElse(""));
logger.debug("Exception on read data", e);
}
}
Expand Down
5 changes: 4 additions & 1 deletion ...main/java/org/openhab/binding/dsmr/internal/device/connector/DSMRConnectorErrorEvent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
*/
package org.openhab.binding.dsmr.internal.device.connector;

import java.util.Locale;

import org.eclipse.jdt.annotation.NonNullByDefault;

/**
Expand All @@ -26,12 +28,13 @@ public enum DSMRConnectorErrorEvent {
IN_USE,
INTERNAL_ERROR,
NOT_COMPATIBLE,
PARSE_ERROR,
READ_ERROR;

/**
* @return the event details
*/
public String getEventDetails() {
return "@text/error.connector." + name().toLowerCase();
return "@text/binding.dsmr.error.connector." + name().toLowerCase(Locale.ROOT);
}
}
3 changes: 2 additions & 1 deletion ...c/main/java/org/openhab/binding/dsmr/internal/device/connector/DSMRConnectorListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ public interface DSMRConnectorListener {
* Callback for {@link DSMRConnectorErrorEvent} events.
*
* @param portEvent {@link DSMRConnectorErrorEvent} that has occurred
* @param message Additional error message
*/
public void handleErrorEvent(DSMRConnectorErrorEvent portEvent);
void handleErrorEvent(DSMRConnectorErrorEvent portEvent, String message);

/**
* Handle data.
Expand Down
Loading

0 comments on commit 137dbba

Please sign in to comment.