Conversation
replace libudev with IOKit/CoreFoundation for device monitoring, swap hidapi-hidraw for the generic hidapi (IOKit backend), handle macOS-specific HID report ID stripping, and add nanosleep compat for thrd_sleep. all changes behind #ifdef __APPLE__ — linux code is untouched. tested on macOS sequoia (arm64) over both USB and bluetooth.
There was a problem hiding this comment.
Pull request overview
This PR adds macOS support to dualsensectl by introducing macOS-specific HID handling and device monitoring while keeping Linux behavior intact via platform conditionals.
Changes:
- Adds platform-conditional Meson dependencies to switch between Linux (
hidapi-hidraw+libudev) and macOS (hidapi+ IOKit/CoreFoundation frameworks). - Introduces macOS-specific handling for HID input report layout differences (report ID stripping).
- Implements a macOS
monitorcommand backend usingIOHIDManagercallbacks.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| meson.build | Adds OS-conditional dependencies and linker args for macOS vs Linux. |
| main.c | Adds macOS-specific includes/shims, input report parsing adjustments, and an IOKit-based monitor implementation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- fix potential NULL dereference in serial number fprintf - handle macOS report ID stripping in update command (not just battery) - fix CF object leak on IOHIDManagerOpen error path - use bracket syntax for meson array appends
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
hello! |
|
bump |
egormanga
left a comment
There was a problem hiding this comment.
My two cents; never used a Mac.
| ds_report = (struct dualsense_input_report *)&data[1]; | ||
| #ifdef __APPLE__ | ||
| } else if (res == DS_INPUT_REPORT_USB_SIZE - 1) { | ||
| /* macOS IOKit strips report ID */ |
There was a problem hiding this comment.
Maybe replace the entire branch on Darwin instead of falling back?
There was a problem hiding this comment.
in my testing macos always stripped the report id, but i kept the generic branch so a build where hidapi doesn't strip still works. i'd rather not hard-assume iokit behavior here lol
| size_t len = strlen(buf); | ||
| if (len == 17) { | ||
| /* Replace dashes with colons if needed, uppercase */ | ||
| for (int i = 0; i < 17; i++) { |
There was a problem hiding this comment.
Trap for future refactoring!
| for (int i = 0; i < 17; i++) { | |
| for (int i = 0; i < len; i++) { |
| if (buf[i] == '-') buf[i] = ':'; | ||
| serial_number[i] = toupper(buf[i]); | ||
| } | ||
| serial_number[17] = '\0'; |
There was a problem hiding this comment.
| serial_number[17] = '\0'; | |
| serial_number[len] = '\0'; |
| static void iokit_device_added(void *context, IOReturn result, void *sender, IOHIDDeviceRef device) | ||
| { | ||
| (void)context; (void)result; (void)sender; | ||
| char serial_number[18] = "00:00:00:00:00:00"; |
There was a problem hiding this comment.
| char serial_number[18] = "00:00:00:00:00:00"; | |
| char serial_number[] = "00:00:00:00:00:00"; |
| static void iokit_device_removed(void *context, IOReturn result, void *sender, IOHIDDeviceRef device) | ||
| { | ||
| (void)context; (void)result; (void)sender; | ||
| char serial_number[18] = "00:00:00:00:00:00"; |
There was a problem hiding this comment.
| char serial_number[18] = "00:00:00:00:00:00"; | |
| char serial_number[] = "00:00:00:00:00:00"; |
|
|
||
| IOReturn ret = IOHIDManagerOpen(manager, kIOHIDOptionsTypeNone); | ||
| if (ret != kIOReturnSuccess) { | ||
| fprintf(stderr, "Failed to open IOHIDManager: 0x%x\n", ret); |
There was a problem hiding this comment.
| fprintf(stderr, "Failed to open IOHIDManager: 0x%x\n", ret); | |
| fprintf(stderr, "Failed to open IOHIDManager: %#04x\n", ret); |
|
Can you please put all the macos specific code to a new file (eg. macos.c)? |
well most of the macos stuff is a couple lines inside shared functions, splitting those out would need either duplicating them or a short shim, do we want that?. the monitor code is self contained though, i can move that plus the serial helper into macos.c and leave the small ifdefs where they are. would that work? |
|
moved the self-contained monitor implementation to macos.c; retained the small platform-specific report branches in main.c |
| return; | ||
| } |
There was a problem hiding this comment.
Can we fail-fast instead? Missed that on my initial review, but a proper (and functionally equivalent) practice would be if (len < 17) continue; and flattening the rest.
There was a problem hiding this comment.
agreed, will flatten it to one early-out and keep the dash/uppercase pass at top level.
| (void)result; | ||
| (void)sender; |
There was a problem hiding this comment.
Perhaps start these names with an underscore as well?
There was a problem hiding this comment.
there's no underscore convention anywhere in main.c. and the (void) casts are what actually silence -Wunused-parameter portably. if you feel strongly about ts, it's a two-line rename, just say so.
There was a problem hiding this comment.
There isn't a single (void) in there, either. But it's your call.
| /* Portable thrd_sleep replacement using nanosleep on macOS */ | ||
| #ifdef __APPLE__ | ||
| #define thrd_sleep(ts, rem) nanosleep((ts), (rem)) | ||
| #endif |
There was a problem hiding this comment.
Perhaps move that to macos.h too?
There was a problem hiding this comment.
yep, makes sense will move it there rq
| } | ||
| } | ||
| } | ||
| strncpy(serial_number, "00:00:00:00:00:00", 18); |
There was a problem hiding this comment.
nit: this assumes serial_number size is enough which it may or may not be depending on API/ABI.
There was a problem hiding this comment.
will pass the buffer size in explicitly and snprintf into it
| (void)sender; | ||
|
|
||
| struct monitor_context *monitor = context; | ||
| char serial_number[] = "00:00:00:00:00:00"; |
There was a problem hiding this comment.
Why do we need a default if that gets filled in in the get_serial_from_hid_device()?
Also specify an explicit size to avoid a possible future bounds fault.
There was a problem hiding this comment.
will make it a plain char serial_number[18] and hand sizeof() to the getter, which also covers your strncpy note above
|
will push a quick fix |
| }; | ||
|
|
||
| static void get_serial_from_hid_device(IOHIDDeviceRef device, char serial_number[18]) | ||
| static void get_serial_from_hid_device(IOHIDDeviceRef device, char *serial_number, size_t size) |
There was a problem hiding this comment.
Note you can char serial_number[static size] for sanity & documentation purposes as well
| static void get_serial_from_hid_device(IOHIDDeviceRef device, char serial_number[18]) | ||
| static void get_serial_from_hid_device(IOHIDDeviceRef device, char *serial_number, size_t size) | ||
| { | ||
| char buf[64]; |
There was a problem hiding this comment.
why 64 btw? a magic number
summary
adds macOS support to dualsensectl. all changes are behind
#ifdef __APPLE__— linux code is untouched.libudevwithIOKit/CoreFoundationfor device monitoring (IOHIDManagercallbacks for hotplug)hidapi-hidrawfor the generichidapi(uses IOKit backend on macOS)nanosleepcompat shim forthrd_sleepmeson.build(auto-detects macOS vs Linux dependencies)tested on
build on macOS