Skip to content

Handle partial query failures in refresh()#20

Merged
SteveEasley merged 3 commits intoSteveEasley:mainfrom
robotdan:fix/refresh-partial-failure
Apr 5, 2026
Merged

Handle partial query failures in refresh()#20
SteveEasley merged 3 commits intoSteveEasley:mainfrom
robotdan:fix/refresh-partial-failure

Conversation

@robotdan
Copy link
Copy Markdown
Contributor

@robotdan robotdan commented Apr 3, 2026

Summary

  • Add missing error code 028 (ERROR_INCOMPATIBLE_VIDEO_CONFIG) to the protocol constants, per the Kaleidescape Control Protocol Reference Manual
  • Use return_exceptions=True in refresh() so that a single query failure doesn't discard results from the other 6 queries
  • Log a warning for each failed query and continue applying successful results

Problem

refresh() runs 7 state queries via asyncio.gather() without return_exceptions=True. If any single query fails, the entire gather raises and none of the _update_* calls execute — even for queries that completed successfully.

In practice, GET_SCREEN_MASK2 returns error 028 ("Incompatible video configuration") on devices that lack masking calibration, and GET_CINEMASCAPE_MASK returns the same error when not in CinemaScope mode (documented on p. 173 of the Control Protocol Reference Manual). This causes refresh() to discard all results, leaving the device state unpopulated.

Test plan

  • New test test_refresh_partial_failure registers GET_SCREEN_MASK2 to return error 028 and verifies the other 6 query results are still applied to the device state
  • All 56 existing tests continue to pass

robotdan added 2 commits April 3, 2026 17:13
Add missing error code 028 (ERROR_INCOMPATIBLE_VIDEO_CONFIG) to the
protocol constants, per the Kaleidescape Control Protocol Reference
Manual. Use return_exceptions=True in refresh() so that a single query
failure doesn't discard results from the other 6 queries. Log a warning
for each failed query and continue applying successful results.

Devices that lack masking calibration return error 028 for
GET_SCREEN_MASK2, and devices not in CinemaScope mode return the same
error for GET_CINEMASCAPE_MASK. Previously this caused asyncio.gather
to raise, discarding all 7 query results including the successful ones.
The loop-based approach with heterogeneous updater functions caused a
mypy "Cannot call function of unknown type" error. Use explicit
isinstance checks for each result instead, matching the existing code
style in _handle_event().
Copy link
Copy Markdown
Owner

@SteveEasley SteveEasley left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@SteveEasley SteveEasley merged commit bf2f7c9 into SteveEasley:main Apr 5, 2026
0 of 4 checks passed
SteveEasley pushed a commit that referenced this pull request May 6, 2026
PR #18 (Refresh device state after auto-reconnect) added two tests that
wait on `connect_signal` (STATE_CONNECTED) after the initial connect.
PR #19 (Dispatch STATE_CONNECTED only on auto-reconnect) then moved the
STATE_CONNECTED dispatch out of `_connect()` and into `_reconnect()`.
PR #19 updated other tests with the same pattern, but #19 was opened
before #18 merged, so the two tests added by #18 weren't updated.

Result: `await connect_signal.wait()` after initial connect waits
forever, hanging CI for the full 6h job timeout. Every CI run on main
since #18 has been cancelled this way (#19, #20, #21, v1.1.6, #22).

Match the pattern used by other tests updated in #19: assert
`state == STATE_CONNECTED` directly after the initial connect, and
keep `connect_signal.wait()` only for the post-reconnect signal where
STATE_CONNECTED is actually dispatched.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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