Merge pull request #11578 from mikeysklar/uart-write-ctrl-c

Let Ctrl-C stop a UART write that is stuck on flow control

Author: dhalbert

ports/espressif/common-hal/busio/UART.c

@@ -374,12 +374,16 @@ size_t common_hal_busio_uart_write(busio_uart_obj_t *self, const uint8_t *data,
         left_to_write -= count;
         data += count;
         RUN_BACKGROUND_TASKS;
+        // Allow user to break out of a write blocked by flow control.
+        if (mp_hal_is_interrupted()) {
+            break;
+        }
     }
-    while (uart_wait_tx_done(self->uart_num, 0) == ESP_ERR_TIMEOUT) {
+    while (uart_wait_tx_done(self->uart_num, 0) == ESP_ERR_TIMEOUT && !mp_hal_is_interrupted()) {
         RUN_BACKGROUND_TASKS;
     }
 
-    return len;
+    return len - left_to_write;
 }
 
 uint32_t common_hal_busio_uart_get_baudrate(busio_uart_obj_t *self) {

ports/raspberrypi/common-hal/busio/UART.c

@@ -231,12 +231,16 @@ size_t common_hal_busio_uart_write(busio_uart_obj_t *self, const uint8_t *data,
             left_to_write--;
         }
         RUN_BACKGROUND_TASKS;
+        // Allow user to break out of a write blocked by flow control.
+        if (mp_hal_is_interrupted()) {
+            break;
+        }
     }
     if (self->rs485_dir_pin != NO_PIN) {
         uart_tx_wait_blocking(self->uart);
         gpio_put(self->rs485_dir_pin, self->rs485_invert);
     }
-    return len;
+    return len - left_to_write;
 }
 
 // Read characters.

ports/zephyr-cp/common-hal/busio/UART.c

@@ -36,8 +36,8 @@ static void resume_rx(busio_uart_obj_t *self) {
 }
 
 /*
- * Read characters from UART until line end is detected. Afterwards push the
- * data to the message queue.
+ * Feed pending TX bytes to the driver, then move received characters into
+ * the message queue.
  */
 static void serial_cb(const struct device *dev, void *user_data) {
     busio_uart_obj_t *self = (busio_uart_obj_t *)user_data;
@@ -46,6 +46,18 @@ static void serial_cb(const struct device *dev, void *user_data) {
 
     uart_irq_update(dev);
 
+    if (self->tx_len > 0 && uart_irq_tx_ready(dev)) {
+        int sent = uart_fifo_fill(dev, self->tx_data, self->tx_len);
+        if (sent > 0) {
+            self->tx_data += sent;
+            self->tx_len -= sent;
+        }
+        if (self->tx_len == 0) {
+            uart_irq_tx_disable(dev);
+            k_sem_give(&self->tx_done);
+        }
+    }
+
     if (!uart_irq_rx_ready(dev)) {
         return;
     }
@@ -102,6 +114,8 @@ mp_obj_t common_hal_busio_uart_construct_from_device(busio_uart_obj_t *self, con
 
     self->timeout = K_FOREVER;
     self->write_timeout = K_FOREVER;
+    self->tx_len = 0;
+    k_sem_init(&self->tx_done, 0, 1);
     self->rx_paused = false;
     self->flow_control = false;
     self->sigint_enabled = sigint_enabled;
@@ -233,13 +247,36 @@ size_t common_hal_busio_uart_read(busio_uart_obj_t *self, uint8_t *data, size_t
     return count;
 }
 
-// Write characters.
+// Write characters. serial_cb() feeds the driver from the TX interrupt, so
+// this thread only waits and can stop on Ctrl-C.
 size_t common_hal_busio_uart_write(busio_uart_obj_t *self, const uint8_t *data, size_t len, int *errcode) {
-    for (int i = 0; i < len; i++) {
-        uart_poll_out(self->uart_device, data[i]);
+    if (len == 0) {
+        return 0;
     }
+    k_sem_reset(&self->tx_done);
+    self->tx_data = data;
+    self->tx_len = len;
+    uart_irq_tx_enable(self->uart_device);
+    while (k_sem_take(&self->tx_done, K_MSEC(10)) != 0) {
+        RUN_BACKGROUND_TASKS;
+        if (mp_hal_is_interrupted()) {
+            break;
+        }
+    }
+    uart_irq_tx_disable(self->uart_device);
+
+    unsigned int key = irq_lock();
+    size_t not_sent = self->tx_len;
+    self->tx_len = 0;
+    irq_unlock(key);
+
+    return len - not_sent;
+}
 
-    return len;
+void zephyr_uart_write_blocking(busio_uart_obj_t *self, const uint8_t *data, size_t len) {
+    for (size_t i = 0; i < len; i++) {
+        uart_poll_out(self->uart_device, data[i]);
+    }
 }
 
 uint32_t common_hal_busio_uart_get_baudrate(busio_uart_obj_t *self) {

ports/zephyr-cp/common-hal/busio/UART.h

@@ -20,6 +20,11 @@ typedef struct {
 
     k_timeout_t timeout;
     k_timeout_t write_timeout;
+    // Bytes not yet handed to the driver by serial_cb(), and a semaphore it
+    // gives when they are all sent.
+    const uint8_t *tx_data;
+    size_t tx_len;
+    struct k_sem tx_done;
     byte *receiver_buffer;
     const mcu_pin_obj_t *tx;
     const mcu_pin_obj_t *rx;
@@ -45,6 +50,13 @@ mp_obj_t common_hal_busio_uart_construct_from_device(busio_uart_obj_t *self, con
 // Internal helper for clearing buffer
 void common_hal_busio_uart_clear_rx_buffer(busio_uart_obj_t *self);
 
+// Write all of data with uart_poll_out(). Used by the console and usb_cdc
+// instead of common_hal_busio_uart_write(), which stops early on a pending
+// Ctrl-C or reload and would cut console output short. With the host port
+// closed, the CDC ACM driver drops this output instead of blocking, as on
+// TinyUSB ports.
+void zephyr_uart_write_blocking(busio_uart_obj_t *self, const uint8_t *data, size_t len);
+
 // Zephyr-port-specific write-timeout accessors, used by usb_cdc/Serial.c.
 mp_float_t common_hal_busio_uart_get_write_timeout(busio_uart_obj_t *self);
 void common_hal_busio_uart_set_write_timeout(busio_uart_obj_t *self, mp_float_t write_timeout);

ports/zephyr-cp/common-hal/usb_cdc/Serial.c

@@ -22,7 +22,8 @@ size_t common_hal_usb_cdc_serial_read(usb_cdc_serial_obj_t *self, uint8_t *data,
 }
 
 size_t common_hal_usb_cdc_serial_write(usb_cdc_serial_obj_t *self, const uint8_t *data, size_t len, int *errcode) {
-    return common_hal_busio_uart_write(self, data, len, errcode);
+    zephyr_uart_write_blocking(self, data, len);
+    return len;
 }
 
 uint32_t common_hal_usb_cdc_serial_get_in_waiting(usb_cdc_serial_obj_t *self) {

ports/zephyr-cp/supervisor/serial.c

@@ -80,6 +80,6 @@ void port_serial_write_substring(const char *text, uint32_t length) {
     }
     common_hal_usb_cdc_serial_write(usb_console, text, length, NULL);
     #else
-    common_hal_busio_uart_write(&uart_console, text, length, NULL);
+    zephyr_uart_write_blocking(&uart_console, (const uint8_t *)text, length);
     #endif
 }