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

Option for SOFTWARESERIAL_UNUSED to allow TX or RX only configs #3053

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

Conversation

sbrynen
Copy link

@sbrynen sbrynen commented Apr 29, 2015

This patch allows you to pass a pin# of "SOFTWARESERIAL_UNUSED" for a transmit or receive pin, to avoid having to use two pins in situations where you only want receive or transmit.

fixes #1022

@matthijskooijman
Copy link
Collaborator

This PR contains two commits, the second of which removes a merge conflict introduced in the first. They should be squashed together instead of remaining as two separate commits (you can use git rebase -i origin/master for that, assuming that origin is the upstream repository).

Looking at the first commit, it contains a lot of whitespace changes and introduces a README, both of which are ok changes, but they should really live in their own commits.

As for the actual code changes, they look ok at first glance (though above things make things a bit harder to review). I did notice an if like this a few times:

if ((_receivePin != SOFTWARESERIAL_UNUSED) && (active_object == this))

However, AFAICS active_object can never be set to this for objects without a valid receive pin? Not sure if it's worht checking the receive pin here (though being on the safe side might be ok, not sure).

@sbrynen
Copy link
Author

sbrynen commented Apr 29, 2015

You're right about the active_object. When I 1st hacked this in I wasn't sure if the object existed with portions of it for TX, so I added the extra check. Anyway, I suspect an integer test is somewhat faster than an object test, so I don't really see too much harm

I'll try the rebase, but they never seem to work properly for me, there's something fundamental about git that just doesn't sink into my mind; so I'll skip it.

@cmaglie cmaglie added feature request A request to make an enhancement (not a bug fix) Library: SoftwareSerial The SoftwareSerial Arduino library Architecture: AVR Applies only to the AVR microcontrollers (Uno, etc.) labels May 6, 2015
facchinm pushed a commit to arduino/ArduinoCore-avr that referenced this pull request Nov 13, 2017
@CLAassistant
Copy link

CLAassistant commented Apr 9, 2021

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture: AVR Applies only to the AVR microcontrollers (Uno, etc.) feature request A request to make an enhancement (not a bug fix) Library: SoftwareSerial The SoftwareSerial Arduino library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants