From cbbaf51e7590d4d7f786393fd2fdeefea1ef0a90 Mon Sep 17 00:00:00 2001 From: Paul Bottein Date: Thu, 27 Aug 2026 16:14:47 +0200 Subject: [PATCH] Keep the automation config when replacing a device in device automations (#53805) --- .../device/ha-device-automation-picker.ts | 33 ++-- src/components/device/ha-device-picker.ts | 23 ++- src/data/device/device_automation.ts | 73 +++++-- .../types/ha-automation-action-device_id.ts | 43 +++- .../types/ha-automation-condition-device.ts | 43 +++- .../target/ha-automation-row-targets.ts | 7 +- .../types/ha-automation-trigger-device.ts | 43 +++- test/data/device_automation.test.ts | 187 ++++++++++++++++++ 8 files changed, 411 insertions(+), 41 deletions(-) create mode 100644 test/data/device_automation.test.ts diff --git a/src/components/device/ha-device-automation-picker.ts b/src/components/device/ha-device-automation-picker.ts index 02862788a5..6e06e0e343 100644 --- a/src/components/device/ha-device-automation-picker.ts +++ b/src/components/device/ha-device-automation-picker.ts @@ -12,7 +12,6 @@ import { fullEntitiesContext } from "../../data/context"; import type { DeviceAutomation } from "../../data/device/device_automation"; import { deviceAutomationsEqual, - deviceAutomationsSimilar, sortDeviceAutomations, } from "../../data/device/device_automation"; import type { EntityRegistryEntry } from "../../data/entity/entity_registry"; @@ -180,12 +179,15 @@ export abstract class HaDeviceAutomationPicker< (a, idx) => value === `${a.device_id}_${idx}` ); - const text = automation + const described = + automation ?? (this.value?.domain ? this.value : undefined); + + const text = described ? this._localizeDeviceAutomation( this.hass.localize, this.hass.states, this._entityReg, - automation + described ) : value === NO_AUTOMATION_KEY ? this.NO_AUTOMATION_TEXT @@ -195,29 +197,24 @@ export abstract class HaDeviceAutomationPicker< }; private async _updateDeviceInfo() { + // Asking a removed device for its automations fails rather than returning + // an empty list. this._automations = this.deviceId ? ( - await this._fetchDeviceAutomations(this.hass.callWS, this.deviceId) + await this._fetchDeviceAutomations( + this.hass.callWS, + this.deviceId + ).catch(() => [] as T[]) ).sort(sortDeviceAutomations) : // No device, clear the list of automations []; - // If there is no value, or if we have changed the device ID, reset the - // value. When the device changed (for example after replacing a removed - // device), try to keep the same automation type/subtype on the new device - // before falling back to the first available automation. + // If there is no value, or if we have changed the device ID, reset the value. if (!this.value || this.value.device_id !== this.deviceId) { - const equivalent = - this.value && this.deviceId - ? this._automations.find((automation) => - deviceAutomationsSimilar(automation, this.value!) - ) - : undefined; this._setValue( - equivalent || - (this._automations.length - ? this._automations[0] - : this._createNoAutomation(this.deviceId)) + this._automations.length + ? this._automations[0] + : this._createNoAutomation(this.deviceId) ); } this._renderEmpty = true; diff --git a/src/components/device/ha-device-picker.ts b/src/components/device/ha-device-picker.ts index 84fe6bdf69..c9c98643c8 100644 --- a/src/components/device/ha-device-picker.ts +++ b/src/components/device/ha-device-picker.ts @@ -105,6 +105,14 @@ export class HaDevicePicker extends LitElement { @property({ attribute: "hide-clear-icon", type: Boolean }) public hideClearIcon = false; + /** + * The split devices that can actually replace the current value, when the + * caller knows better than this picker. Narrows the replacement candidates, + * so the user is only asked to choose when there is a real choice. + */ + @property({ attribute: false }) + public replacementDeviceIds?: string[]; + @query("ha-generic-picker") private _picker?: HaGenericPicker; @state() private _configEntryLookup: Record = {}; @@ -188,7 +196,8 @@ export class HaDevicePicker extends LitElement { value: string | undefined, _devices: HomeAssistant["devices"], compositeSplits: DeviceCompositeSplits | undefined, - items: (DevicePickerItem | string)[] + items: (DevicePickerItem | string)[], + replacementDeviceIds: string[] | undefined ) => { if (!value || !compositeSplits || this.hass.devices[value]) { return undefined; @@ -204,7 +213,11 @@ export class HaDevicePicker extends LitElement { .filter((item): item is DevicePickerItem => typeof item !== "string") .map((item) => item.id) ); - const candidates = split.split_ids.filter((id) => selectableIds.has(id)); + const candidates = split.split_ids.filter( + (id) => + selectableIds.has(id) && + (!replacementDeviceIds || replacementDeviceIds.includes(id)) + ); return { candidates, primaryId: split.primary_id }; } ); @@ -400,7 +413,8 @@ export class HaDevicePicker extends LitElement { this.value, this.hass.devices, this._compositeSplits, - this._getItems() + this._getItems(), + this.replacementDeviceIds ) : undefined; @@ -507,7 +521,8 @@ export class HaDevicePicker extends LitElement { this.value, this.hass.devices, this._compositeSplits, - this._getItems() + this._getItems(), + this.replacementDeviceIds ); if (!replacement?.candidates.length) { return; diff --git a/src/data/device/device_automation.ts b/src/data/device/device_automation.ts index 6340160491..37fc41a52e 100644 --- a/src/data/device/device_automation.ts +++ b/src/data/device/device_automation.ts @@ -182,26 +182,69 @@ export const deviceAutomationEditorMode = ( : "unknown-device"; }; -// Like deviceAutomationsEqual, but ignores device_id and entity_id so an -// automation can be matched to the equivalent one on a different device (for -// example when a referenced device was replaced by a split device). -export const deviceAutomationsSimilar = ( +const deviceAutomationsSameType = (a: DeviceAutomation, b: DeviceAutomation) => + deviceAutomationIdentifiers + .filter((property) => property !== "device_id" && property !== "entity_id") + .every((property) => Object.is(a[property], b[property])); + +// An entity can be referenced by its registry id or by its entity id, and the +// two sides do not have to agree. +const deviceAutomationsSameEntity = ( + entityRegistry: EntityRegistryEntry[], a: DeviceAutomation, b: DeviceAutomation ) => { - if (typeof a !== typeof b) { + if (!a.entity_id && !b.entity_id) { + return true; + } + if (!a.entity_id || !b.entity_id) { return false; } - return deviceAutomationIdentifiers - .filter((property) => property !== "device_id" && property !== "entity_id") - .every((property) => { - const inA = property in a; - const inB = property in b; - if (!inA && !inB) { - return true; - } - return Object.is(a[property], b[property]); - }); + return ( + a.entity_id === b.entity_id || + compareEntityIdWithEntityRegId(entityRegistry, a.entity_id, b.entity_id) + ); +}; + +// A device exposes the same automation type once per entity, so the same type +// on another entity is a different automation, not an equivalent one. +export const findEquivalentDeviceAutomation = ( + entityRegistry: EntityRegistryEntry[], + automations: T[], + automation: DeviceAutomation +): T | undefined => + automations.find( + (candidate) => + deviceAutomationsSameType(candidate, automation) && + deviceAutomationsSameEntity(entityRegistry, candidate, automation) + ); + +// Among the split devices that replaced a removed device, the ones that offer +// the given automation. Nothing in the registry says which of them took it over, +// so each candidate has to be asked. +export const fetchReplacementDevices = async ( + hass: HomeAssistant, + entityRegistry: EntityRegistryEntry[], + automation: DeviceAutomation, + compositeSplits: DeviceCompositeSplits, + fetchDeviceAutomations: (callWS: CallWS, deviceId: string) => Promise +): Promise => { + const candidates = + compositeSplits[automation.device_id]?.split_ids.filter( + (id) => id in hass.devices + ) ?? []; + const automationsPerCandidate = await Promise.all( + candidates.map((id) => + fetchDeviceAutomations(hass.callWS, id).catch(() => [] as T[]) + ) + ); + return candidates.filter((_id, index) => + findEquivalentDeviceAutomation( + entityRegistry, + automationsPerCandidate[index], + automation + ) + ); }; const compareEntityIdWithEntityRegId = ( diff --git a/src/panels/config/automation/action/types/ha-automation-action-device_id.ts b/src/panels/config/automation/action/types/ha-automation-action-device_id.ts index 2343f27905..498b708a23 100644 --- a/src/panels/config/automation/action/types/ha-automation-action-device_id.ts +++ b/src/panels/config/automation/action/types/ha-automation-action-device_id.ts @@ -14,6 +14,8 @@ import type { } from "../../../../../data/device/device_automation"; import { deviceAutomationEditorMode, + fetchReplacementDevices, + fetchDeviceActions, deviceAutomationsEqual, fetchDeviceActionCapabilities, localizeExtraFieldsComputeHelperCallback, @@ -40,6 +42,8 @@ export class HaDeviceAction extends LitElement { @state() private _compositeSplits?: DeviceCompositeSplits; + @state() private _replacementDeviceIds?: string[]; + private _loadingCompositeSplits = false; @state() @@ -95,13 +99,27 @@ export class HaDeviceAction extends LitElement { return true; } + private async _resolveReplacements(compositeSplits: DeviceCompositeSplits) { + this._replacementDeviceIds = await fetchReplacementDevices( + this.hass, + this._entityReg, + this.action, + compositeSplits, + fetchDeviceActions + ); + } + private async _loadCompositeSplits() { if (this._loadingCompositeSplits) { return; } this._loadingCompositeSplits = true; try { - this._compositeSplits = await fetchDeviceCompositeSplits(this.hass); + // Resolve the candidates before exposing the split map, so the picker + // never offers one that cannot host the automation. + const compositeSplits = await fetchDeviceCompositeSplits(this.hass); + await this._resolveReplacements(compositeSplits); + this._compositeSplits = compositeSplits; } catch (_err) { this._compositeSplits = {}; } finally { @@ -115,6 +133,7 @@ export class HaDeviceAction extends LitElement { return html` ) { + // The picked device only lives here until the configuration catches up. + // Once it points somewhere else, undo and redo included, it is stale. + const previous = changedProps.get("action"); + if (previous && previous.device_id !== this.action.device_id) { + this._deviceId = undefined; + this._replacementDeviceIds = undefined; + if (this._compositeSplits) { + this._resolveReplacements(this._compositeSplits); + } + } + } + protected firstUpdated() { this.hass.loadBackendTranslation("device_automation"); if (!this._capabilities) { @@ -185,6 +217,15 @@ export class HaDeviceAction extends LitElement { private _devicePicked(ev) { ev.stopPropagation(); + // The automation exists as is on the replacement, so only the reference + // changes and the rest of the configuration is left untouched. + if (this._replacementDeviceIds?.includes(ev.target.value)) { + this._deviceId = undefined; + fireEvent(this, "value-changed", { + value: { ...this.action, device_id: ev.target.value }, + }); + return; + } this._deviceId = ev.target.value; if (this._deviceId === undefined) { fireEvent(this, "value-changed", { diff --git a/src/panels/config/automation/condition/types/ha-automation-condition-device.ts b/src/panels/config/automation/condition/types/ha-automation-condition-device.ts index 7f78b569a3..ffc4bd4349 100644 --- a/src/panels/config/automation/condition/types/ha-automation-condition-device.ts +++ b/src/panels/config/automation/condition/types/ha-automation-condition-device.ts @@ -14,6 +14,8 @@ import type { } from "../../../../../data/device/device_automation"; import { deviceAutomationEditorMode, + fetchReplacementDevices, + fetchDeviceConditions, deviceAutomationsEqual, fetchDeviceConditionCapabilities, localizeExtraFieldsComputeHelperCallback, @@ -40,6 +42,8 @@ export class HaDeviceCondition extends LitElement { @state() private _compositeSplits?: DeviceCompositeSplits; + @state() private _replacementDeviceIds?: string[]; + private _loadingCompositeSplits = false; @state() @@ -96,13 +100,27 @@ export class HaDeviceCondition extends LitElement { return true; } + private async _resolveReplacements(compositeSplits: DeviceCompositeSplits) { + this._replacementDeviceIds = await fetchReplacementDevices( + this.hass, + this._entityReg, + this.condition, + compositeSplits, + fetchDeviceConditions + ); + } + private async _loadCompositeSplits() { if (this._loadingCompositeSplits) { return; } this._loadingCompositeSplits = true; try { - this._compositeSplits = await fetchDeviceCompositeSplits(this.hass); + // Resolve the candidates before exposing the split map, so the picker + // never offers one that cannot host the automation. + const compositeSplits = await fetchDeviceCompositeSplits(this.hass); + await this._resolveReplacements(compositeSplits); + this._compositeSplits = compositeSplits; } catch (_err) { this._compositeSplits = {}; } finally { @@ -116,6 +134,7 @@ export class HaDeviceCondition extends LitElement { return html` ) { + // The picked device only lives here until the configuration catches up. + // Once it points somewhere else, undo and redo included, it is stale. + const previous = changedProps.get("condition"); + if (previous && previous.device_id !== this.condition.device_id) { + this._deviceId = undefined; + this._replacementDeviceIds = undefined; + if (this._compositeSplits) { + this._resolveReplacements(this._compositeSplits); + } + } + } + protected firstUpdated() { this.hass.loadBackendTranslation("device_automation"); if (!this._capabilities) { @@ -187,6 +219,15 @@ export class HaDeviceCondition extends LitElement { private _devicePicked(ev) { ev.stopPropagation(); + // The automation exists as is on the replacement, so only the reference + // changes and the rest of the configuration is left untouched. + if (this._replacementDeviceIds?.includes(ev.target.value)) { + this._deviceId = undefined; + fireEvent(this, "value-changed", { + value: { ...this.condition, device_id: ev.target.value }, + }); + return; + } this._deviceId = ev.target.value; if (this._deviceId === undefined) { fireEvent(this, "value-changed", { diff --git a/src/panels/config/automation/target/ha-automation-row-targets.ts b/src/panels/config/automation/target/ha-automation-row-targets.ts index ccdf223fc0..b1ed2f183c 100644 --- a/src/panels/config/automation/target/ha-automation-row-targets.ts +++ b/src/panels/config/automation/target/ha-automation-row-targets.ts @@ -479,7 +479,12 @@ export class HaAutomationRowTargets extends LitElement { warning = true; badgeTargetId = undefined; badgeTargetType = undefined; - if (targetType === "device" && this._compositeSplits?.[targetId]) { + if ( + targetType === "device" && + this._compositeSplits?.[targetId]?.split_ids.some( + (id) => id in this._registries.devices + ) + ) { // The device was replaced by one or more split devices; make clear // this reference needs to be updated, distinct from "unknown device". icon = mdiSwapHorizontal; diff --git a/src/panels/config/automation/trigger/types/ha-automation-trigger-device.ts b/src/panels/config/automation/trigger/types/ha-automation-trigger-device.ts index c62dae26f1..012885423b 100644 --- a/src/panels/config/automation/trigger/types/ha-automation-trigger-device.ts +++ b/src/panels/config/automation/trigger/types/ha-automation-trigger-device.ts @@ -16,6 +16,8 @@ import type { } from "../../../../../data/device/device_automation"; import { deviceAutomationEditorMode, + fetchReplacementDevices, + fetchDeviceTriggers, deviceAutomationsEqual, fetchDeviceTriggerCapabilities, localizeExtraFieldsComputeHelperCallback, @@ -42,6 +44,8 @@ export class HaDeviceTrigger extends LitElement { @state() private _compositeSplits?: DeviceCompositeSplits; + @state() private _replacementDeviceIds?: string[]; + private _loadingCompositeSplits = false; @state() @@ -100,13 +104,27 @@ export class HaDeviceTrigger extends LitElement { return true; } + private async _resolveReplacements(compositeSplits: DeviceCompositeSplits) { + this._replacementDeviceIds = await fetchReplacementDevices( + this.hass, + this._entityReg, + this.trigger, + compositeSplits, + fetchDeviceTriggers + ); + } + private async _loadCompositeSplits() { if (this._loadingCompositeSplits) { return; } this._loadingCompositeSplits = true; try { - this._compositeSplits = await fetchDeviceCompositeSplits(this.hass); + // Resolve the candidates before exposing the split map, so the picker + // never offers one that cannot host the automation. + const compositeSplits = await fetchDeviceCompositeSplits(this.hass); + await this._resolveReplacements(compositeSplits); + this._compositeSplits = compositeSplits; } catch (_err) { this._compositeSplits = {}; } finally { @@ -120,6 +138,7 @@ export class HaDeviceTrigger extends LitElement { return html` ) { + // The picked device only lives here until the configuration catches up. + // Once it points somewhere else, undo and redo included, it is stale. + const previous = changedProps.get("trigger"); + if (previous && previous.device_id !== this.trigger.device_id) { + this._deviceId = undefined; + this._replacementDeviceIds = undefined; + if (this._compositeSplits) { + this._resolveReplacements(this._compositeSplits); + } + } + } + protected firstUpdated() { this.hass.loadBackendTranslation("device_automation"); if (!this._capabilities) { @@ -208,6 +240,15 @@ export class HaDeviceTrigger extends LitElement { private _devicePicked(ev) { ev.stopPropagation(); + // The automation exists as is on the replacement, so only the reference + // changes and the rest of the configuration is left untouched. + if (this._replacementDeviceIds?.includes(ev.target.value)) { + this._deviceId = undefined; + fireEvent(this, "value-changed", { + value: { ...this.trigger, device_id: ev.target.value }, + }); + return; + } this._deviceId = ev.target.value; if (this._deviceId === undefined) { fireEvent(this, "value-changed", { diff --git a/test/data/device_automation.test.ts b/test/data/device_automation.test.ts new file mode 100644 index 0000000000..2978aef964 --- /dev/null +++ b/test/data/device_automation.test.ts @@ -0,0 +1,187 @@ +import { describe, expect, it } from "vitest"; +import type { DeviceTrigger } from "../../src/data/device/device_automation"; +import { + fetchReplacementDevices, + findEquivalentDeviceAutomation, +} from "../../src/data/device/device_automation"; +import type { DeviceCompositeSplits } from "../../src/data/device/device_registry"; +import type { EntityRegistryEntry } from "../../src/data/entity/entity_registry"; +import type { HomeAssistant } from "../../src/types"; + +const entityRegistry = [ + { id: "regid1", entity_id: "binary_sensor.one" }, + { id: "regid2", entity_id: "binary_sensor.two" }, +] as EntityRegistryEntry[]; + +const trigger = (partial: Partial): DeviceTrigger => + ({ + trigger: "device", + domain: "binary_sensor", + device_id: "device1", + ...partial, + }) as DeviceTrigger; + +describe("findEquivalentDeviceAutomation", () => { + it("picks the automation on the same entity among several of the same type", () => { + const automations = [ + trigger({ device_id: "device2", type: "turned_on", entity_id: "regid1" }), + trigger({ device_id: "device2", type: "turned_on", entity_id: "regid2" }), + ]; + + expect( + findEquivalentDeviceAutomation( + entityRegistry, + automations, + trigger({ type: "turned_on", entity_id: "regid2" }) + ) + ).toBe(automations[1]); + }); + + it("matches an entity referenced by entity id against one referenced by registry id", () => { + const automations = [ + trigger({ device_id: "device2", type: "turned_on", entity_id: "regid1" }), + trigger({ device_id: "device2", type: "turned_on", entity_id: "regid2" }), + ]; + + expect( + findEquivalentDeviceAutomation( + entityRegistry, + automations, + trigger({ type: "turned_on", entity_id: "binary_sensor.two" }) + ) + ).toBe(automations[1]); + }); + + it("returns undefined when the same type is only offered for another entity", () => { + const automations = [ + trigger({ device_id: "device2", type: "turned_on", entity_id: "regid1" }), + trigger({ + device_id: "device2", + type: "turned_off", + entity_id: "regid1", + }), + ]; + + expect( + findEquivalentDeviceAutomation( + entityRegistry, + automations, + trigger({ type: "turned_on", entity_id: "regid2" }) + ) + ).toBeUndefined(); + }); + + it("matches entity-less automations on their subtype", () => { + const automations = [ + trigger({ + device_id: "device2", + domain: "zha", + type: "remote_button_short_press", + subtype: "button_1", + }), + trigger({ + device_id: "device2", + domain: "zha", + type: "remote_button_short_press", + subtype: "button_2", + }), + ]; + + expect( + findEquivalentDeviceAutomation( + entityRegistry, + automations, + trigger({ + domain: "zha", + type: "remote_button_short_press", + subtype: "button_2", + }) + ) + ).toBe(automations[1]); + }); + + it("returns undefined when the device offers no automation of that type", () => { + const automations = [ + trigger({ + device_id: "device2", + type: "turned_off", + entity_id: "regid1", + }), + ]; + + expect( + findEquivalentDeviceAutomation( + entityRegistry, + automations, + trigger({ type: "turned_on", entity_id: "regid1" }) + ) + ).toBeUndefined(); + }); +}); + +describe("fetchReplacementDevices", () => { + const hass = { + callWS: () => Promise.resolve([]), + devices: { device2: {}, device3: {} }, + } as unknown as HomeAssistant; + + const compositeSplits = { + removed: { split_ids: ["device2", "device3"], primary_id: "device2" }, + } as unknown as DeviceCompositeSplits; + + const value = trigger({ + device_id: "removed", + type: "turned_on", + entity_id: "regid1", + }); + + it("keeps only the devices that offer the automation", async () => { + const offers = { + device2: [ + trigger({ + device_id: "device2", + type: "turned_on", + entity_id: "regid1", + }), + ], + device3: [ + trigger({ + device_id: "device3", + type: "turned_on", + entity_id: "regid2", + }), + ], + }; + + expect( + await fetchReplacementDevices( + hass, + entityRegistry, + value, + compositeSplits, + (_callWS, deviceId) => Promise.resolve(offers[deviceId]) + ) + ).toEqual(["device2"]); + }); + + it("drops a device whose automations cannot be listed", async () => { + expect( + await fetchReplacementDevices( + hass, + entityRegistry, + value, + compositeSplits, + (_callWS, deviceId) => + deviceId === "device2" + ? Promise.reject(new Error("unknown device")) + : Promise.resolve([ + trigger({ + device_id: "device3", + type: "turned_on", + entity_id: "regid1", + }), + ]) + ) + ).toEqual(["device3"]); + }); +});