Skip to content

HardwareSerial: fix issue with swapping pins twice #3258

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
Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

HardwareSerial: fix issue with swapping pins twice #3258

wants to merge 2 commits into from

Conversation

igrr
Copy link
Member

@igrr igrr commented May 18, 2017

Previously calling HardwareSerial::swap twice would set the pins back
to the original setting (1, 3). This feature was lost when swap function
got an optional argument for the TX pin. This change reverts that,
restoring ability to swap pins back.
To use TX pin 2, HardwareSerial::pins or HardwareSerial::set_tx methods
can be used.

@igrr
Copy link
Member Author

igrr commented May 18, 2017

Ref #3192

@igrr
HardwareSerial: fix issue with swapping pins twice
3931249 
Previously calling HardwareSerial::swap twice would set the pins back
to the original setting (1, 3). This feature was lost when swap function
got an optional argument for the TX pin. This change reverts that,
restoring ability to swap pins back.
To use TX pin 2, HardwareSerial::pins or HardwareSerial::set_tx methods
can be used.
This change also simplifies UART pin configuration code.
@igrr igrr force-pushed the bugfix/hardwareserial_swap branch from b809a90 to 3931249 Compare May 18, 2017 16:02
@devyte
Copy link
Collaborator

devyte commented Dec 28, 2017

@igrr what is the status of this? Assuming it's ok'd in #3192 by the OP, can it just be merged, or is there anything pending?

@igrr
Copy link
Member Author

igrr commented Dec 28, 2017

This was long time ago... I don't remember how thoroughly this was tested. Will revisit this and try to test all possible pin options.

@igrr
Copy link
Member Author

igrr commented Dec 31, 2017

I've done some testing on this and there are still some issues. Will consider #3192 a known issue for 2.4.0 and work on this fix again after the release.

@igrr igrr added this to the 2.5.0 milestone Dec 31, 2017
@igrr
Copy link
Member Author

igrr commented Mar 8, 2018

#hwtest

1 similar comment
@igrr
Copy link
Member Author

igrr commented Mar 8, 2018

#hwtest

@devyte
Copy link
Collaborator

devyte commented Jul 4, 2018

@igrr what is needed to move forward on this?

@devyte
Copy link
Collaborator

devyte commented Dec 2, 2018

This doesn't fit into 2.5.0. Pushing milestone back.

@devyte devyte modified the milestones: 2.5.0, 2.6.0 Dec 2, 2018
@earlephilhower earlephilhower added merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels Oct 1, 2019
@earlephilhower
Copy link
Collaborator

Thanks for your PR, but the core and libraries have changed enough that this PR now has a merge conflict.

Could you merge it manually with the latest core, so we can consider it for future releases?

@devyte
Copy link
Collaborator

devyte commented Oct 29, 2019

This needs work, pushing back.

@devyte devyte modified the milestones: 2.6.0, 2.7.0 Oct 29, 2019
@devyte devyte added the help wanted Help needed from the community label Oct 29, 2019
@d-a-v d-a-v modified the milestones: 2.7.0, 3.0.0 Feb 21, 2020
@d-a-v d-a-v modified the milestones: 3.0.0, 3.0.1 Mar 31, 2021
@dok-net
Copy link
Contributor

dok-net commented Apr 3, 2021

@devyte Regarding the issue mentioned in the title - if there's anything else to this PR beyond that, I have not looked for it and am disregarding it - it's not present in master, and this PR could be closed. I have successfully swap()ed multiple times, and used the pins() function as well, no unexpected behavior occurs.

@d-a-v d-a-v modified the milestones: 3.0.1, 3.1 Jun 16, 2021
@mcspr mcspr closed this Oct 12, 2021
@mcspr mcspr reopened this Oct 12, 2021
@d-a-v d-a-v modified the milestones: 3.1, 4.0.0 Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
help wanted Help needed from the community merge-conflict PR has a merge conflict that needs manual correction waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@igrr @devyte @earlephilhower @dok-net @mcspr @d-a-v