Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify determination of transport #9

Closed
wants to merge 2 commits into from

Conversation

dsandber
Copy link

@dsandber dsandber commented Jul 3, 2021

This code change makes so that a port is a considered a USB port if and only if it has a USB Vendor ID / Product ID combo. The old method was more complicated and was failing for the CP2102 chip.

@martinling
Copy link
Contributor

Cross-referencing discussion at: https://sigrok.org/bugzilla/show_bug.cgi?id=1698

@MCUdude
Copy link
Contributor

MCUdude commented Sep 19, 2023

@martinling is there any progress regarding this issue? We're currently implementing libserialport into the Avrdude project for supporting "Automatic port discovery", and no Silabs USB to serial adapter chip (I've tried with CP2102N and CP2104) can be used with libserialport on MacOS (10.14). I've tried a bunch of other adapters, both "standard" ones (FT232R, PL2303, FT231X, CH340, CP210x) and manufacturer-specific ones (Arduino UNO/MEGA/UNO Wifi Rev2/Nano Every, Microchip ATmega4809 Curiosity Nano), and I've only had issues with the CP210x chips.

It would be awesome if someone could look into this!

@MCUdude
Copy link
Contributor

MCUdude commented Sep 19, 2023

This code change makes so that a port is a considered a USB port if and only if it has a USB Vendor ID / Product ID combo.

For the Avrdude project, it's very convenient that libserialport is able to list ports that are native as well. It's nice to provide a list to the user of all available serial ports, not just those that are native. Determining if a port is native is as simple as checking if sp_get_port_usb_vid_pid() doesn't return SP_OK.
So for us, merging this PR as it is would make this library way less useful for us. See comment below

@MCUdude
Copy link
Contributor

MCUdude commented Sep 19, 2023

It looks like I was a bit too quick to conclude.

After a bit of testing, it seems like this PR works great, and it's still able to find and list native serial ports (in my case,/dev/cu.Bluetooth-Incoming-Port).

So after all, it might be a good idea to flag a port as native if there's no VID and PID.

@MCUdude
Copy link
Contributor

MCUdude commented Sep 19, 2023

@martinling Sorry for all the noise, but I went down the rabbit hole to try to figure out why the Silabs chips isn't "detected" by CFSTR("IOClass") or CFSTR("IOProviderClass"), which it should.

And it turns out that it's a very easy fix to all this. Have a look at the following code:

libserialport/macosx.c

Lines 65 to 87 in 6f9b03e

IORegistryEntryGetParentEntry(ioport, kIOServicePlane, &ioparent);
if ((cf_property=IORegistryEntrySearchCFProperty(ioparent,kIOServicePlane,
CFSTR("IOClass"), kCFAllocatorDefault,
kIORegistryIterateRecursively | kIORegistryIterateParents))) {
if (CFStringGetCString(cf_property, class, sizeof(class),
kCFStringEncodingASCII) &&
strstr(class, "USB")) {
DEBUG("Found USB class device");
port->transport = SP_TRANSPORT_USB;
}
CFRelease(cf_property);
}
if ((cf_property=IORegistryEntrySearchCFProperty(ioparent,kIOServicePlane,
CFSTR("IOProviderClass"), kCFAllocatorDefault,
kIORegistryIterateRecursively | kIORegistryIterateParents))) {
if (CFStringGetCString(cf_property, class, sizeof(class),
kCFStringEncodingASCII) &&
strstr(class, "USB")) {
DEBUG("Found USB class device");
port->transport = SP_TRANSPORT_USB;
}
CFRelease(cf_property);
}

It turns out that the class string that's returned from CFSTR("IOClass") is com_silabs_driver_CP210xVCPDriver, and the string returned from CFSTR("IOProviderClass") is IOUSBHostInterface.

The string com_silabs_driver_CP210xVCPDriver does not contains "USB", so there won't be any match here.

The second string, IOUSBHostInterface does contain "USB, but it is longer than the 16 characters allocated for class, and thus it fails. So the simple solution to all this is to just increase the size of class to hold enough characters; at least 64.

The reason why this works for other USB to serial chips is that their CFSTR("IOProviderClass") is IOUSBInterface, which does fit in the 16-character class string.

So this is the solution:

diff --git a/macosx.c b/macosx.c
index a0df84e..6c93d2c 100644
--- a/macosx.c
+++ b/macosx.c
@@ -36,7 +36,7 @@ SP_PRIV enum sp_return get_port_details(struct sp_port *port)
        io_object_t ioport, ioparent;
        CFTypeRef cf_property, cf_bus, cf_address, cf_vendor, cf_product;
        Boolean result;
-       char path[PATH_MAX], class[16];
+       char path[PATH_MAX], class[64];
 
        DEBUG("Getting serial port list");
        if (!(classes = IOServiceMatching(kIOSerialBSDServiceValue)))

@mcuee
Copy link

mcuee commented Jan 29, 2024

I have tested this PR and it is working with CH340, under macOS 14.3 with Mac Mini M1 2020.

Current git does not work.

@mcuee
Copy link

mcuee commented Jan 29, 2024

This PR also works with FT232R.

mcuee@mcuees-Mac-mini examples % ./list_ports                         
Getting port list.
Found port: /dev/cu.wlan-debug
Found port: /dev/cu.Bluetooth-Incoming-Port
Found port: /dev/cu.usbserial-A50285BI
Found port: /dev/cu.usbserial-3
Found 4 ports.
Freeing port list.
mcuee@mcuees-Mac-mini examples % ./port_info /dev/cu.usbserial-3      
Looking for port /dev/cu.usbserial-3.
Port name: /dev/cu.usbserial-3
Description: FT232R USB UART
Type: USB
Manufacturer: FTDI
Product: FT232R USB UART
Serial: A50285BI
VID: 0403 PID: 6001
Bus: 1 Address: 4463960
Freeing port.
mcuee@mcuees-Mac-mini examples % ./port_info /dev/cu.usbserial-A50285BI 
Looking for port /dev/cu.usbserial-A50285BI.
Port name: /dev/cu.usbserial-A50285BI
Description: FT232R USB UART
Type: USB
Manufacturer: FTDI
Product: FT232R USB UART
Serial: A50285BI
VID: 0403 PID: 6001
Bus: 1 Address: 73063768
Freeing port.

Current git does not work.

mcuee@mcuees-Mac-mini bin_org % ./port_info /dev/cu.usbserial-3       
Looking for port /dev/cu.usbserial-3.
Port name: /dev/cu.usbserial-3
Description: FT232R USB UART
Type: Native
Freeing port.
mcuee@mcuees-Mac-mini bin_org % ./port_info /dev/cu.usbserial-A50285BI
Looking for port /dev/cu.usbserial-A50285BI.
Port name: /dev/cu.usbserial-A50285BI
Description: FT232R USB UART
Type: Native
Freeing port.

@mcuee
Copy link

mcuee commented Jan 29, 2024

This PR works with CP2102 as well.

mcuee@mcuees-Mac-mini examples % pwd
/Users/mcuee/build/avr/avrdude_test/libserialport_pr9/examples

mcuee@mcuees-Mac-mini examples % ./list_ports
Getting port list.
Found port: /dev/cu.wlan-debug
Found port: /dev/cu.Bluetooth-Incoming-Port
Found port: /dev/cu.usbserial-0001
Found port: /dev/cu.SLAB_USBtoUART
Found 4 ports.
Freeing port list.

mcuee@mcuees-Mac-mini examples % ./port_info /dev/cu.usbserial-0001 
Looking for port /dev/cu.usbserial-0001.
Port name: /dev/cu.usbserial-0001
Description: CP2102 USB to UART Bridge Controller
Type: USB
Manufacturer: Silicon Labs
Product: CP2102 USB to UART Bridge Controller
Serial: 0001
VID: 10C4 PID: EA60
Bus: 1 Address: 42474840
Freeing port.

mcuee@mcuees-Mac-mini examples % ./port_info /dev/cu.SLAB_USBtoUART 
Looking for port /dev/cu.SLAB_USBtoUART.
Port name: /dev/cu.SLAB_USBtoUART
Description: CP210x USB to UART Bridge Controller
Type: USB
Manufacturer: Silicon Labs
Product: CP210x USB to UART Bridge Controller
Serial: 0001
VID: 10C4 PID: EA60
Bus: 1 Address: 82877784
Freeing port.

Current git does not work.

mcuee@mcuees-Mac-mini examples % cd bin_org 
mcuee@mcuees-Mac-mini bin_org % ./port_info /dev/cu.SLAB_USBtoUART
Looking for port /dev/cu.SLAB_USBtoUART.
Port name: /dev/cu.SLAB_USBtoUART
Description: CP210x USB to UART Bridge Controller
Type: Native
Freeing port.

mcuee@mcuees-Mac-mini bin_org % ./port_info /dev/cu.usbserial-0001
Looking for port /dev/cu.usbserial-0001.
Port name: /dev/cu.usbserial-0001
Description: CP2102 USB to UART Bridge Controller
Type: Native
Freeing port.

@abraxa
Copy link
Member

abraxa commented Aug 30, 2024

Closing this PR as #9 has been merged instead. Thanks to everyone who worked on this topic!

@abraxa abraxa closed this Aug 30, 2024
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.

5 participants