diff --git a/custom_components/localthings/__init__.py b/custom_components/localthings/__init__.py index 91ccf21..3b0803f 100644 --- a/custom_components/localthings/__init__.py +++ b/custom_components/localthings/__init__.py @@ -2,8 +2,10 @@ from __future__ import annotations +import inspect import logging import re +from typing import Any from homeassistant.config_entries import ConfigEntry from homeassistant.const import ( @@ -138,15 +140,15 @@ def _repair_placeholder_keys(hass: HomeAssistant, entry: ConfigEntry, serial: st _PARTICULATE_TYPED_IN_V3 = frozenset({"air_purifier", "air_monitor"}) # unique_id is f"{DOMAIN}_{serial}_{state_key}"; state_key is the descriptor -# key, optionally carrying a subdevice prefix and a trailing instance number -# (registry/adapter._key). Matching the tail rather than rebuilding the whole -# id keeps this working for a renamed entity, whose entity_id -- and so its -# statistic_id -- no longer follows from the key at all. -_PARTICULATE_KEY_RE = re.compile(r"_(?:super_fine_dust|fine_dust|dust)\d*$") +# key, optionally carrying a subdevice prefix and a trailing `_` instance +# (registry/adapter._key, discovery.instance_suffix). Matching the tail rather +# than rebuilding the whole id keeps this working for a renamed entity, whose +# entity_id -- and so its statistic_id -- no longer follows from the key. +_PARTICULATE_KEY_RE = re.compile(r"_(?:super_fine_dust|fine_dust|dust)(?:_\d+)?$") @callback -def _relabel_particulate_statistics(hass: HomeAssistant, entry: ConfigEntry) -> None: +def _relabel_particulate_statistics(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Point existing particulate statistics at the unit they always were. These sensors recorded long-term statistics with no unit, and the @@ -165,29 +167,39 @@ def _relabel_particulate_statistics(hass: HomeAssistant, entry: ConfigEntry) -> `async_update_statistics_metadata` and not `change_statistics_unit`, which would scale every stored value. - A no-op when the recorder isn't loaded, when this device family isn't - one that gained the unit, or when the device never recorded any - statistics -- the underlying UPDATE simply matches no rows. + A no-op when this device family isn't one that gained the unit, or when + the device never recorded any statistics -- the underlying UPDATE simply + matches no rows. + + Returns False only when the recorder wasn't loaded, meaning the caller + should leave the entry on its old version and try again next start. """ if entry.data.get(CONF_DEVICE_TYPE) not in _PARTICULATE_TYPED_IN_V3: - return + return True if "recorder" not in hass.config.components: - # after_dependencies pulls the recorder in when it's configured at - # all; a setup running without it has no statistics to relabel. - _LOGGER.debug("recorder not loaded, skipping statistics relabel") - return + # after_dependencies orders the recorder ahead of us when it's + # configured, so this is either an install without it (nothing to + # relabel, and the retry costs one set lookup per start) or a boot + # where it failed to come up. Not distinguishable here, and burning + # the one-shot migration on the second case would leave the + # statistics suppressed for good. + _LOGGER.debug("recorder not loaded, deferring statistics relabel") + return False from homeassistant.components.recorder.statistics import ( STATISTIC_UNIT_TO_UNIT_CONVERTER, async_update_statistics_metadata, ) - # µg/m³ has a converter (MassVolumeConcentrationConverter), so its class - # is 'concentration', not None. Read rather than hardcoded: passing a - # class that disagrees with HA's own table raises, and passing none at - # all is deprecated and stops working in HA Core 2026.11. - converter = STATISTIC_UNIT_TO_UNIT_CONVERTER.get(PARTICULATE_UNIT) - unit_class = converter.UNIT_CLASS if converter is not None else None + kwargs: dict[str, Any] = {"new_unit_of_measurement": PARTICULATE_UNIT} + # `new_unit_class` only exists from HA 2025.11; hacs.json still supports + # 2025.1, where passing it is a TypeError -- which would propagate out of + # async_migrate_entry and fail the whole entry. Where it is supported it + # must be named, since omitting it is deprecated from HA 2026.11. µg/m³ + # has a converter, so the value is 'concentration' rather than None. + if "new_unit_class" in inspect.signature(async_update_statistics_metadata).parameters: + converter = STATISTIC_UNIT_TO_UNIT_CONVERTER.get(PARTICULATE_UNIT) + kwargs["new_unit_class"] = converter.UNIT_CLASS if converter is not None else None ent_reg = er.async_get(hass) for registry_entry in er.async_entries_for_config_entry(ent_reg, entry.entry_id): @@ -196,12 +208,21 @@ def _relabel_particulate_statistics(hass: HomeAssistant, entry: ConfigEntry) -> if not _PARTICULATE_KEY_RE.search(registry_entry.unique_id): continue _LOGGER.debug("relabelling statistics unit for %s", registry_entry.entity_id) - async_update_statistics_metadata( - hass, - registry_entry.entity_id, - new_unit_class=unit_class, - new_unit_of_measurement=PARTICULATE_UNIT, - ) + try: + async_update_statistics_metadata(hass, registry_entry.entity_id, **kwargs) + except Exception: + # Relabelling is a convenience: without it the user gets Home + # Assistant's own units_changed repair, which is where they were + # before this migration existed. Never worth failing setup over, + # so no recorder-side surprise can cost them the integration. + _LOGGER.warning( + "Could not relabel statistics unit for %s; Home Assistant will " + "offer a units-changed repair for it instead", + registry_entry.entity_id, + exc_info=True, + ) + return True + return True async def async_migrate_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: @@ -229,8 +250,7 @@ async def async_migrate_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: _repair_placeholder_keys(hass, entry, serial) _LOGGER.debug("migrated entry %s to version 2 (serial=%s)", entry.entry_id, serial) - if entry.version == 2: - _relabel_particulate_statistics(hass, entry) + if entry.version == 2 and _relabel_particulate_statistics(hass, entry): hass.config_entries.async_update_entry(entry, version=3) _LOGGER.debug("migrated entry %s to version 3", entry.entry_id) diff --git a/custom_components/localthings/registry/capabilities/airconditioner.py b/custom_components/localthings/registry/capabilities/airconditioner.py index ebcc5b9..a44fc2d 100644 --- a/custom_components/localthings/registry/capabilities/airconditioner.py +++ b/custom_components/localthings/registry/capabilities/airconditioner.py @@ -1558,12 +1558,10 @@ AIR_QUALITY = Capability( # CO2 (PR #316, ACA-KR-TP2-21-AN9000) -- a type this file's other AC # families don't report. Same field/shape air_monitor.SENSORS # already models with device_class='carbon_dioxide'/unit='ppm', so - # this matches that descriptor rather than guessing fresh -- unlike - # the pm10/pm25/pm1 mapping air_monitor.py's own docstring - # deliberately rejects for the three dust-type keys above (Samsung's - # two-tier PM10/PM2.5 convention doesn't confirm where a third tier - # or PM1 fits), ppm for a field literally named CO2 isn't a guess of - # that kind. + # this matches that descriptor rather than guessing fresh. The dust + # keys above stay untyped for the reason in _sensor_item_value: the + # pm10/pm25/pm1 mapping is confirmed for the purifier and monitor + # families (issue #325), but no AC family has evidence of its own. SensorDesc( key="co2", field="x.com.samsung.da.items", diff --git a/tests/localthings/test_statistics_migration.py b/tests/localthings/test_statistics_migration.py index df1e3b0..27fd7cd 100644 --- a/tests/localthings/test_statistics_migration.py +++ b/tests/localthings/test_statistics_migration.py @@ -31,7 +31,7 @@ RELABEL = "homeassistant.components.recorder.statistics.async_update_statistics_ @pytest.fixture(autouse=True) def _recorder_loaded(hass: HomeAssistant): """Most tests here assume a normal install, where after_dependencies has - pulled the recorder in. test_is_a_no_op_without_the_recorder undoes it.""" + pulled the recorder in. The deferral test below undoes it.""" hass.config.components.add("recorder") return hass @@ -63,7 +63,7 @@ async def test_relabels_every_particulate_sensor(hass: HomeAssistant) -> None: _add_sensor(hass, entry, key).entity_id for key in ("dust", "fine_dust", "super_fine_dust") } - with patch(RELABEL) as relabel: + with patch(RELABEL, autospec=True) as relabel: assert await async_migrate_entry(hass, entry) is True assert {call.args[1] for call in relabel.call_args_list} == expected @@ -82,7 +82,7 @@ async def test_leaves_other_sensors_on_the_same_device_alone(hass: HomeAssistant for key in ("odor", "clean_level", "co2", "dustbag_usage", "dustbin_auto_close"): _add_sensor(hass, entry, key) - with patch(RELABEL) as relabel: + with patch(RELABEL, autospec=True) as relabel: assert await async_migrate_entry(hass, entry) is True assert [call.args[1] for call in relabel.call_args_list] == [dust.entity_id] @@ -98,24 +98,73 @@ async def test_skips_families_that_did_not_gain_the_unit(hass: HomeAssistant) -> _add_sensor(hass, entry, "dust") _add_sensor(hass, entry, "fine_dust") - with patch(RELABEL) as relabel: + with patch(RELABEL, autospec=True) as relabel: assert await async_migrate_entry(hass, entry) is True assert relabel.call_args_list == [], device_type assert entry.version == 3 -async def test_is_a_no_op_without_the_recorder(hass: HomeAssistant) -> None: - """A setup running without the recorder has no statistics to relabel, - and importing recorder internals there would fail outright.""" +async def test_defers_rather_than_consuming_the_migration_without_the_recorder( + hass: HomeAssistant, +) -> None: + """A boot where the recorder didn't come up must not burn the one-shot + migration -- doing so would leave the statistics suppressed for good. + The entry stays on v2 so the next start retries.""" entry = _entry(hass, "air_purifier") _add_sensor(hass, entry, "dust") hass.config.components.remove("recorder") - with patch(RELABEL) as relabel: + with patch(RELABEL, autospec=True) as relabel: assert await async_migrate_entry(hass, entry) is True assert relabel.call_args_list == [] + assert entry.version == 2 + + # ...and the retry lands once the recorder is there. + hass.config.components.add("recorder") + with patch(RELABEL, autospec=True) as relabel: + assert await async_migrate_entry(hass, entry) is True + + assert len(relabel.call_args_list) == 1 + assert entry.version == 3 + + +async def test_omits_unit_class_on_an_older_home_assistant(hass: HomeAssistant) -> None: + """`new_unit_class` only exists from HA 2025.11, and hacs.json still + declares 2025.1 as the minimum. Passing it to the older signature is a + TypeError out of async_migrate_entry, which fails the whole entry -- so + the kwarg is feature-detected rather than assumed. + + Stands in for an older HA by patching in that exact signature; the + autospec'd tests above cover the modern one. + """ + entry = _entry(hass, "air_purifier") + _add_sensor(hass, entry, "dust") + + seen: list[dict] = [] + + def old_signature(hass, statistic_id, *, new_statistic_id=None, **kwargs): + seen.append(kwargs) + + with patch(RELABEL, old_signature): + assert await async_migrate_entry(hass, entry) is True + + assert seen == [{"new_unit_of_measurement": CONCENTRATION_MICROGRAMS_PER_CUBIC_METER}] + assert entry.version == 3 + + +async def test_a_relabel_failure_never_fails_the_entry(hass: HomeAssistant) -> None: + """Relabelling is a convenience -- without it the user gets HA's own + units_changed repair, which is where they were before. An older HA whose + async_update_statistics_metadata has a different signature, or any other + recorder-side surprise, must not cost them the integration.""" + entry = _entry(hass, "air_purifier") + _add_sensor(hass, entry, "dust") + + with patch(RELABEL, autospec=True, side_effect=TypeError("older HA signature")): + assert await async_migrate_entry(hass, entry) is True + assert entry.version == 3 @@ -130,7 +179,7 @@ async def test_follows_a_renamed_entity_rather_than_rebuilding_its_id( renamed = _add_sensor(hass, entry, "dust", suggested_object_id="living_room_pm10") assert renamed.entity_id == "sensor.living_room_pm10" - with patch(RELABEL) as relabel: + with patch(RELABEL, autospec=True) as relabel: assert await async_migrate_entry(hass, entry) is True assert [call.args[1] for call in relabel.call_args_list] == ["sensor.living_room_pm10"] @@ -138,10 +187,11 @@ async def test_follows_a_renamed_entity_rather_than_rebuilding_its_id( async def test_matches_subdevice_prefixed_and_instanced_keys(hass: HomeAssistant) -> None: """_key() can prefix a subdevice and append an instance number, so the - match is on the tail rather than the whole unique_id.""" + match is on the tail rather than the whole unique_id. The instance form + is `_` (discovery.instance_suffix), not a bare digit.""" entry = _entry(hass, "air_purifier") ent_reg = er.async_get(hass) - for unique_suffix in ("indoor_0_dust", "fine_dust1", "super_fine_dust"): + for unique_suffix in ("indoor_0_dust", "fine_dust_1", "super_fine_dust"): ent_reg.async_get_or_create( "sensor", DOMAIN, @@ -157,7 +207,7 @@ async def test_matches_subdevice_prefixed_and_instanced_keys(hass: HomeAssistant config_entry=entry, ) - with patch(RELABEL) as relabel: + with patch(RELABEL, autospec=True) as relabel: assert await async_migrate_entry(hass, entry) is True assert len(relabel.call_args_list) == 3