Skip to content

Fix macOS volume and media key posting#184

Open
jonstuebe wants to merge 1 commit into
AprilNEA:masterfrom
jonstuebe:fix/macos-volume-media-keys
Open

Fix macOS volume and media key posting#184
jonstuebe wants to merge 1 commit into
AprilNEA:masterfrom
jonstuebe:fix/macos-volume-media-keys

Conversation

@jonstuebe

Copy link
Copy Markdown

Summary

  • Post macOS volume and media controls as NSSystemDefined subtype 8 NX events instead of standard keyboard events
  • Wire Volume Up/Down/Mute plus Play/Pause/Next/Previous through the shared media-key helper
  • Add macOS objc2 AppKit/CoreGraphics bindings needed to create and post NSEvents

Validation

  • cargo check -p openlogi-core
  • cargo fmt --check

Fixes mouse-button bindings for Volume Up/Down/Mute on macOS, where kVK_Volume* keyboard events are ignored by the system volume handler.

@greptile-apps

greptile-apps Bot commented Jun 8, 2026

Copy link
Copy Markdown

Greptile Summary

This PR replaces the placeholder post_media_key stub with a real NSSystemDefined subtype-8 event pipeline using the objc2 AppKit/CoreGraphics bindings, and routes all six media/volume actions (VolumeUp, VolumeDown, MuteVolume, PlayPause, NextTrack, PrevTrack) through it instead of the broken kVK_Volume* keyboard-event path.

  • NX_KEYTYPE_* constants match <IOKit/hidsystem/ev_keymap.h>, and the data1 layout — (keyCode << 16) | (state << 8) — is the standard encoding used by well-known reference implementations.
  • post_media_key loops over a (key-down, key-up) pair, creating and posting an NSEventCGEvent for each; error branches inside the loop use return rather than break, which can leave a key-down posted without a matching key-up if the second iteration fails.

Confidence Score: 3/5

The approach is correct and the constants are accurate, but the loop's early-return on failure can leave a key-down event posted without a paired key-up, which is worth fixing before merge.

The post_media_key function posts both a key-down and a key-up NSSystemDefined event inside a single for loop. If the key-down iteration succeeds and the key-up iteration then fails, the return in the else branch exits the function entirely, leaving the key-down event in the HID stream with no matching key-up.

crates/openlogi-core/src/binding.rs — specifically the error branches inside the post_media_key loop

Important Files Changed

Filename Overview
crates/openlogi-core/src/binding.rs Replaces stub media-key implementation with real NSSystemDefined subtype-8 posting using objc2 AppKit/CoreGraphics bindings; NX_KEYTYPE constants are correct, data1 encoding matches the standard layout, but return inside the down/up loop can leave a key-down posted without a matching key-up if the second iteration hits an error branch.
crates/openlogi-core/Cargo.toml Adds objc2 ecosystem crates (objc2, objc2-app-kit, objc2-core-graphics, objc2-foundation) under the macOS target; versions are internally consistent and the NSGraphicsContext feature is included (likely needed for NSEvent.CGEvent() via AppKit category).
Cargo.lock Lock file update reflecting new objc2-app-kit transitive deps (block2, libc, objc2-metal); purely mechanical change consistent with the Cargo.toml additions.

Sequence Diagram

sequenceDiagram
    participant A as Action::execute_macos
    participant M as macos::post_media_key
    participant NS as NSEvent (AppKit)
    participant CG as CGEvent (CoreGraphics)
    participant OS as macOS HID System

    A->>M: "post_media_key(NX_KEYTYPE_*)"
    loop key-down then key-up
        M->>NS: "otherEventWithType(SystemDefined, subtype=8, data1)"
        NS-->>M: "Option<NSEvent>"
        M->>NS: ns_event.CGEvent()
        NS-->>M: "Option<CGEvent>"
        M->>CG: CGEvent::post(HIDEventTap, cg_event)
        CG->>OS: deliver NX_SUBTYPE_AUX_CONTROL_BUTTONS event
    end
Loading

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "Fix macOS volume and media key posting" | Re-trigger Greptile

Comment on lines +1004 to +1013
) else {
tracing::warn!(nx_key, phase, "NSEvent::otherEventWithType failed");
return;
};
let Some(cg_event) = ns_event.CGEvent() else {
tracing::warn!(nx_key, phase, "NSEvent::CGEvent failed");
return;
};
CGEvent::post(CGEventTapLocation::HIDEventTap, Some(&cg_event));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Early return can leave key-down posted without matching key-up

If the first iteration (key-down) posts successfully and the second iteration (key-up) then hits either NSEvent::otherEventWithType returning None or ns_event.CGEvent() returning None, the return exits the function entirely — leaving a key-down event in the system with no paired key-up. The post_click function in the same module handles this more gracefully with if let Ok. Using break instead of return in the failure branches would avoid posting orphaned key-down events for volume/media while still aborting any remaining work.

Fix in Codex Fix in Claude Code

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