Fix AI energy level platform-flip bug found in Opus review
The switch and select shared a stub-time asymmetry: only the select had a `not rep` carve-out, so an unfetched-stub rep at the moment platforms are set up (entity creation runs once, ever) would instantiate a Select, while flatten() re-evaluates exists_fn every poll against live data -- once the resource populated to a single-level list, the switch's exists_fn would win instead and feed the already-created Select a bool through their shared 'ai_energy_level' key, which isn't a valid select option. Dropped the stub carve-out from the select's exists_fn so both sides require real, populated data to decide the platform -- on a device that stubs this cold-tier href on its very first poll, the entity now simply doesn't appear until a reload, instead of appearing as the wrong widget type. Added tests for the missing/empty-list supportedAiLevel shapes on both widgets and a regression test locking in the stub behavior.
This commit is contained in:
2 files changed
+57
-8
No files matched your search
@@ -277,6 +277,22 @@ AI_ENERGY_LEVEL = Capability(
|
||||
href='/energy/ailevel/vs/0',
|
||||
poll_tier='cold',
|
||||
entities=(
|
||||
# No `not rep` stub carve-out on either side, unlike most exists_fn
|
||||
# gates in this file -- entity creation only ever runs once, against
|
||||
# whichever snapshot happens to be current the moment platforms are
|
||||
# set up (see entity._is_included / __init__.py's
|
||||
# async_config_entry_first_refresh-before-forward-entry-setups
|
||||
# ordering), while flatten() re-evaluates exists_fn every poll
|
||||
# against live data. Both descriptors share key='ai_energy_level',
|
||||
# so if a stub carve-out let one of them win at setup time while the
|
||||
# other wins once real data lands, flatten() would feed the
|
||||
# instantiated entity a value shaped for the other platform (e.g. a
|
||||
# bool into a Select). Requiring real, populated data on both sides
|
||||
# keeps the entity-creation decision and the live-value decision in
|
||||
# permanent agreement -- the cost is this entity doesn't appear
|
||||
# until a reload if the device's very first poll stubs this
|
||||
# cold-tier href, the same reload already required to fix which
|
||||
# platform got picked in that case.
|
||||
SwitchDesc(key='ai_energy_level', field='aiLevel',
|
||||
name='AI energy level', icon='mdi:leaf',
|
||||
entity_category='config',
|
||||
@@ -289,7 +305,7 @@ AI_ENERGY_LEVEL = Capability(
|
||||
entity_category='config',
|
||||
options=_ai_energy_level_options,
|
||||
exists_fn=lambda rep, resources: (
|
||||
not rep or len(_ai_energy_supported_levels(rep)) > 1),
|
||||
len(_ai_energy_supported_levels(rep)) > 1),
|
||||
write_fn=_ai_energy_level_write),
|
||||
),
|
||||
)
|
||||
|
||||
@@ -166,10 +166,15 @@ class TestAiEnergyLevelSwitch:
|
||||
desc = self._desc()
|
||||
assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': '1'}, {}) is False
|
||||
|
||||
def test_hidden_when_supported_level_missing(self):
|
||||
desc = self._desc()
|
||||
assert desc.exists_fn({'aiLevel': '1'}, {}) is False
|
||||
|
||||
def test_hidden_when_supported_level_empty_list(self):
|
||||
desc = self._desc()
|
||||
assert desc.exists_fn({'aiLevel': '0', 'supportedAiLevel': []}, {}) is False
|
||||
|
||||
def test_hidden_on_empty_stub_rep(self):
|
||||
"""Unlike the select, the switch requires a confirmed single-entry
|
||||
list -- an unfetched stub defaults to the select branch instead (see
|
||||
TestAiEnergyLevelSelect.test_shown_for_empty_stub_rep)."""
|
||||
desc = self._desc()
|
||||
assert desc.exists_fn({}, {}) is False
|
||||
|
||||
@@ -201,11 +206,17 @@ class TestAiEnergyLevelSelect:
|
||||
assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1', '2']}, {}) is True
|
||||
assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': ['1']}, {}) is False
|
||||
|
||||
def test_shown_for_empty_stub_rep(self):
|
||||
"""An empty {} rep is /device/0's not-yet-fetched-stub carve-out --
|
||||
defaults to the select so sub-polls can populate it either way."""
|
||||
def test_hidden_when_supported_level_is_non_list_scalar(self):
|
||||
desc = self._desc()
|
||||
assert desc.exists_fn({}, {}) is True
|
||||
assert desc.exists_fn({'aiLevel': '1', 'supportedAiLevel': '12'}, {}) is False
|
||||
|
||||
def test_hidden_when_supported_level_missing(self):
|
||||
desc = self._desc()
|
||||
assert desc.exists_fn({'aiLevel': '1'}, {}) is False
|
||||
|
||||
def test_hidden_when_supported_level_empty_list(self):
|
||||
desc = self._desc()
|
||||
assert desc.exists_fn({'aiLevel': '0', 'supportedAiLevel': []}, {}) is False
|
||||
|
||||
def test_no_translation_key(self):
|
||||
"""aiLevel's values are plain digits that render fine untranslated
|
||||
@@ -232,6 +243,28 @@ class TestAiEnergyLevelSelect:
|
||||
assert body == {'aiLevel': '2'}
|
||||
|
||||
|
||||
class TestAiEnergyLevelStubDoesNotDecideThePlatform:
|
||||
"""Issue found in review: entity *creation* runs once, against whatever
|
||||
snapshot is current when platforms are set up (see
|
||||
__init__.py's async_config_entry_first_refresh-before-forward-entry-setups
|
||||
ordering), while flatten() re-evaluates exists_fn every poll against live
|
||||
data. Both descriptors share key='ai_energy_level' (see adapter._key), so
|
||||
if a stub carve-out let one of them win at setup time while the other
|
||||
wins once real data lands, flatten() would feed the already-instantiated
|
||||
entity a value shaped for the other platform (e.g. a bool into a Select
|
||||
expecting a string option). Neither side gets a `not rep` carve-out, so
|
||||
an unfetched stub can never win entity creation for either platform --
|
||||
the entity simply doesn't appear until a reload happens with real data,
|
||||
same as any other exists_fn-gated entity in this codebase that's unlucky
|
||||
on first-poll timing, instead of appearing as the wrong widget type."""
|
||||
|
||||
def test_neither_widget_exists_on_empty_stub_rep(self):
|
||||
switch = _ai_energy_level_desc('SwitchDesc')
|
||||
select = _ai_energy_level_desc('SelectDesc')
|
||||
assert switch.exists_fn({}, {}) is False
|
||||
assert select.exists_fn({}, {}) is False
|
||||
|
||||
|
||||
class TestSelfCheckError:
|
||||
"""Self-check diagnostic error list -- surfaced on hardware that reports
|
||||
x.com.samsung.da.error, joined into a single display string."""
|
||||
|
||||
Reference in new issue
Block a user