Skip to content

Commit

Permalink
Implemented #280: ESPHomeSwitch is now instantiated optimistic, all o…
Browse files Browse the repository at this point in the history
…thers not
  • Loading branch information
climategadgets committed Sep 8, 2023
1 parent 5661b57 commit 73dd95c
Show file tree
Hide file tree
Showing 18 changed files with 165 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,6 @@ public interface SwitchConfig {
Optional<Duration> heartbeat();
@JsonProperty("pace")
Optional<Duration> pace();
@JsonProperty("optimistic")
Optional<Boolean> optimistic();
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public interface InterfaceRecordMapper {
@Mapping(expression = "java(source.reversed().orElse(false))", target = "reversed")
@Mapping(expression = "java(source.heartbeat().orElse(null))", target = "heartbeat")
@Mapping(expression = "java(source.pace().orElse(null))", target = "pace")
@Mapping(expression = "java(source.optimistic().orElse(null))", target = "optimistic")
net.sf.dz3.runtime.config.hardware.SwitchConfig switchConfig(SwitchConfig source);

@Mapping(expression = "java(source.serialPort())", target = "serialPort")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import net.sf.dz3r.device.mqtt.v1.MqttAdapter;

import java.util.Map;
import java.util.Optional;
import java.util.Set;

public class EspSensorSwitchResolver extends MqttSensorSwitchResolver<MqttDeviceConfig, ESPHomeListener, ESPHomeSwitch> {
Expand All @@ -21,7 +22,15 @@ protected ESPHomeListener createSensorListener(MqttAdapter adapter, String rootT
}

@Override
protected ESPHomeSwitch createSwitch(MqttAdapter adapter, String rootTopic) {
return new ESPHomeSwitch(adapter, rootTopic, null);
protected ESPHomeSwitch createSwitch(MqttAdapter adapter, String rootTopic, Boolean optimistic) {

// Optimistic defaults to true for this switch only
// https://github.com/home-climate-control/dz/issues/280

return new ESPHomeSwitch(
adapter,
rootTopic,
Optional.ofNullable(optimistic).orElse(true),
null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ private Flux<Map.Entry<String, S>> getSwitches(Map<MqttEndpointSpec, MqttAdapter

var id = c.switchConfig().id();
var address = c.switchConfig.address();
var s = createSwitch(adapter, address);
var s = createSwitch(adapter, address, c.switchConfig.optimistic());

// ID takes precedence over address
var key = id == null ? address : id;
Expand All @@ -166,8 +166,7 @@ private Flux<Map.Entry<String, S>> getSwitches(Map<MqttEndpointSpec, MqttAdapter
});
}

protected abstract S createSwitch(MqttAdapter adapter, String rootTopic);

protected abstract S createSwitch(MqttAdapter adapter, String rootTopic, Boolean optimistic);

public record MqttSensorConfig(
MqttBrokerSpec mqttBrokerSpec,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import net.sf.dz3r.signal.SignalSource;

import java.util.Map;
import java.util.Optional;
import java.util.Set;

public class ZWaveSensorSwitchResolver extends MqttSensorSwitchResolver<MqttDeviceConfig, SignalSource<String, Double, Void>, ZWaveBinarySwitch> {
Expand All @@ -22,7 +23,11 @@ protected SignalSource<String, Double, Void> createSensorListener(MqttAdapter ad
}

@Override
protected ZWaveBinarySwitch createSwitch(MqttAdapter adapter, String rootTopic) {
return new ZWaveBinarySwitch(adapter, rootTopic, null);
protected ZWaveBinarySwitch createSwitch(MqttAdapter adapter, String rootTopic, Boolean optimistic) {
return new ZWaveBinarySwitch(
adapter,
rootTopic,
Optional.ofNullable(optimistic).orElse(false),
null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import net.sf.dz3r.device.z2m.v1.Z2MSwitch;

import java.util.Map;
import java.util.Optional;
import java.util.Set;

public class ZigbeeSensorSwitchResolver extends MqttSensorSwitchResolver<MqttDeviceConfig, Z2MJsonListener, Z2MSwitch> {
Expand All @@ -21,7 +22,11 @@ protected Z2MJsonListener createSensorListener(MqttAdapter adapter, String rootT
}

@Override
protected Z2MSwitch createSwitch(MqttAdapter adapter, String rootTopic) {
return new Z2MSwitch(adapter, rootTopic, null);
protected Z2MSwitch createSwitch(MqttAdapter adapter, String rootTopic, Boolean optimistic) {
return new Z2MSwitch(
adapter,
rootTopic,
Optional.ofNullable(optimistic).orElse(false),
null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
* @param reversed {@code true} if the switch must be reversed.
* @param heartbeat Issue control commands to this switch at least this often, repeat if necessary.
* @param pace Issue identical control commands to this switch at most this often.
* @param optimistic See <a href="https://github.com/home-climate-control/dz/issues/280">issue 280</a>.
*/
public record SwitchConfig(
String id,
String address,
boolean reversed,
Duration heartbeat,
Duration pace
Duration pace,
Boolean optimistic
) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,13 @@ public abstract class AbstractSwitch<A extends Comparable<A>> implements Switch<
*/
private final Clock clock;

private Sinks.Many<Signal<State, String>> stateSink;
private Flux<Signal<State, String>> stateFlux;
/**
* Assume that {@link #setState(boolean)} always worked without checking if it did. Caveat emptor.
*/
protected final boolean optimistic;

private final Sinks.Many<Signal<State, String>> stateSink;
private final Flux<Signal<State, String>> stateFlux;
private Boolean lastKnownState;

/**
Expand All @@ -59,7 +64,7 @@ public abstract class AbstractSwitch<A extends Comparable<A>> implements Switch<
* @param address Switch address.
*/
protected AbstractSwitch(@NonNull A address) {
this(address, Schedulers.newSingle("switch:" + address, true), null, null);
this(address, false, Schedulers.newSingle("switch:" + address, true), null, null);
}

/**
Expand All @@ -70,10 +75,11 @@ protected AbstractSwitch(@NonNull A address) {
* @param pace Issue identical control commands to this switch at most this often.
* @param clock Clock to use. Pass {@code null} except when testing.
*/
protected AbstractSwitch(@NonNull A address, @Nullable Scheduler scheduler, @Nullable Duration pace, @Nullable Clock clock) {
protected AbstractSwitch(@NonNull A address, boolean optimistic, @Nullable Scheduler scheduler, @Nullable Duration pace, @Nullable Clock clock) {

// VT: NOTE: @NonNull seems to have no effect, what enforces it?
this.address = HCCObjects.requireNonNull(address,"address can't be null");
this.optimistic = optimistic;

this.scheduler = scheduler;
this.pace = pace;
Expand All @@ -82,7 +88,7 @@ protected AbstractSwitch(@NonNull A address, @Nullable Scheduler scheduler, @Nul
stateSink = Sinks.many().multicast().onBackpressureBuffer();
stateFlux = stateSink.asFlux();

logger.info("{}: created AbstractSwitch({}) with pace={}", Integer.toHexString(hashCode()), getAddress(), pace);
logger.info("{}: created AbstractSwitch({}) with optimistic={}, pace={}", Integer.toHexString(hashCode()), getAddress(), optimistic, pace);
}

@Override
Expand Down Expand Up @@ -112,7 +118,9 @@ public final Mono<Boolean> setState(boolean state) {

lastSetAt = clock.instant();

return getState();
return optimistic
? Mono.just(state)
: getState();

} catch (IOException e) {
return Mono.create(sink -> sink.error(e));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,53 @@ public class NullSwitch extends AbstractSwitch<String> {
private Boolean state;

/**
* Create an instance without delay running on the default scheduler.
* Create a pessimistic instance without delay running on the default scheduler.
*
* @param address Address to use.
*/
public NullSwitch(String address) {
this(address, 0, 0, Schedulers.newSingle("NullSwitch:" + address, true));
this(address, false, 0, 0, Schedulers.newSingle("NullSwitch:" + address, true));
}

/**
* Create an instance without delay running on the provided scheduler.
* Create an instance without delay running on the default scheduler.
*
* @param address Address to use.
* @param optimistic See <a href="https://github.com/home-climate-control/dz/issues/280">issue 280</a>.
*/
public NullSwitch(String address, boolean optimistic) {
this(address, optimistic, 0, 0, Schedulers.newSingle("NullSwitch:" + address, true));
}

/**
* Create a pessimistic instance without delay running on the provided scheduler.
*
* @param address Address to use.
* @param scheduler Scheduler to use.
*/
public NullSwitch(String address, Scheduler scheduler) {
this(address, 0, 0, scheduler);
this(address, false, 0, 0, scheduler);
}

/**
* Create an instance without delay running on the provided scheduler.
*
* @param address Address to use.
* @param scheduler Scheduler to use.
*/
public NullSwitch(String address, boolean optimistic, Scheduler scheduler) {
this(address, optimistic, 0, 0, scheduler);
}

/**
* Create an instance with delay.
*
* @param address Address to use.
* @param minDelayMillis Minimim switch deley, milliseconds.
* @param maxDelayMillis Max delay. Total delay is calculated as {@code minDelay + rg.nextInt(maxDelay)}.
*/
public NullSwitch(String address, long minDelayMillis, int maxDelayMillis, Scheduler scheduler) {
super(address, scheduler, null, null);
public NullSwitch(String address, boolean optimistic, long minDelayMillis, int maxDelayMillis, Scheduler scheduler) {
super(address, optimistic, scheduler, null, null);

if (minDelayMillis < 0 || maxDelayMillis < 0 || (maxDelayMillis > 0 && (minDelayMillis >= maxDelayMillis))) {
throw new IllegalArgumentException("invalid delays min=" + minDelayMillis + ", max=" + maxDelayMillis);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class AbstractSwitchTest {
void testFirst() {


var s = new TestSwitch("A", null, null, null);
var s = new TestSwitch("A", false, null, null, null);

var state = s.setState(true).block();

Expand All @@ -30,7 +30,7 @@ void testFirst() {
void testNullDelay() {


var s = new TestSwitch("A", null, null, null);
var s = new TestSwitch("A", false, null, null, null);

s.setState(true).block();
s.setState(true).block();
Expand All @@ -44,7 +44,7 @@ void testIdenticalTooClose() {
var clock = new TestClock(Clock.fixed(Instant.now(), ZoneId.systemDefault()));
var rateAllowed = Duration.ofSeconds(1);

var s = new TestSwitch("A", null, rateAllowed, clock);
var s = new TestSwitch("A", false, null, rateAllowed, clock);

s.setState(true).block();

Expand All @@ -60,7 +60,7 @@ void testIdenticalFarEnough() {
var clock = new TestClock(Clock.fixed(Instant.now(), ZoneId.systemDefault()));
var rateAllowed = Duration.ofSeconds(1);

var s = new TestSwitch("A", null, rateAllowed, clock);
var s = new TestSwitch("A", false, null, rateAllowed, clock);

var state1 = s.setState(true).block();
assertThat(state1).isTrue();
Expand All @@ -77,7 +77,7 @@ void testCloseButDifferent() {
var clock = new TestClock(Clock.fixed(Instant.now(), ZoneId.systemDefault()));
var rateAllowed = Duration.ofSeconds(1);

var s = new TestSwitch("A", null, rateAllowed, clock);
var s = new TestSwitch("A", false, null, rateAllowed, clock);

var state1 = s.setState(true).block();

Expand All @@ -93,7 +93,7 @@ void testFarEnoughAndDifferent() {
var clock = new TestClock(Clock.fixed(Instant.now(), ZoneId.systemDefault()));
var rateAllowed = Duration.ofSeconds(1);

var s = new TestSwitch("A", null, rateAllowed, clock);
var s = new TestSwitch("A", false, null, rateAllowed, clock);

var state1 = s.setState(true).block();

Expand All @@ -109,8 +109,8 @@ private static class TestSwitch extends AbstractSwitch<String> {
private Boolean state;
public final AtomicInteger counter = new AtomicInteger(0);

protected TestSwitch(String address, Scheduler scheduler, Duration pace, Clock clock) {
super(address, scheduler, pace, clock);
protected TestSwitch(String address, boolean optimistic, Scheduler scheduler, Duration pace, Clock clock) {
super(address, optimistic, scheduler, pace, clock);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ private SwitchPack getSwitchPack() {

// VT: NOTE: Might need to use this for running parameterized tests with different schedulers
return new SwitchPack(
new NullSwitch("mode", scheduler),
new NullSwitch("running", scheduler),
new NullSwitch("fan", scheduler)
new NullSwitch("mode", true, scheduler),
new NullSwitch("running", true, scheduler),
new NullSwitch("fan", true, scheduler)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,22 +99,38 @@ void passSetStateWithFlux() throws InterruptedException {
}

@Test
void passSetStateWithBlockDelaySingleScheduler() {
void passSetStateWithBlockDelaySingleSchedulerPessimistic() {

assertThatCode(() -> {
delay(null);
delay(null, false);
}).doesNotThrowAnyException();
}

@Test
void passSetStateWithBlockDelayElasticScheduler() {
void passSetStateWithBlockDelaySingleSchedulerOptimistic() {

assertThatCode(() -> {
delay(Schedulers.newBoundedElastic(1, 10, "e-single"));
delay(null, true);
}).doesNotThrowAnyException();
}

private void delay(Scheduler scheduler) {
assertThat(new NullSwitch("D", 10, 50, scheduler).setState(true).block()).isTrue();
@Test
void passSetStateWithBlockDelayElasticSchedulerPessimistic() {

assertThatCode(() -> {
delay(Schedulers.newBoundedElastic(1, 10, "e-single"), false);
}).doesNotThrowAnyException();
}

@Test
void passSetStateWithBlockDelayElasticSchedulerOptimistic() {

assertThatCode(() -> {
delay(Schedulers.newBoundedElastic(1, 10, "e-single"), true);
}).doesNotThrowAnyException();
}

private void delay(Scheduler scheduler, boolean optimistic) {
assertThat(new NullSwitch("D", optimistic, 10, 50, scheduler).setState(true).block()).isTrue();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void lifecycle() {
var now = Instant.now();
var minDelayMillis = 50;
var maxDelayMillis = 200;
var s = new NullSwitch("a", minDelayMillis, maxDelayMillis, null);
var s = new NullSwitch("a", false, minDelayMillis, maxDelayMillis, null);
var d = new SwitchableHvacDevice("d", HvacMode.COOLING, s);

var sequence = Flux.just(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,11 @@ private void testSync(String marker, Class<? extends AbstractDamperController> c

Scheduler switchScheduler = Schedulers.newSingle("switch scheduler", true);

var switchLivingRoom = new NullSwitch("switch_livingroom_damper", minDelay, maxDelay, switchScheduler);
var switchKitchen = new NullSwitch("switch_kitchen_damper", minDelay, maxDelay, switchScheduler);
var switchWestBathroom = new NullSwitch("switch_west_bathroom_damper", minDelay, maxDelay, switchScheduler);
var switchWestDamper = new NullSwitch("switch_west_damper", minDelay, maxDelay, switchScheduler);
var switchWestBoosterFan = new NullSwitch("switch_west_boosterfan", minDelay, maxDelay, switchScheduler);
var switchLivingRoom = new NullSwitch("switch_livingroom_damper", false, minDelay, maxDelay, switchScheduler);
var switchKitchen = new NullSwitch("switch_kitchen_damper", false, minDelay, maxDelay, switchScheduler);
var switchWestBathroom = new NullSwitch("switch_west_bathroom_damper", false, minDelay, maxDelay, switchScheduler);
var switchWestDamper = new NullSwitch("switch_west_damper", false, minDelay, maxDelay, switchScheduler);
var switchWestBoosterFan = new NullSwitch("switch_west_boosterfan", false, minDelay, maxDelay, switchScheduler);

var damperLivingRoom = new SwitchDamper<>("damper_livingroom", switchLivingRoom, 0.8, 1.0);
var damperKitchen = new SwitchDamper<>("damper_kitchen", switchKitchen, 0.8, 1.0);
Expand Down
Loading

0 comments on commit 73dd95c

Please sign in to comment.