Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve UX of WindowCovering by using motor_state #854

Merged
merged 11 commits into from
Jun 30, 2024
110 changes: 91 additions & 19 deletions src/converters/cover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,28 @@ export class CoverCreator implements ServiceCreator {
exposesHasFeatures(e) &&
!accessory.isServiceHandlerIdKnown(CoverHandler.generateIdentifier(e.endpoint))
)
.forEach((e) => this.createService(e as ExposesEntryWithFeatures, accessory));
.forEach((e) => {
let exposesMotorState: boolean = false;
const motorStateExpose = exposes.find(
(m) =>
CoverHandler.generateIdentifier(m.endpoint) === CoverHandler.generateIdentifier(e.endpoint) &&
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved
m.type === ExposesKnownTypes.ENUM &&
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved
m.name === 'motor_state' &&
exposesHasEnumProperty(m) &&
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also check if it is published by Zigbee2MQTT.

Suggested change
exposesHasEnumProperty(m) &&
exposesIsPublished(m) &&
exposesHasEnumProperty(m) &&

m.values.includes(CoverHandler.MOTOR_STATE_OPENING) &&
m.values.includes(CoverHandler.MOTOR_STATE_CLOSING) &&
m.values.includes(CoverHandler.MOTOR_STATE_STOPPED)
) as ExposesEntry[] | undefined;
if (motorStateExpose !== undefined) {
exposesMotorState = true;
}
this.createService(e as ExposesEntryWithFeatures, accessory, exposesMotorState);
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved
});
}

private createService(expose: ExposesEntryWithFeatures, accessory: BasicAccessory): void {
private createService(expose: ExposesEntryWithFeatures, accessory: BasicAccessory, modern: boolean): void {
try {
const handler = new CoverHandler(expose, accessory);
const handler = new CoverHandler(expose, accessory, modern);
accessory.registerServiceHandler(handler);
} catch (error) {
accessory.log.warn(`Failed to setup cover for accessory ${accessory.displayName} from expose "${JSON.stringify(expose)}": ${error}`);
Expand All @@ -42,6 +58,9 @@ export class CoverCreator implements ServiceCreator {

class CoverHandler implements ServiceHandler {
private static readonly STATE_HOLD_POSITION = 'STOP';
public static readonly MOTOR_STATE_OPENING = 'opening';
public static readonly MOTOR_STATE_CLOSING = 'closing';
public static readonly MOTOR_STATE_STOPPED = 'stopped';
private readonly positionExpose: ExposesEntryWithNumericRangeProperty;
private readonly tiltExpose: ExposesEntryWithNumericRangeProperty | undefined;
private readonly stateExpose: ExposesEntryWithEnumProperty | undefined;
Expand All @@ -58,12 +77,16 @@ class CoverHandler implements ServiceHandler {
private ignoreNextUpdateIfEqualToTarget: boolean;
private lastPositionSet = -1;
private positionCurrent = -1;
private motorState: string;
private motorStatePrevious: string;
private setTargetPositionHandled: boolean;

public readonly mainCharacteristics: Characteristic[] = [];

constructor(
expose: ExposesEntryWithFeatures,
private readonly accessory: BasicAccessory
private readonly accessory: BasicAccessory,
private readonly modern: boolean
) {
const endpoint = expose.endpoint;
this.identifier = CoverHandler.generateIdentifier(endpoint);
Expand Down Expand Up @@ -151,11 +174,14 @@ class CoverHandler implements ServiceHandler {
getOrAddCharacteristic(this.service, hap.Characteristic.HoldPosition).on('set', this.handleSetHoldPosition.bind(this));
}

if (exposesCanBeGet(this.positionExpose)) {
if (exposesCanBeGet(this.positionExpose) && !modern) {
this.updateTimer = new ExtendedTimer(this.requestPositionUpdate.bind(this), 4000);
}
this.waitingForUpdate = false;
this.ignoreNextUpdateIfEqualToTarget = false;
this.setTargetPositionHandled = false;
this.motorState = 'none';
this.motorStatePrevious = 'none';
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved
}

identifier: string;
Expand All @@ -173,6 +199,23 @@ class CoverHandler implements ServiceHandler {
updateState(state: Record<string, unknown>): void {
this.monitors.forEach((m) => m.callback(state));

if ('motor_state' in state) {
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved
const latestMotorState = state['motor_state'] as string;
switch (latestMotorState) {
case 'opening':
this.motorState = CoverHandler.MOTOR_STATE_OPENING;
break;
case 'closing':
this.motorState = CoverHandler.MOTOR_STATE_CLOSING;
break;
case 'stopped':
case 'pause':
default:
this.motorState = CoverHandler.MOTOR_STATE_STOPPED;
break;
}
}
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved

if (this.positionExpose.property in state) {
const latestPosition = state[this.positionExpose.property] as number;

Expand All @@ -192,7 +235,7 @@ class CoverHandler implements ServiceHandler {
let didStop = true;

// As long as the update timer is running, we are expecting updates.
if (this.updateTimer !== undefined && this.updateTimer.isActive) {
if (this.updateTimer !== undefined && this.updateTimer.isActive && !this.modern) {
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved
if (latestPosition === this.positionCurrent) {
// Stop requesting frequent updates if no change is detected.
this.updateTimer.stop();
Expand Down Expand Up @@ -252,14 +295,39 @@ class CoverHandler implements ServiceHandler {
this.current_max
);

this.service.updateCharacteristic(hap.Characteristic.CurrentPosition, characteristicValue);

if (isStopped) {
// Update target position and position state
// This should improve the UX in the Home.app
this.accessory.log.debug(`${this.accessory.displayName}: cover: assume movement stopped`);
if (this.motorState === CoverHandler.MOTOR_STATE_CLOSING && this.motorStatePrevious !== CoverHandler.MOTOR_STATE_CLOSING) {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.DECREASING);
this.accessory.log.debug(`${this.accessory.displayName}: cover: closing via motor_state`);
if (!this.setTargetPositionHandled) {
this.service.updateCharacteristic(hap.Characteristic.TargetPosition, 0);
}
this.motorStatePrevious = CoverHandler.MOTOR_STATE_CLOSING;
this.setTargetPositionHandled = false;
} else if (this.motorState === CoverHandler.MOTOR_STATE_OPENING && this.motorStatePrevious !== CoverHandler.MOTOR_STATE_OPENING) {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.INCREASING);
this.accessory.log.debug(`${this.accessory.displayName}: cover: opening via motor_state`);
if (!this.setTargetPositionHandled) {
this.service.updateCharacteristic(hap.Characteristic.TargetPosition, 100);
}
this.motorStatePrevious = CoverHandler.MOTOR_STATE_OPENING;
this.setTargetPositionHandled = false;
} else if (this.motorState === CoverHandler.MOTOR_STATE_STOPPED && this.motorStatePrevious !== CoverHandler.MOTOR_STATE_STOPPED) {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
this.service.updateCharacteristic(hap.Characteristic.CurrentPosition, characteristicValue);
this.service.updateCharacteristic(hap.Characteristic.TargetPosition, characteristicValue);
this.accessory.log.debug(`${this.accessory.displayName}: cover: stopped via motor_state`);
this.motorStatePrevious = CoverHandler.MOTOR_STATE_STOPPED;
this.setTargetPositionHandled = false;
} else if (!this.modern) {
burmistrzak marked this conversation as resolved.
Show resolved Hide resolved
this.service.updateCharacteristic(hap.Characteristic.CurrentPosition, characteristicValue);

if (isStopped) {
// Update target position and position state
// This should improve the UX in the Home.app
this.accessory.log.debug(`${this.accessory.displayName}: cover: assume movement stopped`);
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
this.service.updateCharacteristic(hap.Characteristic.TargetPosition, characteristicValue);
}
}
}

Expand All @@ -276,15 +344,19 @@ class CoverHandler implements ServiceHandler {
data[this.positionExpose.property] = target;
this.accessory.queueDataForSetAction(data);

// Assume position state based on new target
if (target > this.positionCurrent) {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.INCREASING);
} else if (target < this.positionCurrent) {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.DECREASING);
if (!this.modern) {
// Assume position state based on new target
if (target > this.positionCurrent) {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.INCREASING);
} else if (target < this.positionCurrent) {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.DECREASING);
} else {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
}
} else {
this.service.updateCharacteristic(hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED);
this.service.updateCharacteristic(hap.Characteristic.TargetPosition, target);
this.setTargetPositionHandled = true;
}

// Store last sent position for future reference
this.lastPositionSet = target;

Expand Down
103 changes: 103 additions & 0 deletions test/cover.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,4 +413,107 @@ describe('Cover', () => {
harness.checkHomeKitUpdate(hap.Service.WindowCovering, 'state', true, { state: 'STOP' });
});
});

describe('Bosch Light/shutter control unit II', () => {
// Shared "state"
let deviceExposes: ExposesEntry[] = [];
let harness: ServiceHandlersTestHarness;

beforeEach(() => {
// Only test service creation for first test case and reuse harness afterwards
if (deviceExposes.length === 0 && harness === undefined) {
// Load exposes from JSON
deviceExposes = loadExposesFromFile('bosch/bmct-slz.json');
expect(deviceExposes.length).toBeGreaterThan(0);
const newHarness = new ServiceHandlersTestHarness();

// Check service creation
const windowCovering = newHarness
.getOrAddHandler(hap.Service.WindowCovering)
.addExpectedCharacteristic('position', hap.Characteristic.CurrentPosition, false)
.addExpectedCharacteristic('target_position', hap.Characteristic.TargetPosition, true)
.addExpectedCharacteristic('position_state', hap.Characteristic.PositionState, false)
.addExpectedCharacteristic('state', hap.Characteristic.HoldPosition, true);
newHarness.prepareCreationMocks();

const positionCharacteristicMock = windowCovering.getCharacteristicMock('position');
if (positionCharacteristicMock !== undefined) {
positionCharacteristicMock.props.minValue = 0;
positionCharacteristicMock.props.maxValue = 100;
}

const targetPositionCharacteristicMock = windowCovering.getCharacteristicMock('target_position');
if (targetPositionCharacteristicMock !== undefined) {
targetPositionCharacteristicMock.props.minValue = 0;
targetPositionCharacteristicMock.props.maxValue = 100;
}

newHarness.callCreators(deviceExposes);

newHarness.checkCreationExpectations();
newHarness.checkHasMainCharacteristics();
newHarness.checkExpectedGetableKeys(['position']);
harness = newHarness;
}
harness?.clearMocks();
});

afterEach(() => {
verifyAllWhenMocksCalled();
resetAllWhenMocks();
});

test('Status update is handled: Position changes', () => {
expect(harness).toBeDefined();

// First update (previous state is unknown, so)
harness.checkUpdateState(
'{"position":100, "motor_state":"stopped"}',
hap.Service.WindowCovering,
new Map([
[hap.Characteristic.CurrentPosition, 100],
[hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED],
[hap.Characteristic.TargetPosition, 100],
])
);
harness.clearMocks();
});

test('HomeKit: Hold position', () => {
expect(harness).toBeDefined();

harness.checkHomeKitUpdate(hap.Service.WindowCovering, 'state', true, { state: 'STOP' });
});

test('HomeKit: Change target position', () => {
expect(harness).toBeDefined();

// Ignore known stopped position
harness.checkUpdateStateIsIgnored('{"position":100, "motor_state":"stopped"}');
harness.clearMocks();

// Check changing the position to a lower value
harness.checkHomeKitUpdate(hap.Service.WindowCovering, 'target_position', 51, { position: 51 });
harness.getOrAddHandler(hap.Service.WindowCovering).checkCharacteristicUpdates(new Map([[hap.Characteristic.TargetPosition, 51]]));
harness.clearMocks();

harness.checkUpdateState(
'{"position":41, "motor_state":"closing"}',
hap.Service.WindowCovering,
new Map([[hap.Characteristic.PositionState, hap.Characteristic.PositionState.DECREASING]])
);
harness.clearMocks();

harness.checkUpdateState(
'{"position":51, "motor_state":"stopped"}',
hap.Service.WindowCovering,
new Map([
[hap.Characteristic.CurrentPosition, 51],
[hap.Characteristic.PositionState, hap.Characteristic.PositionState.STOPPED],
[hap.Characteristic.TargetPosition, 51],
])
);
harness.clearMocks();
});
});
});
103 changes: 103 additions & 0 deletions test/exposes/bosch/bmct-slz.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
[
{
"access": 7,
"description": "Module controlled by a rocker switch or a button",
"label": "Switch type",
"name": "switch_type",
"property": "switch_type",
"type": "enum",
"values": [
"button",
"button_key_change",
"rocker_switch",
"rocker_switch_key_change"
]
},
{
"access": 1,
"category": "diagnostic",
"description": "Link quality (signal strength)",
"label": "Linkquality",
"name": "linkquality",
"property": "linkquality",
"type": "numeric",
"unit": "lqi",
"value_max": 255,
"value_min": 0
},
{
"features": [
{
"access": 3,
"label": "State",
"name": "state",
"property": "state",
"type": "enum",
"values": [
"OPEN",
"CLOSE",
"STOP"
]
},
{
"access": 7,
"description": "Position of this cover",
"label": "Position",
"name": "position",
"property": "position",
"type": "numeric",
"unit": "%",
"value_max": 100,
"value_min": 0
}
],
"type": "cover"
},
{
"access": 1,
"description": "Shutter motor actual state ",
"label": "Motor state",
"name": "motor_state",
"property": "motor_state",
"type": "enum",
"values": [
"stopped",
"opening",
"closing"
]
},
{
"access": 7,
"description": "Enable/Disable child lock",
"label": "Child lock",
"name": "child_lock",
"property": "child_lock",
"type": "binary",
"value_off": "OFF",
"value_on": "ON"
},
{
"access": 7,
"description": "Calibration closing time",
"endpoint": "closing_time",
"label": "Calibration",
"name": "calibration",
"property": "calibration_closing_time",
"type": "numeric",
"unit": "s",
"value_max": 90,
"value_min": 1
},
{
"access": 7,
"description": "Calibration opening time",
"endpoint": "opening_time",
"label": "Calibration",
"name": "calibration",
"property": "calibration_opening_time",
"type": "numeric",
"unit": "s",
"value_max": 90,
"value_min": 1
}
]
Loading