Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: Home Assistant: remove duplicate sensor/select for select/number/button entities #25026

Merged
merged 6 commits into from
Dec 7, 2024
Merged

fix!: Home Assistant: remove duplicate sensor/select for select/number/button entities #25026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c49b417
Remove duplicate enum/numeric sensor entities in HA
Drafteed Dec 3, 2024
e9f748d
fix coverage
Drafteed Dec 3, 2024
d293e91
Merge branch 'dev' into remove-ha-enum-sensors
Drafteed Dec 4, 2024
ffc6b3e
remove text sensors
Drafteed Dec 6, 2024
9fc7199
Merge branch 'dev' into remove-ha-enum-sensors
Drafteed Dec 6, 2024
9d5befa
Merge branch 'dev' into remove-ha-enum-sensors
Koenkk Dec 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 76 additions & 79 deletions lib/extension/homeassistant.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading