UART fixes

components/cli/CMakeLists.txt

@@ -1,6 +1,6 @@
 idf_component_register(
   SRC_DIRS .
   INCLUDE_DIRS "include"
-  REQUIRES cmd
+  REQUIRES cmd stdio
   PRIV_REQUIRES logging
 )

components/cli/cli.c

@@ -2,9 +2,12 @@
 #include <cli.h>
 #include <logging.h>
 
-#if CONFIG_NEWLIB_VFS_STDIO
-#define HAVE_STDIO_FCNTL 1
-#include <stdio_fcntl.h>
+#if CONFIG_IDF_TARGET_ESP8266 && CONFIG_NEWLIB_VFS_STDIO
+  #define HAVE_STDIO_FCNTL 1
+  #include <stdio_fcntl.h>
+#elif !CONFIG_IDF_TARGET_ESP8266 && CONFIG_VFS_USE_STDIO
+  #define HAVE_STDIO_FCNTL 1
+  #include <stdio_fcntl.h>
 #endif
 
 #include <errno.h>
@@ -93,18 +96,21 @@ static int cli_open(struct cli *cli, TickType_t timeout)
 #if HAVE_STDIO_FCNTL
   if (fcntl(STDIN_FILENO, F_SET_READ_TIMEOUT, timeout) < 0) {
     LOG_WARN("fcntl stdin: %s", strerror(errno));
+  } else {
+    LOG_INFO("fcntl stdin F_SET_READ_TIMEOUT %d", timeout);
   }
 #endif
 
   printf("! Use [ENTER] to open console\n");
 
   while ((c = fgetc(stdin)) != EOF) {
-    if (c == '\r') {
-      continue;
-    } else if (c == '\n') {
-      break;
-    } else {
-      printf("! Use [ENTER] to open console, ignoring <%02x>\n", c);
+    switch (c) {
+      case '\r':
+      case '\n':
+        return 0;
+
+      default:
+       printf("! Use [ENTER] to open console, ignoring <%02x>\n", c);
     }
   }
 

components/dmx/dmx_input.c

@@ -242,7 +242,8 @@ int dmx_input_read (struct dmx_input *in)
   // read until data -> break
   while (!in->state_len || read) {
     WITH_STATS_TIMER(&in->stats.uart_rx) {
-      if ((read = uart_read(in->uart, buf, sizeof(buf))) < 0) {
+      // TODO: 1s timeout for most protocol states?
+      if ((read = uart_read(in->uart, buf, sizeof(buf), portMAX_DELAY)) < 0) {
         dmx_input_process_error(in, -read);
         return read;
       }

components/dmx/dmx_output.c

@@ -115,20 +115,20 @@ int dmx_output_write (struct dmx_output *out, enum dmx_cmd cmd, void *data, size
 
   WITH_STATS_TIMER(&out->stats.uart_tx) {
     // send break/mark per spec minimums for transmit; actual timings will vary, these are minimums
-    if ((err = uart_break(out->uart, DMX_BREAK_BITS, DMX_MARK_AFTER_BREAK_BITS))) {
+    if ((err = uart_break(out->uart, DMX_BREAK_BITS, DMX_MARK_AFTER_BREAK_BITS, portMAX_DELAY))) {
       LOG_ERROR("uart1_break");
       goto error;
     }
 
-    if ((err = uart_putc(out->uart, cmd)) < 0) {
+    if ((err = uart_putc(out->uart, cmd, portMAX_DELAY)) < 0) {
       LOG_ERROR("uart_putc");
       goto error;
     }
 
     for (uint8_t *ptr = data; len > 0; ) {
       ssize_t write;
 
-      if ((write = uart_write(out->uart, ptr, len)) < 0) {
+      if ((write = uart_write(out->uart, ptr, len, portMAX_DELAY)) < 0) {
         LOG_ERROR("uart_write");
         err = write;
         goto error;
@@ -138,7 +138,7 @@ int dmx_output_write (struct dmx_output *out, enum dmx_cmd cmd, void *data, size
       len -= write;
     }
 
-    if ((err = uart_flush_write(out->uart))) {
+    if ((err = uart_flush_write(out->uart, portMAX_DELAY))) {
       LOG_ERROR("uart_flush_write");
       goto error;
     }
@@ -164,7 +164,7 @@ int dmx_output_close (struct dmx_output *out)
     gpio_out_clear(out->options.gpio_options);
   }
 
-  if (uart_close_tx(out->uart)) {
+  if (uart_close_tx(out->uart, portMAX_DELAY)) {
     LOG_WARN("uart_close_tx");
   }
 

components/dmx/include/dmx_uart.h

@@ -7,17 +7,17 @@
 #define DMX_UART_RX_BUFFER_SIZE (512 + 1)
 #define DMX_UART_TX_BUFFER_SIZE (512 + 1)
 
-#define DMX_UART_MTBP_UNIT (8 * (1000000 / 250000))
-#define DMX_UART_MTBP_MIN (4 * DMX_UART_MTBP_UNIT)
+#define DMX_UART_MTBP_UNIT (8 * (1000000 / 250000)) // us per frame
+#define DMX_UART_MTBP_MIN (4 * DMX_UART_MTBP_UNIT) // 128us
 #define DMX_UART_MTBP_MAX (UART_RX_TIMEOUT_MAX * DMX_UART_MTBP_UNIT)
 
 // SOC supports mapping IO pins
 #define DMX_UART_IO_PINS_SUPPORTED UART_IO_PINS_SUPPORTED
 
 struct dmx_uart_options {
-  // buffer RX FIFO until line idle for this many ~8-bit periods
+  // buffer RX FIFO until line idle for this many us
   // this must be short enough to trigger in the MTBP, or the final bytes in the packet will be lost...
-  uint16_t mtbp_min; // use DMX_UART_MTBP_MIN
+  uint16_t mtbp_min; // us
 
   // Acquire mutex before setting dev interrupts
   SemaphoreHandle_t dev_mutex;

components/leds/interfaces/uart/tx.c

@@ -62,23 +62,40 @@ int leds_interface_uart_init(struct leds_interface_uart *interface, const struct
   return 0;
 }
 
+static int leds_interface_uart_write(struct leds_interface_uart *interface, void *buf, size_t len)
+{
+  const uint8_t *ptr = buf;
+  int write;
+
+  while (len) {
+    if ((write = uart_write(interface->uart, ptr, len, portMAX_DELAY)) < 0) {
+      return write;
+    }
+
+    ptr += write;
+    len -= write;
+  }
+
+  return 0;
+}
+
 int leds_interface_uart_tx_pixel(struct leds_interface_uart *interface, const struct leds_color *pixels, unsigned index, const struct leds_limit *limit)
 {
   switch (interface->mode) {
     case LEDS_INTERFACE_UART_MODE_24B3I7_0U4_80U:
       interface->func.uart_mode_24B3I7(interface->buf.uart_mode_24B3I7, pixels, index, limit);
 
-      return uart_write_all(interface->uart, interface->buf.uart_mode_24B3I7, sizeof(interface->buf.uart_mode_24B3I7));
+      return leds_interface_uart_write(interface, interface->buf.uart_mode_24B3I7, sizeof(interface->buf.uart_mode_24B3I7));
 
     case LEDS_INTERFACE_UART_MODE_24B2I8_0U25_50U:
       interface->func.uart_mode_24B2I8(interface->buf.uart_mode_24B2I8, pixels, index, limit);
 
-      return uart_write_all(interface->uart, interface->buf.uart_mode_24B2I8, sizeof(interface->buf.uart_mode_24B2I8));
+      return leds_interface_uart_write(interface, interface->buf.uart_mode_24B2I8, sizeof(interface->buf.uart_mode_24B2I8));
 
     case LEDS_INTERFACE_UART_MODE_32B2I6_0U3_80U:
       interface->func.uart_mode_32B2I6(interface->buf.uart_mode_32B2I6, pixels, index, limit);
 
-      return uart_write_all(interface->uart, interface->buf.uart_mode_32B2I6, sizeof(interface->buf.uart_mode_32B2I6));
+      return leds_interface_uart_write(interface, interface->buf.uart_mode_32B2I6, sizeof(interface->buf.uart_mode_32B2I6));
 
     default:
       LOG_FATAL("invalid mode=%d", interface->mode);
@@ -89,13 +106,13 @@ int leds_interface_uart_tx_reset(struct leds_interface_uart *interface)
 {
   switch (interface->mode) {
     case LEDS_INTERFACE_UART_MODE_24B3I7_0U4_80U:
-      return uart_mark(interface->uart, 80);
+      return uart_mark(interface->uart, 80, portMAX_DELAY);
 
     case LEDS_INTERFACE_UART_MODE_24B2I8_0U25_50U:
-      return uart_mark(interface->uart, 50);
+      return uart_mark(interface->uart, 50, portMAX_DELAY);
 
     case LEDS_INTERFACE_UART_MODE_32B2I6_0U3_80U:
-      return uart_mark(interface->uart, 80);
+      return uart_mark(interface->uart, 80, portMAX_DELAY);
 
     default:
       LOG_FATAL("invalid mode=%d", interface->mode);
@@ -144,7 +161,7 @@ int leds_interface_uart_tx(struct leds_interface_uart *interface, const struct l
   leds_gpio_close(&interface->gpio);
 #endif
 
-  if ((err = uart_close(interface->uart))) {
+  if ((err = uart_close(interface->uart, portMAX_DELAY))) {
     LOG_ERROR("uart_close");
     return err;
   }

components/stdio/log.c

@@ -89,27 +89,30 @@ int stdio_log_new(struct stdio_log **logp, size_t buffer_size)
   return err;
 }
 
-size_t stdio_log_write(struct stdio_log *log, const void *data, size_t len)
+void stdio_log_write(struct stdio_log *log, const uint8_t *data, size_t len)
 {
   if (!xSemaphoreTakeRecursive(log->mutex, portMAX_DELAY)) {
-    return 0;
+    return;
   }
 
-  size_t size = stdio_log_write_size(log);
+  while (len) {
+    size_t size = stdio_log_write_size(log);
 
-  if (len > size) {
-    len = size;
-  }
+    if (size > len) {
+      size = len;
+    }
 
-  LOG_BOOT_DEBUG("write=%p count=%u size=%u len=%u", log->write, log->write_count, size, len);
+    LOG_BOOT_DEBUG("write=%p count=%u size=%u len=%u", log->write, log->write_count, size, len);
 
-  memcpy(log->write, data, len);
+    memcpy(log->write, data, size);
 
-  stdio_log_write_len(log, len);
+    stdio_log_write_len(log, size);
 
-  xSemaphoreGiveRecursive(log->mutex);
+    data += size;
+    len -= size;
+  }
 
-  return len;
+  xSemaphoreGiveRecursive(log->mutex);
 }
 
 static size_t stdio_log_read_size(struct stdio_log *log)

components/stdio/log.h

@@ -20,9 +20,9 @@ struct stdio_log {
 extern struct stdio_log *stderr_log;
 
 /*
- * Return number of bytes copied.
+ * Guaranteed to succeeed and write all data.
  */
-size_t stdio_log_write(struct stdio_log *log, const void *data, size_t len);
+void stdio_log_write(struct stdio_log *log, const uint8_t *data, size_t len);
 
 /*
  * Return number of bytes copied.

components/stdio/vfs.c

@@ -16,27 +16,35 @@
 #define STDIO_VFS_FD_START 0
 #define STDIO_VFS_FD_COUNT 3
 
+static TickType_t stdin_read_timeout = portMAX_DELAY;
+
 ssize_t stdio_vfs_write(int fd, const void *data, size_t size)
 {
+  ssize_t ret;
+
   switch(fd) {
     case STDOUT_FILENO:
       if (stdio_uart) {
-        return uart_write(stdio_uart, data, size);
+        return uart_write(stdio_uart, data, size, portMAX_DELAY);
       } else {
         os_write(data, size);
         return size;
       }
 
     case STDERR_FILENO:
-      if (stderr_log) {
-        size = stdio_log_write(stderr_log, data, size);
-      }
-
       if (!stdio_uart) {
         os_write(data, size);
-      } else if (uart_write_all(stdio_uart, data, size)) {
-        errno = EIO;
-        return -1;
+      } else {
+        if ((ret = uart_write(stdio_uart, data, size, portMAX_DELAY)) < 0) {
+          errno = EIO;
+          return -1;
+        } else {
+          size = ret;
+        }
+      }
+
+      if (stderr_log) {
+        stdio_log_write(stderr_log, data, size);
       }
 
       return size;
@@ -98,7 +106,7 @@ ssize_t stdio_vfs_read(int fd, void *data, size_t size)
     switch(fd) {
       case STDIN_FILENO:
         if (stdio_uart) {
-          return uart_read(stdio_uart, data, size);
+          return uart_read(stdio_uart, data, size, stdin_read_timeout);
         } else {
           errno = ENODEV;
           return -1;
@@ -125,13 +133,9 @@ int stdio_vfs_fcntl_stdin(int cmd, int arg)
       return O_RDONLY;
 
     case F_SET_READ_TIMEOUT:
-      if (!stdio_uart) {
-        errno = ENODEV;
-        return -1;
-      } else if (uart_set_read_timeout(stdio_uart, arg ? arg : portMAX_DELAY)) {
-        errno = EIO;
-        return -1;
-      }
+      stdin_read_timeout = arg ? arg : portMAX_DELAY;
+
+      LOG_DEBUG("stdin_read_timeout = %d", stdin_read_timeout);
 
       return 0;
 
@@ -191,7 +195,7 @@ int stdio_vfs_fsync(int fd)
     case STDOUT_FILENO:
     case STDERR_FILENO:
       if (stdio_uart) {
-        return uart_flush_write(stdio_uart);
+        return uart_flush_write(stdio_uart, portMAX_DELAY);
       } else {
         errno = ENODEV;
         return -1;

components/uart/esp32/intr.c

@@ -162,7 +162,7 @@ void IRAM_ATTR uart_intr_tx_done_handler(struct uart *uart, BaseType_t *task_wok
     LOG_ISR_DEBUG("notify tx_done_notify_task=%p", uart->tx_done_notify_task);
 
     // FIFO is empty, notify task
-    vTaskNotifyGiveFromISR(uart->tx_done_notify_task, task_woken);
+    xTaskNotifyFromISR(uart->tx_done_notify_task, 0, eNoAction, task_woken);
 
     // only once
     uart->tx_done_notify_task = NULL;

components/uart/esp32/rx.c

@@ -15,7 +15,7 @@ int uart_rx_init(struct uart *uart, size_t rx_buffer_size)
   if (rx_buffer_size == 0) {
     uart->rx_buffer = NULL;
 
-  } else if (!(uart->rx_buffer = xStreamBufferCreate(rx_buffer_size, 1))) {
+  } else if (!(uart->rx_buffer = xStreamBufferCreate(rx_buffer_size, 0))) {
     LOG_ERROR("xStreamBufferCreate");
     return -1;
   }