Update the thing configuration only if the key is different

Ignore messages when the thing handler is already disposed Avoid concurrent edit of thing properties Signed-off-by: Laurent Garnier <[email protected]>
openhab · Mar 28, 2020 · 929988e · 929988e
1 parent d78a703
commit 929988e
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 23 deletions.
diff --git a/...ng.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSHandler.java b/...ng.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSHandler.java
@@ -146,10 +146,10 @@ public void initialize() {
         s.setListener(this);
         socket = s;
 
-        discoveryServiceRegistry.addDiscoveryListener(this);
-
         updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.NONE, "TV is off");
 
+        discoveryServiceRegistry.addDiscoveryListener(this);
+
         startReconnectJob();
     }
 
@@ -255,18 +255,22 @@ public String getKey() {
     }
 
     @Override
-    public void storeKey(@Nullable String key) {
-        // store it current configuration and avoiding complete re-initialization via handleConfigurationUpdate
-        getLGWebOSConfig().key = key;
-
-        // persist the configuration change
-        Configuration configuration = editConfiguration();
-        configuration.put(LGWebOSBindingConstants.CONFIG_KEY, key);
-        updateConfiguration(configuration);
+    public synchronized void storeKey(@Nullable String key) {
+        if (!getKey().equals(key)) {
+            logger.debug("store new key");
+            // store it current configuration and avoiding complete re-initialization via handleConfigurationUpdate
+            getLGWebOSConfig().key = key;
+
+            // persist the configuration change
+            Configuration configuration = editConfiguration();
+            configuration.put(LGWebOSBindingConstants.CONFIG_KEY, key);
+            updateConfiguration(configuration);
+        }
     }
 
     @Override
-    public void storeProperties(Map<String, String> properties) {
+    public synchronized void storeProperties(Map<String, String> properties) {
+        logger.debug("storeProperties {}", properties);
         Map<String, String> map = editProperties();
         map.putAll(properties);
         updateProperties(map);
@@ -330,6 +334,7 @@ public void setOptions(String channelId, List<StateOption> options) {
     }
 
     public void postUpdate(String channelId, State state) {
+        logger.debug("postUpdate channelId {} state {}", channelId, state);
         if (isLinked(channelId)) {
             updateState(channelId, state);
         }
@@ -358,28 +363,22 @@ public void thingDiscovered(DiscoveryService source, DiscoveryResult result) {
                 channelHandlers.get(CHANNEL_POWER).onDeviceReady(CHANNEL_POWER, this);
 
                 // Update the thing properties from the discovery result
-                Map<String, String> map = editProperties();
+                Map<String, String> map = new HashMap<>();
                 String deviceId = (String) properties.get(PROPERTY_DEVICE_ID);
                 if (deviceId != null && !deviceId.isEmpty()) {
                     udn = deviceId;
                     logger.debug("UDN {} considered", udn);
-                    if (!deviceId.equals(map.get(PROPERTY_DEVICE_ID))) {
-                        logger.debug("Update property deviceId with {}", deviceId);
-                        map.put(PROPERTY_DEVICE_ID, deviceId);
-                    }
+                    map.put(PROPERTY_DEVICE_ID, deviceId);
                 }
                 String manufacturer = (String) properties.get(PROPERTY_MANUFACTURER);
-                if (manufacturer != null && !manufacturer.isEmpty()
-                        && !manufacturer.equals(map.get(PROPERTY_MANUFACTURER))) {
-                    logger.debug("Update property manufacturer with {}", manufacturer);
+                if (manufacturer != null && !manufacturer.isEmpty()) {
                     map.put(PROPERTY_MANUFACTURER, manufacturer);
                 }
                 String modelName = (String) properties.get(PROPERTY_MODEL_NAME);
-                if (modelName != null && !modelName.isEmpty() && !modelName.equals(map.get(PROPERTY_MODEL_NAME))) {
-                    logger.debug("Update property modelName with {}", modelName);
+                if (modelName != null && !modelName.isEmpty()) {
                     map.put(PROPERTY_MODEL_NAME, modelName);
                 }
-                updateProperties(map);
+                storeProperties(map);
             }
         }
     }

diff --git a/...g.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSTVSocket.java b/...g.lgwebos/src/main/java/org/openhab/binding/lgwebos/internal/handler/LGWebOSTVSocket.java
@@ -295,7 +295,8 @@ public void sendCommand(ServiceCommand<?> command) {
                 break;
             case REGISTERING:
             case DISCONNECTED:
-                logger.warn("Skipping command {} for {} in state {}", command, command.getTarget(), state);
+                logger.warn("Skipping {} command {} for {} in state {}", command.getType(), command,
+                        command.getTarget(), state);
                 break;
         }
 
@@ -332,6 +333,10 @@ private void sendMessage(JsonObject json) {
     @OnWebSocketMessage
     public void onMessage(String message) {
         logger.trace("Message [in]: {}", message);
+        if (this.listener == null) {
+            logger.debug("Message is ignored because the thing handler is already disposed.");
+            return;
+        }
         Response response = GSON.fromJson(message, Response.class);
         ServiceCommand<?> request = null;