diff --git a/docs/configuration/zones.md b/docs/configuration/zones.md index ed3bc5450..7837a7c16 100644 --- a/docs/configuration/zones.md +++ b/docs/configuration/zones.md @@ -81,7 +81,7 @@ Cooling mode assumed: * `hvac-device`: at this point, the economizer is an on/off device (multistage coming). This is the identifier of the [HVAC device](./hvac.md) acting as an economizer. * `timeout`: treat both indoor and ambient sensors as stale and shut off the economizer after not receiving data from them for this long. Default is 90 seconds. The system will complain at `INFO` level if this is happening. * `settings`: This section is optional. If missing, it is assumed that the [schedule](./schedule.md) will take care of configuring economizer settings dynamically. - * `changeover-delta`: ambient temperature has to be this much lower than indoors for the economizer to start working (subject to PID configuration). + * `changeover-delta`: ambient temperature has to be this much lower than indoors for the economizer to start working (subject to PID configuration). Surprisingly, a value of 0 works well due to [PID control magic](https://en.wikipedia.org/wiki/Proportional%E2%80%93integral%E2%80%93derivative_controller). * `target-temperature`: shut the economizer off when indoor temperature drops to this value. * `keep-hvac-on`: set to `true` if you want the main HVAC to be still working when the economizer is active (maximum comfort), and to `false` if you want to stop it (maximum cost savings). diff --git a/docs/release-notes.md b/docs/release-notes.md index 3e472dbf8..6a1b2200d 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -1,7 +1,10 @@ Home Climate Control: Release Notes == -## Coming Up +## Coming Up: v4.3.0 + +* Bugfix: [#320 Economizer runs forever when there is an ambient sensor failure](https://github.com/home-climate-control/dz/issues/320) +* Breaking change: [economizer configuration syntax](../configuration/zones.md#economizer) changed to allow future enahancements ## v4.2.0 (2024-01-17) diff --git a/docs/release-notes/v4.1.0.md b/docs/release-notes/v4.1.0.md index 0e8739609..cf86df0d7 100644 --- a/docs/release-notes/v4.1.0.md +++ b/docs/release-notes/v4.1.0.md @@ -3,7 +3,7 @@ Home Climate Control: v4.1.0. Release Notes Release focus: stability improvements. -Bugfixes and enhancements: +## Bugfixes and enhancements * [#298 HCC now advertizes itself over mDNS](https://github.com/home-climate-control/dz/issues/298) * [#296 MQTT devices relying on retained messages may not work correctly](https://github.com/home-climate-control/dz/issues/296) * [#295 Multiple ESPHome logical devices per physical device will report "available" only for one random](https://github.com/home-climate-control/dz/issues/295) diff --git a/docs/release-notes/v4.2.0.md b/docs/release-notes/v4.2.0.md index a900775df..a81d26f65 100644 --- a/docs/release-notes/v4.2.0.md +++ b/docs/release-notes/v4.2.0.md @@ -3,7 +3,7 @@ Home Climate Control: v4.2.0. Release Notes Release focus: project structure. -Bugfixes and enhancements: +## Bugfixes and enhancements * [#304 Simplified project layout](https://github.com/home-climate-control/dz/issues/304) * [#303 Single invalid Zigbee2MQTT no longer causes a cascading failure](https://github.com/home-climate-control/dz/issues/303) * [#300 Ported the raise() logic from imperative to reactive code](https://github.com/home-climate-control/dz/issues/300) diff --git a/docs/release-notes/v4.3.0.md b/docs/release-notes/v4.3.0.md index dd9e1aa56..1f3f71304 100644 --- a/docs/release-notes/v4.3.0.md +++ b/docs/release-notes/v4.3.0.md @@ -3,6 +3,9 @@ Home Climate Control: v4.3.0. Release Notes - DRAFT Release focus: [Economizer and Scheduler integration](https://github.com/home-climate-control/dz/milestone/18). +## Bugfixes and enhancements +* [#320 Economizer runs forever when there is an ambient sensor failure](https://github.com/home-climate-control/dz/issues/320) + ### System Wide Measurement Units Support Not even quarter of a century later, [Fahrenheit degrees are supported](../configuration/home-climate-control.md#measurement-units). diff --git a/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/AbstractEconomizer.java b/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/AbstractEconomizer.java index 87557774a..14f06f8cc 100644 --- a/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/AbstractEconomizer.java +++ b/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/AbstractEconomizer.java @@ -176,7 +176,7 @@ protected final void initFluxes(Flux> ambientFlux) { // Get the signal var stage2 = computeDeviceState(stage1) - .map(this::recordDeviceState); + .map(this::computeDeviceState); // And act on it stage2 @@ -234,7 +234,13 @@ private void connectCombined(FluxSink sink) { logger.debug("{}: combined sink ready", getAddress()); } - private Boolean recordDeviceState(Signal> stateSignal) { + /** + * Compute the device state based on the state signal. + * + * @param stateSignal Device state as computed by the pipeline. + * @return State to send to the device. + */ + private Boolean computeDeviceState(Signal> stateSignal) { var sample = stateSignal.payload == null ? null : ((HysteresisController.HysteresisStatus) stateSignal.payload).sample; var demand = stateSignal.payload == null ? 0 : stateSignal.payload.signal; @@ -317,14 +323,14 @@ private Signal computeCombined(IndoorAmbientPair pair) { // No go, incomplete information logger.debug("{}: null signals? {}", getAddress(), pair); - return new Signal<>(clock.instant(), -1d); + return new Signal<>(clock.instant(), null, null, Signal.Status.FAILURE_TOTAL, new IllegalStateException("null signals, see the log above")); } if (pair.indoor.isError() || pair.ambient.isError()) { // Absolutely not logger.warn("{}: error signals? {}", getAddress(), pair); - return new Signal<>(clock.instant(), -1d); + return new Signal<>(clock.instant(), null, null, Signal.Status.FAILURE_TOTAL, new IllegalStateException("error signals, see the log above")); } // Let's be generous; Zigbee sensors can fall back to 60 seconds interval even if configured faster, diff --git a/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/v2/PidEconomizer.java b/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/v2/PidEconomizer.java index 69fae8db0..864afa323 100644 --- a/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/v2/PidEconomizer.java +++ b/modules/hcc-model/src/main/java/net/sf/dz3r/device/actuator/economizer/v2/PidEconomizer.java @@ -9,7 +9,6 @@ import net.sf.dz3r.device.actuator.economizer.EconomizerConfig; import net.sf.dz3r.model.Thermostat; import net.sf.dz3r.signal.Signal; -import org.apache.logging.log4j.ThreadContext; import reactor.core.publisher.Flux; import java.time.Clock; @@ -21,6 +20,8 @@ * More information: HVAC Device: Economizer * * @param Actuator device address type. + * + * @author Copyright © Vadim Tkachenko 2001-2024 */ public class PidEconomizer> extends AbstractEconomizer { @@ -73,32 +74,35 @@ public PidEconomizer( @Override protected Flux>> computeDeviceState(Flux> pv) { - ThreadContext.push("computeDeviceState"); - - try { - - // Compute the control signal to feed to the renderer. - // Might want to make this available to outside consumers for instrumentation. - var stage1 = controller - .compute(pv) - .doOnNext(e -> logger.debug("controller/{}: {}", getAddress(), e)); - - // Discard things the renderer doesn't understand. - // Total failure is denoted by NaN by stage 1, it will get through. - // The PID controller output value becomes the extra payload but is ignored at the moment (unlike Thermostat#compute()). - var stage2 = stage1 - .map(s -> new Signal<>(s.timestamp, s.getValue().signal, s.getValue(), s.status, s.error)); + // Compute the control signal to feed to the renderer. + // Might want to make this available to outside consumers for instrumentation. + var stage1 = controller + .compute(pv) + .doOnNext(e -> logger.debug("controller/{}: {}", getAddress(), e)); + + // Interpret things the renderer doesn't understand + var stage2 = stage1.map(this::computeRendererInput); + + // Deliver the signal + // Might want to expose this as well + return signalRenderer + .compute(stage2) + .doOnNext(e -> logger.debug("renderer/{}: {}", getAddress(), e)) + .map(this::mapOutput); + } - // Deliver the signal - // Might want to expose this as well - return signalRenderer - .compute(stage2) - .doOnNext(e -> logger.debug("renderer/{}: {}", getAddress(), e)) - .map(this::mapOutput); + /** + * Convert a possibly error signal from the computing pipeline into an actionable signal for the hysteresis controller. + */ + private Signal> computeRendererInput(Signal, Void> signal) { - } finally { - ThreadContext.pop(); + if (signal.isOK()) { + // PID controller output value becomes the extra payload but is ignored at the moment (unlike Thermostat#compute()). + return new Signal<>(signal.timestamp, signal.getValue().signal, signal.getValue(), signal.status, signal.error); } + + // Any kind of errors at this point must be interpreted as "turn it off" + return new Signal<>(signal.timestamp, -1d); } private Signal> mapOutput(Signal, ProcessController.Status> source) { diff --git a/modules/hcc-model/src/test/java/net/sf/dz3r/device/actuator/economizer/v2/PidEconomizerTest.java b/modules/hcc-model/src/test/java/net/sf/dz3r/device/actuator/economizer/v2/PidEconomizerTest.java new file mode 100644 index 000000000..ae0f8a4a8 --- /dev/null +++ b/modules/hcc-model/src/test/java/net/sf/dz3r/device/actuator/economizer/v2/PidEconomizerTest.java @@ -0,0 +1,101 @@ +package net.sf.dz3r.device.actuator.economizer.v2; + +import net.sf.dz3r.device.DeviceState; +import net.sf.dz3r.device.actuator.NullCqrsSwitch; +import net.sf.dz3r.device.actuator.SwitchableHvacDevice; +import net.sf.dz3r.device.actuator.economizer.EconomizerConfig; +import net.sf.dz3r.device.actuator.economizer.EconomizerSettings; +import net.sf.dz3r.model.HvacMode; +import net.sf.dz3r.signal.Signal; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.junit.jupiter.api.Test; +import reactor.core.publisher.Sinks; +import reactor.core.scheduler.Schedulers; + +import java.time.Clock; +import java.time.Duration; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; + +import static org.assertj.core.api.Assertions.assertThat; + +class PidEconomizerTest { + + private final Logger logger = LogManager.getLogger(); + + /** + * Make sure the economizer stops upon loss of the ambient sensor signal. + * + * See #320. + */ + @Test + void ambientSensorLoss() throws InterruptedException { + + var actuator = new NullCqrsSwitch("test"); + var hvac = new SwitchableHvacDevice( + Clock.systemUTC(), + "test", + HvacMode.COOLING, + actuator, + false, + null + ); + + Sinks.Many> ambientSink = Sinks.many().multicast().onBackpressureBuffer(); + Sinks.Many> indoorSink = Sinks.many().multicast().onBackpressureBuffer(); + + var ambientFlux = ambientSink.asFlux(); + var indoorFlux = indoorSink.asFlux(); + + var eco = new PidEconomizer<>( + Clock.systemUTC(), + "test", + new EconomizerConfig( + HvacMode.COOLING, + 1.0, + 0.0000008, + 1.1, + new EconomizerSettings( + 0, + 20, + true, 1.0 + ) + + ), + ambientFlux, + hvac, + Duration.ofSeconds(10) + ); + + var output = eco.compute(indoorFlux); + var start = Clock.systemUTC().instant(); + var timeStep = 10L; + var offset = new AtomicLong(0); + + output + .publishOn(Schedulers.boundedElastic()) + .subscribe(step -> logger.info("step: {}", step)); + + var deviceState = new LinkedBlockingQueue, String>>(); + + actuator.getFlux().subscribe(deviceState::add); + + // Setup complete, let's push data now + + // After receiving just the indoor signal (but not ambient), the economizer is off + indoorSink.tryEmitNext(new Signal<>(start, 25.0, "indoor")); + assertThat(deviceState.take().getValue().requested).isFalse(); + + // When both indoor and ambient signals are available, it is now on + ambientSink.tryEmitNext(new Signal<>(start.plus(Duration.ofMillis(offset.addAndGet(timeStep))), 20.0)); + assertThat(deviceState.take().getValue().requested).isTrue(); + + // It should turn off now + ambientSink.tryEmitNext(new Signal<>(start.plus(Duration.ofMillis(offset.addAndGet(timeStep))), null, null,Signal.Status.FAILURE_TOTAL, new TimeoutException("oops"))); + + // ...and it does. + assertThat(deviceState.take().getValue().requested).isFalse(); + } +}