Skip to content

Add rotary gear support (Phase 2) for Philips AMBx speakers#7

Open
eirvandelden wants to merge 20 commits into
mainfrom
ai/philips-ambx-rotary-gear-support
Open

Add rotary gear support (Phase 2) for Philips AMBx speakers#7
eirvandelden wants to merge 20 commits into
mainfrom
ai/philips-ambx-rotary-gear-support

Conversation

@eirvandelden

Copy link
Copy Markdown
Owner

Summary

  • Adds AmbxInput: a singleton background polling thread that reads USB interrupt transfers from the AMBx device and fires callbacks for :volume and :brightness rotary gear events
  • Adds RotaryDecoder (stubbed pending Phase 1 USB capture), MacOSIntegration (system volume via osascript), and BrightnessController (AMBx light brightness multiplier with instant reapplication across all tracked lights)
  • Extends Ambx with handles reader and light state tracking so BrightnessController can replay all lit lights at the new brightness level instantly
  • Also includes: menubar app, build tooling, config, and a cspell project dictionary for AMBx-specific technical terms (all pre-existing work from this branch)

Test Plan

  • RotaryDecoder.decode stub returns nil for any input (safe until Phase 1 fills it in)
  • MacOSIntegration.adjust_volume(1) raises macOS system volume by 5%
  • BrightnessController.adjust(-1) lowers @level and calls Ambx.reapply_brightness
  • After Phase 1: rotating volume knob prints Volume delta: +1/-1, brightness knob prints Brightness delta: +1/-1

🤖 Generated with Claude Code

Comment thread libcombustd/communication/ambx.rb Outdated
def self.write_device(handle, bytes)
# Track light state for brightness reapplication.
# Light writes: [0xA1, light_id, SET_LIGHT_COLOR, r, g, b]
if bytes[0] == 0xA1 && bytes[2] == ProtocolDefinitions::SET_LIGHT_COLOR

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This condition also captures fan writes, because the fan helpers send the same SET_LIGHT_COLOR opcode with Lights::LEFT_FAN/RIGHT_FAN. Once a fan speed has been set, reapply_brightness will replay it through BrightnessController.apply, so turning the brightness knob also changes fan speed. For example, writing [0xA1, Lights::LEFT_FAN, 0x03, 0, 0, 200] and then adjusting brightness by -1 replays [0,0,190] to the fan endpoint.

Comment thread libcombustd/communication/ambx.rb Outdated
# Track light state for brightness reapplication.
# Light writes: [0xA1, light_id, SET_LIGHT_COLOR, r, g, b]
if bytes[0] == 0xA1 && bytes[2] == ProtocolDefinitions::SET_LIGHT_COLOR
@light_state[bytes[1]] = [ bytes[3], bytes[4], bytes[5] ]

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This stores every SET_LIGHT_COLOR payload, including the replay writes from reapply_brightness. That makes brightness changes destructive: after a light is set to [100,0,0], calling BrightnessController.adjust(-10) rewrites the cached state to [50,0,0], so adjusting back up can only replay 50 and never restore the original 100. The cache needs to preserve the unscaled source color instead of overwriting it during replay.

@eirvandelden eirvandelden force-pushed the ai/philips-ambx-rotary-gear-support branch from eb44992 to 58047f9 Compare March 25, 2026 21:04
@eirvandelden eirvandelden changed the base branch from main to eirvandelden/menubar-ambx-complete March 25, 2026 21:04
@eirvandelden eirvandelden force-pushed the ai/philips-ambx-rotary-gear-support branch 2 times, most recently from aead327 to 60013b6 Compare March 26, 2026 15:47
Base automatically changed from eirvandelden/menubar-ambx-complete to main March 27, 2026 12:14
eirvandelden and others added 6 commits March 27, 2026 13:16
Implements event-driven input reading infrastructure for the two rotary
gears (volume and brightness) on the AMBx SGC5103BD. Adds AmbxInput
(background polling thread with callback registration), RotaryDecoder
(stubbed pending Phase 1 USB capture), MacOSIntegration (system volume
via osascript), and BrightnessController (AMBx light brightness
multiplier with instant reapplication). Extends Ambx with handle
exposure and light state tracking for brightness reapplication.
Adds cspell project dictionary for AMBx-specific technical terms.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
reapply_brightness was calling write() which updated @light_state with
scaled values, causing each subsequent brightness adjustment to scale
already-scaled colors. Fan writes (LEFT_FAN) shared the SET_LIGHT_COLOR
command byte and were incorrectly tracked and replayed through brightness
scaling.

Moves state tracking from write_device into write, guarded by a
source_light_command? predicate that checks membership in LIGHT_IDS
(the five real light IDs only). reapply_brightness now calls the private
write_to_handles directly, bypassing the tracking path entirely.
Adds a regression test suite confirming both fixes and that @light_state
is preserved across close for post-reconnect reapplication.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
reapply_brightness called write_to_handles which called @handles.each
directly. When @Handles was nil — either because the device was never
opened or because Errno::ENXIO during a previous transfer caused
Ambx.close to nil it out mid-replay — this raised NoMethodError and
crashed the process.

Adding a nil guard at the top of write_to_handles makes brightness
adjustment a safe no-op until the device is re-opened, consistent with
the existing guard in Ambx.write. The fix covers both the disconnected
case and the mid-replay disconnect case.

Adds two regression tests:
- brightness adjust is safe no-op when @Handles is nil
- mid-replay Errno::ENXIO disconnect does not raise

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The rebase incorporated the upstream Gemfile change (Add minitest to
development group), which triggered the brakeman and bundle-audit
pre-push hooks that had previously been skipping due to no matching
push files.

Three issues fixed:
- bin/bundler-audit binstub was missing; generated via `bundle binstubs`
- brakeman exits 4 on non-Rails projects without --force; added
  .lefthook-local.yml override and .brakeman.yml to enable forced scan
- brakeman flagged a medium-confidence command injection in
  MacOSIntegration.adjust_volume; switched system() to array form so
  osascript receives the expression directly without shell interpolation

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The advisory db was updated as part of binstub setup. Running
--update again in the hook causes a flaky git fetch failure against
the local advisory-db clone. Checking without --update uses the
already-current db and exits cleanly.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@eirvandelden eirvandelden force-pushed the ai/philips-ambx-rotary-gear-support branch from 60013b6 to c13dd8b Compare March 27, 2026 12:16
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.

1 participant