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

UARTSerial enhancement request: flow control #4428

Closed
RobMeades opened this issue Jun 2, 2017 · 13 comments
Closed

UARTSerial enhancement request: flow control #4428

RobMeades opened this issue Jun 2, 2017 · 13 comments

Comments

@RobMeades
Copy link
Contributor

RobMeades commented Jun 2, 2017

Description

  • Type: Enhancement
  • Priority: Minor

Enhancement

#4119 introduced UARTSerial, which provides a very neat interface to buffered serial operations. One thing it is missing, though, is flow control. If UARTSerial receives data faster than it can get rid of it, characters are lost without notice. This is a particular issue when using the recv() method of the equally new ATCmdParser with the serial stream, since the sscanf()ing involved can take quite some time and could cause data loss, especially if there are out of band thingies to capture.

All that is required is for UARTSerial to expose the set_flow_control() bit of SerialBase, I think.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 5, 2017

cc @hasnainvirk @kjbracey-arm

@hasnainvirk
Copy link
Contributor

@RobMeades Yes this work item is on the to-do list. We are even thinking a step further:

  • HW/HW flow control
  • HW/SW flow control
  • SW flow control (XON/XOFF)

@kjbracey-arm will be the architectural planner. Thank you for highlighting this.
You can keep this open for traction.

@kjbracey
Copy link
Contributor

kjbracey commented Jun 7, 2017

Looking through the targets, it seems almost every target does implement the set_flow_control() API via hardware assist, so maybe we don't need to worry about software-controlled version in the first pass.

However, XON/XOFF could be useful for the cases where RTS and CTS aren't connected - which is actually quite common.

So maybe we just don't bother with the middle case (RTS and CTS handled as GPIO).

Step 1 is then pass through the set_flow_control, and modify the RX data pump to not read data when the buffer is full, so the RX HW flow control works.

@RobMeades
Copy link
Contributor Author

Sounds good to me.

@RobMeades
Copy link
Contributor Author

RobMeades commented Jun 29, 2017

It would be good to get this flow control cooking, it might help with my attempts to run the modem interface at higher data rates.

@RobMeades
Copy link
Contributor Author

@kjbracey-arm: is it possible to get some idea when HW flow control might be available in a PR? I've some work that needs higher data rates that I can only realistically schedule with this in place.

@kjbracey
Copy link
Contributor

kjbracey commented Jul 6, 2017

Not sure we've got the resource available to work on testing this at our end in the next couple of weeks. Seems like it should be basically making the Rx IRQ handling more similar to the Tx IRQ handling - IRQ handler can disable it, and foreground read routine re-enables it.

If I knock out a quick patch proposal can you test it?

@RobMeades
Copy link
Contributor Author

Yup, absolutely.

@kjbracey
Copy link
Contributor

kjbracey commented Jul 6, 2017

Try this. Untested apart from compilation.

https://github.com/kjbracey-arm/mbed-os/tree/uartserial_flow

@RobMeades
Copy link
Contributor Author

RobMeades commented Jul 6, 2017

Testing now, will continue via e-mail shortly.

@kjbracey
Copy link
Contributor

kjbracey commented Jul 6, 2017

It occurred to me that it might get a bit stuck without the same basic "do it in the foreground" logic as TX. Extra tweak added to branch.

@ciarmcom
Copy link
Member

ciarmcom commented Jun 1, 2018

ARM Internal Ref: MBOTRIAGE-330

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 15, 2018

Resolved via 5088 (referenced above)

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

No branches or pull requests

5 participants