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

Increase max class string length to 64 #14

Closed
wants to merge 1 commit into from

Conversation

MCUdude
Copy link
Contributor

@MCUdude MCUdude commented Sep 19, 2023

#9 points out that Silabs CP210x chips don't work properly with libserialport. However, the attempted fix is a bit drastic, and only fix the symptom and not the problem.

After a bit of research, I'm pretty sure I've figured out the root cause.

char path[PATH_MAX], class[16];

The class string can only hold 16 characters, and this is fine if the assigned string is 16 characters long or less. However, the Silabs CFSTR("IOProviderClass") returns "IOUSBHostInterface", which is longer than 16 characters. Other USB to serial chips returns "IOUSBInterface", which is barely less than 16 bytes.

So all this simple PR does is increase the maximum length the class string can have.

Here's the output from the port_info example:

Without this PR:

$ ./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.

With this PR:

$ ./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: 00FEBF3C
VID: 10C4 PID: EA60
Bus: 0 Address: 0
Freeing port.

@martinling it would be great if you could review this and hopefully merge it. It would be awesome to have a libserialport version that works with Silabs chips on macOS!

@MCUdude
Copy link
Contributor Author

MCUdude commented Jan 29, 2024

@gsigh it would be great if you could have a look at this PR. It resolved an obvious overflow error that causes libserialport not not behave the way it should.

@mcuee
Copy link

mcuee commented Jan 29, 2024

Tested to be working with CH340, under macOS 14.3 with Mac Mini M1 2020. Take note 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: 81419608
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: 48815448
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 % ./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: 18636120
Freeing port.
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: 79371608
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.

@MCUdude
Copy link
Contributor Author

MCUdude commented Jan 29, 2024

@gsigh @martinling please have a look at this when you have time. Currently, libserialport does not work with several USB to serial chips on the latest macOS 14.3, due to the descriptor string being longer than 16 characters. Most likely because Apple has made some changes to the serial driver(s). #9 is a different approach to the same problem. However, this one is a lot simpler, and just fixes the bug.

@abraxa
Copy link
Member

abraxa commented Aug 30, 2024

@abraxa abraxa closed this Aug 30, 2024
@abraxa
Copy link
Member

abraxa commented Aug 30, 2024

Does the merging of this PR mean that #9 can be closed without merging or are there any other issues that this PR doesn't cover?

@MCUdude
Copy link
Contributor Author

MCUdude commented Aug 30, 2024

Thanks for merging @abraxa! Would it be possible to do a new release? This is kind of a big deal on macOS, since lots of USB to serial adapters doesn't work with macOS without this PR (CP210x, CH340, etc). Avrdude is using libserialport, and the only alternative for macOS users is to recommend them to installed a patched version of libavrdude before building and installing Avrdude.

You can safely close #9. I did a lot of research before I created this PR, and the root cause was that Apple had changed the device descriptor, so that it would no longer fit inside the 16 byte buffer.

@abraxa
Copy link
Member

abraxa commented Aug 30, 2024

Yeah, I'm currently merging what can be safely merged so that I can make a release. Thanks, I'll close #9 then.

@MCUdude
Copy link
Contributor Author

MCUdude commented Aug 30, 2024

That's great news! Thank you for doing this @abraxa!
I'll double check that brew picks up the release and installs the newly released version of libserialport when running brew install libserialport.

EDIT: I forgot to mention that when I first submitted this PR, I was using macOS 10.14 (Mojave), and only the Silabs CH210x chips were affected by the bug. However, when I upgraded to macOS 12 (Monterey), the CH340 chips was suddenly also affected.

@MCUdude MCUdude mentioned this pull request Oct 9, 2024
3 tasks
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.

3 participants