[powermax] Improved error handling for Powermax binding (#9817)

* Improved error handling for Powermax binding

Catch all exceptions, and take all Powermax things offline in the event
of a communication failure. Also adds a 5-minute watchdog for
connections in Standard (non-Powerlink) mode.

* More Powermax error handling improvements

1. Mark the bridge and all things as offline when communication is lost
2. Bubble all I/O exceptions up to the bridge status
3. Set all channels to UnDefType.NULL when bridge goes offline
4. Ignore CRC errors on messages of type 0xF1
5. Don't try and download partition settings for panels that don't support partitions
6. Fix logging levels to avoid unnecessary non-DEBUG messages

Signed-off-by: Ron Isaacson <isaacson.ron@gmail.com>
This commit is contained in:
Ron Isaacson 2021-02-08 16:48:38 -05:00 committed by GitHub
parent b65e932650
commit 7898f4f911
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 160 additions and 130 deletions

View File

@ -47,7 +47,7 @@ public abstract class PowermaxConnector implements PowermaxConnectorInterface {
}
@Override
public abstract void open();
public abstract void open() throws Exception;
@Override
public abstract void close();
@ -101,9 +101,15 @@ public abstract class PowermaxConnector implements PowermaxConnectorInterface {
PowermaxBaseMessage.getMessageHandler(incomingMessage));
// send message to event listeners
for (int i = 0; i < listeners.size(); i++) {
listeners.get(i).onNewMessageEvent(event);
}
listeners.forEach(listener -> listener.onNewMessageEvent(event));
}
/**
* Handles a communication failure
*/
public void handleCommunicationFailure(String message) {
close();
listeners.forEach(listener -> listener.onCommunicationFailure(message));
}
@Override
@ -113,7 +119,7 @@ public abstract class PowermaxConnector implements PowermaxConnectorInterface {
output.flush();
} catch (IOException e) {
logger.debug("sendMessage(): Writing error: {}", e.getMessage(), e);
setConnected(false);
handleCommunicationFailure(e.getMessage());
}
}

View File

@ -26,7 +26,7 @@ public interface PowermaxConnectorInterface {
/**
* Method for opening a connection to the Visonic alarm panel.
*/
public void open();
public void open() throws Exception;
/**
* Method for closing a connection to the Visonic alarm panel.

View File

@ -50,7 +50,7 @@ public class PowermaxReaderThread extends Thread {
@Override
public void run() {
logger.debug("Data listener started");
logger.info("Data listener started");
byte[] readDataBuffer = new byte[READ_BUFFER_SIZE];
byte[] dataBuffer = new byte[MAX_MSG_SIZE];
@ -129,9 +129,14 @@ public class PowermaxReaderThread extends Thread {
logger.debug("Interrupted via InterruptedIOException");
} catch (IOException e) {
logger.debug("Reading failed: {}", e.getMessage(), e);
connector.handleCommunicationFailure(e.getMessage());
} catch (Exception e) {
String msg = e.getMessage() != null ? e.getMessage() : e.toString();
logger.debug("Error reading or processing message: {}", msg, e);
connector.handleCommunicationFailure(msg);
}
logger.debug("Data listener stopped");
logger.info("Data listener stopped");
}
/**
@ -143,6 +148,11 @@ public class PowermaxReaderThread extends Thread {
* @return true if the CRC is valid or false if not
*/
private boolean checkCRC(byte[] data, int len) {
// Messages of type 0xF1 are always sent with a bad CRC (possible panel bug?)
if (len == 9 && (data[1] & 0xFF) == 0xF1) {
return true;
}
byte checksum = PowermaxCommManager.computeCRC(data, len);
byte expected = data[len - 2];
if (checksum != expected) {

View File

@ -13,16 +13,12 @@
package org.openhab.binding.powermax.internal.connector;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.util.TooManyListenersException;
import org.openhab.core.io.transport.serial.PortInUseException;
import org.openhab.core.io.transport.serial.SerialPort;
import org.openhab.core.io.transport.serial.SerialPortEvent;
import org.openhab.core.io.transport.serial.SerialPortEventListener;
import org.openhab.core.io.transport.serial.SerialPortIdentifier;
import org.openhab.core.io.transport.serial.SerialPortManager;
import org.openhab.core.io.transport.serial.UnsupportedCommOperationException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -58,59 +54,39 @@ public class PowermaxSerialConnector extends PowermaxConnector implements Serial
}
@Override
public void open() {
public void open() throws Exception {
logger.debug("open(): Opening Serial Connection");
try {
SerialPortIdentifier portIdentifier = serialPortManager.getIdentifier(serialPortName);
if (portIdentifier != null) {
SerialPort commPort = portIdentifier.open(this.getClass().getName(), 2000);
serialPort = commPort;
serialPort.setSerialPortParams(baudRate, SerialPort.DATABITS_8, SerialPort.STOPBITS_1,
SerialPort.PARITY_NONE);
serialPort.enableReceiveThreshold(1);
serialPort.enableReceiveTimeout(250);
setInput(serialPort.getInputStream());
setOutput(serialPort.getOutputStream());
getOutput().flush();
if (getInput().markSupported()) {
getInput().reset();
}
// RXTX serial port library causes high CPU load
// Start event listener, which will just sleep and slow down event
// loop
try {
serialPort.addEventListener(this);
serialPort.notifyOnDataAvailable(true);
} catch (TooManyListenersException e) {
logger.debug("Too Many Listeners Exception: {}", e.getMessage(), e);
}
setReaderThread(new PowermaxReaderThread(this, readerThreadName));
getReaderThread().start();
setConnected(true);
} else {
logger.debug("open(): No Such Port: {}", serialPortName);
setConnected(false);
}
} catch (PortInUseException e) {
logger.debug("open(): Port in Use Exception: {}", e.getMessage(), e);
setConnected(false);
} catch (UnsupportedCommOperationException e) {
logger.debug("open(): Unsupported Comm Operation Exception: {}", e.getMessage(), e);
setConnected(false);
} catch (UnsupportedEncodingException e) {
logger.debug("open(): Unsupported Encoding Exception: {}", e.getMessage(), e);
setConnected(false);
} catch (IOException e) {
logger.debug("open(): IO Exception: {}", e.getMessage(), e);
setConnected(false);
SerialPortIdentifier portIdentifier = serialPortManager.getIdentifier(serialPortName);
if (portIdentifier == null) {
throw new IOException("No Such Port: " + serialPortName);
}
SerialPort commPort = portIdentifier.open(this.getClass().getName(), 2000);
serialPort = commPort;
serialPort.setSerialPortParams(baudRate, SerialPort.DATABITS_8, SerialPort.STOPBITS_1, SerialPort.PARITY_NONE);
serialPort.enableReceiveThreshold(1);
serialPort.enableReceiveTimeout(250);
setInput(serialPort.getInputStream());
setOutput(serialPort.getOutputStream());
getOutput().flush();
if (getInput().markSupported()) {
getInput().reset();
}
// RXTX serial port library causes high CPU load
// Start event listener, which will just sleep and slow down event
// loop
serialPort.addEventListener(this);
serialPort.notifyOnDataAvailable(true);
setReaderThread(new PowermaxReaderThread(this, readerThreadName));
getReaderThread().start();
setConnected(true);
}
@Override

View File

@ -16,9 +16,7 @@ import java.io.IOException;
import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.SocketAddress;
import java.net.SocketException;
import java.net.SocketTimeoutException;
import java.net.UnknownHostException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -53,35 +51,21 @@ public class PowermaxTcpConnector extends PowermaxConnector {
}
@Override
public void open() {
public void open() throws Exception {
logger.debug("open(): Opening TCP Connection");
try {
tcpSocket = new Socket();
tcpSocket.setSoTimeout(250);
SocketAddress socketAddress = new InetSocketAddress(ipAddress, tcpPort);
tcpSocket.connect(socketAddress, connectTimeout);
tcpSocket = new Socket();
tcpSocket.setSoTimeout(250);
SocketAddress socketAddress = new InetSocketAddress(ipAddress, tcpPort);
tcpSocket.connect(socketAddress, connectTimeout);
setInput(tcpSocket.getInputStream());
setOutput(tcpSocket.getOutputStream());
setInput(tcpSocket.getInputStream());
setOutput(tcpSocket.getOutputStream());
setReaderThread(new PowermaxReaderThread(this, readerThreadName));
getReaderThread().start();
setReaderThread(new PowermaxReaderThread(this, readerThreadName));
getReaderThread().start();
setConnected(true);
} catch (UnknownHostException e) {
logger.debug("open(): Unknown Host Exception: {}", e.getMessage(), e);
setConnected(false);
} catch (SocketException e) {
logger.debug("open(): Socket Exception: {}", e.getMessage(), e);
setConnected(false);
} catch (IOException e) {
logger.debug("open(): IO Exception: {}", e.getMessage(), e);
setConnected(false);
} catch (Exception e) {
logger.debug("open(): Exception: {}", e.getMessage(), e);
setConnected(false);
}
setConnected(true);
}
@Override

View File

@ -27,6 +27,7 @@ import org.openhab.binding.powermax.internal.config.PowermaxIpConfiguration;
import org.openhab.binding.powermax.internal.config.PowermaxSerialConfiguration;
import org.openhab.binding.powermax.internal.discovery.PowermaxDiscoveryService;
import org.openhab.binding.powermax.internal.message.PowermaxCommManager;
import org.openhab.binding.powermax.internal.message.PowermaxSendType;
import org.openhab.binding.powermax.internal.state.PowermaxArmMode;
import org.openhab.binding.powermax.internal.state.PowermaxPanelSettings;
import org.openhab.binding.powermax.internal.state.PowermaxPanelSettingsListener;
@ -48,6 +49,7 @@ import org.openhab.core.thing.binding.BaseBridgeHandler;
import org.openhab.core.thing.binding.ThingHandlerService;
import org.openhab.core.types.Command;
import org.openhab.core.types.RefreshType;
import org.openhab.core.types.UnDefType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -64,6 +66,7 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
private final TimeZoneProvider timeZoneProvider;
private static final long ONE_MINUTE = TimeUnit.MINUTES.toMillis(1);
private static final long FIVE_MINUTES = TimeUnit.MINUTES.toMillis(5);
/** Default delay in milliseconds to reset a motion detection */
private static final long DEFAULT_MOTION_OFF_DELAY = TimeUnit.MINUTES.toMillis(3);
@ -252,24 +255,31 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
}
/*
* Check that we receive a keep alive message during the last minute
* Check that we're actively communicating with the panel
*/
private void checkKeepAlive() {
long now = System.currentTimeMillis();
if (Boolean.TRUE.equals(currentState.powerlinkMode.getValue())
&& (currentState.lastKeepAlive.getValue() != null)
&& ((now - currentState.lastKeepAlive.getValue()) > ONE_MINUTE)) {
// Let Powermax know we are alive
// In Powerlink mode: let Powermax know we are alive
commManager.sendRestoreMessage();
currentState.lastKeepAlive.setValue(now);
} else if (!Boolean.TRUE.equals(currentState.downloadMode.getValue())
&& (currentState.lastMessageReceived.getValue() != null)
&& ((now - currentState.lastMessageReceived.getValue()) > FIVE_MINUTES)) {
// In Standard mode: ping the panel every so often to detect disconnects
commManager.sendMessage(PowermaxSendType.STATUS);
}
}
private void tryReconnect() {
logger.debug("trying to reconnect...");
logger.info("Trying to connect or reconnect...");
closeConnection();
currentState = commManager.createNewState();
if (openConnection()) {
try {
openConnection();
logger.debug("openConnection(): connected");
updateStatus(ThingStatus.ONLINE);
if (forceStandardMode) {
currentState.powerlinkMode.setValue(false);
@ -278,8 +288,10 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
} else {
commManager.startDownload();
}
} else {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Reconnection failed");
} catch (Exception e) {
logger.debug("openConnection(): {}", e.getMessage(), e);
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
setAllChannelsOffline();
}
}
@ -288,14 +300,12 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
*
* @return true if the connection has been opened
*/
private synchronized boolean openConnection() {
private synchronized void openConnection() throws Exception {
if (commManager != null) {
commManager.addEventListener(this);
commManager.open();
}
remainingDownloadAttempts = MAX_DOWNLOAD_ATTEMPTS;
logger.debug("openConnection(): {}", isConnected() ? "connected" : "disconnected");
return isConnected();
}
/**
@ -474,6 +484,12 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
}
}
@Override
public void onCommunicationFailure(String message) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, message);
setAllChannelsOffline();
}
private void processPanelSettings() {
if (commManager.processPanelSettings(Boolean.TRUE.equals(currentState.powerlinkMode.getValue()))) {
for (PowermaxPanelSettingsListener listener : listeners) {
@ -489,10 +505,10 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
}
updatePropertiesFromPanelSettings();
if (Boolean.TRUE.equals(currentState.powerlinkMode.getValue())) {
logger.debug("Powermax alarm binding: running in Powerlink mode");
logger.info("Powermax alarm binding: running in Powerlink mode");
commManager.sendRestoreMessage();
} else {
logger.debug("Powermax alarm binding: running in Standard mode");
logger.info("Powermax alarm binding: running in Standard mode");
commManager.getInfosWhenInStandardMode();
}
}
@ -513,7 +529,7 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
* @param state: the alarm system state
*/
private synchronized void updateChannelsFromAlarmState(String channel, PowermaxState state) {
if (state == null) {
if (state == null || !isConnected()) {
return;
}
@ -560,6 +576,13 @@ public class PowermaxBridgeHandler extends BaseBridgeHandler implements Powermax
}
}
/**
* Update all channels to an UNDEF state to indicate that communication with the panel is offline
*/
private synchronized void setAllChannelsOffline() {
getThing().getChannels().forEach(c -> updateState(c.getUID(), UnDefType.UNDEF));
}
/**
* Update properties to match the alarm panel settings
*/

View File

@ -34,6 +34,7 @@ import org.openhab.core.thing.binding.BaseThingHandler;
import org.openhab.core.thing.binding.ThingHandler;
import org.openhab.core.types.Command;
import org.openhab.core.types.RefreshType;
import org.openhab.core.types.UnDefType;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -113,6 +114,7 @@ public class PowermaxThingHandler extends BaseThingHandler implements PowermaxPa
}
} else {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE);
setAllChannelsOffline();
logger.debug("Set handler status to OFFLINE for thing {} (bridge OFFLINE)", getThing().getUID());
}
} else {
@ -121,6 +123,13 @@ public class PowermaxThingHandler extends BaseThingHandler implements PowermaxPa
}
}
/**
* Update all channels to an UNDEF state to indicate that the bridge is offline
*/
private synchronized void setAllChannelsOffline() {
getThing().getChannels().forEach(c -> updateState(c.getUID(), UnDefType.UNDEF));
}
@Override
public void dispose() {
logger.debug("Handler disposed for thing {}", getThing().getUID());

View File

@ -176,13 +176,12 @@ public class PowermaxCommManager implements PowermaxMessageEventListener {
*
* @return true if connected or false if not
*/
public boolean open() {
public void open() throws Exception {
if (connector != null) {
connector.open();
}
lastSendMsg = null;
msgQueue = new ConcurrentLinkedQueue<>();
return isConnected();
}
/**
@ -255,28 +254,37 @@ public class PowermaxCommManager implements PowermaxMessageEventListener {
}
PowermaxState updateState = message.handleMessage(this);
if (updateState != null) {
if (updateState.getUpdateSettings() != null) {
panelSettings.updateRawSettings(updateState.getUpdateSettings());
}
if (!updateState.getUpdatedZoneNames().isEmpty()) {
for (Integer zoneIdx : updateState.getUpdatedZoneNames().keySet()) {
panelSettings.updateZoneName(zoneIdx, updateState.getUpdatedZoneNames().get(zoneIdx));
}
}
if (!updateState.getUpdatedZoneInfos().isEmpty()) {
for (Integer zoneIdx : updateState.getUpdatedZoneInfos().keySet()) {
panelSettings.updateZoneInfo(zoneIdx, updateState.getUpdatedZoneInfos().get(zoneIdx));
}
}
PowermaxStateEvent newEvent = new PowermaxStateEvent(this, updateState);
if (updateState == null) {
updateState = createNewState();
}
// send message to event listeners
for (int i = 0; i < listeners.size(); i++) {
listeners.get(i).onNewStateEvent(newEvent);
updateState.lastMessageReceived.setValue(System.currentTimeMillis());
if (updateState.getUpdateSettings() != null) {
panelSettings.updateRawSettings(updateState.getUpdateSettings());
}
if (!updateState.getUpdatedZoneNames().isEmpty()) {
for (Integer zoneIdx : updateState.getUpdatedZoneNames().keySet()) {
panelSettings.updateZoneName(zoneIdx, updateState.getUpdatedZoneNames().get(zoneIdx));
}
}
if (!updateState.getUpdatedZoneInfos().isEmpty()) {
for (Integer zoneIdx : updateState.getUpdatedZoneInfos().keySet()) {
panelSettings.updateZoneInfo(zoneIdx, updateState.getUpdatedZoneInfos().get(zoneIdx));
}
}
PowermaxStateEvent newEvent = new PowermaxStateEvent(this, updateState);
// send message to event listeners
listeners.forEach(listener -> listener.onNewStateEvent(newEvent));
}
@Override
public void onCommunicationFailure(String message) {
close();
listeners.forEach(listener -> listener.onCommunicationFailure(message));
}
/**

View File

@ -28,4 +28,9 @@ public interface PowermaxMessageEventListener extends EventListener {
* @param event the event object
*/
public void onNewMessageEvent(EventObject event);
/**
* Event handler method to indicate that communication has been lost
*/
public void onCommunicationFailure(String message);
}

View File

@ -512,16 +512,19 @@ public class PowermaxPanelSettings {
result = false;
}
// Check if partitions are enabled
byte[] partitions = readSettings(PowermaxSendType.DL_PARTITIONS, 0, 0x10 + zoneCnt);
if (partitions != null) {
partitionsEnabled = (partitions[0] & 0x000000FF) == 1;
} else {
logger.debug("Cannot get partitions information");
result = false;
}
if (!partitionsEnabled) {
partitionCnt = 1;
// Check if partitions are enabled (only on panels that support partitions)
byte[] partitions = null;
if (partitionCnt > 1) {
partitions = readSettings(PowermaxSendType.DL_PARTITIONS, 0, 0x10 + zoneCnt);
if (partitions != null) {
partitionsEnabled = (partitions[0] & 0x000000FF) == 1;
} else {
logger.debug("Cannot get partitions information");
result = false;
}
if (!partitionsEnabled) {
partitionCnt = 1;
}
}
// Process zone settings

View File

@ -47,6 +47,7 @@ public class PowermaxState extends PowermaxStateContainer {
public StringValue armMode = new StringValue(this, "_arm_mode");
public BooleanValue downloadSetupRequired = new BooleanValue(this, "_download_setup_required");
public DateTimeValue lastKeepAlive = new DateTimeValue(this, "_last_keepalive");
public DateTimeValue lastMessageReceived = new DateTimeValue(this, "_last_message_received");
public StringValue panelStatus = new StringValue(this, "_panel_status");
public StringValue alarmType = new StringValue(this, "_alarm_type");
public StringValue troubleType = new StringValue(this, "_trouble_type");

View File

@ -28,4 +28,9 @@ public interface PowermaxStateEventListener extends EventListener {
* @param event the event object
*/
public void onNewStateEvent(EventObject event);
/**
* Event handler method to indicate that communication has been lost
*/
public void onCommunicationFailure(String message);
}