Skip to content

Commit 9678c80

Browse files
committed
Make UARTSerial send all data when blocking
Previously, write() was somewhat soft - it only ever made one attempt to wait for buffer space, so it would take as much data as would fit in the buffer in one call. This is not the intent of a POSIX filehandle write. It should try to send everything if blocking, and only send less if interrupted by a signal: - If the O_NONBLOCK flag is clear, write() shall block the calling thread until the data can be accepted. - If the O_NONBLOCK flag is set, write() shall not block the thread. If some data can be written without blocking the thread, write() shall write what it can and return the number of bytes written. Otherwise, it shall return -1 and set errno to [EAGAIN]. This "send all" behaviour is of slightly limited usefulness in POSIX, as you still usually have to worry about the interruption possibility: - If write() is interrupted by a signal before it writes any data, it shall return -1 with errno set to [EINTR]. - If write() is interrupted by a signal after it successfully writes some data, it shall return the number of bytes written. But as mbed OS does not have the possibility of signal interruption, if we strengthen write to write everything, we can make applications' lives easier - they can just do "write(large amount)" confident that it will all go in one call (if no errors). So, rework to make multiple writes to the buffer, blocking as necessary, until all data is written. This change does not apply to read(), which is correct in only blocking until some data is available: - If O_NONBLOCK is set, read() shall return -1 and set errno to [EAGAIN]. - If O_NONBLOCK is clear, read() shall block the calling thread until some data becomes available. - The use of the O_NONBLOCK flag has no effect if there is some data available.
1 parent 67b97d3 commit 9678c80

File tree

3 files changed

+48
-21
lines changed

3 files changed

+48
-21
lines changed

drivers/UARTSerial.cpp

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -138,36 +138,47 @@ ssize_t UARTSerial::write(const void* buffer, size_t length)
138138
size_t data_written = 0;
139139
const char *buf_ptr = static_cast<const char *>(buffer);
140140

141+
if (length == 0) {
142+
return 0;
143+
}
144+
141145
api_lock();
142146

143-
while (_txbuf.full()) {
144-
if (!_blocking) {
145-
api_unlock();
146-
return -EAGAIN;
147+
// Unlike read, we should write the whole thing if blocking. POSIX only
148+
// allows partial as a side-effect of signal handling; it normally tries to
149+
// write everything if blocking. Without signals we can always write all.
150+
while (data_written < length) {
151+
152+
if (_txbuf.full()) {
153+
if (!_blocking) {
154+
break;
155+
}
156+
do {
157+
api_unlock();
158+
wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ?
159+
api_lock();
160+
} while (_txbuf.full());
147161
}
148-
api_unlock();
149-
wait_ms(1); // XXX todo - proper wait, WFE for non-rtos ?
150-
api_lock();
151-
}
152162

153-
while (data_written < length && !_txbuf.full()) {
154-
_txbuf.push(*buf_ptr++);
155-
data_written++;
156-
}
163+
while (data_written < length && !_txbuf.full()) {
164+
_txbuf.push(*buf_ptr++);
165+
data_written++;
166+
}
157167

158-
core_util_critical_section_enter();
159-
if (!_tx_irq_enabled) {
160-
UARTSerial::tx_irq(); // only write to hardware in one place
161-
if (!_txbuf.empty()) {
162-
SerialBase::attach(callback(this, &UARTSerial::tx_irq), TxIrq);
163-
_tx_irq_enabled = true;
168+
core_util_critical_section_enter();
169+
if (!_tx_irq_enabled) {
170+
UARTSerial::tx_irq(); // only write to hardware in one place
171+
if (!_txbuf.empty()) {
172+
SerialBase::attach(callback(this, &UARTSerial::tx_irq), TxIrq);
173+
_tx_irq_enabled = true;
174+
}
164175
}
176+
core_util_critical_section_exit();
165177
}
166-
core_util_critical_section_exit();
167178

168179
api_unlock();
169180

170-
return data_written;
181+
return data_written != 0 ? (ssize_t) data_written : (ssize_t) -EAGAIN;
171182
}
172183

173184
ssize_t UARTSerial::read(void* buffer, size_t length)
@@ -176,6 +187,10 @@ ssize_t UARTSerial::read(void* buffer, size_t length)
176187

177188
char *ptr = static_cast<char *>(buffer);
178189

190+
if (length == 0) {
191+
return 0;
192+
}
193+
179194
api_lock();
180195

181196
while (_rxbuf.empty()) {

drivers/UARTSerial.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,12 @@ class UARTSerial : private SerialBase, public FileHandle, private NonCopyable<UA
7070
using FileHandle::writable;
7171

7272
/** Write the contents of a buffer to a file
73+
*
74+
* Follows POSIX semantics:
75+
*
76+
* * if blocking, block until all data is written
77+
* * if no data can be written, and non-blocking set, return -EAGAIN
78+
* * if some data can be written, and non-blocking set, write partial
7379
*
7480
* @param buffer The buffer to write from
7581
* @param length The number of bytes to write

platform/FileHandle.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class FileHandle : private NonCopyable<FileHandle> {
5151
* Devices acting as FileHandles should follow POSIX semantics:
5252
*
5353
* * if no data is available, and non-blocking set return -EAGAIN
54-
* * if no data is available, and blocking set, wait until data is available
54+
* * if no data is available, and blocking set, wait until some data is available
5555
* * If any data is available, call returns immediately
5656
*
5757
* @param buffer The buffer to read in to
@@ -61,6 +61,12 @@ class FileHandle : private NonCopyable<FileHandle> {
6161
virtual ssize_t read(void *buffer, size_t size) = 0;
6262

6363
/** Write the contents of a buffer to a file
64+
*
65+
* Devices acting as FileHandles should follow POSIX semantics:
66+
*
67+
* * if blocking, block until all data is written
68+
* * if no data can be written, and non-blocking set, return -EAGAIN
69+
* * if some data can be written, and non-blocking set, write partial
6470
*
6571
* @param buffer The buffer to write from
6672
* @param size The number of bytes to write

0 commit comments

Comments
 (0)