Bind wired SDL vendor commands to the correct physical controller

.github/workflows/sdl-regressions.yml

@@ -18,26 +18,38 @@ jobs:
           ref: 147a8ee32dbf9ac02f3794964490687b6bbda1bc
           path: .ci-sdl
           persist-credentials: false
-      - name: Build baseline and reproduce missing events
+      - name: Ensure the real wired driver is compiled
+        run: brew list libusb >/dev/null 2>&1 || brew install libusb
+      - name: Build baseline and reproduce missing events and wrong USB selection
         shell: bash
         run: |
           set -euo pipefail
           bash -n sdl/build-sdl.sh sdl/make-gopher64-both.sh
           git -C .ci-sdl apply "$GITHUB_WORKSPACE/sdl/sdl3-3.4.14-s2udp.patch"
-          cmake -S .ci-sdl -B "$RUNNER_TEMP/sdl-build" -DSDL_SHARED=ON -DSDL_STATIC=OFF -DSDL_TESTS=OFF
-          cmake --build "$RUNNER_TEMP/sdl-build" --parallel 3
+          bash tests/sdl-usb/check-sdl.sh .ci-sdl before
+          cmake -S .ci-sdl -B "$RUNNER_TEMP/sdl-build" -DSDL_SHARED=ON -DSDL_STATIC=OFF -DSDL_TESTS=OFF -DSDL_HIDAPI_LIBUSB=ON > "$RUNNER_TEMP/sdl-baseline.log" 2>&1
+          cmake --build "$RUNNER_TEMP/sdl-build" --parallel 3 >> "$RUNNER_TEMP/sdl-baseline.log" 2>&1 || { tail -80 "$RUNNER_TEMP/sdl-baseline.log"; exit 1; }
+          grep '^#define HAVE_LIBUSB 1' "$RUNNER_TEMP/sdl-build"/include-config-*/build_config/SDL_build_config.h
           cc -I .ci-sdl/include tests/sdl-edges/check.c -L "$RUNNER_TEMP/sdl-build" -Wl,-rpath,"$RUNNER_TEMP/sdl-build" -lSDL3 -o "$RUNNER_TEMP/check-edges"
           set +e
           "$RUNNER_TEMP/check-edges"
           result=$?
           set -e
           test "$result" -eq 42
-      - name: Build corrected library with the documented script and verify override
+      - name: Build corrected library and verify the same production functions
         shell: bash
         run: |
           set -euo pipefail
-          bash sdl/build-sdl.sh .ci-sdl
+          bash sdl/build-sdl.sh .ci-sdl > "$RUNNER_TEMP/sdl-corrected.log" 2>&1 || { tail -80 "$RUNNER_TEMP/sdl-corrected.log"; exit 1; }
+          git -C .ci-sdl apply "$GITHUB_WORKSPACE/sdl/s2usb-device-identity.patch"
+          bash tests/sdl-usb/check-sdl.sh .ci-sdl after
           SDL3_DYNAMIC_API="$GITHUB_WORKSPACE/build/sdl/libSDL3.0.dylib" "$RUNNER_TEMP/check-edges"
+      - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02
+        if: always()
+        with:
+          name: sdl-build-diagnostics
+          path: ${{ runner.temp }}/sdl-*.log
+          retention-days: 7
       - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02
         with:
           name: corrected-sdl-arm64

sdl/USB-IDENTITY.md

@@ -0,0 +1,29 @@
+# Wired USB command ownership
+
+The upstream SDL addition selected the first openable device with the same
+VID/PID. With identical controllers this can initialize or send feedback to
+the wrong controller while HID input comes from another one.
+
+The added s2usb-device-identity.patch resolves the exact HID DevSrvsID path
+through IOKit to its USB device ancestor. It reads locationID and USB Address
+(or USBDeviceAddress), matches both libusb bus and device address plus VID/PID,
+and opens only a unique match. Registry entries and properties are released.
+Unknown paths, missing/invalid properties and ambiguous matches do not guess.
+
+**Compatibility tradeoff:** this acquisition path also performs wired
+initialization. If identity cannot be established on a particular Mac/USB
+backend, that wired SDL device may fail to initialize instead of falling back
+to a potentially different controller. BLE/UDP output is unaffected. Capture
+the actual HID/IOKit/libusb identities before qualifying supported hardware;
+no physical-controller or driver-restoration test has been performed here.
+
+CI compiles the real wired driver with libusb and Apple IOKit headers. A small
+harness extracts the actual acquisition/identity function bodies and supplies
+fake platform boundaries: the original selects the wrong identical device
+(exit 42), the correction passes reverse-order, missing-peer, duplicate-address,
+failed-claim, alternate-property, malformed-path and cleanup tests. The existing
+real SDL/UDP edge test also runs against the combined rebuilt library.
+
+Build with sdl/build-sdl.sh as documented in INPUT-DELIVERY.md. The tracked
+upstream dylib is not updated; the builder applies all three patches and
+produces build/sdl/libSDL3.0.dylib. No controller command bytes are changed.

sdl/build-sdl.sh

@@ -10,7 +10,8 @@ trap 'rm -rf "$work"' EXIT
 git -C "$source_dir" archive "$revision" | tar -x -C "$work"
 git -C "$work" apply "$root/sdl/sdl3-3.4.14-s2udp.patch"
 git -C "$work" apply "$root/sdl/s2udp-input-edges.patch"
-cmake -S "$work" -B "$work/build" -DSDL_SHARED=ON -DSDL_STATIC=OFF -DSDL_TESTS=OFF
+git -C "$work" apply "$root/sdl/s2usb-device-identity.patch"
+cmake -S "$work" -B "$work/build" -DSDL_SHARED=ON -DSDL_STATIC=OFF -DSDL_TESTS=OFF -DSDL_HIDAPI_LIBUSB=ON
 cmake --build "$work/build" --parallel 3
 mkdir -p "$root/build/sdl"
 cp "$work/build/libSDL3.0.dylib" "$root/build/sdl/libSDL3.0.dylib"

sdl/s2usb-device-identity.patch

@@ -0,0 +1,86 @@
+--- a/src/joystick/hidapi/SDL_hidapi_switch2.c
++++ b/src/joystick/hidapi/SDL_hidapi_switch2.c
+@@ -417,10 +417,56 @@
++#ifdef SDL_PLATFORM_MACOS
++#include <CoreFoundation/CFNumber.h>
++#include <IOKit/IOKitLib.h>
++
++/* Match the opened HID service's USB ancestor, never just its model. */
++static bool S2USB_GetIdentity(const char *path, Uint8 *bus, Uint8 *address)
++{
++    unsigned long long identifier = 0;
++    char trailing;
++    if (!path || SDL_sscanf(path, "DevSrvsID:%llu%c", &identifier, &trailing) != 1) {
++        return false;
++    }
++    io_registry_entry_t entry = IOServiceGetMatchingService(kIOMainPortDefault,
++                                                            IORegistryEntryIDMatching(identifier));
++    while (entry && !IOObjectConformsTo(entry, "IOUSBHostDevice") &&
++           !IOObjectConformsTo(entry, "IOUSBDevice")) {
++        io_registry_entry_t parent = IO_OBJECT_NULL;
++        kern_return_t result = IORegistryEntryGetParentEntry(entry, kIOServicePlane, &parent);
++        IOObjectRelease(entry);
++        if (result != KERN_SUCCESS) return false;
++        entry = parent;
++    }
++    if (!entry) return false;
++    CFTypeRef location = IORegistryEntryCreateCFProperty(entry, CFSTR("locationID"), kCFAllocatorDefault, 0);
++    CFTypeRef device_address = IORegistryEntryCreateCFProperty(entry, CFSTR("USB Address"), kCFAllocatorDefault, 0);
++    if (!device_address) {
++        device_address = IORegistryEntryCreateCFProperty(entry, CFSTR("USBDeviceAddress"), kCFAllocatorDefault, 0);
++    }
++    Sint64 loc = -1, addr = -1;
++    bool valid = location && device_address &&
++        CFGetTypeID(location) == CFNumberGetTypeID() &&
++        CFGetTypeID(device_address) == CFNumberGetTypeID() &&
++        CFNumberGetValue((CFNumberRef)location, kCFNumberSInt64Type, &loc) &&
++        CFNumberGetValue((CFNumberRef)device_address, kCFNumberSInt64Type, &addr) &&
++        loc >= 0 && loc <= 0xffffffffLL && addr > 0 && addr <= 127;
++    if (location) CFRelease(location);
++    if (device_address) CFRelease(device_address);
++    IOObjectRelease(entry);
++    if (!valid) return false;
++    *bus = (Uint8)((Uint32)loc >> 24);
++    *address = (Uint8)addr;
++    return true;
++}
++#endif
++
+ static bool AcquireVendorInterface(SDL_DriverSwitch2_Context *ctx)
+ {
+ #ifdef SDL_PLATFORM_MACOS
+     if (ctx->device_handle) {
+         return true;
+     }
+-    if (!ctx->libusb) {
++    Uint8 bus = 0, address = 0;
++    if (!ctx->libusb || !S2USB_GetIdentity(ctx->device->path, &bus, &address)) {
+         return false;
+     }
+     if (ctx->libusb->init(&ctx->usb_context) != 0) {
+@@ -430,16 +476,20 @@
+     {
+         libusb_device **usb_list = NULL;
+         ssize_t usb_count = ctx->libusb->get_device_list(ctx->usb_context, &usb_list);
++        libusb_device *matched = NULL;
+         for (ssize_t di = 0; di < usb_count; di++) {
+             struct libusb_device_descriptor desc;
+             if (ctx->libusb->get_device_descriptor(usb_list[di], &desc) == 0 &&
+                 desc.idVendor == ctx->device->vendor_id &&
+-                desc.idProduct == ctx->device->product_id) {
+-                if (ctx->libusb->open(usb_list[di], &ctx->device_handle) == 0) {
+-                    ctx->own_device_handle = true;
+-                    break;
+-                }
++                desc.idProduct == ctx->device->product_id &&
++                ctx->libusb->get_bus_number(usb_list[di]) == bus &&
++                ctx->libusb->get_device_address(usb_list[di]) == address) {
++                if (matched) { matched = NULL; break; } // ambiguous: no command
++                matched = usb_list[di];
+             }
++        }
++        if (matched && ctx->libusb->open(matched, &ctx->device_handle) == 0) {
++            ctx->own_device_handle = true;
+         }
+         if (usb_list) {
+             ctx->libusb->free_device_list(usb_list, 1);

tests/sdl-usb/check-sdl.sh

@@ -0,0 +1,38 @@
+#!/bin/bash
+# Test functions from the actual patched SDL source; fake only IOKit/libusb.
+set -euo pipefail
+root=$(cd "$(dirname "$0")/../.." && pwd)
+source_dir=${1:?Provide the SDL source directory with patches applied}
+mode=${2:-after}
+work=$(mktemp -d)
+trap 'rm -rf "$work"' EXIT
+mkdir -p "$work/IOKit"
+printf '/* Boundary declarations are in the harness. */\n' > "$work/IOKit/IOKitLib.h"
+python3 - "$source_dir/src/joystick/hidapi/SDL_hidapi_switch2.c" "$work/production.c" "$mode" <<'PY'
+from pathlib import Path
+import sys
+s=Path(sys.argv[1]).read_text()
+def function(name):
+    a=s.index('static bool '+name+'(')
+    # Skip forward declarations, if present.
+    while s.index(';',a) < s.index('{',a):
+        a=s.index('static bool '+name+'(',a+1)
+    b=s.index('{',a);depth=1;c=b+1
+    while depth:
+        depth += (s[c]=='{') - (s[c]=='}');c+=1
+    return s[a:c]+'\n'
+text='' if sys.argv[3]=='before' else function('S2USB_GetIdentity')
+Path(sys.argv[2]).write_text(text+function('AcquireVendorInterface'))
+PY
+flags=()
+[ "$mode" != before ] || flags+=(-DBASELINE)
+cc "${flags[@]}" -I "$work" "$root/tests/sdl-usb/harness.c" -o "$work/test"
+if [ "$mode" = before ]; then
+    set +e
+    "$work/test"
+    result=$?
+    set -e
+    test "$result" -eq 42
+else
+    "$work/test"
+fi

tests/sdl-usb/harness.c

@@ -0,0 +1,116 @@
+#include <stdbool.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#define SDL_PLATFORM_MACOS 1
+#define SDL_sscanf sscanf
+#define CHECK(x) do { if (!(x)) { fprintf(stderr,"line %d: %s\n",__LINE__,#x); exit(2); } } while(0)
+typedef uint8_t Uint8; typedef uint32_t Uint32; typedef int64_t Sint64;
+// Fake only the platform/USB boundary; included function bodies are production.
+typedef int io_registry_entry_t; typedef int kern_return_t;
+#define kIOMainPortDefault 0
+#define IO_OBJECT_NULL 0
+#define KERN_SUCCESS 0
+#define kIOServicePlane "IOService"
+#define kCFAllocatorDefault 0
+#define kCFNumberSInt64Type 0
+#define CFSTR(x) (x)
+typedef struct { int type; Sint64 value; } Number;
+typedef Number *CFTypeRef; typedef Number *CFNumberRef;
+static Number location = {1, 0x02000000}, address = {1, 5};
+static bool missing_address, alternate_address;
+static int released_entries, released_properties;
+static unsigned long long IORegistryEntryIDMatching(unsigned long long id) {return id;}
+static int IOServiceGetMatchingService(int port, unsigned long long id) {return id == 42 ? 3 : 0;}
+static bool IOObjectConformsTo(int entry, const char *name) {return entry == 1 && strcmp(name,"IOUSBHostDevice") == 0;}
+static int IORegistryEntryGetParentEntry(int entry, const char *plane, int *parent) {*parent = entry - 1; return 0;}
+static void IOObjectRelease(int entry) {CHECK(entry > 0); released_entries++;}
+static CFTypeRef IORegistryEntryCreateCFProperty(int entry, const char *key, int alloc, int options) {
+    if (!strcmp(key,"locationID")) return &location;
+    if (missing_address) return NULL;
+    if (!strcmp(key,alternate_address ? "USBDeviceAddress" : "USB Address")) return &address;
+    return NULL;
+}
+static int CFGetTypeID(CFTypeRef v) {return v->type;}
+static int CFNumberGetTypeID(void) {return 1;}
+static bool CFNumberGetValue(CFNumberRef n, int type, Sint64 *out) {*out=n->value;return true;}
+static void CFRelease(CFTypeRef p) {CHECK(p);released_properties++;}
+
+typedef struct {int index; Uint8 bus, address; unsigned vendor, product;} libusb_device;
+typedef libusb_device libusb_device_handle;
+typedef int libusb_context;
+struct libusb_device_descriptor {unsigned idVendor,idProduct;};
+static libusb_device devices[]={{0,2,4,0x57e,0x2073},{1,2,5,0x57e,0x2073},{2,2,6,0x57e,0x2073}};
+static libusb_device *ordered[]={&devices[0],&devices[1],&devices[2]};
+static int count=2, opened, claimed, exited, freed, closed, claim_result;
+static int usb_init(libusb_context **c) {static int context;*c=&context;return 0;}
+static void usb_exit(libusb_context *c) {exited++;}
+static ssize_t usb_list(libusb_context *c, libusb_device ***out) {*out=ordered;return count;}
+static void usb_free(libusb_device **list,int unref) {freed++;}
+static int usb_desc(libusb_device *d,struct libusb_device_descriptor *out) {out->idVendor=d->vendor;out->idProduct=d->product;return 0;}
+static Uint8 usb_bus(libusb_device *d){return d->bus;}
+static Uint8 usb_address(libusb_device *d){return d->address;}
+static int usb_open(libusb_device *d,libusb_device_handle **out){opened++;*out=d;return 0;}
+static int usb_claim(libusb_device_handle *d,Uint8 interface){claimed++;return claim_result;}
+typedef struct {
+    int (*init)(libusb_context **);void (*exit)(libusb_context *);
+    ssize_t (*get_device_list)(libusb_context *,libusb_device ***);
+    void (*free_device_list)(libusb_device **,int);
+    int (*get_device_descriptor)(libusb_device *,struct libusb_device_descriptor *);
+    Uint8 (*get_bus_number)(libusb_device *);Uint8 (*get_device_address)(libusb_device *);
+    int (*open)(libusb_device *,libusb_device_handle **);
+    int (*claim_interface)(libusb_device_handle *,Uint8);
+} API;
+typedef struct {unsigned vendor_id,product_id; const char *path;} HIDDevice;
+typedef struct {
+    API *libusb;HIDDevice *device;libusb_device_handle *device_handle;
+    libusb_context *usb_context;bool own_device_handle,interface_claimed;
+    Uint8 interface_number,out_endpoint,in_endpoint;
+} SDL_DriverSwitch2_Context;
+static bool FindBulkEndpoints(API *api,libusb_device_handle *d,Uint8 *i,Uint8 *out,Uint8 *in){*i=1;*out=2;*in=0x82;return true;}
+static void ReleaseVendorInterface(SDL_DriverSwitch2_Context *ctx){
+    if(ctx->device_handle)closed++;
+    if(ctx->usb_context)ctx->libusb->exit(ctx->usb_context);
+    ctx->device_handle=NULL;ctx->usb_context=NULL;ctx->own_device_handle=false;ctx->interface_claimed=false;
+}
+#include "production.c"
+
+int main(void) {
+    API api={usb_init,usb_exit,usb_list,usb_free,usb_desc,usb_bus,usb_address,usb_open,usb_claim};
+    HIDDevice hid={0x57e,0x2073,"DevSrvsID:42"};
+    SDL_DriverSwitch2_Context c={.libusb=&api,.device=&hid};
+    CHECK(AcquireVendorInterface(&c));
+    if(c.device_handle != &devices[1]) {fputs("Wrong identical USB controller selected\n",stderr);return 42;}
+    CHECK(opened==1 && claimed==1 && freed==1);
+    ReleaseVendorInterface(&c);
+    ordered[0]=&devices[1];ordered[1]=&devices[0];
+    CHECK(AcquireVendorInterface(&c) && c.device_handle==&devices[1]);
+    ReleaseVendorInterface(&c);
+    // One visible wrong peer is not proof of association.
+    ordered[0]=&devices[0];count=1;int old=opened;
+    CHECK(!AcquireVendorInterface(&c) && opened==old && c.usb_context==NULL);
+    // Duplicate address records fail closed rather than choosing either.
+    ordered[0]=&devices[1];ordered[1]=&devices[2];devices[2].address=5;count=2;
+    CHECK(!AcquireVendorInterface(&c) && opened==old);
+    count=1;claim_result=-1;
+    CHECK(!AcquireVendorInterface(&c) && c.device_handle==NULL && c.usb_context==NULL);
+    CHECK(closed==3);claim_result=0;
+#ifndef BASELINE
+    Uint8 bus=0,addr=0;
+    released_entries=released_properties=0;
+    CHECK(S2USB_GetIdentity("DevSrvsID:42",&bus,&addr) && bus==2 && addr==5);
+    CHECK(released_entries==3 && released_properties==2);
+    CHECK(!S2USB_GetIdentity(NULL,&bus,&addr));
+    CHECK(!S2USB_GetIdentity("DevSrvsID:42junk",&bus,&addr));
+    CHECK(!S2USB_GetIdentity("DevSrvsID:999",&bus,&addr));
+    alternate_address=true;CHECK(S2USB_GetIdentity(hid.path,&bus,&addr));
+    missing_address=true;CHECK(!S2USB_GetIdentity(hid.path,&bus,&addr));missing_address=false;
+    location.type=2;CHECK(!S2USB_GetIdentity(hid.path,&bus,&addr));location.type=1;
+    address.value=128;CHECK(!S2USB_GetIdentity(hid.path,&bus,&addr));address.value=5;
+    hid.path="DevSrvsID:999";old=opened;CHECK(!AcquireVendorInterface(&c) && opened==old);
+#endif
+    puts("USB identity, reversed enumeration, ambiguity and cleanup regressions passed.");
+    return 0;
+}