Skip to content

Fix 1200-bps touch DTR handling (Windows)#2234

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 30, 2023
Merged

Fix 1200-bps touch DTR handling (Windows) #2234

merged 1 commit into from
Jun 30, 2023

Conversation

cmaglie
Copy link
Member

@cmagliecmaglie commented Jun 30, 2023•
edited
Loading

Please check if the PR fulfills these requirements

See how to contribute

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • The PR follows
    our contributing guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • UPGRADING.md has been updated with a migration guide (for breaking changes)
  • configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

This PR disables the step that sets DTR low on the 1200-bps touch subroutine.

This is the reason why it was originally introduced: arduino/Arduino@a6909bd

Fix auto-reset on Leonardo-derived boards from Linux hosts Also renamed the touchPort() function, as it's now unambiguously single-purpose. The 1200bps reset from Linux hosts wasn't working with these newer JSSC-based versions. Adding a step which explicitly sets DTR low (via a TIOCMSET ioctl clearing DTR) fixes this. I'm fairly sure the reason why this worked on older Arduino with librxtx and not with jssc is that librxtx appears to keep HUPCL in the termio flags, but jssc appears to remove it. If HUPCL ("hangup on close") is set, it causes DTR to be explicitly pulled low on close.

Why we are removing it now?

  • Windows does preserve the state of the RTS/DTR bits on the next opening of the serial port.
  • The serial library used in the Arduino IDE 1.8.x has a bug when trying to set DTR=false, on successive opening of the port the DTR line is set back high by the USB serial driver. This works differently from the serial library we use in the Arduino CLI, which sets DTR=false for good and this setting is preserved on the next opening of the port.
  • Having the serial port left in a state with DTR=false may cause problems with tools uploading later.

It may probably be applied to all OS but, for now, it will be disabled only for Windows to reduce the testing surface.

What is the current behavior?

Upload may fail on some boards (arduino/ArduinoCore-renesas#10).

What is the new behavior?

Uploads should be successful...

Does this PR introduce a breaking change, and is titled accordingly?

No, in theory. A lot of testing is required to be sure.

Other information

@cmaglie
Disable DTR clearing on 1200-bps touch (only on Windows)
31d9c34
The reason why it was originally introduced: arduino/Arduino@a6909bd Why we are removing it now? * Windows does preserve the state of the RTS/DTR bits on successive opening of the serial port. * The serial library used in the Arduino IDE 1.8.x has a bug when trying to set DTR=false, on successive opening of the port the DTR line is set back high by the USB serial driver. This works differently from the serial library we use in the Arduino CLI, that sets DTR=false for good and this change is preserved on the successive opening of the port. * Having the serial port left in a state with DTR=false may cause problems to tools uploading later. It may probably completely removed, but for now, to reduce the testing surface, it will be disabled only for Windows.
@cmagliecmaglie self-assigned this Jun 30, 2023
@kittaakoskittaakos mentioned this pull request Jun 30, 2023
Closed
4 tasks
@cmaglie
Copy link
MemberAuthor

Failing checks are OK ✅. They are happening because this PR is against an old release branch.
It will be eventually cherry-picked on master.

@cmagliecmaglie added topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project priority: high Resolution is a high priority criticality: highest Of highest impact labels Jun 30, 2023
@kittaakoskittaakos mentioned this pull request Jun 30, 2023
Merged
4 tasks
@cmagliecmaglie merged commit b08dbd5 into arduino:0.32.xJun 30, 2023
@cmagliecmaglie deleted the fix_touch_dtr_handling branch June 30, 2023 11:39
kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Jun 30, 2023
chore(cli): Bumped the CLI version to 0.32.3
a03d1be
Ref: arduino/arduino-cli#2234 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
@kittaakoskittaakos mentioned this pull request Jun 30, 2023
Merged
4 tasks
kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Jun 30, 2023
@kittaakos
chore(cli): Bumped the CLI version to 0.32.3
f3b99b0
Ref: arduino/arduino-cli#2234 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
kittaakos pushed a commit to arduino/arduino-ide that referenced this pull request Jun 30, 2023
chore(cli): Bumped the CLI version to 0.32.3
c318223
Ref: arduino/arduino-cli#2234 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
cmaglie added a commit that referenced this pull request Jun 30, 2023
@cmaglie
Disable DTR clearing on 1200-bps touch (only on Windows) (#2234)
347bfeb
The reason why it was originally introduced: arduino/Arduino@a6909bd Why we are removing it now? * Windows does preserve the state of the RTS/DTR bits on successive opening of the serial port. * The serial library used in the Arduino IDE 1.8.x has a bug when trying to set DTR=false, on successive opening of the port the DTR line is set back high by the USB serial driver. This works differently from the serial library we use in the Arduino CLI, that sets DTR=false for good and this change is preserved on the successive opening of the port. * Having the serial port left in a state with DTR=false may cause problems to tools uploading later. It may probably completely removed, but for now, to reduce the testing surface, it will be disabled only for Windows.
cmaglie added a commit that referenced this pull request Jun 30, 2023
@cmaglie
Disable DTR clearing on 1200-bps touch (only on Windows) (#2234)
fe6d749
The reason why it was originally introduced: arduino/Arduino@a6909bd Why we are removing it now? * Windows does preserve the state of the RTS/DTR bits on successive opening of the serial port. * The serial library used in the Arduino IDE 1.8.x has a bug when trying to set DTR=false, on successive opening of the port the DTR line is set back high by the USB serial driver. This works differently from the serial library we use in the Arduino CLI, that sets DTR=false for good and this change is preserved on the successive opening of the port. * Having the serial port left in a state with DTR=false may cause problems to tools uploading later. It may probably completely removed, but for now, to reduce the testing surface, it will be disabled only for Windows.
@per1234per1234 added the os: windows Specific to Windows operating system label Jun 30, 2023
@per1234per1234 mentioned this pull request Jul 1, 2023
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
Labels
criticality: highestOf highest impactos: windowsSpecific to Windows operating systempriority: highResolution is a high prioritytopic: codeRelated to content of the project itselftype: imperfectionPerceived defect in any part of project
3 participants
@cmaglie@umbynos@per1234
close