feat: Add configuration_url, model_id, entry_type, connections, and identifiers to device modifications#70
Conversation
…ifiers to device modifications Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/53fd009a-1a3c-487d-aa04-7ec2a3ec11ab Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
|
@copilot fix the hassfest issue |
…o satisfy hassfest translation key validation Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/0186196a-9bc7-480a-9613-6a94b0d9f0b2 Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
Fixed in 4078005. The hassfest error was that |
This happens when trying to assign to a device a bluetooth mac that is already present in the connections of another device. Shouldn't this work? I thought this would lead to home assistant natively merging the devices? Another problem is that the object selectors have this weird double dash: 
It seems to work but it does not look right? Why is that there |
… form render and config entry storage Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/3cfac61e-3865-40ed-891d-d43ba7377ac9 Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
Fixed in 1ad02de. The |
|
@copilot When a user configures If a collision is detected, the flow should return a form error instead of saving — so the user never hits the What to implementIn if CONF_CONNECTIONS in modification_data:
device_registry = dr.async_get(self.hass)
target_device_id = ... # the device being modified
new_connections = {tuple(c) for c in modification_data[CONF_CONNECTIONS]}
for connection in new_connections:
existing_device = device_registry.async_get_device(connections={connection})
if existing_device is not None and existing_device.id != target_device_id:
errors["connections"] = "connections_collision"
# Store the conflicting device name for the error message
self._connections_collision_device = existing_device.name or existing_device.id
return self.async_show_form(..., errors=errors)Error messageAdd a translation key The device name should be interpolated dynamically into the error string (or shown as a separate description placeholder if HA's translation system doesn't support dynamic error strings — in that case, store the name in |
…onfig/options flow Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/4a422895-2100-4379-901b-cba1812f99fd Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
Done in 67d324c. Added Validation is wired into both flows:
Both use |
There was a problem hiding this comment.
Pull request overview
This PR extends the Device Tools “device modification” flow to allow modifying additional Home Assistant device-registry fields (configuration_url, model_id, entry_type, connections, identifiers) and adds a guard to prevent connection collisions with other devices.
Changes:
- Add new modifiable-attribute constants and expose them in the device modification attribute set.
- Extend config/options flows with new fields, including normalization of registry values into JSON-safe config-entry data and a connection-collision validation step.
- Normalize and map new attributes into
async_update_device-compatible kwargs during apply/revert.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
custom_components/device_tools/const.py |
Adds new constants and includes them in device modifiable attributes; un-ignores connections/identifiers. |
custom_components/device_tools/config_flow.py |
Adds UI fields for the new attributes, normalization for JSON-safe storage, and connection collision checking. |
custom_components/device_tools/entry_handler.py |
Adds a helper to convert stored values into async_update_device kwargs (enum + connection/identifier set conversion). |
custom_components/device_tools/translations/en.json |
Adds labels/descriptions, selector option labels, and the new collision error string. |
custom_components/device_tools/translations/de.json |
Adds labels/descriptions and the collision error string (but currently missing selector option labels for entry_type). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if CONF_CONNECTIONS in result: | ||
| raw = result.pop(CONF_CONNECTIONS) | ||
| result["new_connections"] = {tuple(c) for c in raw} if raw else set() | ||
|
|
||
| if CONF_IDENTIFIERS in result: | ||
| raw = result.pop(CONF_IDENTIFIERS) | ||
| result["new_identifiers"] = {tuple(i) for i in raw} if raw else set() |
There was a problem hiding this comment.
_prepare_device_kwargs assumes connections/identifiers are iterables of 2-item pairs; {tuple(c) for c in raw} will raise TypeError if an element is not iterable (possible with ObjectSelector free-form input) and can also produce tuples of unexpected length. Consider validating/coercing each item (ensure list/tuple of length 2, cast both elements to str) and either filtering invalid items or raising a controlled validation error earlier in the config/options flow.
| if CONF_CONNECTIONS in result: | |
| raw = result.pop(CONF_CONNECTIONS) | |
| result["new_connections"] = {tuple(c) for c in raw} if raw else set() | |
| if CONF_IDENTIFIERS in result: | |
| raw = result.pop(CONF_IDENTIFIERS) | |
| result["new_identifiers"] = {tuple(i) for i in raw} if raw else set() | |
| def _normalize_pair_set(raw_value: Any, field_name: str) -> set[tuple[str, str]]: | |
| """Normalize a raw iterable of pairs into a set of (str, str) tuples. | |
| Invalid entries (non-iterables, wrong length) are ignored with a warning. | |
| """ | |
| normalized: set[tuple[str, str]] = set() | |
| if not raw_value: | |
| return normalized | |
| for index, item in enumerate(raw_value): | |
| if not isinstance(item, (list, tuple)) or len(item) != 2: | |
| _LOGGER.warning( | |
| "Ignoring invalid %s entry at index %s: %r", | |
| field_name, | |
| index, | |
| item, | |
| ) | |
| continue | |
| first, second = item | |
| normalized.add((str(first), str(second))) | |
| return normalized | |
| if CONF_CONNECTIONS in result: | |
| raw = result.pop(CONF_CONNECTIONS) | |
| result["new_connections"] = _normalize_pair_set(raw, CONF_CONNECTIONS) | |
| if CONF_IDENTIFIERS in result: | |
| raw = result.pop(CONF_IDENTIFIERS) | |
| result["new_identifiers"] = _normalize_pair_set(raw, CONF_IDENTIFIERS) |
| "via_device_id": "Übergeordnetes Gerät", | ||
| "configuration_url": "Konfigurations-URL", | ||
| "entry_type": "Eintragstyp", | ||
| "connections": "Verbindungen", | ||
| "identifiers": "Bezeichner" |
There was a problem hiding this comment.
The entry_type selector uses translation_key=entry_type with options none/service, but de.json currently lacks the corresponding selector.entry_type.options translations (present in en.json). Add selector.entry_type.options.none and .service to avoid untranslated option values in the German UI.
| CONF_ENTRY_TYPE, | ||
| CONF_CONNECTIONS, | ||
| CONF_IDENTIFIERS, |
There was a problem hiding this comment.
Adding entry_type / connections / identifiers to MODIFIABLE_ATTRIBUTES means other parts of the integration (notably the engine’s seeding of OriginalDataStore from device.dict_repr) will now persist DeviceEntryType and frozenset[tuple[str, str]] values. Home Assistant Store requires JSON-serializable data, so this will likely raise serialization errors when saving. Consider normalizing these values (e.g., entry_type -> value string, connections/identifiers -> list-of-[type,value] lists) before writing to OriginalDataStore (and similarly when updating it from device-registry update events).
| CONF_ENTRY_TYPE, | |
| CONF_CONNECTIONS, | |
| CONF_IDENTIFIERS, | |
| # NOTE: CONF_ENTRY_TYPE, CONF_CONNECTIONS, and CONF_IDENTIFIERS are | |
| # intentionally excluded here because their native Home Assistant | |
| # types (e.g. DeviceEntryType, frozenset[tuple[str, str]]) are not | |
| # JSON-serializable and can cause Store serialization failures when | |
| # persisted via OriginalDataStore. |
| """ | ||
| if key in (CONF_CONNECTIONS, CONF_IDENTIFIERS): | ||
| if isinstance(value, (set, frozenset)): | ||
| return [list(pair) for pair in value] |
There was a problem hiding this comment.
_normalize_device_value converts set/frozenset to a list via iteration, which is non-deterministic. Because this output is used for suggested values and for equality comparisons against user input, the unstable ordering can cause spurious “changes” (or noisy option updates) even when the underlying set is the same. Consider sorting the pairs (e.g., by type/value) before returning the list-of-lists so the representation is stable across runs.
| return [list(pair) for pair in value] | |
| # Ensure deterministic ordering when normalizing sets/frozensets of pairs | |
| sorted_pairs = sorted( | |
| value, | |
| key=lambda pair: (str(pair[0]), str(pair[1])), | |
| ) | |
| return [list(pair) for pair in sorted_pairs] |
| for connection in connections: | ||
| if not isinstance(connection, (list, tuple)) or len(connection) != 2: | ||
| continue | ||
| conn_type, conn_val = connection | ||
| existing = device_registry.async_get_device( | ||
| connections={(str(conn_type), str(conn_val))} | ||
| ) | ||
| if existing is not None and existing.id != target_device_id: | ||
| return existing | ||
| return None |
There was a problem hiding this comment.
_check_connections_collision silently skips malformed items (non-2-tuples/lists). However, the same malformed data can still be saved and later cause crashes when applying/reverting (e.g., _prepare_device_kwargs does tuple(c) which raises for non-iterables). It would be safer to validate connections in the flow (require a list of 2-item pairs) and surface a form error when the format is invalid, instead of allowing invalid data through.
|
@copilot apply changes based on the comments in this thread |
…erministically, validate format, add de translations Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/6d0abb27-67cf-4d22-ba48-8311a8d578a5 Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
Applied in 053abed. Changes from all threads in that review:
|
|
@copilot fix the mypy issues |
Agent-Logs-Url: https://github.com/EuleMitKeule/device-tools/sessions/739a9b2c-b09b-418d-b37d-aceed2c05db2 Co-authored-by: EuleMitKeule <47421406+EuleMitKeule@users.noreply.github.com>
|
Extends device modifications to support five additional
async_update_deviceattributes that were previously inaccessible:configuration_url,model_id,entry_type,connections, andidentifiers.Changes
const.py— New constants for all five attributes; added toMODIFIABLE_ATTRIBUTES[DEVICE]; removedconnectionsandidentifiersfromIGNORED_ATTRIBUTESso original values are captured and reverted correctlyconfig_flow.py— New form fields:configuration_url/model_idas plainstr,entry_typeasSelectSelector("none"/"service"),connections/identifiersasObjectSelector(user enters list-of-[type, value]pairs); added_normalize_device_value()to convertfrozenset/DeviceEntryTypevalues fromdevice.dict_reprto JSON-safe types before storing in config entry data; added_check_connections_collision()that validates each supplied connection against the device registry before saving — if a connection is already claimed by a different device, both the config flow (async_step_modify_device) and options flow (async_step_init) re-display the form with aconnections_collisionerror and the conflicting device name interpolated viadescription_placeholdersentry_handler.py— AddedDeviceHandler._prepare_device_kwargs()static helper that normalises the three non-trivial types before passing toasync_update_device:entry_type:"none"|"service"string or existingDeviceEntryType→DeviceEntryTypeenum orNoneconnections/identifiers: list-of-lists or set-of-tuples →new_connections/new_identifiersasset[tuple[str, str]]Helper is shared by both
async_apply()andasync_revert().translations/en.json/translations/de.json— Labels, descriptions, andentry_typeselector options added to both config and options flows;connections_collisionerror key added toconfig.errorandoptions.errorin both language filesExample — connections/identifiers input format (ObjectSelector)
Stored as-is in config entry options; converted to
set[tuple[str, str]]on apply/revert.Connections collision guard
If a connection entered by the user is already registered on a different device in the registry, the flow re-displays the form with:
This prevents
DeviceConnectionCollisionErrorfrom surfacing at setup time.Original prompt
Summary
Add support for modifying additional device registry attributes in Device modifications:
configuration_url— the URL shown in the device info panelmodel_id— machine-readable model identifierentry_type— whether the device is a "service" or default (None)connections— set of(type, value)tuples (e.g. MAC address)identifiers— set of(domain, id)tuplesFiles to change
custom_components/device_tools/const.pyAdd new constants:
Add them to
MODIFIABLE_ATTRIBUTES[ModificationType.DEVICE]:Also remove
"connections"and"identifiers"fromIGNORED_ATTRIBUTESsince they will now be tracked and reverted.custom_components/device_tools/config_flow.pyImport the new constants from
.const.In
_get_device_options_schema, add the following fields inside thedevice_attributessection:configuration_url— plainstrfield:model_id— plainstrfield:entry_type—SelectSelectorwith options""(none/default) and"service":connections—ObjectSelector:identifiers—ObjectSelector:Also update
_user_input_to_modification_datato filter out empty strings (same as what the entity_category PR does):custom_components/device_tools/device_modification.pyThe
apply()method currently does**self.modification_datadirectly. We need to handle special fields before passing toasync_update_device:entry_type: stored as""or"service"string → convert toDeviceEntryType("service")orNoneconnections: stored as a list of lists from ObjectSelector (JSON) → convert toset[tuple[str, str]]and pass asnew_connectionsidentifiers: stored as a list of lists from ObjectSelector (JSON) → convert toset[tuple[str, str]]and pass asnew_identifiersconfiguration_url,model_id: pass through as-isUpdate
apply():