Skip to content

Commit

Permalink
fix!: Home Assistant: remove duplicate sensor/select for select…
Browse files Browse the repository at this point in the history 
…/`number`/`button` entities (#25026)

Co-authored-by: Koen Kanters <[email protected]>
  • Loading branch information
@Drafteed @Koenkk
Drafteed and Koenkk authored Dec 7, 2024
1 parent 1a9c79b commit ba45445
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 86 deletions.
155 changes: 76 additions & 79 deletions lib/extension/homeassistant.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -973,15 +973,50 @@ 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
if (firstExpose.unit && ['Wh', 'kWh'].includes(firstExpose.unit)) {
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.
Expand Down Expand Up @@ -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': {
Expand Down Expand Up @@ -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,
Expand All @@ -1117,29 +1122,24 @@ 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) {
/* istanbul ignore else */
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],
},
});
Expand All @@ -1149,37 +1149,34 @@ export default class HomeAssistant extends Extension {
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],
},
});
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/frontend.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand All @@ -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},
Expand Down
33 changes: 26 additions & 7 deletions test/extensions/homeassistant.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit ba45445

Please sign in to comment.