Skip to content

Commit

Permalink
Fixed #51, added test case.
Browse files Browse the repository at this point in the history
  • Loading branch information
climategadgets committed May 18, 2021
1 parent 13bd518 commit 03059b2
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ public void testParkAllCustomMulti() throws InterruptedException, ExecutionExcep

m.park().get();

// Note: None of the dampers have their park position specified, hence,
// they will be parked where multiplexer wants

assertEquals("wrong parked position - a", parkPosition, a.getPosition(), 0.001);
assertEquals("wrong parked position - b", parkPosition, b.getPosition(), 0.001);
assertEquals("wrong parked position - m", parkPosition, m.getPosition(), 0.001);
Expand All @@ -98,10 +101,12 @@ public void testParkAllCustom() throws InterruptedException, ExecutionException,

Damper a = new NullDamper("a");
Damper b = new NullDamper("b");
Damper c = new NullDamper("c");
Set<Damper> dampers = new HashSet<>();

dampers.add(a);
dampers.add(b);
dampers.add(c);

DamperMultiplexer m = new DamperMultiplexer("m", dampers);

Expand All @@ -113,10 +118,13 @@ public void testParkAllCustom() throws InterruptedException, ExecutionException,
b.setParkPosition(parkPositionB);
m.setParkPosition(parkPositionM);

// Note: Damper 'c' will have the default park position (none), hence it will be overridden

m.park().get();

assertEquals("wrong parked position - a", parkPositionA, a.getPosition(), 0.001);
assertEquals("wrong parked position - b", parkPositionB, b.getPosition(), 0.001);
assertEquals("wrong parked position - c", parkPositionM, c.getPosition(), 0.001);
assertEquals("wrong parked position - m", parkPositionM, m.getPosition(), 0.001);

} finally {
Expand Down
10 changes: 10 additions & 0 deletions dz3-model/src/main/java/net/sf/dz3/device/actuator/Damper.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,16 @@ public interface Damper extends DataSink<Double>, DataSource<Double>, JmxAware {
@JmxAttribute(description = "Parked position")
double getParkPosition();

/**
* Determine whether the park position has been explicitly specified.
*
* See https://github.com/home-climate-control/dz/issues/51 diffs for details.
*
* @return {@code true} if park position was specified either via constructor, or by calling
* {@link #setParkPosition(double)}.
*/
boolean isCustomParkPosition();

/**
* 'Park' the damper.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,28 @@ public abstract class AbstractDamper extends LogAware implements Damper {

private final DataBroadcaster<Double> dataBroadcaster = new DataBroadcaster<Double>();

/**
* Position to park if there was no park position {@link #setParkPosition(double) explicitly specified}.
*
* Normally, the damper should be fully open in this position.
*/
private double defaultParkPosition = 1.0;

/**
* A damper position defined as 'parked'.
*
* Default value is 1 (fully open).
* Default is {@code null} - none. See commits related to https://github.com/home-climate-control/dz/issues/51
* for more details.
*
* If the value is {@code null} and {@link #park()} method is called, the value of
* {@link #defaultParkPosition} is used.
*/
private double parkPosition = 1;
private Double parkPosition = null;

/**
* Current position.
*/
private double position = parkPosition;
private double position = defaultParkPosition;

public AbstractDamper(String name) {

Expand Down Expand Up @@ -73,7 +84,13 @@ public final void setParkPosition(double parkPosition) {
@Override
public final double getParkPosition() {

return parkPosition;
return parkPosition == null ? defaultParkPosition : parkPosition;
}

@Override
public boolean isCustomParkPosition() {

return parkPosition != null;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,20 +39,31 @@ public class DamperMultiplexer extends AbstractDamper {
* Dampers to control.
*/
private final Set<Damper> dampers = new HashSet<Damper>();


/**
* Our own position. Different from {@link AbstractDamper#position}.
*
* This variable doesn't participate in transitions, but is rather for reporting purposes.
*/
private double multiPosition;

/**
* Create an instance.
*
* @param name Name to use.
* @param dampers Set of dampers to control.
* @param parkPosition Park position.
*/
public DamperMultiplexer(String name, Set<Damper> dampers, double parkPosition) {
public DamperMultiplexer(String name, Set<Damper> dampers, Double parkPosition) {
super(name);

this.dampers.addAll(dampers);

setParkPosition(parkPosition);
if (parkPosition != null) {
setParkPosition(parkPosition);
}

multiPosition = getParkPosition();
}

/**
Expand All @@ -63,19 +74,26 @@ public DamperMultiplexer(String name, Set<Damper> dampers, double parkPosition)
*/
public DamperMultiplexer(String name, Set<Damper> dampers) {

this(name, dampers, 1.0);
this(name, dampers, null);
}

@Override
protected synchronized Future<TransitionStatus> moveDamper(double position) {

multiPosition = position;

Map<Damper, Double> targetPosition = new HashMap<>();

for (Iterator<Damper> i = dampers.iterator(); i.hasNext(); ) {

targetPosition.put(i.next(), position);
}

return moveDampers(targetPosition);
}

private Future<TransitionStatus> moveDampers(Map<Damper, Double> targetPosition) {

transitionCompletionService.submit(new Damper.MoveGroup(targetPosition, false));

try {
Expand All @@ -96,12 +114,9 @@ protected synchronized Future<TransitionStatus> moveDamper(double position) {
}

@Override
public double getPosition() throws IOException {
public synchronized double getPosition() throws IOException {

// A random one (HashSet, remember?) is as good as any other,
// they're supposed to be identical anyway

return dampers.iterator().next().getPosition();
return multiPosition;
}

@Override
Expand All @@ -123,15 +138,32 @@ public Future<TransitionStatus> park() {

try {

// VT: FIXME: https://github.com/home-climate-control/dz/issues/41
//
multiPosition = getParkPosition();

// Need to park dampers at their individual positions if they were specified,
// or to the multiplexer parking position if they weren't.
//
// At this point, it is not known whether the parking positions were specified or not
// (the parking position is double, not Double); need to fix this.

return moveDamper(getParkPosition());
Map<Damper, Double> targetPosition = new HashMap<>();

for (Iterator<Damper> i = dampers.iterator(); i.hasNext(); ) {

Damper d = i.next();
boolean custom = d.isCustomParkPosition();
double position = custom ? d.getParkPosition() : getParkPosition();

targetPosition.put(d, position);

logger.debug(d.getName() + ": " + position + " (" + (custom ? "custom" : "multiplexer") + ")");

if (custom) {

// Dude, you better know what you're doing

logger.warn(d.getName() + " will be parked at custom position, consider changing hardware layout so override is not necessary");
}
}

return moveDampers(targetPosition);

} finally {
ThreadContext.pop();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@
*/
public class NullDamper extends AbstractDamper {

public NullDamper(String name) {
super(name);
}

/**
* Current throttle value.
*/
private double throttle = 0.3;
private double throttle;

public NullDamper(String name) {
super(name);

throttle = getParkPosition();
}

@Override
public Future<TransitionStatus> moveDamper(double throttle) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,18 +137,27 @@ public DummyDamper(String name) {
this.name = name;
}

@Override
public String getName() {
return name;
}

@Override
public double getParkPosition() {
return 1.0;
}

@Override
public boolean isCustomParkPosition() {
return true;
}

@Override
public double getPosition() throws IOException {
throw new UnsupportedOperationException("Not Implemented");
}

@Override
public Future<TransitionStatus> park() {
throw new UnsupportedOperationException("Not Implemented");
}
Expand Down

0 comments on commit 03059b2

Please sign in to comment.