Skip to content

Commit

Permalink
[dscalarm] Bridge/things management refactored (openhab#7748)
Browse files Browse the repository at this point in the history 
* [dscalarm] Bridge/things management refactored

Relative to openhab#7704
* Avoid logging the password
* Masking the user code too

Signed-off-by: Laurent Garnier <lg.hc@free.fr>
Signed-off-by: Daan Meijer <daan@studioseptember.nl>
  • Loading branch information
@lolodomo @DaanMeijer
lolodomo authored and DaanMeijer committed Sep 1, 2020
1 parent 3570f39 commit 8ac519c
Show file tree
Hide file tree
Showing 8 changed files with 97 additions and 122 deletions.
63 changes: 39 additions & 24 deletions ...rc/main/java/org/openhab/binding/dscalarm/internal/handler/DSCAlarmBaseBridgeHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public abstract class DSCAlarmBaseBridgeHandler extends BaseBridgeHandler {
private String userCode = null;

// Polling variables
public int pollPeriod = 0;
protected int pollPeriod = 0;
private long pollElapsedTime = 0;
private long pollStartTime = 0;
private long refreshInterval = 5000;
Expand Down Expand Up @@ -133,6 +133,24 @@ public void setBridgeType(DSCAlarmBridgeType dscAlarmBridgeType) {
this.dscAlarmBridgeType = dscAlarmBridgeType;
}

@Override
public void initialize() {
if (this.pollPeriod > 15) {
this.pollPeriod = 15;
} else if (this.pollPeriod < 1) {
this.pollPeriod = 1;
}
updateStatus(ThingStatus.OFFLINE);
startPolling();
}

@Override
public void dispose() {
stopPolling();
closeConnection();
super.dispose();
}

/**
* Register the Discovery Service.
*
Expand Down Expand Up @@ -210,8 +228,7 @@ public void setConnected(boolean connected) {
* @param isOnline
*/
public void setBridgeStatus(boolean isOnline) {
logger.debug("setBridgeConnection(): Setting Bridge to {}",
isOnline ? ThingStatus.ONLINE : ThingStatus.OFFLINE);
logger.debug("setBridgeStatus(): Setting Bridge to {}", isOnline ? ThingStatus.ONLINE : ThingStatus.OFFLINE);

updateStatus(isOnline ? ThingStatus.ONLINE : ThingStatus.OFFLINE);

Expand All @@ -233,8 +250,9 @@ public void setBridgeStatus(boolean isOnline) {
* Method for writing to an open DSC Alarm connection.
*
* @param writeString
* @param doNotLog
*/
public abstract void write(String writeString);
public abstract void write(String writeString, boolean doNotLog);

/**
* Method for reading from an open DSC Alarm connection.
Expand Down Expand Up @@ -276,7 +294,7 @@ public void setUserCode(String userCode) {
/**
* Method to start the polling task.
*/
public void startPolling() {
private void startPolling() {
logger.debug("Starting DSC Alarm Polling Task.");
if (pollingTask == null || pollingTask.isCancelled()) {
pollingTask = scheduler.scheduleWithFixedDelay(this::polling, 0, refreshInterval, TimeUnit.MILLISECONDS);
Expand All @@ -286,7 +304,7 @@ public void startPolling() {
/**
* Method to stop the polling task.
*/
public void stopPolling() {
private void stopPolling() {
logger.debug("Stopping DSC Alarm Polling Task.");
if (pollingTask != null && !pollingTask.isCancelled()) {
pollingTask.cancel(true);
Expand Down Expand Up @@ -316,13 +334,11 @@ public synchronized void polling() {

checkThings();

if (thingsHaveChanged) {
if (allThingsInitialized) {
this.setBridgeStatus(isConnected());
thingsHaveChanged = false;
// Get a status report from DSC Alarm.
sendCommand(DSCAlarmCode.StatusReport);
}
if (thingsHaveChanged && allThingsInitialized) {
this.setBridgeStatus(isConnected());
thingsHaveChanged = false;
// Get a status report from DSC Alarm.
sendCommand(DSCAlarmCode.StatusReport);
}
} else {
logger.error("Not Connected to the DSC Alarm!");
Expand Down Expand Up @@ -353,11 +369,7 @@ public void checkThings() {
logger.debug("***Checking '{}' - Status: {}, Initialized: {}", thing.getUID(), thing.getStatus(),
handler.isThingHandlerInitialized());

if (!handler.isThingHandlerInitialized() || !thing.getStatus().equals(ThingStatus.ONLINE)) {
if (getThing().getStatus().equals(ThingStatus.ONLINE)) {
handler.bridgeStatusChanged(getThing().getStatusInfo());
}

if (!handler.isThingHandlerInitialized() || thing.getStatus() != ThingStatus.ONLINE) {
allThingsInitialized = false;
}

Expand Down Expand Up @@ -508,7 +520,7 @@ public synchronized void handleIncomingMessage(String incomingMessage) {
DSCAlarmBaseThingHandler thingHandler = (DSCAlarmBaseThingHandler) thing.getHandler();

if (thingHandler != null) {
if (thingHandler.isThingHandlerInitialized()) {
if (thingHandler.isThingHandlerInitialized() && thing.getStatus() == ThingStatus.ONLINE) {
thingHandler.dscAlarmEventReceived(event, thing);

} else {
Expand Down Expand Up @@ -617,6 +629,7 @@ public boolean sendCommand(DSCAlarmCode dscAlarmCode, String... dscAlarmData) {

String command = dscAlarmCode.getCode();
String data = "";
boolean confidentialData = false;

switch (dscAlarmCode) {
case Poll: /* 000 */
Expand All @@ -639,6 +652,7 @@ public boolean sendCommand(DSCAlarmCode dscAlarmCode, String... dscAlarmData) {
break;
}
data = password;
confidentialData = true;
validCommand = true;
break;
case DumpZoneTimers: /* 008 */
Expand Down Expand Up @@ -697,7 +711,7 @@ public boolean sendCommand(DSCAlarmCode dscAlarmCode, String... dscAlarmData) {
}

if (userCode == null || userCode.length() < 4 || userCode.length() > 6) {
logger.error("sendCommand(): User Code is invalid, must be between 4 and 6 chars: {}", userCode);
logger.error("sendCommand(): User Code is invalid, must be between 4 and 6 chars");
break;
}

Expand All @@ -707,6 +721,7 @@ public boolean sendCommand(DSCAlarmCode dscAlarmCode, String... dscAlarmData) {
data = dscAlarmData[0] + userCode;
}

confidentialData = true;
validCommand = true;
break;
case VirtualKeypadControl: /* 058 */
Expand Down Expand Up @@ -784,12 +799,12 @@ public boolean sendCommand(DSCAlarmCode dscAlarmCode, String... dscAlarmData) {
break;
case CodeSend: /* 200 */
if (userCode == null || userCode.length() < 4 || userCode.length() > 6) {
logger.error("sendCommand(): Access Code is invalid, must be between 4 and 6 chars: {}",
dscAlarmData[0]);
logger.error("sendCommand(): Access Code is invalid, must be between 4 and 6 chars");
break;
}

data = userCode;
confidentialData = true;
validCommand = true;
break;

Expand All @@ -801,9 +816,9 @@ public boolean sendCommand(DSCAlarmCode dscAlarmCode, String... dscAlarmData) {

if (validCommand) {
String cmd = dscAlarmCommand(command, data);
write(cmd);
write(cmd, confidentialData);
successful = true;
logger.debug("sendCommand(): '{}' Command Sent - {}", dscAlarmCode, cmd);
logger.debug("sendCommand(): '{}' Command Sent - {}", dscAlarmCode, confidentialData ? "***" : cmd);
} else {
logger.error("sendCommand(): Command '{}' Not Sent - Invalid!", dscAlarmCode);
}
Expand Down
43 changes: 16 additions & 27 deletions ...src/main/java/org/openhab/binding/dscalarm/internal/handler/DSCAlarmBaseThingHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.eclipse.smarthome.core.thing.ChannelUID;
import org.eclipse.smarthome.core.thing.Thing;
import org.eclipse.smarthome.core.thing.ThingStatus;
import org.eclipse.smarthome.core.thing.ThingStatusDetail;
import org.eclipse.smarthome.core.thing.ThingStatusInfo;
import org.eclipse.smarthome.core.thing.binding.BaseThingHandler;
import org.eclipse.smarthome.core.thing.binding.ThingHandler;
Expand Down Expand Up @@ -82,25 +83,24 @@ public void initialize() {

getConfiguration(dscAlarmThingType);

// set the Thing offline for now
updateStatus(ThingStatus.OFFLINE);
Bridge bridge = getBridge();
initializeThingHandler(bridge != null ? bridge.getStatus() : null);
this.setThingHandlerInitialized(true);
}

@Override
public void dispose() {
logger.debug("Thing {} disposed.", getThing().getUID());

this.setThingHandlerInitialized(false);

super.dispose();
}

/**
* Method to Initialize Thing Handler.
*/
public void initializeThingHandler() {
if (getDSCAlarmBridgeHandler() != null) {
if (getThing().getStatus().equals(ThingStatus.ONLINE)) {
private void initializeThingHandler(ThingStatus bridgeStatus) {
if (getDSCAlarmBridgeHandler() != null && bridgeStatus != null) {
if (bridgeStatus == ThingStatus.ONLINE) {
Thing thing = getThing();
List<Channel> channels = thing.getChannels();
logger.debug("initializeThingHandler(): Initialize Thing Handler - {}", thing.getUID());
Expand All @@ -117,13 +117,18 @@ public void initializeThingHandler() {
dscAlarmBridgeHandler.setUserCode(getUserCode());
}

this.setThingHandlerInitialized(true);
updateStatus(ThingStatus.ONLINE);

logger.debug("initializeThingHandler(): Thing Handler Initialized - {}", thing.getUID());
} else {
logger.debug("initializeThingHandler(): Thing '{}' Unable To Initialize Thing Handler!: Status - {}",
thing.getUID(), thing.getStatus());
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_OFFLINE);
logger.debug("initializeThingHandler(): Thing '{}' is set to OFFLINE because bridge is OFFLINE",
thing.getUID());
}
} else {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.BRIDGE_UNINITIALIZED);
logger.debug("initializeThingHandler(): Thing '{}' is set to OFFLINE because bridge is uninitialized",
thing.getUID());
}
}

Expand Down Expand Up @@ -164,16 +169,6 @@ public synchronized DSCAlarmBaseBridgeHandler getDSCAlarmBridgeHandler() {
*/
public abstract void updateChannel(ChannelUID channel, int state, String description);

/**
* Method to Update Device Properties.
*
* @param channelUID
* @param state
* @param description
*/
// public abstract void updateProperties(ChannelUID channelUID, int state,
// String description);

/**
* Receives DSC Alarm Events from the bridge.
*
Expand All @@ -188,15 +183,9 @@ public void handleCommand(ChannelUID channelUID, Command command) {

@Override
public void bridgeStatusChanged(ThingStatusInfo bridgeStatusInfo) {
if (bridgeStatusInfo.getStatus().equals(ThingStatus.ONLINE)) {
updateStatus(bridgeStatusInfo.getStatus());
this.initializeThingHandler();
} else {
this.setThingHandlerInitialized(false);
}

logger.debug("bridgeStatusChanged(): Bridge Status: '{}' - Thing '{}' Status: '{}'!", bridgeStatusInfo,
getThing().getUID(), getThing().getStatus());
initializeThingHandler(bridgeStatusInfo.getStatus());
}

/**
Expand Down
32 changes: 12 additions & 20 deletions .../src/main/java/org/openhab/binding/dscalarm/internal/handler/EnvisalinkBridgeHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import org.eclipse.smarthome.core.thing.Bridge;
import org.eclipse.smarthome.core.thing.ThingStatus;
import org.eclipse.smarthome.core.thing.ThingStatusDetail;
import org.openhab.binding.dscalarm.internal.config.EnvisalinkBridgeConfiguration;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -61,38 +62,29 @@ public void initialize() {

EnvisalinkBridgeConfiguration configuration = getConfigAs(EnvisalinkBridgeConfiguration.class);

if (configuration.ipAddress != null) {
ipAddress = configuration.ipAddress;
if (configuration.ipAddress == null || configuration.ipAddress.trim().isEmpty()) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
"Set an IP address in the thing configuration.");
} else if (configuration.port == null) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
"Set a TCP port in the thing configuration.");
} else {
ipAddress = configuration.ipAddress.trim();
tcpPort = configuration.port.intValue();
setPassword(configuration.password);
connectionTimeout = configuration.connectionTimeout.intValue();
pollPeriod = configuration.pollPeriod.intValue();

if (this.pollPeriod > 15) {
this.pollPeriod = 15;
} else if (this.pollPeriod < 1) {
this.pollPeriod = 1;
}
super.initialize();

logger.debug("Envisalink Bridge Handler Initialized");
logger.debug(" IP Address: {},", ipAddress);
logger.debug(" Port: {},", tcpPort);
logger.debug(" Password: {},", getPassword());
logger.debug(" PollPeriod: {},", pollPeriod);
logger.debug(" Connection Timeout: {}.", connectionTimeout);

updateStatus(ThingStatus.OFFLINE);
startPolling();
}
}

@Override
public void dispose() {
stopPolling();
closeConnection();
super.dispose();
}

@Override
public void openConnection() {
try {
Expand Down Expand Up @@ -126,9 +118,9 @@ public void openConnection() {
}

@Override
public void write(String writeString) {
public void write(String writeString, boolean doNotLog) {
try {
logger.debug("write(): Attempting to Send Message: {}", writeString);
logger.debug("write(): Attempting to Send Message: {}", doNotLog ? "***" : writeString);
tcpOutput.write(writeString);
tcpOutput.flush();
} catch (IOException ioException) {
Expand Down
Loading

0 comments on commit 8ac519c

Please sign in to comment.