Simplify semantic-class lookups and drop no-op device class wraps #93
Loading…
Reference in a new issue
No description provided.
Delete branch "refactor/direct-semantic-lookups"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
READOUTS, BINARY and ACTUATED are dicts keyed by IntEnum, so a raw semantic
int already matches them on lookup. Replaces the semantic_of round-trip and
isinstance checks in entity_shape.py with direct dict.get calls, and the
kind_of/semantic_of calls in platforms.py with direct comparisons (an unknown
kind can never equal a known PresentationKind, and IntEnum members hash equal
to their int value).
Also drops the SensorDeviceClass/BinarySensorDeviceClass/NumberDeviceClass
re-wrapping in sensor.py, binary_sensor.py and number.py: the shape tables
already store real enum instances, so re-constructing them was a no-op. The
device_class field stays typed as str since the three platforms each expect
a different enum from it; the properties cast rather than reconstruct.
No behavioural change. tools/check passes (1219 tests).
Reviewed after merge (PR was already closed/merged by the time I got to it). Traced both commits against the description — diff matches it exactly, no scope creep.
Correctness checked and no behavioural change found:
READOUTS/BINARY/ACTUATEDwidened fromdict[SemanticClass, Shape/str/Platform]todict[int, ...]: safe, sinceSemanticClassis anIntEnumand a raw int hashes/compares equal to the member with that value, so.get(semantic)matches exactly whatisinstance(known, SemanticClass) and known in Xused to.platforms.py:endpoint.kind == PresentationKind.LEVEL/SETTINGis equivalent to the oldkind_of(endpoint.kind) is PresentationKind.LEVEL/SETTING, becausekind_ofonly ever normalises an unrecognised raw value down toUNSPECIFIED, never up toLEVEL/SETTING— so the two can only ever agree. Same reasoning forendpoint.semantic in MOMENTARYreplacingsemantic_of(endpoint.semantic) in MOMENTARY.semantic == SemanticClass.DURATIONvs the oldknown is SemanticClass.DURATION: fine,int == IntEnumcompares by value.sensor.py/binary_sensor.py/number.py: checked everyShape(...)construction site inentity_shape.pythat can reach each platform, and each stores the enum type that platform'scastclaims (SensorDeviceClassforPlatform.SENSOR,BinarySensorDeviceClassforBINARY_SENSOR,NumberDeviceClassforNUMBER, including the untouchedNumberDeviceClass(readout.device_class)conversion inside_setting). The casts are safe. Worth noting the oldSensorDeviceClass(named)-style call was also acting as a runtime guard (raisesValueErroron a bad value); the cast removes that. Sincedevice_classonly ever comes from hardcoded literals inentity_shape.py, not wire data, this is a static-only concern, not a live risk.Two minor, non-blocking points:
cast("BinarySensorDeviceClass | None", ...)/cast("SensorDeviceClass | None", ...)/cast("NumberDeviceClass | None", ...)all quote the type as a string, but the class is already imported normally at module scope in each file with no circular-import concern — the quoting buys nothing here and is a little inconsistent with a plaincast(BinarySensorDeviceClass | None, ...).test_a_raw_unknown_semantic_still_falls_to_its_bandis fine but fairly duplicate in spirit with the existingtest_an_unknown_measured_class_still_becomes_a_readable_sensor/test_an_unknown_binary_class_still_becomes_a_binary_sensor, which already prove.get()on an unrecognised int falls through correctly (just reached viaPresentationKind.READOUT/STATEinstead ofUNSPECIFIED). It doesn't target the specific risk this refactor could have introduced — a raw int that does match a knownSemanticClassvalue still hitting the dict via.get()— because that's guaranteed byIntEnum's int-compatible hashing, not something this diff could plausibly get wrong. Not wrong, just marginal.The
Shape.device_classdocstring explaining why it stays typedstris appropriately terse and states a genuine non-obvious constraint — no issue there.Couldn't run
tools/checkmyself: the worktree I was given was removed partway through the review (looks like environment/session cleanup, unrelated to this PR), so this is a static read-through of the diff plus the surrounding files, not a tool-verified one. The PR description statestools/checkpasses with 1219 tests, which I have no reason to doubt but also couldn't independently confirm.