Clean up microwave PR after main merge: dedupe skill doc, derive cook modes live
The merge from main had left the adding-device-support skill with a stale duplicate "Enum selects need translation support" section (referencing a nonexistent strings.json) sitting alongside the canonical version, and main's new "never hard-code the dump" guidance hadn't landed on this branch at all -- restore it and drop the duplicate. Apply that guidance to the microwave capability itself: MICROWAVE_MODE's cook_mode select was carrying a hardcoded _MICROWAVE_MODES tuple lifted from the single issue #66 dump, even though /mode/vs/0 reports its own live supportedModes list. Switch to options_field and validate writes against the live rep instead of the static vocabulary, so a microwave with a narrower or wider mode set (grill-less models, etc.) isn't stuck with this one unit's options. The microwave entities also shipped with no translations/en.json entries at all (cook_mode, cavity_state, power_level, cavity_temp) -- add them, mirrored into nl.json, and update the affected tests.
This commit is contained in:
Marc Billow
@@ -103,7 +103,38 @@ sub-polled between summary polls. Pick descriptor types from `entities.py`
|
||||
as a gap for a human, or ignore it with a documented reason — never invent an
|
||||
entity on a hunch (`ignored.py`'s rule).
|
||||
|
||||
## 5. Parse units out of the value — don't ship them embedded in a string
|
||||
## 5. Never hard-code the one dump's values
|
||||
|
||||
A single `/device/0` dump is **one device on one firmware** — its select options,
|
||||
temperature range/increment, and any other "what values are valid here" data are
|
||||
**that unit's snapshot**, not the field's universe. Other units of the same model
|
||||
(different region, firmware, board revision) can support more, fewer, or
|
||||
differently-stepped values. If the dump reports the live option/range list, wire
|
||||
the descriptor to read it live — don't transcribe what you saw into a Python
|
||||
literal:
|
||||
|
||||
- **Selects**: use `options_field` (a resource field holding the live options
|
||||
list, e.g. `supportedWaterTemperature`, `iceType.supported`) so `select.py`
|
||||
reads the current device's real options every time, not `options=(...)` typed
|
||||
from the dump. Reach for a callable `options` only when the values require
|
||||
cross-resource computation the field alone can't give you — a static tuple is
|
||||
right only for genuinely fixed, spec-defined enums (e.g. an OCF-standard field
|
||||
with a closed value set), never for vendor `supported*` lists.
|
||||
- **Number ranges/steps**: use `range_field` (a `[min, max]`-shaped field) or
|
||||
`native_min_fn`/`native_max_fn`/`step_fn` to read bounds from the live rep —
|
||||
see `oven.py`'s `_setpoint_bounds`. Only fall back to static `native_min`/
|
||||
`native_max`/`step` when the dump has no such field and the bound is genuinely
|
||||
fixed by spec, not just "the only value this one unit happened to report."
|
||||
- **Anywhere else** a field's presence, count, or shape looks like it could vary
|
||||
by model/config (course lists, capability flags, supported-mode arrays):
|
||||
check whether the resource carries its own `supported*` companion field before
|
||||
assuming the observed value is exhaustive.
|
||||
|
||||
When you do hard-code something (a genuinely fixed enum, a spec constant), that's
|
||||
a judgement call worth a one-line comment saying why it's safe — the default
|
||||
assumption should be "derive it," not "copy it."
|
||||
|
||||
## 6. Parse units out of the value — don't ship them embedded in a string
|
||||
|
||||
Samsung reps sometimes encode a numeral and its unit as one string
|
||||
(`x.com.samsung.da.powerLevel: "700W"`; a `desired`/`current` temperature whose
|
||||
@@ -131,17 +162,6 @@ Before wiring up a numeric-looking field:
|
||||
genuinely non-numeric state (mode names, enum-like text) — reserve it for
|
||||
that, not as a shortcut past parsing a numeral.
|
||||
|
||||
## 6. Enum selects need translation support
|
||||
|
||||
Any select whose options are raw device codes (course/cycle, and code-valued
|
||||
settings) must render through translations, not Python:
|
||||
- Set `translation_key='<family>_cycle'` (or similar) on the `SelectDesc`;
|
||||
`options`/`options_field` supply the **raw** codes.
|
||||
- Add the labels to **both** `strings.json` and `translations/en.json` under
|
||||
`entity.select.<translation_key>.state.<code>`, with the code **lowercased**
|
||||
(e.g. `"16": "Cotton"`). Codes with no entry render as the raw code — that's
|
||||
the cue to identify and name them.
|
||||
|
||||
## 7. Names and enum labels live in translations, never in Python
|
||||
|
||||
Descriptors have **no `name` field**. Every entity is named from the shipped
|
||||
|
||||
@@ -7,10 +7,12 @@ progress_percentage, operation_time_minutes, finish_time, cook_time, stop),
|
||||
so those capabilities are reused directly from oven.py rather than
|
||||
re-declared here -- see by_type/microwave.py.
|
||||
|
||||
/mode/vs/0's cook-mode vocabulary (MicroWave/MicroWaveGrill/Grill/Autocook)
|
||||
and options-array toggles (only a Sound_On/Off slot on this dump -- no
|
||||
UpperLamp/fastpreheat/NaturalSteam like the oven family) are microwave-
|
||||
specific, so it gets its own capability here. /oven/vs/0 additionally
|
||||
/mode/vs/0's cook-mode select reads its options live from supportedModes
|
||||
(MicroWave/MicroWaveGrill/Grill/Autocook on this dump -- no oven-style
|
||||
Bake/Broil vocabulary applies, and another model's supported set may
|
||||
differ) and its options-array toggles (only a Sound_On/Off slot on this
|
||||
dump -- no UpperLamp/fastpreheat/NaturalSteam like the oven family) are
|
||||
microwave-specific, so it gets its own capability here. /oven/vs/0 additionally
|
||||
reports a powerLevel field (a wattage with the unit embedded in the string,
|
||||
e.g. "700W") the oven family's cavity capability doesn't carry, so it also
|
||||
gets its own.
|
||||
@@ -85,20 +87,11 @@ def _sound_write(p, rep, href=None):
|
||||
}
|
||||
|
||||
|
||||
# Cook modes as reported by /mode/vs/0's supportedModes (issue #66 dump) --
|
||||
# no oven-style Bake/Broil vocabulary applies to a microwave.
|
||||
_MICROWAVE_MODES = (
|
||||
'NoOperation',
|
||||
'MicroWave',
|
||||
'MicroWaveGrill',
|
||||
'Grill',
|
||||
'Autocook',
|
||||
'AutocookCustom',
|
||||
)
|
||||
|
||||
|
||||
def _mode_write(p, rep, href=None):
|
||||
if p not in _MICROWAVE_MODES:
|
||||
# rep is this capability's own /mode/vs/0 rep, so the live
|
||||
# supportedModes list is right here -- no static vocabulary to keep in
|
||||
# sync with devices whose mode set differs (grill-less models, etc.).
|
||||
if p not in (rep.get('x.com.samsung.da.supportedModes') or ()):
|
||||
return None
|
||||
return ['mode', 'vs', '0'], {'x.com.samsung.da.modes': [p]}
|
||||
|
||||
@@ -109,7 +102,7 @@ MICROWAVE_MODE = Capability(
|
||||
entities=(
|
||||
SelectDesc(key='cook_mode', field='x.com.samsung.da.modes',
|
||||
icon='mdi:tune',
|
||||
options=_MICROWAVE_MODES,
|
||||
options_field='x.com.samsung.da.supportedModes',
|
||||
value_fn=lambda v: v[0] if v else None,
|
||||
write_fn=_mode_write),
|
||||
SwitchDesc(key='sound', field='x.com.samsung.da.options',
|
||||
|
||||
@@ -154,6 +154,17 @@
|
||||
"on": "On"
|
||||
}
|
||||
},
|
||||
"cook_mode": {
|
||||
"name": "Cook mode",
|
||||
"state": {
|
||||
"no_operation": "No operation",
|
||||
"micro_wave": "Microwave",
|
||||
"micro_wave_grill": "Microwave + grill",
|
||||
"grill": "Grill",
|
||||
"autocook": "Autocook",
|
||||
"autocook_custom": "Autocook custom"
|
||||
}
|
||||
},
|
||||
"cycle": {
|
||||
"name": "Cycle"
|
||||
},
|
||||
@@ -438,6 +449,12 @@
|
||||
"burner_state": {
|
||||
"name": "Burner {number} state"
|
||||
},
|
||||
"cavity_state": {
|
||||
"name": "Cavity state"
|
||||
},
|
||||
"cavity_temp": {
|
||||
"name": "Cavity temperature"
|
||||
},
|
||||
"clean_level": {
|
||||
"name": "Clean level"
|
||||
},
|
||||
@@ -571,6 +588,9 @@
|
||||
"power_energy_kwh": {
|
||||
"name": "Power energy"
|
||||
},
|
||||
"power_level": {
|
||||
"name": "Power level"
|
||||
},
|
||||
"power_watts": {
|
||||
"name": "Power"
|
||||
},
|
||||
|
||||
@@ -154,6 +154,17 @@
|
||||
"on": "Aan"
|
||||
}
|
||||
},
|
||||
"cook_mode": {
|
||||
"name": "Kookmodus",
|
||||
"state": {
|
||||
"no_operation": "Niet actief",
|
||||
"micro_wave": "Magnetron",
|
||||
"micro_wave_grill": "Magnetron + grill",
|
||||
"grill": "Grill",
|
||||
"autocook": "Automatisch koken",
|
||||
"autocook_custom": "Automatisch koken (aangepast)"
|
||||
}
|
||||
},
|
||||
"cycle": {
|
||||
"name": "Programma"
|
||||
},
|
||||
@@ -438,6 +449,12 @@
|
||||
"burner_state": {
|
||||
"name": "Status brander {number}"
|
||||
},
|
||||
"cavity_state": {
|
||||
"name": "Status ovenruimte"
|
||||
},
|
||||
"cavity_temp": {
|
||||
"name": "Temperatuur ovenruimte"
|
||||
},
|
||||
"clean_level": {
|
||||
"name": "Reinigingsniveau"
|
||||
},
|
||||
@@ -571,6 +588,9 @@
|
||||
"power_energy_kwh": {
|
||||
"name": "Energieverbruik"
|
||||
},
|
||||
"power_level": {
|
||||
"name": "Vermogensniveau"
|
||||
},
|
||||
"power_watts": {
|
||||
"name": "Vermogen"
|
||||
},
|
||||
|
||||
@@ -27,24 +27,38 @@ def test_microwave_fixture_resolves_and_has_no_unbound_hrefs():
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# MICROWAVE_MODE -- SelectDesc with non-empty options
|
||||
# MICROWAVE_MODE -- SelectDesc reads its options live from supportedModes
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_microwave_mode_options_nonempty():
|
||||
assert len(microwave.MICROWAVE_MODE.entities[0].options) > 0
|
||||
_SUPPORTED_MODES_REP = {'x.com.samsung.da.supportedModes': [
|
||||
'NoOperation', 'MicroWave', 'MicroWaveGrill', 'Grill', 'Autocook', 'AutocookCustom',
|
||||
]}
|
||||
|
||||
|
||||
def test_microwave_mode_options_field():
|
||||
desc = microwave.MICROWAVE_MODE.entities[0]
|
||||
assert desc.options_field == 'x.com.samsung.da.supportedModes'
|
||||
|
||||
|
||||
def test_microwave_mode_write_round_trips():
|
||||
desc = microwave.MICROWAVE_MODE.entities[0]
|
||||
valid_mode = desc.options[1] # e.g. 'MicroWave'
|
||||
path, body = desc.write_fn(valid_mode, {})
|
||||
path, body = desc.write_fn('MicroWave', _SUPPORTED_MODES_REP)
|
||||
assert path == ['mode', 'vs', '0']
|
||||
assert body['x.com.samsung.da.modes'] == [valid_mode]
|
||||
assert body['x.com.samsung.da.modes'] == ['MicroWave']
|
||||
|
||||
|
||||
def test_microwave_mode_rejects_unknown():
|
||||
desc = microwave.MICROWAVE_MODE.entities[0]
|
||||
assert desc.write_fn('SpaghettiMode', {}) is None
|
||||
assert desc.write_fn('SpaghettiMode', _SUPPORTED_MODES_REP) is None
|
||||
|
||||
|
||||
def test_microwave_mode_rejects_when_not_in_live_supported_list():
|
||||
"""A mode absent from *this* device's live supportedModes is rejected
|
||||
even if another microwave model supports it -- the write must not fall
|
||||
back to a static vocabulary."""
|
||||
desc = microwave.MICROWAVE_MODE.entities[0]
|
||||
narrower_rep = {'x.com.samsung.da.supportedModes': ['NoOperation', 'MicroWave']}
|
||||
assert desc.write_fn('Grill', narrower_rep) is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user