From 3bd61280cbd6cb6b0806efab3623ae8d36e271f1 Mon Sep 17 00:00:00 2001 From: Pierre-Gilles Leymarie Date: Thu, 9 May 2024 17:10:04 +0200 Subject: [PATCH] Zigbee2mqtt : Keep keep_history when updating feature + fix update from devices with duplicate features (#2072) --- .../zigbee2mqtt/lib/getDiscoveredDevices.js | 26 +++++------ server/test/utils/device.test.js | 44 ++++++++++++++++++- server/utils/device.js | 14 +++--- 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/server/services/zigbee2mqtt/lib/getDiscoveredDevices.js b/server/services/zigbee2mqtt/lib/getDiscoveredDevices.js index 9906f80ab5..62ed9387c8 100644 --- a/server/services/zigbee2mqtt/lib/getDiscoveredDevices.js +++ b/server/services/zigbee2mqtt/lib/getDiscoveredDevices.js @@ -14,6 +14,19 @@ function getDiscoveredDevices(filters = {}) { .filter((d) => d.definition !== null) // Convert to Gladys device .map((d) => convertDevice(d, this.serviceId)) + .map((d) => { + // Remove features with duplicate external_id + // This code is needed because AQARA motion sensor + // Returns 2 illuminance features and it doesn't work with Gladys + d.features = d.features.reduce((acc, current) => { + const isDuplicate = acc.some((feature) => feature.external_id === current.external_id); + if (!isDuplicate) { + acc.push(current); + } + return acc; + }, []); + return d; + }) .map((d) => { const existingDevice = this.gladys.stateManager.get('deviceByExternalId', d.external_id); // Merge with existing device. @@ -25,19 +38,6 @@ function getDiscoveredDevices(filters = {}) { devices = devices.filter((device) => device.id === undefined || device.updatable); } - devices.forEach((device) => { - // Remove features with duplicate external_id - // This code is needed because AQARA motion sensor - // Returns 2 illuminance features and it doesn't work with Gladys - device.features = device.features.reduce((acc, current) => { - const isDuplicate = acc.some((feature) => feature.external_id === current.external_id); - if (!isDuplicate) { - acc.push(current); - } - return acc; - }, []); - }); - return devices; } diff --git a/server/test/utils/device.test.js b/server/test/utils/device.test.js index 10fcf7e152..525a945bd1 100644 --- a/server/test/utils/device.test.js +++ b/server/test/utils/device.test.js @@ -253,9 +253,49 @@ describe('mergeFeature', () => { const oldFeature = buildFeature('old'); const mergedFeature = mergeFeatures(newFeature, oldFeature); - const expectedFeature = { ...newFeature, name: 'feature-old-name' }; + const expectedFeature = { ...newFeature, name: 'feature-old-name', keep_history: 'feature-old-keep_history' }; expect(mergedFeature).to.deep.equal(expectedFeature); }); + it('should keep keep_history from old feature', () => { + const newFeature = { + name: 'Temp sensor', + category: 'humidity-sensor', + type: 'decimal', + external_id: 'second-humidity', + selector: 'second-humidity', + read_only: true, + keep_history: true, + has_feedback: false, + min: -50, + max: 150, + }; + const oldFeature = { + name: 'My custom name', + category: 'humidity-sensor', + type: 'decimal', + external_id: 'second-humidity', + selector: 'second-humidity', + read_only: true, + keep_history: false, + has_feedback: false, + min: -50, + max: 100, + }; + const mergedFeature = mergeFeatures(newFeature, oldFeature); + + expect(mergedFeature).to.deep.equal({ + name: 'My custom name', + category: 'humidity-sensor', + type: 'decimal', + external_id: 'second-humidity', + selector: 'second-humidity', + read_only: true, + keep_history: false, + has_feedback: false, + min: -50, + max: 150, + }); + }); }); describe('mergeDevice', () => { @@ -307,7 +347,7 @@ describe('mergeDevice', () => { const mergedDevice = mergeDevices(newDevice, oldDevice); - const expectedFeature = { ...newFeature, name: 'feature-old1-name' }; + const expectedFeature = { ...newFeature, name: 'feature-old1-name', keep_history: 'feature-old1-keep_history' }; const expectedDevice = { ...newDevice, features: [expectedFeature], updatable: true }; expect(mergedDevice).to.deep.equal(expectedDevice); }); diff --git a/server/utils/device.js b/server/utils/device.js index b37facfff8..ac5d5c37b1 100644 --- a/server/utils/device.js +++ b/server/utils/device.js @@ -135,7 +135,7 @@ function hasDeviceChanged(newDevice, existingDevice = {}) { /** * @description Merge feature attributes from existing with the new one. - * It keeps 'name' attribute from existing. + * It keeps 'name' and 'keep_history' attribute from existing. * @param {object} newFeature - Newly created feature. * @param {object} existingFeature - Already existing feature. * @returns {object} A new feature merged with existing one. @@ -143,13 +143,17 @@ function hasDeviceChanged(newDevice, existingDevice = {}) { * mergeFeatures({ name: 'Default name' }, { name: 'Overriden name' }) */ function mergeFeatures(newFeature, existingFeature = {}) { - const { name } = existingFeature || {}; + const featureToReturn = { ...newFeature }; - if (name) { - return { ...newFeature, name }; + if (existingFeature && existingFeature.name) { + featureToReturn.name = existingFeature.name; } - return newFeature; + if (existingFeature && existingFeature.keep_history !== undefined) { + featureToReturn.keep_history = existingFeature.keep_history; + } + + return featureToReturn; } /**