Skip to content

fix(sound): WHO 22 area parameter and area-derived routing, modulation-aware tuner decode, station vs preset; WHO 16 volume 0..31, amplifier range, named WHATs (Encyclopedia audit, group 5) - #158

Closed
gdluck wants to merge 10 commits into
OpenWebNet-HA:masterfrom
gdluck:fix/who22-who16-area-volume

Conversation

@gdluck

@gdluck gdluck commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Requested by A

Review follow-up (GreenGrassBlueOcean, 2026-11-04) — applied in edf9ea8

The rule is understood: no change to decoded values, log strings or accepted builder domains without committed on-wire plant traces. Where the original description below still describes a reverted item, the diff and the CHANGELOG are the truth.

Blocker Action taken
1. turn_on_with_source The runtime DeprecationWarning and import warnings removed; the WHAT#MULTIMEDIA_TYPE#AREA vs source ambiguity is an evidence-boundary note in the docstring only (frame, log and validation as on master).
2. turn_off_atomic / route_source / source_address Everything back to master: no area= parameter or area helpers, turn_off_atomic("3#2#1") -> *22*0#4#0*3#2#1##, route_source default matrix_area=1 and master's log, no ValueError on route_source("2#1", 7) / ("6", 7), source_id / source_address not bounded to 1..9, no raise on 3#12#1.
3. OWNSoundDiffusionEvent requested_source on WHAT 1, preset, frequency_khz, human_readable_log and the area fallback back to master; the ~20 new decoders / properties (DIMENSION 2, 3, 4, 7, 17, 18, 19, 20, 21; WHAT 2, 22, 36..43, 55, 56) and builders (turn_on_atomic, select_station, request_station, store_station, set_equalizer_preset, request_equalizer_preset, next_preset, previous_preset) removed; master's isdigit() guards on WHERE / WHAT parameters, which the branch had dropped, are back.
4. WHO 16 All runtime changes reverted (set_volume 0..100, volume not None above 31, select_source("20", 2) rejected again, WHERE 100 is_source_event / log as master, the new WHAT properties removed); tests/test_sound.py and tests/test_sound_diffusion.py are byte-identical to master; tests/test_sound_audit_fixes.py deleted (its remaining tests asserted branch wording).

Kept: docstring notes only, on volume_up / volume_down (bare step-1 vs 3#STEP, the bt_multi oracle rows), turn_on_with_source (the ambiguity, written as an evidence boundary, not as a deprecation), send_rds_text (the bt_multi NACK note plus the absence of a client dimension 10 write in the published grammar) and open_paging / start_paging_voice (trace provenance). No crash-guard was kept because the branch added none: the dimension 5 / 11 trailing-empty-field change was a decoded-value change (None -> 101500), not a crash fix, so it was reverted. Every changed line in sound.py is a docstring line; frames, decoded values, logs and accepted domains are master's.

Committed plant evidence. There is no physical sound-plant trace for any of the reverted items beyond what master already pins; the MyHOME oracle_who22_tuner fixtures are emulator-derived, not on-wire. The only new WHO 22 evidence in this round is the bare-power-frame crash, which goes through #163 with its own oracle suite and golden pin. If a physical WHO 22 / WHO 16 capture would be worth having, say which frames; the capture instructions can be prepared for any user with a sound plant.

Summary

Before: in *22*WHAT#4#X*3#A#P## for WHAT 0 / 1 / 2 / 34 OWNd reads the second parameter X as a source, so turn_off_atomic hard-codes 0#4#0 and route_source defaults the target's area to 1 (misrouting area-2 speakers); the dimension 5 / 11 frequency decode applies the FM 10 / 100 kHz heuristic to AM bands and crashes on a trailing empty value; "preset" names the stored station (dimension 6) while the page's preset is dimension 19; several dimensions and WHATs the page defines are logged as unknown; some builders emit frames outside the published grammar without saying so; WHO 16 set_volume accepts 0..100 (page: 0..31, volume 50 refused by all twelve emulated gateways), select_source refuses amplifiers 20..90 (the six WHO 16 gateways accept them), WHERE 100 (all sources) is logged as a zone and the page's WHATs are "received command: N".

After: the second parameter is exposed as what_area (and requested_source only for WHAT 35), turn_on_atomic / turn_off_atomic / route_source derive the area from a 3#A#P / 4#A / 5#3#A#P target (explicit area= allowed, range 0..9 checked; 2#S / 6 targets keep master's area 0 since no area can be derived); the tuner decode is modulation-aware (AM in kHz marked as an inference) and never raises; station (dimension 6) and preset (dimension 19) are distinct with the old names kept as deprecated aliases; the page's dimensions 2 / 3 / 4 / 7 / 17 / 18 / 20 / 21#x and WHATs 5 / 6 / 22 / 36..43 / 55 / 56 decode with names; undocumented builders say so in their docstrings and source_address accepts 1..9 (published 1..4, 5..9 plant evidence); WHO 16 volume 0..31 (replies above 31 give None), amplifiers [1-9][0-9], WHERE 100 = all sources (is_source_event), and the page's WHATs carry is_* properties while the log keeps the "received command: N (meaning)" text MyHOME pins.

Context and how it was found. A asked for OWNd master to be checked against the OpenWebNet Encyclopedia functional pages (audit of 2026-10-28; 94 rows reproduced by running the code; confirmed by a firmware-oracle replay on the twelve emulated gateways and a whole-Encyclopedia search). This is the WHO 22 / WHO 16 group (rows D1..D7, S1..S4), built on #153. Oracle confirmation: *22*35#4#1#7*3#2#1## produces area-1 bus bytes and 35#4#2#7 area-2 bytes, so a defaulted area 1 misroutes area-2 speakers on nine gateways; WHO 16 volume 50 refused 12/12, 31 out on 6; *16*3*20## accepted by the six WHO 16 gateways; the bare *22*3*W## step sends step 00 on F454 / F459 / MyHomeServer1 and the same bytes as 3#1 elsewhere. The area reading of the second parameter is corroborated by the plant captures in the Encyclopedia's sound-matrix-routing review (*22*2#4#1*5#2#1## = area 1, source 1); the oracle could not distinguish the two readings (gateways copy the parameter verbatim), so turn_on_with_source is not changed, only deprecated.

Changes

  • OWNd/message/sound.py, WHO 22 (D1..D7, who-22-sound-diffusion/README.md "WHAT", "Historical touchscreen syntax", "Payloads", "Core parameters", "DIMENSION"): what_area for WHAT 0 / 1 / 2 / 34, requested_source for WHAT 35 only; turn_on_atomic (new) / turn_off_atomic / route_source with _validate_area (derived from the target or explicit, 0..9); turn_on_with_source(where, source_id, multimedia_type=4) keeps master's frame and emits a DeprecationWarning pointing to turn_on_atomic + route_source; modulation-aware dimension 5 / 11 decode; station vs preset (dimension 19) with deprecated aliases; volume_up / volume_down keep master's bare *22*3*W## / *22*4*W## for step 1 (fix(sound): reject non-positive step values in OWNSoundDiffusionCommand.volume_up/down #127 kept that form on purpose) and 3#STEP / 4#STEP above; named decoders for the page's dimensions and WHATs; source_address 1..9; send_rds_text, set_preset(..., freq), open_paging marked as outside the published grammar in their docstrings, not removed.
  • OWNd/message/sound.py, WHO 16 (S1..S4, who-16-sound-system/README.md "WHERE", "Dimension values", "WHAT values"): set_volume 0..31, replies above 31 -> None; select_source amplifiers [1-9][0-9]; WHERE 100 = all sources; is_* properties for WHATs 20 / 23 / 30 / 33 / 40 / 50 / 53 / 101 / 102 with the log text "received command: N (meaning)"; module docstring "Sound System".
  • CHANGELOG.md: [Unreleased] Added / Changed / Fixed bullets with the frames and sections; OWNd/message/__init__.py exports.

State Diagram

stateDiagram-v2
    direction TB
    state "WHO 22 area derivation for turn_on_atomic / turn_off_atomic / route_source" as A {
        [*] --> FromTarget : 3#A#P, 4#A, 5#3#A#P -> area A (0..9 checked)
        [*] --> Explicit : area= given -> used, 0..9 checked
        [*] --> SourceTarget : 2#S or 6 -> turn_on / turn_off keep area 0, route_source needs area=
        FromTarget --> Frame : *22*WHAT#4#AREA*TARGET## / *22*35#4#AREA#SOURCE*TARGET##
    }
    state "WHO 16 volume / amplifier" as V {
        [*] --> Volume : set_volume 0..31 -> *#16*W*#1*V##, reply above 31 -> None
        [*] --> Amplifier : select_source amplifier 01..99 (page WHERE table)
        [*] --> AllSources : WHERE 100 -> is_source_event
    }
Loading

Where are the traces

Subsystem Capture Plant Frames verified by this PR
WHO 22 area vs source OpenWebNet-Encyclopedia reverse-engineering/sound-matrix-routing.md plant captures; MyHOME#839 F500 traces (H/L4684 6.0.11) sound plant, F500 tuner *22*2#4#1*5#2#1## = area 1 / source 1; every OWNSoundDiffusionCommand call of MyHOME#839 keeps its signature and frame
WHO 22 / 16 builders own-firmware-oracle ownd-audit-frames replay (2026-10-28, emulated, two identical runs) and tests/golden/firmware_oracle.json twelve emulated gateways 35#4#1#7 vs 35#4#2#7 area bytes, volume 50 refused 12/12, amplifier 20 accepted on the six WHO 16 gateways, bare *22*3*W## step 00 on three

What does the Oracle say

Frame Result
*22*35#4#1#7*3#2#1## / *22*35#4#2#7*3#2#1## area-1 / area-2 telegram bytes on 9 gateways (area derived from the WHAT parameter)
*22*0#4#0*3#2#1## vs *22*0#4#2*3#2#1## both forwarded, parameter copied verbatim: the reading is not distinguishable on the bus, hence the deprecation rather than a change
*#16*21*#1*50## / *#16*21*#1*31## refused 12/12 / out on 6
*16*3*20## accepted by the six WHO 16 gateways
*22*3*3#1#1## vs *22*3#1*3#1#1## step 00 vs 01 on F454 / F459 / MyHomeServer1, identical bytes on the other six (#127 kept the bare form)

Testing

  • tests/test_sound_audit_fixes.py (new, 54 tests) plus updates in tests/test_sound.py and tests/test_sound_diffusion.py; pytest.deprecated_call on turn_on_with_source (the project turns DeprecationWarning into errors).
  • Full suite: 2855 passed (plus the environment-dependent test_runtime_version_matches_installed_package); scripts/verify_coverage.py 7,297 / 7,297, sound.py 100 % statements and branches (master had 11 partial branches); ruff check, mypy OWNd, scripts/verify_library_standards.py, scripts/sync_documentation.py --check pass.
  • MyHOME v2-phase1-architecture suite against this branch: 5187 passed, 1 deselected (the pin-bump doc test).

Precedent check

Kept from #153: store_station 1..5, steps 5001.. / 5101.. and 5#STEP / 6#STEP, bare *22*5*W## = auto-seek, no WHAT 7 / 8, no dimension 10 decode, the ownkb:* ids; from #127: the bare single-step volume frame (the 3#1 change from the first revision was reverted); from #128: volume bounds on WHO 22; profile wording untouched. Changed back after review: derived areas range-checked, source_address 1..9, 2#S / 6 targets do not raise, turn_on_with_source deprecated instead of silently changed; the generic "received command: N" log text kept for MyHOME's pin.

🤖 Generated with Claude Code

gdluck and others added 6 commits November 3, 2026 09:50
…named WHATs

Audit 2026-10-28 vs Encyclopedia, rows S1, S2, S3, S4 (sound_cen_doorentry.md).

- S1: set_volume refuses values above 31 and a dimension 1 report above 31
  reads as None. Encyclopedia who-16-sound-system#dimension-values and
  #volume (Volume 0..31); firmware oracle: *OpenWebNet-HA#16*21*michnovka#1*50## nack/silent on
  12/12 gateways, *OpenWebNet-HA#16*21*michnovka#1*31## out on the six WHO 16 gateways
  (ORACLE_CONFIRM S1); BtExperience writes P*31/100 (ENCY_CONFIRM SD-S1).
- S2: select_source accepts amplifiers [1-9][0-9] (page WHERE 01..99,
  #amplifier-address-structure, #matrix-source-routing); *16*3*20## is
  accepted by the six WHO 16-forwarding gateways (ORACLE_CONFIRM S2).
  Environment 0 stays refused (10S is a source address).
- S3: WHERE 100 is every source (is_source_event, is_all_sources); module
  docstring names WHO 16 Sound System, WHO 22 Sound Diffusion and the
  WHO 7 OWNAVCommand instead of "audio/video door entry".
- S4: WHAT 20/23, 30/33, 40, 50/53, 101/102, 2001..2015 / 2101..2115
  decode by name with log lines (#what-values; *16*20*100## / *16*23*100##
  cycle sources on the sound-matrix-routing.md plant, ENCY_CONFIRM SD-S4);
  is_stereo_channel tells the base-band and stereo-channel variants apart.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… preset, volume step

Audit 2026-10-28 vs Encyclopedia, rows D1..D7 (sound_cen_doorentry.md).

- D1: the second WHAT parameter of WHAT 0/1/2/34 is the AREA
  (who-22-sound-diffusion#what "WHAT#MULTIMEDIA_TYPE#AREA",
  #historical-touchscreen-syntax-and-extensions *22*0#4#AREA*3#AREA#POINT##,
  #source-activity-and-rds WHAT 2#4#AREA; plant captures *22*2#4#1*5#2#1##
  = area 1 / source 1, ENCY_CONFIRM SD-D1). The event exposes it as
  what_area (and area for a source WHERE); requested_source is only the
  WHAT 35 source. turn_on_atomic (new; turn_on_with_source is its
  deprecated spelling, the source is not on the wire) and turn_off_atomic
  put the target's area in that slot instead of the source / a constant 0.
  Gateways copy the parameter verbatim onto the bus (ORACLE_CONFIRM D1).
- D2: route_source derives the area from 3#A#P / 4#A / 5#3#A#P, accepts
  area= (matrix_area= kept as the older spelling) and never defaults to 1;
  the gateways address the bus from that parameter (ORACLE_CONFIRM D2:
  35#4#1#7 -> $04A10F8113, 35#4#2#7 -> $04A10F8123). Source 1..9.
- D3: dimension 5/11 decode follows the modulation (1 FM 10/100 kHz
  heuristic; 2/3/4 AM in kHz, an inference); unknown modulation and a
  trailing empty field fail closed / are ignored (#payloads-and-concrete-
  operations, #core-parameters, #published-inconsistencies).
- D4: dimension 19 is the preset (2..11, 16..25), dimension 6/11 and WHAT 33
  the station: new station, is_station_cleared, equalizer_preset; preset
  reads dimension 19 and stays a deprecated alias of station otherwise;
  select_station / request_station / store_station / set_equalizer_preset /
  request_equalizer_preset / next_preset / previous_preset builders, the
  old names kept as deprecated aliases (ENCY_CONFIRM SD-D4).
- D5: volume_up / volume_down always emit 3#STEP / 4#STEP; the bare form is
  not on the page and F454, F459, MyHomeServer1 send it with step 00
  (ORACLE_CONFIRM D5); OpenWebNet-HA#127 kept it on purpose, noted in the docstring.
- D6: dimensions 2/3/4/7/17/18/20/21#x and WHATs 22, 36..43, 55/56 decode
  into named properties with log lines (ENCY_CONFIRM SD-D6).
- D7: source_address limited to 1..4 (#core-parameters); send_rds_text,
  set_preset(..., frequency_khz=), open_paging and start_paging_voice are
  marked "not in the published grammar" and kept (ENCY_CONFIRM SD-D7,
  ORACLE_CONFIRM D7).

Oracle rows produced by the fixed builders are pinned from
tests/golden/firmware_oracle.json (sound-who22): *22*0#4#1*3#1#1##,
*22*1#4#1*3#1#1##, *22*35#4#1#1*3#1#1##, *22*35#4#2#7*3#2#0##,
*22*3#1*3#1#1##.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Precedent gate on audit 2026-10-28 D5: OpenWebNet-HA#127 deliberately preserves both
the bare single-step `*22*3*WHERE##` / `*22*4*WHERE##` and the multi-step
`*22*3#STEP*WHERE##` / `*22*4#STEP*WHERE##` forms, and OpenWebNet-HA#128 bounds the
step to 1..31; changing the step-1 frame needs a OpenWebNet-HA#127 reopen with the
maintainer. `volume_up` / `volume_down` build the master frames again;
the ORACLE_CONFIRM D5 finding (F454 / F459 / MyHomeServer1 put the bare
form on the bus with step `00`) stays as a docstring note and in the
CHANGELOG, and the D5 test pins both forms.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…HOME pin)

MyHOME tests/test_message.py::test_own_sound_event_unknown_command pins
`received command: 30` in the log of `*16*30*22##`. The S4 named WHATs
(20 / 23 / 30 / 33 / 40 / 50 / 53 / 101 / 102) keep that generic text and
add the name in parentheses; the new `is_*` properties are unchanged.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…urce targets, turn_on_with_source deprecated not changed (review follow-up)

Review follow-up on audit 2026-10-28 D1, D2, D7:

- `_validate_area` range-checks the area derived from `3#A#P`, `4#A`,
  `5#3#A#P` (0..9) like an explicit `area=`; `3#12#1` raises.
- `source_address` accepts 1..9 like `route_source`: the page's 1..4 is
  the published domain, 5..9 is plant evidence (source 7 is the traced FM
  tuner, MyHOME#839; `*22*35#4#2#7*3#2#0##` in the firmware oracle).
- `turn_on_atomic` / `turn_off_atomic` keep master's area `0` for a
  source (`2#S`) or general (`6`) target instead of raising (the area
  cannot be derived from such a target; `area=` still overrides);
  `route_source` still requires `area=` there.
- `turn_on_with_source` builds master's `*22*1#TYPE#SOURCE*WHERE##`
  unchanged and emits a DeprecationWarning pointing to `turn_on_atomic`
  + `route_source`; the docstring records that the oracle could not
  distinguish the two readings of the second parameter (ORACLE_CONFIRM
  D1, NOT DISTINGUISHED) while the plant captures read it as the area.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

…= left 10)

The audit's D6 row was right: the Encyclopedia who-22
#tone-balance-and-presets gives the balance magnitude as the digits
after the side divided by 3 (030 -> left 10, 115 -> right 5), and the
`balance` docstring already said "030 is left 10", but `balance_level`,
the human-readable log and the CHANGELOG reported the raw digits (left
30 / 15). `balance_level` now returns the divided-by-3 magnitude so the
log reads "left 10"; the raw string stays available as `balance`.
MyHOME reads none of these properties. Tones stay raw 1..63 (the
published domain; the raw/3-10 form is the power-amp UI only).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@gdluck

gdluck commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Fixed the balance magnitude (commit e173498, normal push). A spec-vs-code check flagged that the balance log and CHANGELOG said 030 is "left 30" while this PR's own balance docstring, and the Encyclopedia (who-22-sound-diffusion#tone-balance-and-presets: "remaining digits divided by 3 give magnitude", tests 030 -> left 10, 115 -> right 5), say left 10. balance_level now returns the divided-by-3 magnitude so the log reads "left 10" / "right 5"; the raw string stays on balance ("030"). The touchscreen steps balance in threes, so the division is exact for the values it emits. Tones stay raw 1..63 (the published domain; the raw/3-10 form is the power-amp UI only, per the same page). balance_level and balance are not read by MyHOME. Audit tests 70 passed, full OWNd suite green, MyHOME sound tests 203 passed.

…§3.4.1.1), not undocumented

A re-check against the official Legrand WHO 22 v1.1 PDF (§3.4 "Dimension
writing" → 3.4.1.1 "State Memorized Station - Dimension = 11",
*OpenWebNet-HA#22*5#2#S*OpenWebNet-HA#11*MOD*FREQ*ST## -> Ack) shows set_preset's dimension 11
write IS in the published grammar; the Encyclopedia functional summary
only omits it. Corrected the set_preset docstring, CHANGELOG and the D7
test to cite §3.4.1.1, keeping the honest note that the effect on a
physical tuner is not yet confirmed (D50). The dimension 10 RDS-text,
WHAT 19/20 paging builders stay marked undocumented; they are unrelated.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@gdluck

gdluck commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Correction on D7, following a re-check against the official Legrand WHO 22 v1.1 PDF (commit 06c51a1, normal push). My earlier "not in the published grammar" for the dimension 11 write was wrong: §3.4 "Dimension writing" → 3.4.1.1 "State Memorized Station - Dimension = 11" documents the client write *#22*5#2#S*#11*MOD*FREQ*ST## with an Ack. So set_preset(..., frequency_khz=) builds a published frame; the Encyclopedia functional summary just omits it (worth a note upstream). The docstring, CHANGELOG and the D7 test now cite §3.4.1.1, and keep only the honest caveat that the effect on a physical tuner is not yet confirmed (consistent with the WHO 22-only tuner path noted in MyHOME #839). The genuinely undocumented builders here (RDS-text dimension 10, WHAT 19/20 paging) are unrelated and still marked as such. Sound tests 227 passed, ruff and mypy clean.

@gdluck

gdluck commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

One finding from the 2026-11-04 cross-check that lands squarely in this PR's area. The bare WHO 22 power frame, WHAT 0 / 1 without #mmtype#area (*22*1*WHERE##, *22*0*WHERE##), crashes the sound daemon on F461 020011 and F454 020051 on every target type (source 2#S, amplifier 3#A#P, area 4#A, all sources 6: all nack/crash in the firmware oracle, two byte-identical runs each), while MH202 refuses it without crashing. The published forms, *22*1#<mmtype>#<area>*…## / *22*0#<mmtype>#<area>*…## (WHO 22 PDF 3.1.1 / 3.1.2 / 3.2.1, <area> the output area 1..9), go out (ack/out) on every target.

That also confirms, from the PDF, the reading this PR already takes in turn_on_atomic / turn_off_atomic: the second parameter is the area, not the source. The pre-b13 turn_on_with_source writes the source into that slot, so the fix is to point callers at the atomic builders rather than add an OFF twin of turn_on_with_source.

OWNSoundDiffusionCommand.turn_on / turn_off still build exactly the bare form. I've prepared a single commit on top of this branch (fix/who22-bare-power-guard-on-158) that refuses the bare frame in the send layer the way the WHO 4 dimension 22 and WHO 18 dimension 52 crash frames are refused, deprecates turn_on / turn_off (they keep their frame for callers that pin it, with a DeprecationWarning pointing to the atomic builders), and adds no new builder. OWNCommand.parse stays permissive. Full suite and MyHOME's conformance suite pass.

MyHOME is unaffected: it powers audio zones through WHO 16 (OWNSoundCommand.turn_on -> *16*3*WHERE##, turn_off -> *16*13*WHERE##) and never builds the WHO 22 form. Happy to push it onto this PR, or open it as a follow-up stacked on #158 — your call.

@GreenGrassBlueOcean GreenGrassBlueOcean left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the WHO 22 / WHO 16 audit and for carefully recording where the firmware oracle could and could not distinguish parameters (ORACLE_CONFIRM D1, NOT DISTINGUISHED).

Under our standing rule — if we don't have real, committed on-wire plant traces from physical hardware, we don't change behaviour — virtually all runtime changes in this PR rest on WHO 22 / WHO 16 specification tables, archived touchscreen code (libqtdevices), or empty-bus gateway forwarding rather than real physical sound-system captures, and several break existing master contracts and tests.

What can stay

  1. Docstring evidence-boundary notes: Recording the #127 / bt_multi step-1 vs. 3#1 oracle notes on volume_up / volume_down, the bt_multi NACK note on send_rds_text, the paging trace provenance notes on open_paging / start_paging_voice, and the WHAT#MULTIMEDIA_TYPE#AREA (ownkb:claim:c002727) vs. turn_on_with_source ambiguity in docstrings.
  2. Pure parser crash-guards (if any malformed/truncated frame previously raised uncaught ValueError / IndexError during OWNMessage.parse), without changing decoded values on valid frames.

Blockers — Behaviour changes without committed plant traces (please revert)

# Symbol / File Behaviour change without real plant trace Required action
1 OWNSoundDiffusionCommand.turn_on_with_source in OWNd/message/sound.py Emits a runtime DeprecationWarning (warnings.warn(...)). Because OWNd and downstream suites run with filterwarnings = ["error"], emitting DeprecationWarning is a breaking runtime change — and the docstring itself notes ORACLE_CONFIRM D1, NOT DISTINGUISHED (gateways copy the nibble verbatim onto the bus and cannot settle area vs. source without physical sound hardware). Remove warnings.warn(..., DeprecationWarning); keep the explanation in the docstring only.
2 OWNSoundDiffusionCommand.turn_off_atomic, route_source, & source_address in OWNd/message/sound.py Changes turn_off_atomic("3#2#1") wire output (*22*0#4#0*3#2#1## -> *22*0#4#2*3#2#1##), changes route_source("3#2#1", 7) default matrix_area (1 -> 2) and human_readable_log, newly raises ValueError on route_source("2#1", 7) / route_source("6", 7) without area=, restricts source_id / source_address to 1..9 (source_address(10) now raises), and raises on multi-digit areas like 3#12#1 — all without multi-area physical sound-plant traces confirming speaker/matrix behaviour (ownkb:claim:moc-c007654 explicitly cautions that libqtdevices does not validate every WHAT/WHERE combination). Revert turn_off_atomic, route_source, and source_address wire output, parameter defaults, validation bounds, and logs to master.
3 OWNSoundDiffusionEvent existing properties (requested_source, preset, frequency_khz, human_readable_log) & ~20 new speculative builders/decoders in OWNd/message/sound.py Drops requested_source to None on WHAT 1, repurposes preset to DIMENSION 19 equalizer preset, decodes AM frequencies in kHz (explicitly noted as "an inference, the page gives the bands but no AM unit"), and adds speculative builders/decoders for equalizer/tone/balance/loudness/3D/play-status (DIMENSION 2, 3, 4, 7, 17, 18, 19, 20, 21, WHAT 22, 36..43, 55, 56, turn_on_atomic, select_station, set_equalizer_preset, etc.) with zero physical sound-plant traces (DIMENSION 19 is not even in the firmware oracle). Revert OWNSoundDiffusionEvent changes and hold back unverified WHO 22 builders/decoders until backed by real sound-system bus captures.
4 OWNSoundCommand & OWNSoundEvent (WHO 16) in OWNd/message/sound.py Narrows set_volume from 0..100 to 0..31 and drops OWNSoundEvent.volume to None above 31 (forcing edits to tests/test_sound.py where volume == 37 and set_volume("21", 40) were pinned!), loosens select_source("20", 2) (reversing an explicit pytest.raises(ValueError) assertion in tests/test_sound.py), changes WHERE 100 is_source_event / subject log, and adds speculative WHAT 20/23/30/33/40/50/53/101/102/2001..2115 properties (forcing test_unknown_sound_command_has_diagnostic_log to change from *16*30*22## to *16*60*22##), all with zero physical WHO 16 plant traces. Revert all WHO 16 runtime changes in OWNSoundCommand and OWNSoundEvent to master.

gdluck and others added 2 commits November 4, 2026 16:46
…d plant traces; keep the docstring notes

Maintainer review on OpenWebNet-HA#158: without real, committed on-wire plant traces from
physical hardware, runtime behaviour does not change. Spec tables, the
Encyclopedia, libqtdevices and empty-bus gateway forwarding are context only.
OWNd/message/sound.py, tests/test_sound.py and tests/test_sound_diffusion.py
are back at master except for docstrings; tests/test_sound_audit_fixes.py is
removed.

Blocker 1, turn_on_with_source:
- reverted: the runtime warnings.warn(DeprecationWarning) and the `import warnings`.
- kept: the WHAT#MULTIMEDIA_TYPE#AREA vs source ambiguity, as a docstring note
  only (frame, log and validation unchanged).

Blocker 2, turn_off_atomic / route_source / source_address:
- reverted: the area= parameter and derived-area logic, _who22_target_area /
  _validate_area helpers, turn_off_atomic back to `*22*0#TYPE#0*WHERE##`,
  route_source back to matrix_area=1 default and master's log, no ValueError
  on route_source("2#1", 7) / ("6", 7) without area=, source_id /
  source_address back to "positive" (no 1..9 bound), no raise on multi-digit
  areas such as 3#12#1.
- kept: nothing at runtime.

Blocker 3, OWNSoundDiffusionEvent:
- reverted: requested_source on WHAT 1, preset (dimension 19 reading and the
  station alias), frequency_khz (modulation-aware AM / trailing empty field),
  human_readable_log changes, the what_area fallback into area, the removal of
  master's isdigit() guards on WHERE / WHAT parameters, and the new decoders
  for dimensions 2, 3, 4, 7, 17, 18, 19, 20, 21 and WHATs 2, 22, 36..43, 55,
  56 with their properties (station, is_station_cleared, what_area,
  frequency_band_name, high/medium/low_tones, play_status, balance*, sound_3d,
  loudness, equalizer_*, tone_band, tone_step, balance_step,
  is_sliding_request, is_next_preset, is_previous_preset, is_source_active).
- reverted builders: turn_on_atomic, select_station, request_station,
  store_station, set_equalizer_preset, request_equalizer_preset, next_preset,
  previous_preset, the deprecated-alias wording of set_preset / request_preset
  / store_preset, and the module docstring rewrite.
- kept: the volume_up / volume_down docstrings (OpenWebNet-HA#127 / OpenWebNet-HA#128 bare step-1 vs
  3#STEP form and the bt_multi oracle note), the send_rds_text bt_multi NACK
  note (master's text plus the published-grammar sentence), and the paging
  trace-provenance notes on open_paging / start_paging_voice.

Blocker 4, OWNSoundCommand / OWNSoundEvent (WHO 16):
- reverted: set_volume back to 0..100, OWNSoundEvent.volume no longer None
  above 31, select_source("20", 2) back to master's rejection, WHERE 100
  is_source_event / is_all_sources / "All audio sources" log back to master,
  the WHAT 20 / 23 / 30 / 33 / 40 / 50 / 53 / 101 / 102 / 2001..2115
  properties and logs removed; tests/test_sound.py back to master
  (volume == 37, set_volume("21", 40), ValueError on select_source("20", 2),
  test_unknown_sound_command_has_diagnostic_log on *16*30*22##).
- kept: nothing at runtime.

Crash-guards: none kept; the branch added no parser guard that prevented an
uncaught exception in OWNMessage.parse (it removed master's isdigit() guards,
which are restored).

CHANGELOG: the [Unreleased] Added / Changed / Fixed entries of this branch are
removed; one Changed entry describes the docstring notes and states that
frames, decoded values, logs and accepted domains are unchanged. The file now
carries master's 2.0.0b13 heading.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@gdluck

gdluck commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Applied in edf9ea8 (pushed; the description now carries the same follow-up table at the top).

Blocker Action taken
1. turn_on_with_source The runtime DeprecationWarning and import warnings removed; the WHAT#MULTIMEDIA_TYPE#AREA vs source ambiguity is an evidence-boundary note in the docstring only (frame, log and validation as on master).
2. turn_off_atomic / route_source / source_address Everything back to master: no area= parameter or area helpers, turn_off_atomic("3#2#1") -> *22*0#4#0*3#2#1##, route_source default matrix_area=1 and master's log, no ValueError on route_source("2#1", 7) / ("6", 7), source_id / source_address not bounded to 1..9, no raise on 3#12#1.
3. OWNSoundDiffusionEvent requested_source on WHAT 1, preset, frequency_khz, human_readable_log and the area fallback back to master; the ~20 new decoders / properties (DIMENSION 2, 3, 4, 7, 17, 18, 19, 20, 21; WHAT 2, 22, 36..43, 55, 56) and builders (turn_on_atomic, select_station, request_station, store_station, set_equalizer_preset, request_equalizer_preset, next_preset, previous_preset) removed; master's isdigit() guards on WHERE / WHAT parameters, which the branch had dropped, are back.
4. WHO 16 All runtime changes reverted (set_volume 0..100, volume not None above 31, select_source("20", 2) rejected again, WHERE 100 is_source_event / log as master, the new WHAT properties removed); tests/test_sound.py and tests/test_sound_diffusion.py are byte-identical to master; tests/test_sound_audit_fixes.py deleted (its remaining tests asserted branch wording).

Kept: docstring notes only, on volume_up / volume_down (bare step-1 vs 3#STEP, the bt_multi oracle rows), turn_on_with_source (the ambiguity, written as an evidence boundary, not as a deprecation), send_rds_text (the bt_multi NACK note plus the absence of a client dimension 10 write in the published grammar) and open_paging / start_paging_voice (trace provenance). No crash-guard was kept because the branch added none: the dimension 5 / 11 trailing-empty-field change was a decoded-value change (None -> 101500), not a crash fix, so it was reverted. Every changed line in sound.py is a docstring line; frames, decoded values, logs and accepted domains are master's.

Committed plant evidence. There is no physical sound-plant trace for any of the reverted items beyond what master already pins; the MyHOME oracle_who22_tuner fixtures are emulator-derived, not on-wire. The only new WHO 22 evidence in this round is the bare-power-frame crash, which goes through #163 with its own oracle suite and golden pin. If a physical WHO 22 / WHO 16 capture would be worth having, say which frames; the capture instructions can be prepared for any user with a sound plant.

@gdluck
gdluck marked this pull request as draft November 4, 2026 15:14
@gdluck
gdluck marked this pull request as ready for review November 4, 2026 16:45

@GreenGrassBlueOcean GreenGrassBlueOcean left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Review: PR #158

Verdict: REQUEST CHANGES / RECOMMEND CLOSING
Resolution on core question: Does this solve a real problem? No.


1. Executive Summary & Problem Assessment

This PR does not solve an active production problem.

  • In its current state: It is a hollowed-out PR. Following the reversion in edf9ea8, the diff touches only two files, introducing 42 lines of docstring commentary across four methods in OWNd/message/sound.py and a 3-line changelog addition. Furthermore, it sits in a broken merge state (mergeable_state: dirty) due to an unresolved conflict in CHANGELOG.md.
  • In its original state: It did not solve a problem either; it introduced multiple regressions. The initial commits attempted a speculative rewrite of WHO 22 (Sound Diffusion) and WHO 16 (Sound System) based solely on reading documentation tables. Merging those changes would have broken Home Assistant volume sliders, raised unexpected ValueErrors in automations, and degraded parser sanitization.
  • The one genuine issue unearthed during this investigation: The discovery that bare WHO 22 power frames (*22*1*WHERE## / *22*0*WHERE##) crash gateway daemons on 8 out of 12 firmware models is a real issue. However, that fix does not live in this PR—it was properly isolated and implemented in PR #163 with dedicated oracle test suites.

2. Protocol & Architecture Review

The "Spec Audit" Anti-Pattern

This PR began as an audit against OpenWebNet documentation tables and manufacturer PDF specifications. In OpenWebNet development, the epistemic hierarchy must always be:

$$\text{Spec PDF} < \text{Disassembly / Daemon strings} < \text{Firmware Oracle (Emulated)} < \text{Committed On-Wire Plant Traces}$$

Manufacturer documentation frequently contains draft definitions, copy-paste errors, and syntax that never shipped to physical hardware. Modifying runtime decoding, builder parameters, or domain validation without committed on-wire bus captures violates our empirical standard.

Why the Runtime Changes Had to Be Reverted (edf9ea8)

Commit edf9ea8 correctly rolled back the speculative changes after review. Specifically:

  1. WHO 22 Area vs. Source Ambiguity (WHAT#4#X):
    The PR originally asserted that the second parameter of *22*1#4#X*WHERE## is strictly an output AREA. However, firmware oracle analysis shows that gateways (bt_multi) copy that nibble verbatim into bus telegrams ($0490018213), making emulator behavior indifferent to semantics. In real-world single-area installations, traces show this field carrying the source ID. Deprecating turn_on_with_source and forcing derived area routing would have broken working single-zone setups.
  2. WHO 16 Volume Clamping (0..31 vs. 0..100):
    Restricting set_volume to <= 31 and returning None for reports > 31 violated downstream client contracts. Home Assistant's media_player expects a 0..100% / float scale. Any physical gateway or software component operating on a percentage basis would cause the entity state to collapse to None or throw exceptions on volume changes.
  3. Domain & Route Restrictions:
    Enforcing strict 1..9 boundaries and raising ValueError on source targets in route_source added runtime failure modes to existing scripts without evidence of bus errors.
  4. Parser Sanitization:
    The initial revision dropped existing isdigit() checks on WHERE/WHAT parameters, weakening parser resilience against malformed frames.

3. PR Hygiene & Git Integrity

  1. Severe Description/Diff Desynchronization:
    The PR title (fix(sound): WHO 22 area parameter and area-derived routing, modulation-aware tuner decode, station vs preset; WHO 16 volume 0..31, amplifier range, named WHATs...) and the PR body describe 15+ substantive features, new properties, and state diagrams that are completely absent from the actual diff. Merging this PR under its current title and description would severely degrade git history and changelog traceability.
  2. Unresolved Merge Conflict:
    Commit b0b5efeb introduced an unmerged conflict marker in CHANGELOG.md. The branch cannot be merged cleanly.

4. Breakdown of Claims vs. Reality

Claimed Fix Does it solve a real problem? Analysis
WHO 22 Area vs. Source routing ❌ No Speculative. Conflates theoretical documentation with physical multi-area bus behavior. Reverted.
WHO 16 Volume clamped to 0..31 ❌ No Causes downstream breaking changes in Home Assistant media_player. Reverted.
Station (Dim 6) vs. Preset (Dim 19) ❌ No Synthetic API churn with no reported integration defect. Reverted.
AM Tuner trailing field handling ❌ No Changed decoded values without crash evidence; was not a parser panic fix. Reverted.
Bare Power Frame Crash (*22*1*W##) ⚠️ Yes (Real Bug) Not resolved here. Extracted and properly addressed in PR #163 with dedicated oracle suites.
Retained Docstrings (Current Diff) ❌ Trivial Clarifies documentation discrepancies, but does not warrant keeping an open, conflicted 10-commit PR.

5. Recommendation & Next Steps

  1. Close this PR without merging.
  2. If the docstring notes detailing the evidence boundaries on volume_up, turn_on_with_source, send_rds_text, and open_paging are desired in the codebase, open a clean, single-commit PR titled:
    docs(sound): add evidence boundary notes to WHO 22 sound diffusion docstrings
  3. Direct testing and review efforts to PR #163, which correctly protects gateways from the daemon crash without breaking API contracts or downstream consumers.

@gdluck gdluck closed this Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants