Skip to content

Expose which outputs a button's programming can change - #135

Open
rqi14 wants to merge 3 commits into
thecynic:masterfrom
rqi14:add-button-affected-outputs
Open

rqi14 wants to merge 3 commits into
thecynic:masterfrom
rqi14:add-button-affected-outputs

Conversation

@rqi14

@rqi14 rqi14 commented Sep 2, 2026

Copy link
Copy Markdown

What this adds

Button.affected_outputs — a mapping of output integration id to the level the
button's programming assigns.

The XML database already describes what every keypad button does. Its Press
action carries presets whose PresetAssignments either set an output level
directly (GOTO_LEVEL) or recall a scene in some area (GOTO_SCENE), which in
turn holds level assignments. The parser skipped all of it, so a consumer
holding a Button had no way to know which loads it drives.

Scene references are resolved through a table built up front, because the area
that owns a scene is usually not the area that owns the button recalling it —
wall keypads tend to live in their own area.

Why it is useful

A consumer that learns a button was pressed currently has two options: refresh
every output in the system, or refresh nothing. With this it can refresh
exactly the outputs that button can change.

On the project I tested against that is the difference between 112 queries and
a median of 8.

It also opens the door to updating state optimistically on a press and
confirming afterwards, though that is the caller's choice — see the caveat
below.

What the values do and don't mean

The keys are dependable: pressing the button can change exactly those
outputs and no others.

The values are what the programming assigns, which is not always where the
outputs land. A toggle button's result depends on the current state, and a
raise/lower button's depends on how long it is held. The docstring says so, and
callers that need certainty should use the keys and query.

Compatibility

No behaviour change for existing consumers. The new constructor argument
defaults to empty, and nothing in the library reads it.

Testing

Four tests in tests/test_parser.py against a new fixture covering a direct
GOTO_LEVEL assignment, a GOTO_SCENE resolved across areas, a button with no
programming, and that the returned mapping is a copy.

Full suite: 73 tests pass, mypy clean.

Also validated against a real HomeWorks QS project — 153 buttons, 112 outputs,
75 buttons with programming, the largest recalling a scene touching 106 outputs
across several areas. Results matched an independently written parser on all 75.


🤖 Generated with Claude Code

The XML database already describes what every keypad button does: its Press
action carries presets whose assignments either set an output level directly
(GOTO_LEVEL) or recall a scene in some area (GOTO_SCENE), which in turn holds
level assignments. The parser skipped all of it, so a consumer holding a Button
had no way to know which loads it drives.

Parse it and expose it as `Button.affected_outputs`, a mapping of output
integration id to the level the programming assigns. Scene references are
resolved through a table built up front, because the area owning a scene is
usually not the area owning the button that recalls it -- wall keypads tend to
live in their own area.

The keys are dependable: pressing the button can change exactly those outputs.
That is enough to refresh the right subset after a press instead of querying
every output in the system. The values are what the programming assigns, which
is not always where the outputs land -- a toggle depends on current state, a
raise/lower on how long it is held -- so they are documented as a prediction to
confirm rather than as fact.

No behaviour changes for existing consumers: the new constructor argument
defaults to empty and nothing else reads it.

Validated against a real HomeWorks QS project: 153 buttons, 112 outputs, 75
buttons with programming, the largest recalling a scene that touches 106
outputs across several areas.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@rqi14

rqi14 commented Sep 2, 2026

Copy link
Copy Markdown
Author

The red checks are the same repository CI issue described in #133, not anything from this change: all seven jobs fail at actions/checkout because python-app.yml and type_checking.yml trigger on pull_request_target, which GitHub Actions now refuses to check out fork code under. Nothing in the branch runs.

Locally on this branch: full suite passes and mypy is clean; the added tests fail on master as intended.

@rqi14

rqi14 commented Sep 10, 2026

Copy link
Copy Markdown
Author

Updated this branch to current master (including the CI trigger fix from #137). Local verification on the merged result passes: 74 unit tests and mypy. GitHub created fresh pull_request workflow runs, but they are waiting for repository-admin approval: Python application and Type Checking. Once approved, these will test head 8bd3f37 normally.

@cdheiser

Copy link
Copy Markdown
Collaborator

Nice parsing work, but I want to understand the motivation before reviewing it.

Zone monitoring is already on — _do_login sends #MONITORING,5,1, so the repeater pushes ~OUTPUT,<id>,1,<level> whenever a level changes, including from a button press or scene recall. That lands in Output.handle_update and dispatches LEVEL_CHANGED to anything subscribed. Home Assistant runs entirely off those callbacks with should_poll = False and issues zero queries on a keypad press.

So the "112 queries vs a median of 8" comparison is against a refresh that shouldn't need to happen at all. What are you seeing that makes it necessary? If you're subscribed and ~OUTPUT genuinely doesn't arrive after a press, that's a bug I'd rather find than work around — debug logs of the raw lines after a press would tell us.

One bug regardless: the docstring says this reads the button's Press action, but findall('./Actions/Action') takes every action, and affected.update(levels) means Press/Release/Hold merge with last-one-wins. The fixture only has a single Press action, so no test catches it.

A button is programmed per action: Press, Release, Hold and MultiTap can each
drive a different set of outputs to different levels. The previous revision
folded every action into one mapping, so a conflicting output took whichever
level the XML happened to list last -- a value with no meaning.

affected_outputs is now the union of output ids across actions, which is what a
consumer needs to refresh the right subset after a button event. The level
assignments move to affected_outputs_by_action, keyed by the action name, so a
caller that knows which action fired can use the levels that belong to it.

The fixture gains a button whose Press and Release drive the same output to
conflicting levels; the old code could not have passed it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rqi14

rqi14 commented Sep 14, 2026

Copy link
Copy Markdown
Author

The behavior of my HomeWorks QS processor running OS 15.13.2 (June 2, 2023) motivates this PR. With #MONITORING,5,1 enabled, keypad ~DEVICE events were received normally. I then pressed a Palladiom keypad scene which changed DALI outputs. However, the session received no ~OUTPUT messages for those level changes. Keypad, LED, and occupancy notifications were present. Only the zone-level notifications were absent.

Everything the session received across the whole capture:

    8.6  TX ?MONITORING,5
    8.6  ~MONITORING,5,1
  136.5  ~DEVICE,138,1,3        <- keypad 138, component 1, press
  136.9  ~DEVICE,146,81,9,1     <- LED on

Outputs 57-60 read 0.00 before the press and 50.00 after it. Sixteen outputs changed. ?OUTPUT on the same session returns the correct levels, so the session is not being starved.

As a result, Home Assistant's push-only lutron entities remained at their startup values. affected_outputs allows a caller to identify the outputs that may be changed by the button and query only those outputs. It is a fallback for this observed QS behavior, not a replacement for ~OUTPUT notifications on controllers where they work.

I have not yet tested a direct GOTO_LEVEL/manual dimmer change because my contractor did not configure these functionalities for any button. I cannot test the behavior on these scenarios.

You are right about the action merge, and I have fixed it. Press, Release, Hold and MultiTap are no longer folded together: affected_outputs is now the union of output ids, and the level assignments moved to affected_outputs_by_action, keyed by action name. The fixture now covers a button whose Press and Release drive the same output to conflicting levels.

@cdheiser

cdheiser commented Sep 14, 2026 via email

Copy link
Copy Markdown
Collaborator

@rqi14

rqi14 commented Sep 14, 2026

Copy link
Copy Markdown
Author

Tried #130. It doesn't fix this, and testing it answered the question you raised above.

I reproduced its architecture with raw telnet rather than the patched library: session A with the full #MONITORING set (12,2 / 255,2 / 3,4,5,6,8=1) and never sending anything, session B with no monitoring, used only to send.

 2.1  B →  #OUTPUT,57,1,80
 8.1  B →  ?OUTPUT,57,1
 8.1  B ←  ~OUTPUT,57,1,80.00
10.1  A →  ?OUTPUT,57,1
10.1  A ←  ~OUTPUT,57,1,80.00
12.2  B →  #OUTPUT,57,1,50
18.2  B →  ?OUTPUT,57,1
18.3  B ←  ~OUTPUT,57,1,50.00

Output 57 really did go 50 → 80 → 50, as the two queries confirm. Across the whole capture, 26 seconds, neither A nor B received an unsolicited ~OUTPUT.

That was a direct #OUTPUT, no scene involved, which is the case I said I had no way to trigger. So this isn't scene recalls failing to report. Zone monitoring is on, ?MONITORING,5 reads back 1, the command takes effect, and nothing is pushed to any session.

#130 also couldn't have applied here even if its premise held. The capture in my last comment was a physical keypad press against a passive session, so nothing originated from telnet and there was no originating session to exclude.

One processor on one firmware, so I can't say whether this holds for QS in general. But on this one, a caller that wants current levels has to ask for them, which is what affected_outputs is for.

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.

2 participants