Skip to content

Commit

Permalink
Solves two issues:
Browse files Browse the repository at this point in the history
1. If resend is set to true and target position is equal to current position, the previous implementation resulted in sending the close command.
That doesn't make sense. If for example, the window covering is already completely open, the previous implementation made it close instead of keeping it open.

2. If close->open commands are issued in rapid succession, the open command will not get sent because the target position is still equal to current position. This is not really true in reality since the window covering has already started moving by then. The previous implementation resulted in the window covering continuing to close, rather then start opening.

The two issues are somewhat related.
  • Loading branch information
seidnerj committed Nov 29, 2023
1 parent 744f2ce commit 18f6dba
Showing 1 changed file with 47 additions and 20 deletions.
67 changes: 47 additions & 20 deletions accessories/windowCovering.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,22 @@ class WindowCoveringAccessory extends BroadlinkRMAccessory {
const closeCompletely = await this.checkOpenOrCloseCompletely();
if (closeCompletely) {return;}

log(`${name} setTargetPosition: (currentPosition: ${state.currentPosition})`);
log(`${name} setTargetPosition: (set new position)`);

// Determine if we're opening or closing
let difference = state.targetPosition - state.currentPosition;

state.opening = (difference > 0);
if (!state.opening) {difference = -1 * difference;}

hexData = state.opening ? open : close

if (difference > 0) {
state.positionState = Characteristic.PositionState.INCREASING
hexData = open
} else if (difference < 0) {
state.positionState = Characteristic.PositionState.DECREASING
hexData = close
} else {
state.positionState = Characteristic.PositionState.STOPPED
hexData = stop
}

// Perform the actual open/close asynchronously i.e. without await so that HomeKit status can be updated
this.openOrClose({ hexData, previousValue });
});
Expand All @@ -85,23 +91,42 @@ class WindowCoveringAccessory extends BroadlinkRMAccessory {
let { totalDurationOpen, totalDurationClose } = config;
const { stop } = data;

const newPositionState = state.opening ? Characteristic.PositionState.INCREASING : Characteristic.PositionState.DECREASING;
serviceManager.setCharacteristic(Characteristic.PositionState, newPositionState);

log(`${name} setTargetPosition: currently ${state.currentPosition}%, moving to ${state.targetPosition}%`);

await this.performSend(hexData);
serviceManager.setCharacteristic(Characteristic.PositionState, state.positionState);

let difference = state.targetPosition - state.currentPosition
if (!state.opening) {difference = -1 * difference;}

const fullOpenCloseTime = state.opening ? totalDurationOpen : totalDurationClose;
const durationPerPercentage = fullOpenCloseTime / 100;
const totalTime = durationPerPercentage * difference;
const totalTime = difference / 100 * fullOpenCloseTime;

log(`${name} setTargetPosition: ${totalTime}s (${fullOpenCloseTime} / 100 * ${difference}) until auto-stop`);
log(`${name} setTargetPosition: position change ${state.currentPosition}% -> ${state.targetPosition}%`);
log(`${name} setTargetPosition: ${totalTime}s (${difference} / 100 * ${fullOpenCloseTime} ) until auto-stop`);

this.startUpdatingCurrentPositionAtIntervals();
await this.performSend(hexData);

if (state.positionState != Characteristic.PositionState.STOPPED) {
// immediately update position to reflect that there's already some change in the position (even though its fractional,
// we have to add 1 whole %), we then skip incrementing the position within startUpdatingCurrentPositionAtIntervals
// if this is a first iteration, this way at time 0 the position delta is already 1, and so is the position at time 1
// and we do not overshoot the actual position.

// NOTE: ideally send+position update should be an "atomic" operation and the position should change by some
// fractional value (e.g. 0.00001) but that requires significant changes to the code base.

// if we get here, it means that difference != 0, and the minimal abs(difference) is 1 so even by
// increasing/decreasing currentValue its value should not go above 100 or below 0
let currentValue = state.currentPosition || 0;
if (state.opening) {currentValue++;}
if (!state.opening) {currentValue--;}
serviceManager.setCharacteristic(Characteristic.CurrentPosition, currentValue);
this.startUpdatingCurrentPositionAtIntervals(true);
} else {

// there's really no reason to do this is the target position state is "STOPPED" since we risk inadvertently
// increasing the value of currentPosition (even though its unlikely since totalTime is 0 and we issue stopWidnowCovering
// and hence reset() immediately).
this.startUpdatingCurrentPositionAtIntervals(false);
}

this.autoStopPromise = delayForDuration(totalTime);
await this.autoStopPromise;
Expand Down Expand Up @@ -172,7 +197,7 @@ class WindowCoveringAccessory extends BroadlinkRMAccessory {
return durationPerPercentage;
}

async startUpdatingCurrentPositionAtIntervals () {
async startUpdatingCurrentPositionAtIntervals (isFirst) {
catchDelayCancelError(async () => {
const { config, serviceManager, state } = this;
const { totalDurationOpen, totalDurationClose } = config;
Expand All @@ -186,13 +211,15 @@ class WindowCoveringAccessory extends BroadlinkRMAccessory {
// Set the new currentPosition
let currentValue = state.currentPosition || 0;

if (state.opening) {currentValue++;}
if (!state.opening) {currentValue--;}
if (!isFirst) {
if (state.opening) {currentValue++;}
if (!state.opening) {currentValue--;}
}

serviceManager.setCharacteristic(Characteristic.CurrentPosition, currentValue);

// Let's go again
this.startUpdatingCurrentPositionAtIntervals();
this.startUpdatingCurrentPositionAtIntervals(false);
});
}

Expand Down

0 comments on commit 18f6dba

Please sign in to comment.