Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions src/cli/install/tests.zig
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,96 @@ test "install: generateUdevRules produces valid output" {
try testing.expect(std.mem.indexOf(u8, content, "KERNEL==\"uhid\"") != null);
}

const usb_node_rule = "SUBSYSTEM==\"usb\", ENV{DEVTYPE}==\"usb_device\", ATTR{idVendor}==\"37d7\", ATTR{idProduct}==\"2401\", TAG+=\"uaccess\", GROUP=\"input\", MODE=\"0660\"";

test "install: libusb-claimed device gets raw USB node uaccess rule (#355)" {
const testing = std.testing;
const allocator = testing.allocator;

var tmp = testing.tmpDir(.{});
defer tmp.cleanup();
const tmp_path = try tmp.dir.realpathAlloc(allocator, ".");
defer allocator.free(tmp_path);

const devices_dir = try std.fmt.allocPrint(allocator, "{s}/devices", .{tmp_path});
defer allocator.free(devices_dir);
try std.fs.makeDirAbsolute(devices_dir);

const toml_path = try std.fmt.allocPrint(allocator, "{s}/vader5.toml", .{devices_dir});
defer allocator.free(toml_path);
{
var file = try std.fs.createFileAbsolute(toml_path, .{});
defer file.close();
try file.writeAll(
\\[device]
\\name = "Flydigi Vader 5 Pro"
\\vid = 0x37d7
\\pid = 0x2401
\\[[device.interface]]
\\id = 1
\\class = "vendor" # libusb-claimed
\\[[device.interface]]
\\id = 2
\\class = "suppress"
);
}

const rules_path = try std.fmt.allocPrint(allocator, "{s}/60-padctl.rules", .{tmp_path});
defer allocator.free(rules_path);
try generateUdevRules(allocator, devices_dir, rules_path, "/usr");

var file = try std.fs.openFileAbsolute(rules_path, .{});
defer file.close();
const content = try file.readToEndAlloc(allocator, 4096);
defer allocator.free(content);

// Without this grant the libusb claim fails and the device never binds (#355).
try testing.expect(std.mem.indexOf(u8, content, usb_node_rule) != null);
}

test "install: pure-hid device gets no raw USB node rule (#355)" {
const testing = std.testing;
const allocator = testing.allocator;

var tmp = testing.tmpDir(.{});
defer tmp.cleanup();
const tmp_path = try tmp.dir.realpathAlloc(allocator, ".");
defer allocator.free(tmp_path);

const devices_dir = try std.fmt.allocPrint(allocator, "{s}/devices", .{tmp_path});
defer allocator.free(devices_dir);
try std.fs.makeDirAbsolute(devices_dir);

const toml_path = try std.fmt.allocPrint(allocator, "{s}/hidpad.toml", .{devices_dir});
defer allocator.free(toml_path);
{
var file = try std.fs.createFileAbsolute(toml_path, .{});
defer file.close();
try file.writeAll(
\\[device]
\\name = "Plain HID Pad"
\\vid = 0x37d7
\\pid = 0x2401
\\[[device.interface]]
\\id = 0
\\class = "hid"
);
}

const rules_path = try std.fmt.allocPrint(allocator, "{s}/60-padctl.rules", .{tmp_path});
defer allocator.free(rules_path);
try generateUdevRules(allocator, devices_dir, rules_path, "/usr");

var file = try std.fs.openFileAbsolute(rules_path, .{});
defer file.close();
const content = try file.readToEndAlloc(allocator, 4096);
defer allocator.free(content);

// A hidraw-only device must not emit a raw USB node rule.
try testing.expect(std.mem.indexOf(u8, content, "ENV{DEVTYPE}==\"usb_device\"") == null);
try testing.expect(std.mem.indexOf(u8, content, "SUBSYSTEM==\"hidraw\"") != null);
}

test "install: findDevicesSourceDir discovers repo-root devices from zig-out/bin" {
const testing = std.testing;
const allocator = testing.allocator;
Expand Down
39 changes: 39 additions & 0 deletions src/cli/install/udev.zig
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub const UdevEntry = struct {
pid: u16,
block_kernel_drivers: []const []const u8 = &.{},
clone_vid_pid: bool = false,
needs_libusb: bool = false,
};

/// Runtime path checked by installed udev rules. This must never include
Expand Down Expand Up @@ -367,6 +368,9 @@ fn collectDeviceEntries(allocator: std.mem.Allocator, dirs: []const []const u8)
if (entries.items[j].clone_vid_pid) {
entries.items[i].clone_vid_pid = true;
}
if (entries.items[j].needs_libusb) {
entries.items[i].needs_libusb = true;
}
const removed = entries.items[j];
allocator.free(removed.name);
for (removed.block_kernel_drivers) |d| allocator.free(d);
Expand Down Expand Up @@ -432,6 +436,26 @@ pub fn generateUdevRulesFromEntries(allocator: std.mem.Allocator, entries: []con
try buf.appendSlice(allocator, rule);
}

// Devices with vendor/suppress interfaces are claimed via libusb, which needs
// write access to the raw USB device node (/dev/bus/usb/...) plus driver
// detach — the hidraw grant above does not cover that node, so without this
// the user-scope daemon cannot claim the device and the bind fails.
var has_libusb = false;
for (entries) |e| {
if (!e.needs_libusb) continue;
if (!has_libusb) {
try buf.appendSlice(allocator, "\n# Raw USB device node access for libusb-claimed devices\n");
has_libusb = true;
}
const rule = try std.fmt.allocPrint(
allocator,
"ACTION==\"add\", SUBSYSTEM==\"usb\", ENV{{DEVTYPE}}==\"usb_device\", ATTR{{idVendor}}==\"{x:0>4}\", ATTR{{idProduct}}==\"{x:0>4}\", TAG+=\"uaccess\", GROUP=\"input\", MODE=\"0660\"\n# {s}\n",
.{ e.vid, e.pid, e.name },
);
defer allocator.free(rule);
try buf.appendSlice(allocator, rule);
}

var f = try std.fs.createFileAbsolute(rules_path, .{ .truncate = true });
defer f.close();
try f.writeAll(buf.items);
Expand Down Expand Up @@ -758,15 +782,18 @@ fn extractVidPid(allocator: std.mem.Allocator, path: []const u8, entries: *std.A
var name_buf: [256]u8 = undefined;
var name: []const u8 = std.fs.path.stem(path);
var clone_vid_pid: bool = false;
var needs_libusb: bool = false;
var in_device_section = false;
var in_ffb_section = false;
var in_interface_section = false;

var lines = std.mem.splitScalar(u8, content, '\n');
while (lines.next()) |line| {
const trimmed = std.mem.trim(u8, line, " \t\r");
if (trimmed.len > 0 and trimmed[0] == '[') {
in_device_section = std.mem.startsWith(u8, trimmed, "[device]");
in_ffb_section = std.mem.startsWith(u8, trimmed, "[output.force_feedback]");
in_interface_section = std.mem.startsWith(u8, trimmed, "[[device.interface]]");
continue;
}
if (in_device_section) {
Expand All @@ -785,6 +812,17 @@ fn extractVidPid(allocator: std.mem.Allocator, path: []const u8, entries: *std.A
clone_vid_pid = std.mem.eql(u8, val, "true");
}
}
} else if (in_interface_section) {
// vendor/suppress interfaces are claimed via libusb, not hidraw.
if (isFieldKey(trimmed, "class")) {
if (std.mem.indexOfScalar(u8, trimmed, '"')) |q1| {
if (std.mem.indexOfScalarPos(u8, trimmed, q1 + 1, '"')) |q2| {
const val = trimmed[q1 + 1 .. q2];
if (std.mem.eql(u8, val, "vendor") or std.mem.eql(u8, val, "suppress"))
needs_libusb = true;
}
}
}
}
}

Expand All @@ -794,6 +832,7 @@ fn extractVidPid(allocator: std.mem.Allocator, path: []const u8, entries: *std.A
.pid = dev.pid,
.block_kernel_drivers = dev.block_kernel_drivers,
.clone_vid_pid = clone_vid_pid,
.needs_libusb = needs_libusb,
});
}

Expand Down
37 changes: 37 additions & 0 deletions src/config/device.zig
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ pub fn isSuppressClass(class: []const u8) bool {
return std.mem.eql(u8, class, "suppress");
}

/// True when any interface is claimed via libusb (vendor or suppress class),
/// which needs raw USB device-node access rather than a hidraw node.
pub fn usesLibusb(cfg: *const DeviceConfig) bool {
for (cfg.device.interface) |iface| {
if (std.mem.eql(u8, iface.class, "vendor") or isSuppressClass(iface.class)) return true;
}
return false;
}

fn isSuppressInterface(cfg: *const DeviceConfig, iface_id: i64) bool {
for (cfg.device.interface) |iface| {
if (iface.id == iface_id) return isSuppressClass(iface.class);
Expand Down Expand Up @@ -632,6 +641,34 @@ test "device: vader5 IF1 is claimed via libusb (vendor transport)" {
try std.testing.expect(init_cfg.enable != null);
}

test "device: usesLibusb true for vendor/suppress, false for hid-only" {
const allocator = std.testing.allocator;

const vader = try parseFile(allocator, "devices/flydigi/vader5.toml");
defer vader.deinit();
try std.testing.expect(usesLibusb(&vader.value));

const hid_only =
\\[device]
\\name = "H"
\\vid = 1
\\pid = 2
\\[[device.interface]]
\\id = 0
\\class = "hid"
\\[[report]]
\\name = "r"
\\interface = 0
\\size = 1
\\[report.match]
\\offset = 0
\\expect = [0x01]
;
const hid = try parseString(allocator, hid_only);
defer hid.deinit();
try std.testing.expect(!usesLibusb(&hid.value));
}

fn suppressIndexToml(comptime suppress_first: bool) []const u8 {
const report_block =
\\[[device.interface]]
Expand Down
27 changes: 25 additions & 2 deletions src/supervisor.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2177,7 +2177,7 @@ pub const Supervisor = struct {

const inst_ptr = try self.allocator.create(DeviceInstance);
inst_ptr.* = DeviceInstance.init(self.allocator, &cfg_ptr.value, init_mapping, phys, &self.daemon_uniq_counter, .{}) catch |err| {
std.log.warn("DeviceInstance.init for {s}: {}", .{ hidraw_path, err });
logBindFailure(&cfg_ptr.value, hidraw_path, err);
if (isTransientOpenError(err)) self.enqueueHotplugRetryForPath(hidraw_path);
self.allocator.destroy(inst_ptr);
if (default_pr_ptr) |p| {
Expand Down Expand Up @@ -2314,6 +2314,29 @@ pub const Supervisor = struct {
};
}

fn isLibusbClaimError(err: anyerror) bool {
return switch (err) {
error.NotFound, error.Busy, error.ClaimFailed => true,
else => false,
};
Comment on lines +2317 to +2321

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Libusb busy not retried 🐞 Bug ☼ Reliability

isLibusbClaimError now treats error.Busy as a libusb claim failure, but the retry path still
depends on isTransientOpenError; since isTransientOpenError does not include error.Busy, a
LIBUSB_ERROR_BUSY bind failure will log and then drop without scheduling a retry. This can leave a
device missing from padctl status until the user manually replugs/restarts the daemon.
Agent Prompt
## Issue description
`usbraw` can return `error.Busy` (mapped from `LIBUSB_ERROR_BUSY`). The supervisor currently enqueues a hotplug retry only when `isTransientOpenError(err)` is true; `error.Busy` is not considered transient there, so a busy libusb claim can cause a permanent drop until user intervention.

## Issue Context
This PR adds `isLibusbClaimError()` (including `error.Busy`) and `logBindFailure()` for libusb devices, but does not adjust the retry classification to match.

## Fix Focus Areas
- src/supervisor.zig[2300-2315]
- src/supervisor.zig[2317-2338]
- src/io/usbraw.zig[95-114]

### Suggested fix
Add `error.Busy` to `Supervisor.isTransientOpenError()` (or map `usbraw`'s busy condition to an existing transient error like `error.DeviceBusy`) so the existing retry path (`enqueueHotplugRetryForPath`) also covers libusb busy claim failures.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}

/// A libusb-claimed device that fails to open/claim drops out silently
/// (never enters `managed`, so `padctl status` shows nothing). Surface the
/// likely cause and remedy instead, since it almost always means the raw
/// USB device node is not accessible to the unprivileged daemon.
fn logBindFailure(cfg: *const DeviceConfig, key: []const u8, err: anyerror) void {
if (config_device.usesLibusb(cfg) and isLibusbClaimError(err)) {
std.log.warn(
"cannot claim \"{s}\" via libusb ({}): the raw USB device node is not accessible. " ++
"Install the padctl udev rules and add your user to the 'input' group (or run the daemon privileged), then replug.",
.{ cfg.device.name, err },
);
} else {
std.log.warn("DeviceInstance.init for {s}: {}", .{ key, err });
}
}

pub fn attachWithRoot(self: *Supervisor, devname: []const u8, dev_root: []const u8) !void {
var path_buf: [128]u8 = undefined;
const path = try std.fmt.bufPrint(&path_buf, "{s}/{s}", .{ dev_root, devname });
Expand Down Expand Up @@ -2384,7 +2407,7 @@ pub const Supervisor = struct {

const inst_ptr = try self.allocator.create(DeviceInstance);
inst_ptr.* = DeviceInstance.init(self.allocator, cfg.?, init_mapping, phys, &self.daemon_uniq_counter, .{}) catch |err| {
std.log.warn("DeviceInstance.init for {s}: {}", .{ path, err });
logBindFailure(cfg.?, path, err);
self.allocator.destroy(inst_ptr);
if (default_pr_ptr) |p| {
p.deinit();
Expand Down
Loading