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

fix: linux baudRate and latency errors #2253

Merged
merged 1 commit into from
May 26, 2021
Merged

Conversation

reconbot
Copy link
Member

  • linuxGetLowLatencyMode would incorrectly get an error
  • linuxSetLowLatencyMode would incorrectly get an error
  • linuxGetSystemBaudRate would incorrectly get an error
  • linuxSetCustomBaudRate would incorrectly get an error

Thanks to @GazHank for highlighting work was needed in this area

@reconbot
Copy link
Member Author

@GazHank I don't have a linux machine handy, would you be able to test this branch?

@GazHank
Copy link
Contributor

GazHank commented May 26, 2021

With the parentheses change per above, I'm getting successful results:

  • setting DTR true etc seems to work
  • no errors when setting low latency with admin rights
  • suitable error messages when access rights don't permit setting low latency

I've tested on:

  • Ubuntu 20.04.2 LTS
  • Node.js 14.16.0,
  • Chromium 89.0.4389.128,
  • Electron 12.0.9

I should call out that I'm not really able to test that low latency mode works, only that it doesn't seem to break stuff anymore :-)

Additionally I've tried sending some dodgy values (0 and very large numbers) to the update baudrate, and so far I've not managed to trigger any error codes (I double checked and this is the case with or without the change to linuxSetCustomBaudRate, so I don't think we have broken anything.

@reconbot
Copy link
Member Author

I think dodgy baudrates are a serial driver concern less a linux kernel concern and they seem to have chosen to silently ignore them in general. Hense we have this bolted on readBaudrate method, which maybe setBaudrate should be calling to check it's work.

@GazHank
Copy link
Contributor

GazHank commented May 26, 2021

@reconbot let me know if there are any extra tests you'd like me to run. I'm normally on Win10, but can fire up Linux for some testing if I can help

@reconbot
Copy link
Member Author

Thank you =)

- linuxGetLowLatencyMode would incorrectly get an error
- linuxSetLowLatencyMode would incorrectly get an error
- linuxGetSystemBaudRate would incorrectly get an error
- linuxSetCustomBaudRate would incorrectly get an error
@reconbot
Copy link
Member Author

#1676 already exists 🤷

@reconbot reconbot merged commit 015bc17 into master May 26, 2021
@reconbot reconbot deleted the reconbot/linux-errors branch May 26, 2021 21:09
lvogt pushed a commit to lvogt/node-serialport that referenced this pull request Aug 10, 2021
- linuxGetLowLatencyMode would incorrectly get an error
- linuxSetLowLatencyMode would incorrectly get an error
- linuxGetSystemBaudRate would incorrectly get an error
- linuxSetCustomBaudRate would incorrectly get an error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants