Skip to content

Commit

Permalink
Changes after complete review by @fwolter
Browse files Browse the repository at this point in the history
Signed-off-by: Massimo Valla <[email protected]>
  • Loading branch information
mvalla committed Jul 15, 2020
1 parent e8e5308 commit b0f02d2
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 143 deletions.
10 changes: 3 additions & 7 deletions bundles/org.openhab.binding.openwebnet/NOTICE
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ https://github.com/openhab/openhab-addons

## Third-party Content

slf4j-api
* License: MIT
* Source: https://github.com/qos-ch/slf4j

nrjavaserial
* License: LGPL v 2.1 + Linking Over Controlled Interface
* Source: https://github.com/NeuronRobotics/nrjavaserial
openwebnet4j
* License: Eclipse Public License 2.0
* Source: https://github.com/mvalla/openwebnet4j
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class OpenWebNetBindingConstants {

public static final String BINDING_ID = "openwebnet";

public static final int THING_STATE_REQ_TIMEOUT = 5; // seconds
public static final int THING_STATE_REQ_TIMEOUT_SEC = 5;

// #LIST OF Thing Type UIDs
// generic device (used for not identified devices)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public class OpenWebNetBridgeHandler extends ConfigStatusBridgeHandler implement

private final Logger logger = LoggerFactory.getLogger(OpenWebNetBridgeHandler.class);

private static final int GATEWAY_ONLINE_TIMEOUT = 20; // (sec) Time to wait for the gateway to become connected
private static final int GATEWAY_ONLINE_TIMEOUT_SEC = 20; // Time to wait for the gateway to become connected
private static final int CONFIG_GATEWAY_DEFAULT_PORT = 20000;
private static final String CONFIG_GATEWAY_DEFAULT_PASSWD = "12345";
private static final boolean CONFIG_GATEWAY_DEFAULT_DISCVERY_ACTIVATION = false;
Expand Down Expand Up @@ -99,7 +99,6 @@ public boolean isBusGateway() {
@Override
public void initialize() {
ThingTypeUID thingType = getThing().getThingTypeUID();
logger.debug("Initializing Bridge - type: {}", thingType);
OpenGateway gw;
if (thingType.equals(THING_TYPE_ZB_GATEWAY)) {
gw = initZigBeeGateway();
Expand All @@ -112,7 +111,6 @@ public void initialize() {
gw.subscribe(this);
if (gw.isConnected()) { // gateway is already connected, device can go ONLINE
isGatewayConnected = true;
logger.info("------------------- ALREADY CONNECTED -> setting status to ONLINE");
updateStatus(ThingStatus.ONLINE);
} else {
updateStatus(ThingStatus.UNKNOWN);
Expand All @@ -124,9 +122,9 @@ public void initialize() {
if (thing.getStatus().equals(ThingStatus.UNKNOWN)) {
logger.info("status still UNKNOWN. Setting device={} to OFFLINE", thing.getUID());
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR,
"Could not connect to gateway before " + GATEWAY_ONLINE_TIMEOUT + "s");
"Could not connect to gateway before " + GATEWAY_ONLINE_TIMEOUT_SEC + "s");
}
}, GATEWAY_ONLINE_TIMEOUT, TimeUnit.SECONDS);
}, GATEWAY_ONLINE_TIMEOUT_SEC, TimeUnit.SECONDS);
} catch (OWNException e) {
logger.debug("gw.connect() returned: {}", e.getMessage());
// status is updated by callback onConnectionError()
Expand Down Expand Up @@ -225,15 +223,12 @@ public void thingUpdated(Thing thing) {

@Override
public void handleRemoval() {
logger.debug("handleRemoval() for {}", getThing().getUID());
disconnectGateway();
// logger.debug("calling super.handleRemoval()");
super.handleRemoval();
}

@Override
public void dispose() {
logger.debug("dispose() for {}", getThing().getUID());
disconnectGateway();
super.dispose();
}
Expand All @@ -255,14 +250,13 @@ private void disconnectGateway() {
* @param listener to receive device found notifications
*/
public synchronized void searchDevices() {
logger.debug("------$$ BridgeHandler.searchDevices()");
scanIsActive = true;
logger.debug("------$$ scanIsActive={}", scanIsActive);
OpenGateway gw = gateway;
if (gw != null) {
if (!gw.isDiscovering()) {
if (!gw.isConnected()) {
logger.warn("------$$ Gateway is NOT connected, cannot search for devices");
logger.debug("------$$ Gateway is NOT connected, cannot search for devices");
return;
}
logger.info("------$$ STARTED active SEARCH for devices on gateway '{}'", this.getThing().getUID());
Expand All @@ -273,11 +267,11 @@ public synchronized void searchDevices() {
this.getThing().getUID(), e.getMessage());
}
} else {
logger.warn("------$$ Searching devices on gateway {} already activated", this.getThing().getUID());
logger.debug("------$$ Searching devices on gateway {} already activated", this.getThing().getUID());
return;
}
} else {
logger.warn("------$$ Cannot search devices: no gateway associated to this handler");
logger.debug("------$$ Cannot search devices: no gateway associated to this handler");
}
}

Expand Down Expand Up @@ -336,7 +330,7 @@ protected void registerDevice(String ownId, OpenWebNetThingHandler thingHandler)
logger.warn("registering device with an existing ownId={}", ownId);
}
registeredDevices.put(ownId, thingHandler);
logger.info("registered device ownId={}, thing={}", ownId, thingHandler.getThing().getUID());
logger.debug("registered device ownId={}, thing={}", ownId, thingHandler.getThing().getUID());
}

/**
Expand Down Expand Up @@ -378,7 +372,6 @@ public void onEventMessage(@Nullable OpenMessage msg) {
OpenGateway gw = gateway;
if (isBusGateway && ((gw != null && !gw.isDiscovering() && scanIsActive)
|| (discoveryByActivation && !scanIsActive))) {
// try device discovery by activation
discoverByActivation(baseMsg);
} else {
logger.debug("ownId={} has NO DEVICE associated, ignoring it", ownId);
Expand All @@ -387,7 +380,7 @@ public void onEventMessage(@Nullable OpenMessage msg) {
deviceHandler.handleMessage(baseMsg);
}
} else {
logger.debug("BridgeHandler ignoring frame {}. WHO={} is not supported by the binding", baseMsg,
logger.debug("BridgeHandler ignoring frame {}. WHO={} is not supported by this binding", baseMsg,
baseMsg.getWho());
}
}
Expand Down Expand Up @@ -435,7 +428,7 @@ public void onConnectionError(@Nullable OWNException error) {
} else {
errMsg = error.getMessage();
}
logger.warn("------------------- ON CONNECTION ERROR: {}", errMsg);
logger.info("------------------- ON CONNECTION ERROR: {}", errMsg);
isGatewayConnected = false;
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR, errMsg);
tryRecconectGateway();
Expand All @@ -457,7 +450,7 @@ public void onDisconnected(@Nullable OWNException e) {
} else {
errMsg = e.getMessage();
}
logger.warn("------------------- DISCONNECTED from gateway. OWNException={}", errMsg);
logger.info("------------------- DISCONNECTED from gateway. OWNException={}", errMsg);
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR,
"Disconnected from gateway (onDisconnected - " + errMsg + ")");
tryRecconectGateway();
Expand All @@ -472,7 +465,7 @@ private void tryRecconectGateway() {
try {
gw.reconnect();
} catch (OWNAuthException e) {
logger.warn("------------------- AUTH error from gateway. Stopping recconnect");
logger.info("------------------- AUTH error from gateway. Stopping recconnect");
reconnecting = false;
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.CONFIGURATION_ERROR,
"Authentication error. Check gateway password in Thing Configuration Parameters (" + e
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@

/**
* The {@link OpenWebNetGenericHandler} is responsible for handling Generic OpenWebNet
* devices. It does not too much, but it is needed to avoid handler errors.
* devices. It does not too much, but it is needed to avoid handler errors and to tell the user
* that some device has been found by the gateway but it was not recognised.
* It extends the abstract {@link OpenWebNetThingHandler}.
*
* @author Massimo Valla - Initial contribution
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public class OpenWebNetLightingHandler extends OpenWebNetThingHandler {
private final Logger logger = LoggerFactory.getLogger(OpenWebNetLightingHandler.class);

public static final Set<ThingTypeUID> SUPPORTED_THING_TYPES = OpenWebNetBindingConstants.LIGHTING_SUPPORTED_THING_TYPES;
private static final int BRIGHTNESS_CHANGE_DELAY_MSEC = 1500; // delay to wait before sending another brightness
// status request

private double lastBrightnessChangeSentTS = 0; // timestamp when last brightness change was sent to the device
private static final int BRIGHTNESS_CHANGE_DELAY = 1500; // ms delay to wait before sending another brightness
// status request
private long lastBrightnessChangeSentTS = 0; // timestamp when last brightness change was sent to the device

private boolean brightnessLevelRequested = false; // was the brightness level requested ?
private int latestBrightnessWhat = -1; // latest brightness WHAT value (-1 = unknown)
Expand Down Expand Up @@ -136,7 +136,6 @@ private void handleBrightnessCommand(Command command) {
}
} else {
logger.warn("Cannot handle command {} for thing {}", command, getThing().getUID());
return;
}
}

Expand Down Expand Up @@ -228,22 +227,25 @@ private void updateLightState(Lighting msg) {
private void updateLightOnOffState(Lighting msg) {
String channelID;
OpenWebNetBridgeHandler brH = bridgeHandler;
if (brH != null && brH.isBusGateway()) {
channelID = CHANNEL_SWITCH;
} else {
if (WhereZigBee.UNIT_02.equals(((WhereZigBee) (msg.getWhere())).getUnit())) {
channelID = CHANNEL_SWITCH_02;
if (brH != null) {
if (brH.isBusGateway()) {
channelID = CHANNEL_SWITCH;
} else {
channelID = CHANNEL_SWITCH_01;
WhereZigBee w = (WhereZigBee) (msg.getWhere());
if (WhereZigBee.UNIT_02.equals(w.getUnit())) {
channelID = CHANNEL_SWITCH_02;
} else {
channelID = CHANNEL_SWITCH_01;
}
}
if (msg.isOn()) {
updateState(channelID, OnOffType.ON);
} else if (msg.isOff()) {
updateState(channelID, OnOffType.OFF);
} else {
logger.debug("updateLightOnOffState() Ignoring unsupported WHAT for thing {}. Frame={}",
getThing().getUID(), msg.getFrameValue());
}
}
if (msg.isOn()) {
updateState(channelID, OnOffType.ON);
} else if (msg.isOff()) {
updateState(channelID, OnOffType.OFF);
} else {
logger.info("updateLightOnOffState() Ignoring unsupported WHAT for thing {}. Frame={}", getThing().getUID(),
msg.getFrameValue());
}
}

Expand All @@ -257,14 +259,14 @@ private synchronized void updateLightBrightnessState(Lighting msg) {
logger.debug("$BRI updateLightBrightnessState() msg={}", msg);
logger.debug("$BRI updateLightBr() latestBriWhat={} latestBriBeforeOff={} brightnessLevelRequested={}",
latestBrightnessWhat, latestBrightnessWhatBeforeOff, brightnessLevelRequested);
double now = System.currentTimeMillis();
double delta = now - lastBrightnessChangeSentTS;
long now = System.currentTimeMillis();
long delta = now - lastBrightnessChangeSentTS;
logger.debug("$BRI now={} delta={}", now, delta);
if (msg.isOn() && !brightnessLevelRequested) {
if (delta >= BRIGHTNESS_CHANGE_DELAY) {
if (delta >= BRIGHTNESS_CHANGE_DELAY_MSEC) {
// we send a light brightness status request ONLY if last brightness change
// was sent >BRIGHTNESS_CHANGE_DELAY ago
logger.debug("$BRI change sent >={}ms ago, sending requestStatus...", BRIGHTNESS_CHANGE_DELAY);
// was sent >BRIGHTNESS_CHANGE_DELAY_MSEC ago
logger.debug("$BRI change sent >={}ms ago, sending requestStatus...", BRIGHTNESS_CHANGE_DELAY_MSEC);
brightnessLevelRequested = true;
Where w = deviceWhere;
if (w != null) {
Expand All @@ -275,16 +277,16 @@ private synchronized void updateLightBrightnessState(Lighting msg) {
}
}
} else {
logger.debug("$BRI change sent {}<{}ms, NO requestStatus needed", delta, BRIGHTNESS_CHANGE_DELAY);
logger.debug("$BRI change sent {}<{}ms, NO requestStatus needed", delta, BRIGHTNESS_CHANGE_DELAY_MSEC);
}
} else {
logger.debug("$BRI update from network -> level should be present in WHAT part of the message");
if (msg.getWhat() != null) {
int newLevel = msg.getWhat().value();
logger.debug("$BRI current level={} ----> new level={}", latestBrightnessWhat, newLevel);
if (latestBrightnessWhat != newLevel) {
if (delta >= BRIGHTNESS_CHANGE_DELAY) {
logger.debug("$BRI change sent >={}ms ago, updating state...", BRIGHTNESS_CHANGE_DELAY);
if (delta >= BRIGHTNESS_CHANGE_DELAY_MSEC) {
logger.debug("$BRI change sent >={}ms ago, updating state...", BRIGHTNESS_CHANGE_DELAY_MSEC);
updateState(channel, new PercentType(Lighting.levelToPercent(newLevel)));
} else if (msg.isOff()) {
logger.debug("$BRI change just sent, but OFF from network received, updating state...");
Expand Down Expand Up @@ -336,7 +338,6 @@ private synchronized void updateLightBrightnessState(Lighting msg) {
**/
@Nullable
protected String toWhere(String unit) {
logger.debug("toWhere(unit) ownId={}", ownId);
Where w = deviceWhere;
if (w != null) {
OpenWebNetBridgeHandler brH = bridgeHandler;
Expand All @@ -357,7 +358,6 @@ protected String toWhere(String unit) {
**/
@Nullable
protected String toWhere(ChannelUID channel) {
logger.debug("toWhere(ChannelUID) ownId={}", ownId);
Where w = deviceWhere;
if (w != null) {
OpenWebNetBridgeHandler brH = bridgeHandler;
Expand Down
Loading

0 comments on commit b0f02d2

Please sign in to comment.