From c49b417457811e949be437e8a677bc6392bdc80a Mon Sep 17 00:00:00 2001 From: Arem Draft Date: Tue, 3 Dec 2024 05:30:24 +0300 Subject: [PATCH 1/3] Remove duplicate enum/numeric sensor entities in HA --- lib/extension/homeassistant.ts | 119 +++++++++++++------------- test/extensions/frontend.test.ts | 2 + test/extensions/homeassistant.test.ts | 33 +++++-- 3 files changed, 87 insertions(+), 67 deletions(-) diff --git a/lib/extension/homeassistant.ts b/lib/extension/homeassistant.ts index 685a9dc38b..0aef7f9f0e 100644 --- a/lib/extension/homeassistant.ts +++ b/lib/extension/homeassistant.ts @@ -973,6 +973,43 @@ export default class HomeAssistant extends Extension { } case 'numeric': { assertNumericExpose(firstExpose); + const allowsSet = firstExpose.access & ACCESS_SET; + + /** + * If numeric attribute has SET access then expose as SELECT entity. + */ + if (allowsSet) { + const discoveryEntry: DiscoveryEntry = { + type: 'number', + object_id: endpoint ? `${firstExpose.name}_${endpoint}` : `${firstExpose.name}`, + mockProperties: [{property: firstExpose.property, value: null}], + discovery_payload: { + name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, + value_template: `{{ value_json.${firstExpose.property} }}`, + command_topic: true, + command_topic_prefix: endpoint, + command_topic_postfix: firstExpose.property, + ...(firstExpose.unit && {unit_of_measurement: firstExpose.unit}), + ...(firstExpose.value_step && {step: firstExpose.value_step}), + ...NUMERIC_DISCOVERY_LOOKUP[firstExpose.name], + }, + }; + + if (NUMERIC_DISCOVERY_LOOKUP[firstExpose.name]?.device_class === 'temperature') { + discoveryEntry.discovery_payload.device_class = NUMERIC_DISCOVERY_LOOKUP[firstExpose.name]?.device_class; + } else { + delete discoveryEntry.discovery_payload.device_class; + } + + // istanbul ignore else + if (firstExpose.value_min != null) discoveryEntry.discovery_payload.min = firstExpose.value_min; + // istanbul ignore else + if (firstExpose.value_max != null) discoveryEntry.discovery_payload.max = firstExpose.value_max; + + discoveryEntries.push(discoveryEntry); + break; + } + const extraAttrs = {}; // If a variable includes Wh, mark it as energy @@ -980,8 +1017,6 @@ export default class HomeAssistant extends Extension { Object.assign(extraAttrs, {device_class: 'energy', state_class: 'total_increasing'}); } - const allowsSet = firstExpose.access & ACCESS_SET; - let key = firstExpose.name; // Home Assistant uses a different voc device_class for µg/m³ versus ppb or ppm. @@ -1016,42 +1051,6 @@ export default class HomeAssistant extends Extension { } discoveryEntries.push(discoveryEntry); - - /** - * If numeric attribute has SET access then expose as SELECT entity too. - * Note: currently both sensor and number are discovered, this is to avoid - * breaking changes for sensors already existing in HA (legacy). - */ - if (allowsSet) { - const discoveryEntry: DiscoveryEntry = { - type: 'number', - object_id: endpoint ? `${firstExpose.name}_${endpoint}` : `${firstExpose.name}`, - mockProperties: [{property: firstExpose.property, value: null}], - discovery_payload: { - name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, - value_template: `{{ value_json.${firstExpose.property} }}`, - command_topic: true, - command_topic_prefix: endpoint, - command_topic_postfix: firstExpose.property, - ...(firstExpose.unit && {unit_of_measurement: firstExpose.unit}), - ...(firstExpose.value_step && {step: firstExpose.value_step}), - ...NUMERIC_DISCOVERY_LOOKUP[firstExpose.name], - }, - }; - - if (NUMERIC_DISCOVERY_LOOKUP[firstExpose.name]?.device_class === 'temperature') { - discoveryEntry.discovery_payload.device_class = NUMERIC_DISCOVERY_LOOKUP[firstExpose.name]?.device_class; - } else { - delete discoveryEntry.discovery_payload.device_class; - } - - // istanbul ignore else - if (firstExpose.value_min != null) discoveryEntry.discovery_payload.min = firstExpose.value_min; - // istanbul ignore else - if (firstExpose.value_max != null) discoveryEntry.discovery_payload.max = firstExpose.value_max; - - discoveryEntries.push(discoveryEntry); - } break; } case 'enum': { @@ -1085,30 +1084,36 @@ export default class HomeAssistant extends Extension { } const valueTemplate = firstExpose.access & ACCESS_STATE ? `{{ value_json.${firstExpose.property} }}` : undefined; - if (firstExpose.access & ACCESS_STATE) { + + /** + * If enum has only one item and has SET access then expose as BUTTON entity. + */ + if (firstExpose.access & ACCESS_SET && firstExpose.values.length === 1) { discoveryEntries.push({ - type: 'sensor', + type: 'button', object_id: firstExpose.property, mockProperties: [{property: firstExpose.property, value: null}], discovery_payload: { - name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, - value_template: valueTemplate, - enabled_by_default: !(firstExpose.access & ACCESS_SET), + name: endpoint ? /* istanbul ignore next */ `${firstExpose.label} ${endpoint}` : firstExpose.label, + state_topic: false, + command_topic_prefix: endpoint, + command_topic: true, + command_topic_postfix: firstExpose.property, + payload_press: firstExpose.values[0].toString(), ...ENUM_DISCOVERY_LOOKUP[firstExpose.name], }, }); + break; } /** - * If enum attribute has SET access then expose as SELECT entity too. - * Note: currently both sensor and select are discovered, this is to avoid - * breaking changes for sensors already existing in HA (legacy). + * If enum attribute has SET access then expose as SELECT entity. */ if (firstExpose.access & ACCESS_SET) { discoveryEntries.push({ type: 'select', object_id: firstExpose.property, - mockProperties: [], // Already mocked above in case access STATE is supported + mockProperties: [{property: firstExpose.property, value: null}], discovery_payload: { name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, value_template: valueTemplate, @@ -1117,29 +1122,23 @@ export default class HomeAssistant extends Extension { command_topic: true, command_topic_postfix: firstExpose.property, options: firstExpose.values.map((v) => v.toString()), - enabled_by_default: firstExpose.values.length !== 1, // hide if button is exposed ...ENUM_DISCOVERY_LOOKUP[firstExpose.name], }, }); + break; } /** - * If enum has only item and only supports SET then expose as button entity. - * Note: select entity is hidden by default to avoid breaking changes - * for selects already existing in HA (legacy). + * Otherwise expose as SENSOR entity. */ - if (firstExpose.access & ACCESS_SET && firstExpose.values.length === 1) { + if (firstExpose.access & ACCESS_STATE) { discoveryEntries.push({ - type: 'button', + type: 'sensor', object_id: firstExpose.property, - mockProperties: [], + mockProperties: [{property: firstExpose.property, value: null}], discovery_payload: { - name: endpoint ? /* istanbul ignore next */ `${firstExpose.label} ${endpoint}` : firstExpose.label, - state_topic: false, - command_topic_prefix: endpoint, - command_topic: true, - command_topic_postfix: firstExpose.property, - payload_press: firstExpose.values[0].toString(), + name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, + value_template: valueTemplate, ...ENUM_DISCOVERY_LOOKUP[firstExpose.name], }, }); diff --git a/test/extensions/frontend.test.ts b/test/extensions/frontend.test.ts index fc7c634d59..4d51876fdb 100644 --- a/test/extensions/frontend.test.ts +++ b/test/extensions/frontend.test.ts @@ -239,6 +239,7 @@ describe('Extension: Frontend', () => { 'zigbee2mqtt/bulb_color', stringify({ state: 'ON', + effect: null, power_on_behavior: null, linkquality: null, update: {state: null, installed_version: -1, latest_version: -1}, @@ -261,6 +262,7 @@ describe('Extension: Frontend', () => { topic: 'bulb_color', payload: { state: 'ON', + effect: null, power_on_behavior: null, linkquality: null, update: {state: null, installed_version: -1, latest_version: -1}, diff --git a/test/extensions/homeassistant.test.ts b/test/extensions/homeassistant.test.ts index 96a1d82cf4..4c05a92627 100644 --- a/test/extensions/homeassistant.test.ts +++ b/test/extensions/homeassistant.test.ts @@ -1090,6 +1090,7 @@ describe('Extension: HomeAssistant', () => { stringify({ color: {hue: 0, saturation: 100, h: 0, s: 100}, color_mode: 'hs', + effect: null, linkquality: null, state: null, power_on_behavior: null, @@ -1112,6 +1113,7 @@ describe('Extension: HomeAssistant', () => { stringify({ color: {x: 0.4576, y: 0.41}, color_mode: 'xy', + effect: null, linkquality: null, state: null, power_on_behavior: null, @@ -1133,6 +1135,7 @@ describe('Extension: HomeAssistant', () => { 'zigbee2mqtt/bulb_color', stringify({ linkquality: null, + effect: null, state: 'ON', power_on_behavior: null, update: {state: null, installed_version: -1, latest_version: -1}, @@ -1242,6 +1245,8 @@ describe('Extension: HomeAssistant', () => { color_options: null, brightness: 50, color_temp: 370, + effect: null, + identify: null, linkquality: 99, power_on_behavior: null, update: {state: null, installed_version: -1, latest_version: -1}, @@ -1280,6 +1285,8 @@ describe('Extension: HomeAssistant', () => { color_options: null, brightness: 50, color_temp: 370, + effect: null, + identify: null, linkquality: 99, power_on_behavior: null, update: {state: null, installed_version: -1, latest_version: -1}, @@ -1735,18 +1742,31 @@ describe('Extension: HomeAssistant', () => { await flushPromises(); expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/U202DST600ZB', - stringify({state_l2: 'ON', brightness_l2: 20, linkquality: null, state_l1: null, power_on_behavior_l1: null, power_on_behavior_l2: null}), + stringify({ + state_l2: 'ON', + brightness_l2: 20, + linkquality: null, + state_l1: null, + effect_l1: null, + effect_l2: null, + power_on_behavior_l1: null, + power_on_behavior_l2: null, + }), {qos: 0, retain: false}, ); expect(mockMQTT.publishAsync).toHaveBeenCalledWith( 'zigbee2mqtt/U202DST600ZB/l2', - stringify({state: 'ON', brightness: 20, power_on_behavior: null}), + stringify({state: 'ON', brightness: 20, effect: null, power_on_behavior: null}), {qos: 0, retain: false}, ); - expect(mockMQTT.publishAsync).toHaveBeenCalledWith('zigbee2mqtt/U202DST600ZB/l1', stringify({state: null, power_on_behavior: null}), { - qos: 0, - retain: false, - }); + expect(mockMQTT.publishAsync).toHaveBeenCalledWith( + 'zigbee2mqtt/U202DST600ZB/l1', + stringify({state: null, effect: null, power_on_behavior: null}), + { + qos: 0, + retain: false, + }, + ); }); it('Shouldnt crash in onPublishEntityState on group publish', async () => { @@ -2452,7 +2472,6 @@ describe('Extension: HomeAssistant', () => { state_topic: 'zigbee2mqtt/0x18fc26000000cafe', unique_id: '0x18fc26000000cafe_device_mode_zigbee2mqtt', value_template: '{{ value_json.device_mode }}', - enabled_by_default: true, }; expect(mockMQTT.publishAsync).toHaveBeenCalledWith('homeassistant/select/0x18fc26000000cafe/device_mode/config', stringify(payload), { retain: true, From e9f748d1a9c29fd9fb812d1a79b35e265b7651e0 Mon Sep 17 00:00:00 2001 From: Arem Draft Date: Tue, 3 Dec 2024 06:26:11 +0300 Subject: [PATCH 2/3] fix coverage --- lib/extension/homeassistant.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/extension/homeassistant.ts b/lib/extension/homeassistant.ts index 0aef7f9f0e..47975edd8e 100644 --- a/lib/extension/homeassistant.ts +++ b/lib/extension/homeassistant.ts @@ -1131,18 +1131,16 @@ export default class HomeAssistant extends Extension { /** * Otherwise expose as SENSOR entity. */ - if (firstExpose.access & ACCESS_STATE) { - discoveryEntries.push({ - type: 'sensor', - object_id: firstExpose.property, - mockProperties: [{property: firstExpose.property, value: null}], - discovery_payload: { - name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, - value_template: valueTemplate, - ...ENUM_DISCOVERY_LOOKUP[firstExpose.name], - }, - }); - } + discoveryEntries.push({ + type: 'sensor', + object_id: firstExpose.property, + mockProperties: [{property: firstExpose.property, value: null}], + discovery_payload: { + name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, + value_template: valueTemplate, + ...ENUM_DISCOVERY_LOOKUP[firstExpose.name], + }, + }); break; } case 'text': From ffc6b3e575d4d337b34f8ac2481febb74f65975b Mon Sep 17 00:00:00 2001 From: Arem Draft Date: Fri, 6 Dec 2024 03:38:44 +0300 Subject: [PATCH 3/3] remove text sensors --- lib/extension/homeassistant.ts | 58 +++++++++++++++++----------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/lib/extension/homeassistant.ts b/lib/extension/homeassistant.ts index 47975edd8e..4a1c071aa8 100644 --- a/lib/extension/homeassistant.ts +++ b/lib/extension/homeassistant.ts @@ -1131,52 +1131,52 @@ export default class HomeAssistant extends Extension { /** * Otherwise expose as SENSOR entity. */ - discoveryEntries.push({ - type: 'sensor', - object_id: firstExpose.property, - mockProperties: [{property: firstExpose.property, value: null}], - discovery_payload: { - name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, - value_template: valueTemplate, - ...ENUM_DISCOVERY_LOOKUP[firstExpose.name], - }, - }); + /* istanbul ignore else */ + if (firstExpose.access & ACCESS_STATE) { + discoveryEntries.push({ + type: 'sensor', + object_id: firstExpose.property, + mockProperties: [{property: firstExpose.property, value: null}], + discovery_payload: { + name: endpoint ? `${firstExpose.label} ${endpoint}` : firstExpose.label, + value_template: valueTemplate, + ...ENUM_DISCOVERY_LOOKUP[firstExpose.name], + }, + }); + } break; } case 'text': case 'composite': case 'list': { - // legacy: remove text sensor const firstExposeTyped = firstExpose as zhc.Text | zhc.Composite | zhc.List; - const settableText = firstExposeTyped.type === 'text' && firstExposeTyped.access & ACCESS_SET; - if (firstExposeTyped.access & ACCESS_STATE) { - const discoveryEntry: DiscoveryEntry = { - type: 'sensor', + if (firstExposeTyped.type === 'text' && firstExposeTyped.access & ACCESS_SET) { + discoveryEntries.push({ + type: 'text', object_id: firstExposeTyped.property, mockProperties: [{property: firstExposeTyped.property, value: null}], discovery_payload: { name: endpoint ? `${firstExposeTyped.label} ${endpoint}` : firstExposeTyped.label, - // Truncate text if it's too long - // https://github.com/Koenkk/zigbee2mqtt/issues/23199 - value_template: `{{ value_json.${firstExposeTyped.property} | default('',True) | string | truncate(254, True, '', 0) }}`, - enabled_by_default: !settableText, + state_topic: firstExposeTyped.access & ACCESS_STATE, + value_template: `{{ value_json.${firstExposeTyped.property} }}`, + command_topic_prefix: endpoint, + command_topic: true, + command_topic_postfix: firstExposeTyped.property, ...LIST_DISCOVERY_LOOKUP[firstExposeTyped.name], }, - }; - discoveryEntries.push(discoveryEntry); + }); + break; } - if (settableText) { + if (firstExposeTyped.access & ACCESS_STATE) { discoveryEntries.push({ - type: 'text', + type: 'sensor', object_id: firstExposeTyped.property, - mockProperties: firstExposeTyped.access & ACCESS_STATE ? [{property: firstExposeTyped.property, value: null}] : [], + mockProperties: [{property: firstExposeTyped.property, value: null}], discovery_payload: { name: endpoint ? `${firstExposeTyped.label} ${endpoint}` : firstExposeTyped.label, - state_topic: firstExposeTyped.access & ACCESS_STATE, - value_template: `{{ value_json.${firstExposeTyped.property} }}`, - command_topic_prefix: endpoint, - command_topic: true, - command_topic_postfix: firstExposeTyped.property, + // Truncate text if it's too long + // https://github.com/Koenkk/zigbee2mqtt/issues/23199 + value_template: `{{ value_json.${firstExposeTyped.property} | default('',True) | string | truncate(254, True, '', 0) }}`, ...LIST_DISCOVERY_LOOKUP[firstExposeTyped.name], }, });