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

HardwareSerial::attachRts - new optional parameter to invert signal #765

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

Conversation

KurtE
Copy link
Contributor

@KurtE KurtE commented Sep 27, 2024

Added parameter to attachRts that if set will invert the signals:

bool attachRts(uint8_t pin, bool invert_signal=false);

This should allow more flexibility in connecting two teensy 4.x boards to each other and using the CTS/RTS between the two boards for flow control.

This is in response to the thread:
https://forum.pjrc.com/index.php?threads/teensy-4-0-4-1-serial1-attachcts.75899/#post-350107

Where I felt somewhat strange to require someone to have to use hardware inverters to do this.

Added  parameter to attachRts that if set will invert the signals:
```
bool attachRts(uint8_t pin, bool invert_signal=false);
```

This should allow more flexability in connecting two teensy 4.x boards to each other and using the CTS/RTS between the two boards for flow control.

This is in response to the thread:
https://forum.pjrc.com/index.php?threads/teensy-4-0-4-1-serial1-attachcts.75899/#post-350107

Where I felt somewhat strange to require somone to have to use hardware inverters to do this.
@PaulStoffregen
Copy link
Owner

I'm hoping we can avoid more true / false parameters in public APIs. Main issue is they lead to less readable user code, where you would need to look up in the documentation what the true vs false input means to understand what the program is doing.

In this case, let's go with a descriptive function name like attachRtsInverted().

@KurtE
Copy link
Contributor Author

KurtE commented Sep 27, 2024

We can do... But

Personally, I believe that this:

In this case, let's go with a descriptive function name like attachRtsInverted().

Is equally unreadable... Inverted from What?

If we really wanted readability, we could do something like:

typedef enum {
    RTS_ASSERTED_PIN_SIGNAL_LOW = 0,  // default
    RTS_ASSERTED_PIN_SIGNAL_HIGH = 1
} RTS_assert_pin_level_t;

bool attachRts(uint8_t pin, RTS_assert_pin_level_t assert=RTS_ASSERTED_PIN_SIGNAL_LOW); 

Warning typed on fly so probably typos. And the comments above the definition should then
change to match.

But your call

@KurtE
Copy link
Contributor Author

KurtE commented Sep 28, 2024

I pushed up changes to match the API name you requested...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants