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

* [pwm] Fix exception when disabling the module * Improve logging Signed-off-by: Fabian Wolter <[email protected]>
mlobstein · Nov 20, 2022 · d0736bd · d0736bd
1 parent 1036197
commit d0736bd
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 17 deletions.
diff --git a/...wm/src/main/java/org/openhab/automation/pwm/internal/factory/PWMModuleHandlerFactory.java b/...wm/src/main/java/org/openhab/automation/pwm/internal/factory/PWMModuleHandlerFactory.java
@@ -56,7 +56,7 @@ public Collection<String> getTypes() {
     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/...tion.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/PWMTriggerHandler.java b/...tion.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 void setCallback(ModuleHandlerCallback callback) {
         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 void receive(Event 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 @@ private void restartDeadManSwitchTimer() {
     }
 
     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 @@ private double getDutyCycleValueInPercent(State state) throws PWMException {
                 // 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 void dispose() {
             localEventSubscriberRegistration.unregister();
         }
 
-        StateMachine localStateMachine = stateMachine;
-        if (localStateMachine != null) {
-            localStateMachine.stop();
-        }
-
         super.dispose();
     }
 }
diff --git a/...automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/State.java b/...automation.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/State.java
@@ -59,7 +59,8 @@ public synchronized void nextState(Function<StateMachine, ? extends State> nextS
         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/...ion.pwm/src/main/java/org/openhab/automation/pwm/internal/handler/state/StateMachine.java b/...ion.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 void setState(State current) {
         this.state = current;
     }
 
+    public String getRuleUID() {
+        return ruleUID;
+    }
+
     public void controlOutput(boolean on) {
         controlOutput.accept(on);
     }