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

Slight rework of CH341 HAL #5848

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

psiegl
Copy link
Contributor

@psiegl psiegl commented Jan 13, 2025

I noticed, that the CH341 HAL could be a bit cleaned up. First, use the constructor / deconstructor of C++ to initiate/cleanup pinedio. Constructor throws error, in case not possible, which is then subsequently catched. Second, reworked the establishment of a MAC address for the CH341 HAL. The getMacAddr employs its parameter as an output. Thus, definining a similar variable that gets set upfront, it not useful for understanding the code.

@thebentern thebentern requested a review from jp-bennett January 13, 2025 19:11
@jp-bennett
Copy link
Collaborator

Looks good. I'll test it on my devices to confirm there's no surprises.

@jp-bennett
Copy link
Collaborator

This is not calculating MAC Addresses the same way as the old code. It's a surprisingly tricky thing to get this just right. I've run into similar issues.

libusb: warning [libusb_get_string_descriptor_ascii] suspicious bLength 18 for string descriptor (read 127) Serial 10000002 Serial length 8 settingsStrings length 0 MAC Address from CH341: 12f4f0efb6 settingsStrings length a MAC Address: 12f4f0efb6
vs
libusb: warning [libusb_get_string_descriptor_ascii] suspicious bLength 18 for string descriptor (read 127) Serial 10000002 *** Blank MAC Address not allowed!

Debugger shows that after the ch341 driver inits:
settingsStrings[mac_address] =
"9120F04F0EFB6"

@psiegl
Copy link
Contributor Author

psiegl commented Jan 14, 2025

Interesting, seems my CH341 doesn't have any serial that is why I didn't see any issue during rework. With lsusb -v:

Bus 001 Device 021: ID 1a86:5512 QinHeng Electronics CH341 in EPP/MEM/I2C mode, EPP/I2C adapter
Negotiated speed: Full Speed (12Mbps)
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               1.10
  bDeviceClass          255 Vendor Specific Class
  bDeviceSubClass         0 [unknown]
  bDeviceProtocol         2 
  bMaxPacketSize0         8
  idVendor           0x1a86 QinHeng Electronics
  idProduct          0x5512 CH341 in EPP/MEM/I2C mode, EPP/I2C adapter
  bcdDevice            3.04
  iManufacturer           0 
  iProduct                2 USB UART-LPT
  iSerial                 0

@psiegl
Copy link
Contributor Author

psiegl commented Jan 14, 2025

@jp-bennett I reverted the part, which reads the serial. Can you retry?

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