Skip to content

Conversation

@andelf
Copy link
Contributor

@andelf andelf commented Dec 7, 2025

Replace rusb (libusb bindings) with nusb (pure Rust USB library):

  • Remove rusb dependency, add nusb and futures-lite
  • Simplify USB code: DeviceHandle -> Interface
  • Use async bulk_out/bulk_in APIs with block_on for blocking
  • Interface automatically released on drop (no manual release needed)
  • Keep Windows CH375DLL support unchanged

Benefits:

  • Pure Rust, no C library dependency
  • Easier cross-compilation (no libusb to compile)
  • Cleaner API

🤖 Generated with Claude Code

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR migrates the USB transport layer from rusb (libusb FFI bindings) to nusb (pure Rust USB library), eliminating the C library dependency and simplifying cross-compilation.

Key Changes:

  • Replaced rusb::DeviceHandle with nusb::Interface for USB device management
  • Migrated to async bulk I/O APIs with futures-lite::block_on for synchronous execution
  • Simplified resource management by relying on automatic interface release on drop
  • Removed vendored libusb feature and associated build dependencies

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/transport/usb.rs Core migration: replaced rusb API calls with nusb equivalents, simplified device enumeration and opening logic, and migrated to async bulk transfer APIs
Cargo.toml Removed rusb dependency and vendored-libusb feature, added nusb and futures-lite dependencies
Cargo.lock Updated dependency graph reflecting removal of libusb build toolchain (cc, pkg-config, vcpkg) and addition of nusb dependencies
Comments suppressed due to low confidence (1)

src/transport/usb.rs:112

  • The endpoint verification loop doesn't break early once both endpoints are found. This means it will continue iterating through all interfaces and alternate settings even after finding both required endpoints, which is inefficient.

Consider adding if endpoint_out_found && endpoint_in_found { break; } inside the inner loop to exit early once both endpoints are located.

        for iface in active_config.interfaces() {
            for alt in iface.alt_settings() {
                for ep in alt.endpoints() {
                    if ep.address() == ENDPOINT_OUT {
                        endpoint_out_found = true;
                    }
                    if ep.address() == ENDPOINT_IN {
                        endpoint_in_found = true;
                    }
                }
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

anyhow::bail!("Failed to open USB device");
}
};
log::warn!("It's likely the udev rules is not installed properly. Please refer to README.md for more details.");
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

Grammar issue: "the udev rules is" should be "the udev rules are" since "rules" is plural.

Corrected: "It's likely the udev rules are not installed properly. Please refer to README.md for more details."

Suggested change
log::warn!("It's likely the udev rules is not installed properly. Please refer to README.md for more details.");
log::warn!("It's likely the udev rules are not installed properly. Please refer to README.md for more details.");

Copilot uses AI. Check for mistakes.
Comment on lines 169 to 174
fn recv_raw(&mut self, _timeout: Duration) -> Result<Vec<u8>> {
if let Some(ref interface) = self.interface {
let fut = interface.bulk_in(ENDPOINT_IN, RequestBuffer::new(64));
let completion = futures_lite::future::block_on(fut);
completion.status.map_err(|e| anyhow::anyhow!("USB read error: {}", e))?;
Ok(completion.data.to_vec())
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The timeout parameter is being ignored in the new implementation. The old rusb code used handle.read_bulk(ENDPOINT_IN, &mut buf, timeout) which respected the timeout parameter. The new nusb implementation uses block_on with no timeout mechanism, which could cause the application to hang indefinitely if the USB device doesn't respond.

Consider using a timeout mechanism with the async operation, such as futures_lite::future::or() with a timer, or check if nusb provides timeout support for transfers.

Copilot uses AI. Check for mistakes.
if let Some(ref mut handle) = self.device_handle {
handle.write_bulk(ENDPOINT_OUT, raw, Duration::from_millis(USB_TIMEOUT_MS))?;
if let Some(ref interface) = self.interface {
let fut = interface.bulk_out(ENDPOINT_OUT, raw.to_vec());
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The to_vec() call creates an unnecessary copy of the data being sent. Since raw is already a slice, this allocates new memory and copies the data, which impacts performance for every USB write operation.

Consider if nusb's bulk_out API supports borrowing the data instead of requiring owned data. If the API requires owned data due to the async nature, this may be unavoidable, but it's worth documenting or investigating alternatives.

Suggested change
let fut = interface.bulk_out(ENDPOINT_OUT, raw.to_vec());
let fut = interface.bulk_out(ENDPOINT_OUT, raw);

Copilot uses AI. Check for mistakes.
let fut = interface.bulk_in(ENDPOINT_IN, RequestBuffer::new(64));
let completion = futures_lite::future::block_on(fut);
completion.status.map_err(|e| anyhow::anyhow!("USB read error: {}", e))?;
Ok(completion.data.to_vec())
Copy link

Copilot AI Dec 7, 2025

Choose a reason for hiding this comment

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

The to_vec() call creates an unnecessary copy of the received data. The data is already returned by nusb as completion.data, and converting it to a Vec creates another allocation and copy.

Consider if completion.data can be returned directly or converted more efficiently. If it's already a Vec or can be converted without copying, that would improve performance.

Suggested change
Ok(completion.data.to_vec())
Ok(completion.data)

Copilot uses AI. Check for mistakes.
Replace rusb (libusb bindings) with nusb (pure Rust USB library):

- Remove rusb dependency, add nusb, futures-lite, and async-io
- Simplify USB code: DeviceHandle -> Interface
- Use async bulk_out/bulk_in APIs with block_on for blocking
- Implement proper timeout handling for recv_raw using futures_lite::future::or
- Interface automatically released on drop (no manual release needed)
- Keep Windows CH375DLL support unchanged
- Fix grammar: "udev rules is" -> "udev rules are"

Benefits:
- Pure Rust, no C library dependency
- Easier cross-compilation (no libusb to compile)
- Cleaner API

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
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.

2 participants