API to temporarily enable/disable FileHandles #9797
Conversation
Add API to dynamically enable/disable input and output on FileHandles. Aim is to allow power saving by indicating that we permanently or temporarily do not require input, so that for example serial Rx interrupts can be released, and deep sleep permitted.
|
As system suspend is going to take a while longer, here's a plan B to cover one of the most commonly-requested power save issues. |
ciarmcom
requested review from
AriParkkila,
geky,
pan-,
SeppoTakalo,
VeijoPesonen and
a team
|
@kjbracey-arm, thank you for your changes. |
|
@kjbracey-arm Spelling errors were discovered in |
|
Spelling americanised and TX buffer condition fixed. Topic branch remains in master repo (sorry). |
Implement enable calls so that reception can be shut down to save power without closing the device.
Add a necessary helper to allow FileHandle objects to be obtained from POSIX file descriptors. Primary envisaged use case is to act on STDIN_FILENO etc, eg to set it non-blocking or use sigio, or to use the enable API.
There was a problem hiding this comment.
Looks good. I'm assuming that all other serial/uart classes are not affecting sleep in the same way (not holding a sleep lock)?
We need to document it, not only in terms of API, but also in context of sleep as it's not obvious. Maybe a note in handbook's power management section?
|
No other current in-tree |
Please reference this PR in the docs PR |
|
CI started |
Test run: FAILEDSummary: 1 of 1 test jobs failed Failed test jobs:
|
There was a problem hiding this comment.
Implementation looks fine, but I don't see any test case or even use case where the new functionality is implemented and verified.
_tx_enabed and _rx_enabled are default true, so existing functionality is unchanged.
API's to disable RX and TX are provided by enable_input and enable_output but not used anywhere. My understanding was that we will disable Rx before entering deep sleep (if only UART is blocking it).
Is the intention to leave it completely to application developer to disable serial RTX before deep sleep? Also, it will be good to have test case to verify the new API's.
Yes, or more precisely it allows the application developer to disable serial RX so that deep sleep is possible. We can't deep sleep if serial RX is required, but there's currently no way for the application to indicate they don't need RX (other than closing/destroying the UARTSerial).
That would be nearly equivalent to having enable_input false as default - you'd still need an API to indicate that an app actually needed serial reception. An app could just manually call "deep sleep lock" to do this, but that would be an abstraction/layering error - the application shouldn't need to know the details of how a particular FileHandle interacts with power management. We do need this facility for quite specific "you can deep sleep now" reasons with serial, but the API needs to be generic. I discussed this more over in #9676 - this is largely addressing that, although it's closed.
Indeed - I haven't quite figured out how to do this. I don't think we can easily do it in Greentea, because the debug serial is being driven by a RawSerial, and we can't just take it over. If I can slot into a cpu stats test in the Ice tea framework would be ideal. Or maybe it could be slotted into say an ESP8266 test, where we know we're running on a board with that 2nd serial port connected up. |
|
Missing unittest stubs added. |
|
There's fix incoming for psa, will be on master soon. I'll restart travis and as soon as its green will this go into CI |
|
CI restarted |
Assuming we will have test later, can we add the use case of new API as an example for docs? Along with API docs. |
Test run: SUCCESSSummary: 13 of 13 test jobs passed |
|
@AnotherButler Please review (we will fix docs in separate PR as it's close to the code freeze and we have still some work to do there). |
Is this tracked anywhere? |
|
@kjbracey-arm ^^^ |
Cover changes in ARMmbed/mbed-os#9797
Cover changes in ARMmbed/mbed-os#9797
Description
New API to permit input and/or output to be temporarily disabled on a
FileHandle. Implementation forUARTSerialprovided.At present the use of
UARTSerial, directly or by settingplatform.stdio-buffered-serial, blocks deep sleep because it installs an RxIrq handler for input, and there is no way to indicate that you don't require input (ever, or temporarily) except by destroying the object. And that's not possible if it's the console.This PR allows you to do
mbed_file_handle(STDIN_FILENO)->enable_input(false), which will shut down the reception interrupt pump, and deep sleep can be entered. This is done without losing any serial settings (or any data already in the buffer). The pump is just suspended, permitting power save.(For this particular common case, should there be a
platform.stdio-output-onlyto do it automatically?)Pull request type
Reviewers
@geky, @AriParkkila, @VeijoPesonen, @SeppoTakalo, @pan-
Release Notes
UARTSerialcan now be temporarily or permanently disabled for input and/or output. This is particularly useful to save power when reception is not required - disabling input permits deep sleep. To do this more generically for the console, usembed_file_handle(STDIN_FILENO)->enable_input(false).