Zigbee2mqtt : Keep keep_history when updating feature + fix update fr…

…om devices with duplicate features (#2072)

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;
 }
 

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);
   });

server/utils/device.js

@@ -135,21 +135,25 @@ 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.
  * @example
  * 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;
 }
 
 /**