Version: https://github.com/sigrokproject/libserialport/tree/1b011060df5579c15bd4e103c2d8afc4d442fd72 Driver: XR21B1411 USB UART, https://www.maxlinear.com/product/interface/uarts/usb-uarts/xr21b1411 OS: Windows 10 Pro Version 1909 Build 18363.1139 64 bit Problem: When I manually set it to 9600 and then pass it to set_config it work and SetCommState doesn't fail with The parameter is incorrect. As you can see after get_config I have placed a DEBUG_FMT to print the config.baudrate which is zero. It is probably a driver issue with this particular USB UART. Solution: When config.baudrate is zero set it to an supported value of e.g 9600. https://github.com/sigrokproject/libserialport/blob/1b011060df5579c15bd4e103c2d8afc4d442fd72/serialport.c#L586 Log: sp: sp_get_port_by_name(COM5, 009D2090) called. sp: Building structure for port COM5. sp: get_port_details returning SP_OK. sp: sp_get_port_by_name returning SP_OK. sp: sp_open(010C3DD8, 0x3) called. sp: Opening port COM5. sp: New wait running in background. sp: restart_wait returning SP_OK. sp: get_config(010C3DD8, 00EFF494, 00EFF468) called. sp: Getting configuration for port COM5. sp: get_config returning SP_OK. sp: config.baudrate = 0. sp: set_config(010C3DD8, 00EFF494, 00EFF468) called. sp: Setting configuration for port COM5. sp: await_write_completion(010C3DD8) called. sp: await_write_completion returning SP_OK. sp: set_config(baudrate: 0) called. sp: sp_last_error_message() called. sp: sp_last_error_message returning The parameter is incorrect.. sp: set_config returning SP_ERR_FAIL: SetCommState() failed: The parameter is incorrect.. sp: sp_free_error_message(The parameter is incorrect.) called. sp: sp_free_error_message returning. sp: sp_close(010C3DD8) called. sp: Closing port COM5. sp: sp_close returning SP_OK. sp: sp_open returning SP_ERR_FAIL.
Problem (rewritten): When calling sp_open("COM5", SP_MODE_READ_WRITE) it fails in SetCommState which is inside set_config. Before this call get_config on the serial port is called and the config.baudrate field is zero. When setting config.baudrate to 9600 after the get_config call the SetCommState call succeeds.
Thank you, Jerry!! I received my Exar XR21B1411L-0A-EB evaluation board yesterday and had the same problem with libserialport you reported. (And I might have had the same problem, but intermittently, with the Microchip MCP2200 evaluation board.) After making your suggested change to serialport.c and recompiling the library, it worked. I am running Windows 7 x64. By the way, the XR21B1411 seems to have correct RTS/CTS flow control handshaking, unlike so many other devices that are flawed, like the FTDI chips, Microchip MCP2200, etc. (If CTS is negated while they are transmitting a character, they transmit an additional one or more characters, which my system then loses. The XR21B1411 stops transmitting at the end of the current character, until CTS is asserted again.)
PR has been created https://github.com/sigrokproject/libserialport/pull/6
I've commented on the PR - the fix should be to set -1 in the config struct when a value of zero is read in get_config(). We already have logic to handle the case of unknown/unset/invalid settings like this by representing them as -1 in the config struct.
@David could you test https://github.com/sigrokproject/libserialport/pull/6
I briefly tested the patch, and so far it is working for me. Note: - I don't have a GitHub account, and have never used Git - so I used a text editor to manually paste the patch into my copy of serialport.c, and hopefully got it right - there was a bit of back and forth on what the final correct patch should be, but looking at commit c192c6f8df0e828bf36cae889f846fba5de81a67 "Revert old behaviour" it looks like it just inserts these 6 lines after line 592 -- is that correct?: // When the OS gives an invalid baudrate we need to set it to a sane value // See https://sigrok.org/bugzilla/show_bug.cgi?id=1632 if (config.baudrate == 0) { config.baudrate = 9600; }
(In reply to David Wiens from comment #6) > I briefly tested the patch, and so far it is working for me. > > Note: > - I don't have a GitHub account, and have never used Git > - so I used a text editor to manually paste the patch into my copy of > serialport.c, and hopefully got it right > - there was a bit of back and forth on what the final correct patch should > be, but looking at commit c192c6f8df0e828bf36cae889f846fba5de81a67 "Revert > old behaviour" it looks like it just inserts these 6 lines after line 592 -- > is that correct?: > > // When the OS gives an invalid baudrate we need to set it to a sane > value > // See https://sigrok.org/bugzilla/show_bug.cgi?id=1632 > if (config.baudrate == 0) { > config.baudrate = 9600; > } Yes its the correct patch
Jerry, that recent patch of yours did not follow the project's style. That's when I needed to touch it, and it's when I decided to put the motivation into the source code for awareness during maintenance and to not lose this important information, while the URLs to potentially volatile discussion moved to the commit message. When you can agree to the amended version, then I could take the result to mainline. Wrote the comment here since I never got why github users keep closing PRs and open new ones just for another iteration of a commit series. But that's a different story. Don't want to spread a single discussion across several threads. Martin, can you ACK that resulting change and how it'd enter the repo? Do you expect to see some other information that is important to you? Did I summarize appropriately or miss something? See https://repo.or.cz/libserialport/gsi.git/shortlog/refs/heads/wip/zero-baudrate and the commitdiff links there. When you agree I will squash and commit.
(In reply to Gerhard Sittig from comment #8) > Jerry, that recent patch of yours did not follow the project's style. > That's when I needed to touch it, and it's when I decided to put the > motivation into the source code for awareness during maintenance and > to not lose this important information, while the URLs to potentially > volatile discussion moved to the commit message. When you can agree to > the amended version, then I could take the result to mainline. > > Wrote the comment here since I never got why github users keep closing > PRs and open new ones just for another iteration of a commit series. But > that's a different story. Don't want to spread a single discussion across > several threads. > > Martin, can you ACK that resulting change and how it'd enter the repo? > Do you expect to see some other information that is important to you? > Did I summarize appropriately or miss something? > > See > https://repo.or.cz/libserialport/gsi.git/shortlog/refs/heads/wip/zero- > baudrate > and the commitdiff links there. When you agree I will squash and commit. Seems fine for me, just go ahead
Ping. Martin, can I get your ACK or NAK on comment 8 and specifically the commits as they will enter the repo?
ACK. Looks good to me.
Fixed in commit 1abec205027b.