Skip to content

Conversation

@sandeepmistry
Copy link
Contributor

No description provided.


size_t Uart::write(const uint8_t data)
{
if (uc_pinRTS != NO_RTS_PIN) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why? should RTS influence only Rx?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I misunderstood the RTS term - corrected!

if (uc_pinRTS != NO_RTS_PIN) {
// if there is NOT enough space in the RX buffer, de-assert RTS
if (rxBuffer.availableForStore() < RTS_RX_THRESHOLD) {
digitalWrite(uc_pinRTS, HIGH);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

digitalWrite may be slow, we should explore direct register access.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed a commit to leverage SERCOM h/w capabilities.

digitalWrite(uc_pinRTS, LOW);
}

if (uc_pinCTS != NO_CTS_PIN) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmmhhh there is no other way here? Ideally the CTS pin should mask the tx ISR, so you can always write in the buffer without blocking (until the buffer is full). What is not clear to me is if there is a way to trasparently re-enable the tx ISR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latest commit addresses this.

@sandeepmistry
Copy link
Contributor Author

@cmaglie there's one open question, if we expose the attachRts and attachCts methods to the user, we need to validate the inputs are the right pins.

The other option is to pass then in the constructor.

@sandeepmistry
Copy link
Contributor Author

I've removed the attachRts and attachCts API's for now, and made RTS and CTS only configurable via the Uart constructor.

@cmaglie cmaglie added this to the Release 1.6.17 milestone Sep 1, 2017
@arduino arduino deleted a comment from ArduinoBot Sep 5, 2017
@arduino arduino deleted a comment from ArduinoBot Sep 5, 2017
@arduino arduino deleted a comment from ArduinoBot Sep 5, 2017
@ArduinoBot
Copy link

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/samd/package_samd-b180_index.json

ℹ️ To test this build:

  1. Open the Preferences of the Arduino IDE.
  2. Add the Build URL above in the Additional Boards Manager URLs field, and click OK.
  3. Open the Boards Manager (menu Tools->Board->Board Manager...)
  4. Install Arduino SAMD core - Pull Request Add optional RTS + CTS support to UART #257
  5. Select one of the boards under SAMD Pull Request Add optional RTS + CTS support to UART #257 in Tools->Board menu
  6. Compile/Upload as usual

@arduino arduino deleted a comment from ArduinoBot Nov 29, 2017
@arduino arduino deleted a comment from ArduinoBot Nov 29, 2017
@cmaglie cmaglie merged commit af14b97 into arduino:master Nov 29, 2017
@cmaglie cmaglie deleted the uart-rts-cts branch November 29, 2017 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants