From 7d31f51ae4c8521c57a626efb8c302ae0c044c56 Mon Sep 17 00:00:00 2001 From: Arno Moonen Date: Thu, 23 May 2024 22:12:03 +0200 Subject: [PATCH 1/3] Adaptive Lighting: enable by default. Reset reference when brightness is changed or turned on via HomeKit. Still need to fix tests --- config.schema.json | 26 +++++++++++++++++++++++--- docs/light.md | 4 +++- src/converters/light.ts | 41 ++++++++++++++++++++++++----------------- test/light.spec.ts | 9 +++++++-- test/testHelpers.ts | 30 +++++++++++++++--------------- 5 files changed, 72 insertions(+), 38 deletions(-) diff --git a/config.schema.json b/config.schema.json index 72677dc0..e0489e07 100644 --- a/config.schema.json +++ b/config.schema.json @@ -161,9 +161,29 @@ "type": "object", "properties": { "adaptive_lighting": { - "title": "Enable Adaptive Lighting", - "type": "boolean", - "required": false + "title": "Adaptive Lighting", + "type": "object", + "required": false, + "properties": { + "enabled": { + "title": "Enable adaptive lighting", + "type": "boolean", + "default": true, + "required": false + }, + "only_when_on": { + "title": "Only send color temperature updates when light is on", + "type": "boolean", + "default": true, + "required": false + }, + "transition": { + "title": "Transition time", + "type": "integer", + "default": 0, + "required": false + } + } } } } diff --git a/docs/light.md b/docs/light.md index a66a95b7..d0fc8752 100644 --- a/docs/light.md +++ b/docs/light.md @@ -12,8 +12,9 @@ The table below shows how the different features within this `exposes` entry are ## Converter specific configuration (`light`) -- `adaptive_lighting`: Set to `true` to enable [Adaptive Lighting](https://support.apple.com/guide/iphone/control-accessories-iph0a717a8fd/ios#iph79e72e212). Apple requires a home hub for Adaptive Lighting to work. This feature is only available for lights that expose a *Color Temperature* characteristic. +- `adaptive_lighting`: Set to `false` to disable [Adaptive Lighting](https://support.apple.com/guide/iphone/control-accessories-iph0a717a8fd/ios#iph79e72e212). Apple requires a home hub for Adaptive Lighting to work. This feature is only available for lights that expose a *Color Temperature* characteristic. Additionally you can also configure the following options for Adaptive Lighting: + - `enabled`: Set to `true` to enable Adaptive Lighting. Defaults to `true`. (same as setting `adaptive_lighting` to a boolean value) - `only_when_on`: Only update the color temperature when the light is on. Defaults to `true`. - `transition`: Transition time to send along with the color temperature change when the light is on. If not defined, `transition` will not be send. @@ -22,6 +23,7 @@ The table below shows how the different features within this `exposes` entry are "converters": { "light": { "adaptive_lighting": { + "enabled": true, "only_when_on": true, "transition": 0.5 } diff --git a/src/converters/light.ts b/src/converters/light.ts index 97ceb368..3d52ed74 100644 --- a/src/converters/light.ts +++ b/src/converters/light.ts @@ -30,6 +30,7 @@ import { convertHueSatToXy, convertMiredColorTemperatureToHueSat, convertXyToHue import { EXP_COLOR_MODE } from '../experimental'; interface AdaptiveLightingConfig { + enabled?: boolean; only_when_on?: boolean; transition?: number; } @@ -40,6 +41,8 @@ interface LightConfig { const isAdaptiveLightingConfig = (x: unknown): x is AdaptiveLightingConfig => x !== undefined && + typeof x !== 'boolean' && + (typeof (x as AdaptiveLightingConfig).enabled === 'boolean' || (x as AdaptiveLightingConfig).enabled === undefined) && (typeof (x as AdaptiveLightingConfig).only_when_on === 'boolean' || (x as AdaptiveLightingConfig).only_when_on === undefined) && (typeof (x as AdaptiveLightingConfig).transition === 'number' || (x as AdaptiveLightingConfig).transition === undefined); @@ -51,7 +54,8 @@ const isLightConfig = (x: unknown): x is LightConfig => export class LightCreator implements ServiceCreator { public static readonly CONFIG_TAG = 'light'; - private static readonly DEFAULT_CONFIG_WHEN_ON = { + private static readonly DEFAULT_CONFIG = { + enabled: true, only_when_on: true, transition: undefined, }; @@ -74,12 +78,15 @@ export class LightCreator implements ServiceCreator { private createService(expose: ExposesEntryWithFeatures, accessory: BasicAccessory): void { const converterConfig = accessory.getConverterConfiguration(LightCreator.CONFIG_TAG); - let adaptiveLightingConfig: AdaptiveLightingConfig | undefined = undefined; + let adaptiveLightingConfig: AdaptiveLightingConfig = LightCreator.DEFAULT_CONFIG; if (isLightConfig(converterConfig)) { if (isAdaptiveLightingConfig(converterConfig.adaptive_lighting)) { adaptiveLightingConfig = converterConfig.adaptive_lighting; - } else if (converterConfig.adaptive_lighting === true) { - adaptiveLightingConfig = LightCreator.DEFAULT_CONFIG_WHEN_ON; + if (adaptiveLightingConfig.enabled === undefined) { + adaptiveLightingConfig.enabled = true; + } + } else if (converterConfig.adaptive_lighting === false) { + adaptiveLightingConfig.enabled = false; } } @@ -132,13 +139,13 @@ class LightHandler implements ServiceHandler { private received_saturation = false; private get adaptiveLightingEnabled(): boolean { - return this.adaptiveLightingConfig !== undefined; + return this.adaptiveLightingConfig.enabled === true; } constructor( expose: ExposesEntryWithFeatures, private readonly accessory: BasicAccessory, - private readonly adaptiveLightingConfig?: AdaptiveLightingConfig + private readonly adaptiveLightingConfig: AdaptiveLightingConfig ) { const endpoint = expose.endpoint; this.identifier = LightHandler.generateIdentifier(endpoint); @@ -376,8 +383,14 @@ class LightHandler implements ServiceHandler { private handleSetOn(value: CharacteristicValue, callback: CharacteristicSetCallback): void { const data = {}; - data[this.stateExpose.property] = (value as boolean) ? this.stateExpose.value_on : this.stateExpose.value_off; + const is_on = value as boolean; + data[this.stateExpose.property] = is_on ? this.stateExpose.value_on : this.stateExpose.value_off; this.accessory.queueDataForSetAction(data); + + // If turned on, reset the last adaptive lighting temperature. + if (is_on) { + this.resetAdaptiveLightingTemperature(); + } callback(null); } @@ -400,6 +413,8 @@ class LightHandler implements ServiceHandler { ); } this.accessory.queueDataForSetAction(data); + // If brightness is set, reset the last adaptive lighting temperature. + this.resetAdaptiveLightingTemperature(); callback(null); } else { callback(new Error('brightness not supported')); @@ -526,7 +541,7 @@ class LightHandler implements ServiceHandler { // Adaptive Lighting active? if (this.colorTempExpose !== undefined && this.adaptiveLighting !== undefined && this.adaptiveLighting.isAdaptiveLightingActive()) { const lightIsOn = this.service.getCharacteristic(hap.Characteristic.On).value as boolean; - if (this.adaptiveLightingConfig?.only_when_on && lightIsOn === false) { + if (this.adaptiveLightingConfig.only_when_on && lightIsOn === false) { this.accessory.log.debug(`adaptive_lighting: ${this.accessory.displayName}: skipped, light is off`); return false; } @@ -542,7 +557,7 @@ class LightHandler implements ServiceHandler { return false; } - if (lightIsOn && this.adaptiveLightingConfig?.transition !== undefined && this.adaptiveLightingConfig.transition > 0) { + if (lightIsOn && this.adaptiveLightingConfig.transition !== undefined && this.adaptiveLightingConfig.transition > 0) { this.accessory.queueDataForSetAction({ transition: this.adaptiveLightingConfig.transition }); } @@ -555,14 +570,6 @@ class LightHandler implements ServiceHandler { return true; } - - private updateHueAndSaturationBasedOnColorTemperature(value: number): void { - if (this.colorHueCharacteristic !== undefined && this.colorSaturationCharacteristic !== undefined) { - const color = hap.ColorUtils.colorTemperatureToHueAndSaturation(value, true); - this.colorHueCharacteristic.updateValue(color.hue); - this.colorSaturationCharacteristic.updateValue(color.saturation); - } - } } class ColorTemperatureToHueSatMonitor implements CharacteristicMonitor { diff --git a/test/light.spec.ts b/test/light.spec.ts index 5b8ecb8f..f9eb5dd4 100644 --- a/test/light.spec.ts +++ b/test/light.spec.ts @@ -166,6 +166,8 @@ describe('Light', () => { // Check service creation const newHarness = new ServiceHandlersTestHarness(); + newHarness.addConverterConfiguration('light', { adaptive_lighting: { enabled: false } }); + newHarness.numberOfExpectedControllers = 0; const lightbulb = newHarness .getOrAddHandler(hap.Service.Lightbulb) .addExpectedCharacteristic('state', hap.Characteristic.On, true) @@ -491,6 +493,8 @@ describe('Light', () => { // Check service creation const newHarness = new ServiceHandlersTestHarness(); + newHarness.addConverterConfiguration('light', { adaptive_lighting: false }); + newHarness.numberOfExpectedControllers = 0; newHarness.addExperimentalFeatureFlags(EXP_COLOR_MODE); const lightbulb = newHarness .getOrAddHandler(hap.Service.Lightbulb) @@ -672,6 +676,7 @@ describe('Light', () => { // Check service creation const newHarness = new ServiceHandlersTestHarness(); + newHarness.numberOfExpectedControllers = 1; const lightbulb = newHarness .getOrAddHandler(hap.Service.Lightbulb) .addExpectedCharacteristic('state', hap.Characteristic.On, true) @@ -799,6 +804,7 @@ describe('Light', () => { harness.checkSetDataQueued({ color: { hue: 300, saturation: 100 } }); }); }); + describe('Namron Zigbee Dimmer (Adaptive Lighting ignored)', () => { // Shared "state" let deviceExposes: ExposesEntry[] = []; @@ -857,7 +863,7 @@ describe('Light', () => { }); }); - describe('Innr RB-249-T (Adaptive Lighting turned on)', () => { + describe('Innr RB-249-T', () => { // Shared "state" let deviceExposes: ExposesEntry[] = []; let harness: ServiceHandlersTestHarness; @@ -871,7 +877,6 @@ describe('Light', () => { // Check service creation const newHarness = new ServiceHandlersTestHarness(); - newHarness.addConverterConfiguration('light', { adaptive_lighting: true }); newHarness.numberOfExpectedControllers = 1; const lightbulb = newHarness .getOrAddHandler(hap.Service.Lightbulb) diff --git a/test/testHelpers.ts b/test/testHelpers.ts index 8c2a1e5d..72f9211e 100644 --- a/test/testHelpers.ts +++ b/test/testHelpers.ts @@ -213,7 +213,7 @@ class ServiceHandlerTestData implements ServiceHandlerContainer { checkCharacteristicPropertiesHaveBeenSet(identifier: string, props: Partial): ServiceHandlerContainer { const mock = this.getCharacteristicMock(identifier); - expect(mock.setProps).toBeCalledTimes(1).toBeCalledWith(props); + expect(mock.setProps).toHaveBeenCalledTimes(1).toHaveBeenCalledWith(props); return this; } @@ -247,10 +247,10 @@ class ServiceHandlerTestData implements ServiceHandlerContainer { checkCharacteristicUpdates( expectedUpdates: Map | string, CharacteristicValue> ): ServiceHandlerContainer { - expect(this.serviceMock.updateCharacteristic).toBeCalledTimes(expectedUpdates.size); + expect(this.serviceMock.updateCharacteristic).toHaveBeenCalledTimes(expectedUpdates.size); for (const [characteristic, value] of expectedUpdates) { - expect(this.serviceMock.updateCharacteristic).toBeCalledWith(characteristic, value); + expect(this.serviceMock.updateCharacteristic).toHaveBeenCalledWith(characteristic, value); } return this; } @@ -262,13 +262,13 @@ class ServiceHandlerTestData implements ServiceHandlerContainer { checkCharacteristicUpdateValues(expectedUpdates: Map): ServiceHandlerContainer { for (const [identifier, value] of expectedUpdates) { const mock = this.getCharacteristicMock(identifier); - expect(mock.updateValue).toBeCalledTimes(1).toBeCalledWith(value); + expect(mock.updateValue).toHaveBeenCalledTimes(1).toHaveBeenCalledWith(value); } return this; } checkNoCharacteristicUpdates(): ServiceHandlerContainer { - expect(this.serviceMock.updateCharacteristic).not.toBeCalled(); + expect(this.serviceMock.updateCharacteristic).not.toHaveBeenCalled(); return this; } @@ -282,7 +282,7 @@ class ServiceHandlerTestData implements ServiceHandlerContainer { const callbackMock = jest.fn(); mapping.setFunction(setValue, callbackMock); - expect(callbackMock).toBeCalledTimes(1).toBeCalledWith(null); + expect(callbackMock).toHaveBeenCalledTimes(1).toHaveBeenCalledWith(null); return this; } @@ -456,7 +456,7 @@ export class ServiceHandlersTestHarness { let expectedCallsToGetOrAddService = 0; let expectedCallsToRegisterServiceHandler = 0; - expect(this.accessoryMock.configureController).toBeCalledTimes(this.numberOfExpectedControllers); + expect(this.accessoryMock.configureController).toHaveBeenCalledTimes(this.numberOfExpectedControllers); for (const handler of this.handlers.values()) { expect(this.accessoryMock.isServiceHandlerIdKnown).toHaveBeenCalledWith(handler.serviceIdentifier); @@ -470,9 +470,9 @@ export class ServiceHandlersTestHarness { } } - expect(handler.serviceMock.getCharacteristic).toBeCalledTimes(characteristicCount); + expect(handler.serviceMock.getCharacteristic).toHaveBeenCalledTimes(characteristicCount); - expect(handler.serviceMock.addCharacteristic).toBeCalledTimes(characteristicCount); + expect(handler.serviceMock.addCharacteristic).toHaveBeenCalledTimes(characteristicCount); ++expectedCallsToRegisterServiceHandler; expect(this.accessoryMock.registerServiceHandler.mock.calls.length).toBeGreaterThanOrEqual(expectedCallsToRegisterServiceHandler); @@ -487,9 +487,9 @@ export class ServiceHandlersTestHarness { private checkCharacteristicExpectations(handler: ServiceHandlerTestData) { for (const mapping of handler.characteristics.values()) { if (mapping.characteristic !== undefined) { - expect(handler.serviceMock.getCharacteristic).toBeCalledWith(mapping.characteristic); + expect(handler.serviceMock.getCharacteristic).toHaveBeenCalledWith(mapping.characteristic); - expect(handler.serviceMock.addCharacteristic).toBeCalledWith(mapping.characteristic); + expect(handler.serviceMock.addCharacteristic).toHaveBeenCalledWith(mapping.characteristic); if (mapping.doExpectSet && mapping.mock !== undefined) { expect(mapping.mock.on).toHaveBeenCalledTimes(1).toHaveBeenCalledWith(CharacteristicEventTypes.SET, expect.anything()); @@ -578,19 +578,19 @@ export class ServiceHandlersTestHarness { } checkSetDataQueued(expectedData: unknown) { - expect(this.accessoryMock.queueDataForSetAction).toBeCalledTimes(1).toBeCalledWith(expectedData); + expect(this.accessoryMock.queueDataForSetAction).toHaveBeenCalledTimes(1).toHaveBeenCalledWith(expectedData); } checkNoSetDataQueued() { - expect(this.accessoryMock.queueDataForSetAction).not.toBeCalled(); + expect(this.accessoryMock.queueDataForSetAction).not.toHaveBeenCalled(); } checkGetKeysQueued(expectedKeys: string | string[]) { - expect(this.accessoryMock.queueKeyForGetAction).toBeCalledTimes(1).toBeCalledWith(expectedKeys); + expect(this.accessoryMock.queueKeyForGetAction).toHaveBeenCalledTimes(1).toHaveBeenCalledWith(expectedKeys); } checkNoGetKeysQueued() { - expect(this.accessoryMock.queueKeyForGetAction).not.toBeCalled(); + expect(this.accessoryMock.queueKeyForGetAction).not.toHaveBeenCalled(); } clearMocks(): void { From 9ca926a200dd337d85942f8909bfc948226c6399 Mon Sep 17 00:00:00 2001 From: Arno Moonen Date: Thu, 23 May 2024 22:46:20 +0200 Subject: [PATCH 2/3] Fix mistake that changed the default light config instead of cloning it --- src/converters/light.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/converters/light.ts b/src/converters/light.ts index 3d52ed74..506cd741 100644 --- a/src/converters/light.ts +++ b/src/converters/light.ts @@ -78,7 +78,7 @@ export class LightCreator implements ServiceCreator { private createService(expose: ExposesEntryWithFeatures, accessory: BasicAccessory): void { const converterConfig = accessory.getConverterConfiguration(LightCreator.CONFIG_TAG); - let adaptiveLightingConfig: AdaptiveLightingConfig = LightCreator.DEFAULT_CONFIG; + let adaptiveLightingConfig: AdaptiveLightingConfig = { ...LightCreator.DEFAULT_CONFIG }; if (isLightConfig(converterConfig)) { if (isAdaptiveLightingConfig(converterConfig.adaptive_lighting)) { adaptiveLightingConfig = converterConfig.adaptive_lighting; From 1da761117f0d447050aa85310f1d277e6866baf4 Mon Sep 17 00:00:00 2001 From: Arno Moonen Date: Thu, 23 May 2024 22:49:10 +0200 Subject: [PATCH 3/3] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2c7fc4a..382fa285 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ Since version 1.0.0, we try to follow the [Semantic Versioning](https://semver.o ## [Unreleased] +### Changed + +- Adaptive Lighting: + - Enabled by default for lights that support it (can still be disabled). + - Internal color temperature reference is reset when brightness or state is changed via HomeKit. + ## [1.11.0-beta.6] - 2024-06-30 ### Changed