Stop the v1 migration re-keying devices onto a placeholder serial

_serial_from_unique_id took the entry's unique_id at face value. That is
right for an entry whose unique_id holds a real serial, but the unique_id
records what the config flow believed when it ran, not what the registry
holds now -- and for two firmware families those are different things.

Entries added before the placeholder rules landed (issues #83/#189) were
keyed on the placeholder itself: `localthings_Nothing(SVC)` for the
ARTIK051_DONGLE_REF dongles, `localthings_FFFFFFFFFFFFFFF` for the
DA_WM_A51_20_COMMON laundry boards. The coordinator has been resolving
those same boards to the host ever since, so their devices and entities
are host-keyed today. Migration read the placeholder back off the
unique_id, decided the host-keyed rows were the stale ones, and rewrote
them onto the placeholder -- reintroducing exactly the collision those
issues exist to prevent, since every unit of the family reports the same
placeholder and would go back to sharing entity unique_ids.

Run the recovered string through resolve_serial, which is the whole point
of that helper being shared. The old `host:port` special case stays: it's
a config-flow-history artifact rather than a device-reported serial, so
resolve_serial can't recognize it.

The repair pass had a second, narrower way to lose data. Removing a device
takes its entities with it (entity_registry.async_device_modified), and
the removal branch ran after the entity pass -- so an entity that had just
been re-keyed rather than removed, because its serial-keyed key was free,
was destroyed a few lines later along with the entity_id, name and area
the rewrite existed to preserve. Move surviving entities onto the device
they now belong to before removing the duplicate.

Reachable when the serial-keyed device exists but a given entity's
serial-keyed key doesn't -- e.g. the user deleted the visible duplicate by
hand, which is the first thing anyone hitting #236 tries.

Also fold the modelNum `<model>|<board>` split into resolve_model beside
resolve_serial. The config flow and _run_discovery each had their own copy
under a comment promising they matched; a device that renames itself on
the first poll is what a drift there looks like.

custom_components/localthings/__init__.py

@@ -13,32 +13,45 @@ from homeassistant.helpers import entity_registry as er
 
 from .const import CONF_HOST, CONF_PORT, CONF_SERIAL, DOMAIN, PLATFORMS
 from .coordinator import LocalThingsCoordinator
+from .registry.identity import resolve_serial
 
 _LOGGER = logging.getLogger(__name__)
 
 
-def _serial_from_unique_id(entry: ConfigEntry) -> str | None:
+def _serial_from_unique_id(entry: ConfigEntry) -> str:
     """The device identity a pre-v2 entry was created with.
 
     The config flow has always keyed the entry's unique_id on the serial the
     probe read (`localthings_<serial>`), so that string is the identity the
     entry's registry entries were minted from -- there is no need to reach the
-    device to recover it.
+    device to recover it. Anything we can't recover one from resolves to the
+    host, which is what the coordinator seeded such an entry with anyway.
 
-    One wrinkle: for a board reporting a placeholder serial (issues #83/#189)
-    the two sides used to disagree. The config flow fell back to `host:port`
-    while the coordinator fell back to `host`, so the entry and its own
-    devices/entities were keyed differently. Collapse that to the
-    coordinator's form, which is the one the registry actually holds.
+    The recovered string goes back through resolve_serial rather than being
+    taken at face value, because the unique_id records what the flow believed
+    at the time it ran, not what the registry holds now. Entries created
+    before the placeholder rules landed (issues #83/#189) were keyed on the
+    placeholder itself -- `localthings_Nothing(SVC)`, `localthings_FFFF...` --
+    while the coordinator has since been resolving those same boards to the
+    host. Re-keying the registry onto the placeholder to match the unique_id
+    would reintroduce the collision those issues are about: two units of that
+    family report the *same* placeholder, so they'd share entity unique_ids
+    again.
+
+    A later wrinkle, same root cause: for a stretch the two sides disagreed on
+    which fallback to use, the flow writing `host:port` while the coordinator
+    wrote `host`. Collapse that to the coordinator's form too -- the registry
+    is what has to keep working.
     """
+    host = entry.data[CONF_HOST]
     prefix = f"{DOMAIN}_"
     unique_id = entry.unique_id or ""
     if not unique_id.startswith(prefix):
-        return None
+        return host
     serial = unique_id[len(prefix) :]
-    if serial == f"{entry.data[CONF_HOST]}:{entry.data.get(CONF_PORT)}":
-        return entry.data[CONF_HOST]
-    return serial or None
+    if serial == f"{host}:{entry.data.get(CONF_PORT)}":
+        return host
+    return resolve_serial(serial, host)
 
 
 @callback
@@ -92,6 +105,16 @@ def _repair_placeholder_keys(hass: HomeAssistant, entry: ConfigEntry, serial: st
         fresh = {(DOMAIN, f"{serial}{ident[1][len(host) :]}") for ident in stale}
         existing = dev_reg.async_get_device(identifiers=fresh)
         if existing is not None and existing.id != device.id:
+            # Removing a device takes its entities with it. Anything still
+            # attached here came through the pass above re-keyed rather than
+            # removed -- i.e. it's the surviving copy, not a duplicate -- so
+            # move it onto the device it now belongs to first. Otherwise the
+            # rewrite that was supposed to preserve an entity_id, name and
+            # area destroys them a few lines later.
+            for entity in er.async_entries_for_device(
+                ent_reg, device.id, include_disabled_entities=True
+            ):
+                ent_reg.async_update_entity(entity.entity_id, device_id=existing.id)
             _LOGGER.debug("removing orphaned device %s", device.id)
             dev_reg.async_remove_device(device.id)
         else:
@@ -113,9 +136,7 @@ async def async_migrate_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
         return False
 
     if entry.version == 1:
-        serial = (
-            entry.data.get(CONF_SERIAL) or _serial_from_unique_id(entry) or entry.data[CONF_HOST]
-        )
+        serial = entry.data.get(CONF_SERIAL) or _serial_from_unique_id(entry)
         hass.config_entries.async_update_entry(
             entry,
             data={**entry.data, CONF_SERIAL: serial},

custom_components/localthings/config_flow.py

@@ -600,7 +600,7 @@ def _read_device(sess, host: str, port: int) -> dict:
 
     from .registry.batch import parse_device0_batch
     from .registry.by_type import resolve as resolve_registry
-    from .registry.identity import read_identity, resolve_serial
+    from .registry.identity import read_identity, resolve_model, resolve_serial
 
     identity = read_identity(sess, None)
 
@@ -617,14 +617,14 @@ def _read_device(sess, host: str, port: int) -> dict:
     resources = parse_device0_batch(body) if isinstance(body, list) else {}
 
     info = resources.get("/information/vs/0", {})
-    model_num = info.get("x.com.samsung.da.modelNum", "")
     registry = resolve_registry(resources, device_types=identity.device_types)
     return {
         "port": port,
+        # Resolved through the same helpers _run_discovery uses, so the device
+        # the coordinator registers up front is the one discovery would have
+        # produced -- no rename, and no re-key, once the first poll lands.
         "serial": resolve_serial(info.get("x.com.samsung.da.serialNum"), host),
-        # Same derivation _run_discovery uses, so the device the coordinator
-        # registers up front is the one discovery would have produced.
-        "model": model_num.split("|", 1)[0] if model_num else identity.model,
+        "model": resolve_model(info.get("x.com.samsung.da.modelNum", ""), identity),
         "manufacturer": identity.manufacturer or "Samsung",
         "device_type_name": registry.name if registry is not None else None,
         "device_type_recognized": registry is not None,

custom_components/localthings/coordinator.py

@@ -53,6 +53,7 @@ from .registry.identity import (
     DeviceIdentity,
     device_display_name,
     read_identity,
+    resolve_model,
     resolve_serial,
 )
 from .registry.subdevices import (
@@ -777,7 +778,7 @@ class LocalThingsCoordinator(DataUpdateCoordinator[dict[str, Any]]):
         self.device_serial = serial
 
         ident = self._identity
-        model = model_num.split("|", 1)[0] if model_num else (ident.model if ident else "")
+        model = resolve_model(model_num, ident)
         name = device_display_name(device_type_name, model)
         mfr = (ident.manufacturer if ident else "") or "Samsung"
 

custom_components/localthings/registry/identity.py

@@ -62,6 +62,24 @@ def resolve_serial(raw_serial: str | None, host: str) -> str:
     return s
 
 
+def resolve_model(model_num: str, identity: DeviceIdentity | None) -> str:
+    """The model string to name and register a device under.
+
+    `model_num` is /information/vs/0's x.com.samsung.da.modelNum, which many
+    boards report as `<model>|<board>` -- only the part before the pipe is the
+    model a user would recognize. A board that reports no modelNum at all
+    falls back to /oic/p's mnmo, which read_identity already parsed.
+
+    Shared with resolve_serial's motivation: the config flow resolves this
+    once and persists it on the entry, and the coordinator recomputes it after
+    the first poll. Two copies of the split rule would let those two disagree,
+    and a device that renames itself on the first poll is the visible symptom.
+    """
+    if model_num:
+        return model_num.split("|", 1)[0]
+    return identity.model if identity else ""
+
+
 def device_display_name(device_type_name: str | None, model: str) -> str:
     """The HA device name for a resolved device type + model.
 

tests/localthings/test_migration.py

@@ -58,6 +58,93 @@ async def test_migration_collapses_the_host_port_unique_id(
     assert entry.unique_id == f"{DOMAIN}_{MOCK_HOST}"
 
 
+async def test_migration_resolves_a_placeholder_serial_unique_id(
+    hass: HomeAssistant, mock_coordinator_session
+) -> None:
+    """An entry created before the placeholder rules landed was keyed on the
+    placeholder itself (issues #83/#189), while the coordinator has been
+    resolving those boards to the host ever since. The unique_id records what
+    the flow believed then, not what the registry holds -- taking it at face
+    value would re-key working devices back onto a string every unit of the
+    family reports, which is the collision those issues are about."""
+    entry = _legacy_entry(hass, f"{DOMAIN}_Nothing(SVC)")
+    dev_reg = dr.async_get(hass)
+    device = dev_reg.async_get_or_create(
+        config_entry_id=entry.entry_id,
+        identifiers={(DOMAIN, MOCK_HOST)},
+    )
+
+    await hass.config_entries.async_setup(entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert entry.data[CONF_SERIAL] == MOCK_HOST
+    assert entry.unique_id == f"{DOMAIN}_{MOCK_HOST}"
+    unchanged = dev_reg.async_get(device.id)
+    assert unchanged is not None
+    assert unchanged.identifiers == {(DOMAIN, MOCK_HOST)}
+
+
+async def test_migration_resolves_an_all_hex_placeholder_unique_id(
+    hass: HomeAssistant, mock_coordinator_session
+) -> None:
+    """The issue #189 flash-unset sentinel, same reasoning."""
+    entry = _legacy_entry(hass, f"{DOMAIN}_FFFFFFFFFFFFFFF")
+
+    await hass.config_entries.async_setup(entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert entry.data[CONF_SERIAL] == MOCK_HOST
+
+
+async def test_migration_keeps_a_survivor_off_a_removed_duplicate_device(
+    hass: HomeAssistant,
+) -> None:
+    """Removing a device takes its entities with it (entity_registry's
+    async_device_modified), so an entity that came through the pass above
+    re-keyed rather than removed has to move to the surviving device first --
+    otherwise the rewrite that exists to preserve an entity_id, name and area
+    destroys all three a few lines later.
+
+    Migration is called directly here: what the repair leaves behind is the
+    contract, and going through async_setup would let the platform re-adding
+    its entities hide a row that had in fact been deleted."""
+    from custom_components.localthings import async_migrate_entry
+
+    entry = _legacy_entry(hass, f"{DOMAIN}_{MOCK_SERIAL}")
+    dev_reg = dr.async_get(hass)
+    ent_reg = er.async_get(hass)
+
+    real_device = dev_reg.async_get_or_create(
+        config_entry_id=entry.entry_id,
+        identifiers={(DOMAIN, MOCK_SERIAL)},
+    )
+    orphan_device = dev_reg.async_get_or_create(
+        config_entry_id=entry.entry_id,
+        identifiers={(DOMAIN, MOCK_HOST)},
+    )
+    # The serial-keyed device exists, but this entity's serial-keyed *key* is
+    # free -- e.g. the user deleted the visible duplicate by hand -- so the
+    # entity pass rewrites it instead of removing it.
+    survivor = ent_reg.async_get_or_create(
+        "sensor",
+        DOMAIN,
+        f"{DOMAIN}_{MOCK_HOST}_connection_mode",
+        config_entry=entry,
+        device_id=orphan_device.id,
+        suggested_object_id="kitchen_fridge_connection",
+    )
+
+    assert await async_migrate_entry(hass, entry) is True
+    await hass.async_block_till_done()
+
+    assert dev_reg.async_get(orphan_device.id) is None
+    kept = ent_reg.async_get(survivor.entity_id)
+    assert kept is not None
+    assert kept.entity_id == "sensor.kitchen_fridge_connection"
+    assert kept.unique_id == f"{DOMAIN}_{MOCK_SERIAL}_connection_mode"
+    assert kept.device_id == real_device.id
+
+
 async def test_migration_rekeys_an_ip_keyed_device_and_entity(
     hass: HomeAssistant, mock_coordinator_session
 ) -> None: