[pwm] Fix NPE when disabling and improve logging (#13755)

* [pwm] Fix exception when disabling the module
* Improve logging

Signed-off-by: Fabian Wolter <github@fabian-wolter.de>

diff --git a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/factory/PWMModuleHandlerFactory.java b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/factory/PWMModuleHandlerFactory.java
index d2488a124b631d24628bba499c44fcf2acb0288b..1aeaf3c483ca3810c0907c2e6cd1fe10d88689a4 100644 (file)
--- a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/factory/PWMModuleHandlerFactory.java
+++ b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/factory/PWMModuleHandlerFactory.java
@@ -56,7 +56,7 @@ public class PWMModuleHandlerFactory extends BaseModuleHandlerFactory {
     protected @Nullable ModuleHandler internalCreate(Module module, String ruleUID) {
         switch (module.getTypeUID()) {
             case PWMTriggerHandler.MODULE_TYPE_ID:
-                return new PWMTriggerHandler((Trigger) module, itemRegistry, bundleContext);
+                return new PWMTriggerHandler((Trigger) module, itemRegistry, bundleContext, ruleUID);
         }
 
         return null;

diff --git a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/PWMTriggerHandler.java b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/PWMTriggerHandler.java
index 937b33abf7df41f24e720af959fdd576db1d0e2c..4f8c5486a5c8e2ec60de7e889ec852961aadfed2 100644 (file)
--- a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/PWMTriggerHandler.java
+++ b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/PWMTriggerHandler.java
@@ -69,10 +69,12 @@ public class PWMTriggerHandler extends BaseTriggerModuleHandler implements Event
     private @Nullable ServiceRegistration<?> eventSubscriberRegistration;
     private @Nullable ScheduledFuture<?> deadMeanSwitchTimer;
     private @Nullable StateMachine stateMachine;
+    private String ruleUID;
 
-    public PWMTriggerHandler(Trigger module, ItemRegistry itemRegistry, BundleContext bundleContext) {
+    public PWMTriggerHandler(Trigger module, ItemRegistry itemRegistry, BundleContext bundleContext, String ruleUID) {
         super(module);
         this.bundleContext = bundleContext;
+        this.ruleUID = ruleUID;
 
         Configuration config = module.getConfiguration();
 
@@ -99,13 +101,15 @@ public class PWMTriggerHandler extends BaseTriggerModuleHandler implements Event
         super.setCallback(callback);
 
         double periodSec = getDoubleFromConfig(module.getConfiguration(), CONFIG_PERIOD);
-        stateMachine = new StateMachine(getCallback().getScheduler(), this::setOutput, (long) (periodSec * 1000));
+        stateMachine = new StateMachine(getCallback().getScheduler(), this::setOutput, (long) (periodSec * 1000),
+                ruleUID);
 
         eventSubscriberRegistration = bundleContext.registerService(EventSubscriber.class.getName(), this, null);
     }
 
     private double getDoubleFromConfig(Configuration config, String key) {
-        return ((BigDecimal) Objects.requireNonNull(config.get(key), key + " is not set")).doubleValue();
+        return ((BigDecimal) Objects.requireNonNull(config.get(key), ruleUID + ": " + key + " is not set"))
+                .doubleValue();
     }
 
     private Optional<Double> getOptionalDoubleFromConfig(Configuration config, String key) {
@@ -161,17 +165,17 @@ public class PWMTriggerHandler extends BaseTriggerModuleHandler implements Event
                     }
                 }).orElse(newDutycycle);
 
-                logger.debug("Received new duty cycle: {} {}", newDutycycleBeforeLimit,
+                logger.debug("{}: Received new duty cycle: {} {}", ruleUID, newDutycycleBeforeLimit,
                         newDutycycle != newDutycycleBeforeLimit ? "Limited to: " + newDutycycle : "");
 
                 StateMachine localStateMachine = stateMachine;
                 if (localStateMachine != null) {
                     localStateMachine.setDutycycle(newDutycycle);
                 } else {
-                    logger.debug("Initialization not finished");
+                    logger.debug("{}: Initialization not finished", ruleUID);
                 }
             } catch (PWMException e) {
-                logger.warn("{}", e.getMessage());
+                logger.warn("{}: {}", ruleUID, e.getMessage());
             }
         }
     }
@@ -189,7 +193,7 @@ public class PWMTriggerHandler extends BaseTriggerModuleHandler implements Event
     }
 
     private void activateDeadManSwitch() {
-        logger.warn("Dead-man switch activated. Disabling output");
+        logger.warn("{}: Dead-man switch activated. Disabling output", ruleUID);
 
         StateMachine localStateMachine = stateMachine;
         if (localStateMachine != null) {
@@ -220,9 +224,10 @@ public class PWMTriggerHandler extends BaseTriggerModuleHandler implements Event
                 // nothing
             }
         } else if (state instanceof UnDefType) {
-            throw new PWMException("Duty cycle item '" + dutyCycleItem.getName() + "' has no valid value");
+            throw new PWMException(ruleUID + ": Duty cycle item '" + dutyCycleItem.getName() + "' has no valid value");
         }
-        throw new PWMException("Duty cycle item not of type DecimalType: " + state.getClass().getSimpleName());
+        throw new PWMException(
+                ruleUID + ": Duty cycle item not of type DecimalType: " + state.getClass().getSimpleName());
     }
 
     @Override
@@ -242,11 +247,6 @@ public class PWMTriggerHandler extends BaseTriggerModuleHandler implements Event
             localEventSubscriberRegistration.unregister();
         }
 
-        StateMachine localStateMachine = stateMachine;
-        if (localStateMachine != null) {
-            localStateMachine.stop();
-        }
-
         super.dispose();
     }
 }

diff --git a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/State.java b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/State.java
index b0783d932a1b6feca364be2a51cb2b4b96d79729..36b889257fb88a1801b77604c47b64e464e1e8aa 100644 (file)
--- a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/State.java
+++ b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/State.java
@@ -59,7 +59,8 @@ public abstract class State {
         context.getState().dispose();
         State newState = nextState.apply(context);
 
-        logger.trace("{} -> {}", context.getState().getClass().getSimpleName(), newState.getClass().getSimpleName());
+        logger.trace("{}: {} -> {}", context.getRuleUID(), context.getState().getClass().getSimpleName(),
+                newState.getClass().getSimpleName());
 
         context.setState(newState);
     }

diff --git a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/StateMachine.java b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/StateMachine.java
index fc5ca6a86c2171b8a2000fca7b1dfcecd5998efa..e984ea8b3440ec502f1f751b5376bdba6dac4b8a 100644 (file)
--- a/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/StateMachine.java
+++ b/bundles/org.openhab.automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/StateMachine.java
@@ -29,11 +29,14 @@ public class StateMachine {
     private State state;
     private long periodMs;
     private double dutycycle;
+    private String ruleUID;
 
-    public StateMachine(ScheduledExecutorService scheduler, Consumer<Boolean> controlOutput, long periodMs) {
+    public StateMachine(ScheduledExecutorService scheduler, Consumer<Boolean> controlOutput, long periodMs,
+            String ruleUID) {
         this.scheduler = scheduler;
         this.controlOutput = controlOutput;
         this.periodMs = periodMs;
+        this.ruleUID = ruleUID;
         this.state = new AlwaysOffState(this);
     }
 
@@ -66,6 +69,10 @@ public class StateMachine {
         this.state = current;
     }
 
+    public String getRuleUID() {
+        return ruleUID;
+    }
+
     public void controlOutput(boolean on) {
         controlOutput.accept(on);
     }