diff --git a/components/console/esp_console_repl.c b/components/console/esp_console_repl.c index f656cee91f80..fda91fa475ac 100644 --- a/components/console/esp_console_repl.c +++ b/components/console/esp_console_repl.c @@ -30,6 +30,8 @@ typedef enum { CONSOLE_REPL_STATE_DEINIT, CONSOLE_REPL_STATE_INIT, CONSOLE_REPL_STATE_START, + CONSOLE_REPL_STATE_REPL_STOP_REQUESTED, + CONSOLE_REPL_STATE_REPL_TASK_ENDED, } repl_state_t; typedef struct { @@ -409,13 +411,25 @@ static esp_err_t esp_console_repl_uart_delete(esp_console_repl_t *repl) esp_err_t ret = ESP_OK; esp_console_repl_com_t *repl_com = __containerof(repl, esp_console_repl_com_t, repl_core); esp_console_repl_universal_t *uart_repl = __containerof(repl_com, esp_console_repl_universal_t, repl_com); + // check if already de-initialized if (repl_com->state == CONSOLE_REPL_STATE_DEINIT) { ESP_LOGE(TAG, "already de-initialized"); ret = ESP_ERR_INVALID_STATE; goto _exit; } + + // wait for repl thread to stop, if it is running + if (repl_com->state == CONSOLE_REPL_STATE_START) { + repl_com->state = CONSOLE_REPL_STATE_REPL_STOP_REQUESTED; + while (repl_com->state != CONSOLE_REPL_STATE_REPL_TASK_ENDED) { + uart_unblock_reads(); + vTaskDelay(1); + } + } + repl_com->state = CONSOLE_REPL_STATE_DEINIT; + esp_console_deinit(); esp_vfs_dev_uart_use_nonblocking(uart_repl->uart_channel); uart_driver_delete(uart_repl->uart_channel); @@ -429,13 +443,21 @@ static esp_err_t esp_console_repl_usb_cdc_delete(esp_console_repl_t *repl) esp_err_t ret = ESP_OK; esp_console_repl_com_t *repl_com = __containerof(repl, esp_console_repl_com_t, repl_core); esp_console_repl_universal_t *cdc_repl = __containerof(repl_com, esp_console_repl_universal_t, repl_com); + // check if already de-initialized if (repl_com->state == CONSOLE_REPL_STATE_DEINIT) { ESP_LOGE(TAG, "already de-initialized"); ret = ESP_ERR_INVALID_STATE; goto _exit; } + + // TODO: wait for repl thread to stop, if it is running. + // Need to implement a USB CDC driver, and a + // corresponding usb_cdc_unblock_reads() function. + // See other esp_console_repl_X_delete() functions for reference. + repl_com->state = CONSOLE_REPL_STATE_DEINIT; + esp_console_deinit(); free(cdc_repl); _exit: @@ -454,7 +476,18 @@ static esp_err_t esp_console_repl_usb_serial_jtag_delete(esp_console_repl_t *rep ret = ESP_ERR_INVALID_STATE; goto _exit; } + + // wait for repl thread to stop, if it is running + if (repl_com->state == CONSOLE_REPL_STATE_START) { + repl_com->state = CONSOLE_REPL_STATE_REPL_STOP_REQUESTED; + while (repl_com->state != CONSOLE_REPL_STATE_REPL_TASK_ENDED) { + usb_serial_jtag_unblock_reads(); + vTaskDelay(1); + } + } + repl_com->state = CONSOLE_REPL_STATE_DEINIT; + esp_console_deinit(); esp_vfs_usb_serial_jtag_use_nonblocking(); usb_serial_jtag_driver_uninstall(); @@ -535,6 +568,8 @@ static void esp_console_repl_task(void *args) /* linenoise allocates line buffer on the heap, so need to free it */ linenoiseFree(line); } + + repl_com->state = CONSOLE_REPL_STATE_REPL_TASK_ENDED; ESP_LOGD(TAG, "The End"); vTaskDelete(NULL); } diff --git a/components/console/linenoise/linenoise.c b/components/console/linenoise/linenoise.c index a21673184d03..63d057ead48e 100644 --- a/components/console/linenoise/linenoise.c +++ b/components/console/linenoise/linenoise.c @@ -221,8 +221,8 @@ static void flushWrite(void) { } /* Use the ESC [6n escape sequence to query the horizontal cursor position - * and return it. On error -1 is returned, on success the position of the - * cursor. */ + * and return it. On read error -1 is returned, On cursor error, -2 is returned. + * On success the position of the cursor. */ static int getCursorPosition(void) { char buf[LINENOISE_COMMAND_MAX_LEN] = { 0 }; int cols = 0; @@ -248,10 +248,16 @@ static int getCursorPosition(void) { * Stop right before the last character of the buffer, to be able to NULL * terminate it. */ while (i < sizeof(buf)-1) { + + int nread = read(in_fd, buf + i, 1); + if (nread == -1 && errno == EWOULDBLOCK) { + return -1; // read error + } + /* Keep using unistd's functions. Here, using `read` instead of `fgets` * or `fgets` guarantees us that we we can read a byte regardless on * whether the sender sent end of line character(s) (CR, CRLF, LF). */ - if (read(in_fd, buf + i, 1) != 1 || buf[i] == 'R') { + if (nread != 1 || buf[i] == 'R') { /* If we couldn't read a byte from STDIN or if 'R' was received, * the transmission is finished. */ break; @@ -270,7 +276,8 @@ static int getCursorPosition(void) { /* Parse the received data to get the position of the cursor. */ if (buf[0] != ESC || buf[1] != '[' || sscanf(buf+2,"%d;%d",&rows,&cols) != 2) { - return -1; + // cursor error + return -2; } return cols; } @@ -294,7 +301,10 @@ static int getColumns(void) { /* Get the initial position so we can restore it later. */ start = getCursorPosition(); - if (start == -1) { + if (start == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (start == -2) { goto failed; } @@ -308,7 +318,10 @@ static int getColumns(void) { /* After sending this command, we can get the new position of the cursor, * we'd get the size, in columns, of the opened TTY. */ cols = getCursorPosition(); - if (cols == -1) { + if (cols == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (cols == -2) { goto failed; } @@ -819,6 +832,11 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt) int out_fd = fileno(stdout); int in_fd = fileno(stdin); + int cols = getColumns(); + if (cols == -1 && errno == EWOULDBLOCK) { + return -1; + } + /* Populate the linenoise state that we pass to functions implementing * specific editing functionalities. */ l.buf = buf; @@ -827,7 +845,7 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt) l.plen = strlen(prompt); l.oldpos = l.pos = 0; l.len = 0; - l.cols = getColumns(); + l.cols = cols; l.maxrows = 0; l.history_index = 0; @@ -840,11 +858,20 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt) linenoiseHistoryAdd(""); int pos1 = getCursorPosition(); + if (pos1 == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (write(out_fd, prompt,l.plen) == -1) { return -1; } flushWrite(); + int pos2 = getCursorPosition(); + if (pos2 == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (pos1 >= 0 && pos2 >= 0) { l.plen = pos2 - pos1; } @@ -864,7 +891,12 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt) */ t1 = getMillis(); nread = read(in_fd, &c, 1); - if (nread <= 0) return l.len; + if (nread == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (nread <= 0) { + return l.len; + } if ( (getMillis() - t1) < LINENOISE_PASTE_KEY_DELAY && c != ENTER) { /* Pasting data, insert characters without formatting. @@ -940,9 +972,13 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt) case CTRL_N: /* ctrl-n */ linenoiseEditHistoryNext(&l, LINENOISE_HISTORY_NEXT); break; - case ESC: /* escape sequence */ + case ESC: { /* escape sequence */ /* Read the next two bytes representing the escape sequence. */ - if (read(in_fd, seq, 2) < 2) { + int nread = read(in_fd, seq, 2); + if (nread == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (nread < 2) { break; } @@ -950,7 +986,11 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt) if (seq[0] == '[') { if (seq[1] >= '0' && seq[1] <= '9') { /* Extended escape, read additional byte. */ - if (read(in_fd, seq+2, 1) == -1) { + int nread = read(in_fd, seq+2, 1); + if (nread == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (nread == -1) { break; } if (seq[2] == '~') { @@ -996,6 +1036,7 @@ static int linenoiseEdit(char *buf, size_t buflen, const char *prompt) } } break; + } default: if (linenoiseEditInsert(&l,c)) return -1; break; @@ -1054,6 +1095,8 @@ int linenoiseProbe(void) { timeout_ms -= retry_ms; char c; int cb = read(stdin_fileno, &c, 1); + // Note! Due to the fcntl call above, this is read in non-blocking mode! + // So, nread == -1 && errno == EWOULDBLOCK are expected here! if (cb < 0) { continue; } @@ -1084,6 +1127,9 @@ static int linenoiseRaw(char *buf, size_t buflen, const char *prompt) { } count = linenoiseEdit(buf, buflen, prompt); + if (count == -1 && errno == EWOULDBLOCK) { + return -1; + } fputc('\n', stdout); flushWrite(); return count; @@ -1095,9 +1141,13 @@ static int linenoiseDumb(char* buf, size_t buflen, const char* prompt) { size_t count = 0; while (count < buflen) { int c = fgetc(stdin); + if (c == -1 && errno == EWOULDBLOCK) { + return -1; + } + if (c == '\n') { break; - } else if (c >= 0x1c && c <= 0x1f){ + } else if (c >= 0x1c && c <= 0x1f) { continue; /* consume arrow keys */ } else if (c == BACKSPACE || c == 0x8) { if (count > 0) { diff --git a/components/driver/uart/include/driver/uart.h b/components/driver/uart/include/driver/uart.h index ba5f49306ea1..876250782e83 100644 --- a/components/driver/uart/include/driver/uart.h +++ b/components/driver/uart/include/driver/uart.h @@ -823,6 +823,17 @@ esp_err_t uart_wait_tx_idle_polling(uart_port_t uart_num); */ esp_err_t uart_set_loop_back(uart_port_t uart_num, bool loop_back_en); +/** + * @brief Unblocks uart_read_bytes() if it is currently waiting to receive bytes. + * + * This will cause uart_read_bytes() to return -1 with errno set to EWOULDBLOCK. + * + * @return + * - ESP_OK Success + * - ESP_INVALID_STATE If the Uart driver has not been installed + */ +esp_err_t uart_unblock_reads(uart_port_t uart_num); + /** * @brief Configure behavior of UART RX timeout interrupt. * diff --git a/components/driver/uart/uart.c b/components/driver/uart/uart.c index 140bfe469836..a91b0d54f365 100644 --- a/components/driver/uart/uart.c +++ b/components/driver/uart/uart.c @@ -1762,6 +1762,16 @@ esp_err_t uart_set_loop_back(uart_port_t uart_num, bool loop_back_en) return ESP_OK; } +esp_err_t uart_unblock_reads(uart_port_t uart_num) +{ + if (p_uart_obj[uart_num]->rx_ring_buf == NULL) { + return ESP_ERR_INVALID_STATE; + } + + vRingbufferUnblockRx(pp_uart_obj[uart_num]->rx_ring_buf); + return ESP_OK; +} + void uart_set_always_rx_timeout(uart_port_t uart_num, bool always_rx_timeout) { uint16_t rx_tout = uart_hal_get_rx_tout_thr(&(uart_context[uart_num].hal)); diff --git a/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h b/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h index fec968a5bfce..5dd0bf09961d 100644 --- a/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h +++ b/components/driver/usb_serial_jtag/include/driver/usb_serial_jtag.h @@ -47,6 +47,17 @@ typedef struct { */ esp_err_t usb_serial_jtag_driver_install(usb_serial_jtag_driver_config_t *usb_serial_jtag_config); +/** + * @brief Unblocks usb_serial_jtag_read_bytes() if it is currently waiting to receive bytes. + * + * This will cause usb_serial_jtag_read_bytes() to return -1 with errno set to EWOULDBLOCK. + * + * @return + * - ESP_OK Success + * - ESP_INVALID_STATE If the Usb Serial JTAG driver has not been installed + */ +esp_err_t usb_serial_jtag_unblock_reads(); + /** * @brief USB_SERIAL_JTAG read bytes from USB_SERIAL_JTAG buffer * diff --git a/components/driver/usb_serial_jtag/usb_serial_jtag.c b/components/driver/usb_serial_jtag/usb_serial_jtag.c index e1f1354c2aba..4b2d669e9015 100644 --- a/components/driver/usb_serial_jtag/usb_serial_jtag.c +++ b/components/driver/usb_serial_jtag/usb_serial_jtag.c @@ -132,6 +132,16 @@ esp_err_t usb_serial_jtag_driver_install(usb_serial_jtag_driver_config_t *usb_se return err; } +esp_err_t usb_serial_jtag_unblock_reads() +{ + if (p_usb_serial_jtag_obj->rx_ring_buf == NULL) { + return ESP_ERR_INVALID_STATE; + } + + vRingbufferUnblockRx(p_usb_serial_jtag_obj->rx_ring_buf); + return ESP_OK; +} + int usb_serial_jtag_read_bytes(void* buf, uint32_t length, TickType_t ticks_to_wait) { uint8_t *data = NULL; diff --git a/components/esp_ringbuf/include/freertos/ringbuf.h b/components/esp_ringbuf/include/freertos/ringbuf.h index 9a2ee166e2e4..709de0fe1d54 100644 --- a/components/esp_ringbuf/include/freertos/ringbuf.h +++ b/components/esp_ringbuf/include/freertos/ringbuf.h @@ -506,6 +506,16 @@ void vRingbufferGetInfo(RingbufHandle_t xRingbuffer, UBaseType_t *uxAcquire, UBaseType_t *uxItemsWaiting); +/** + * @brief Unblock any read function that is currently waiting. example: xRingbufferReceiveUpTo() + * + * All read functions take a xTicksToWait argument, which can be set up to + * to infinity. This function will unblock any threads currently waiting. + * + * @param[in] xRingbuffer The ring buffer who's rx will be unblocked. + */ +void vRingbufferUnblockRx(RingbufHandle_t xRingbuffer); + /** * @brief Debugging function to print the internal pointers in the ring buffer * diff --git a/components/esp_ringbuf/ringbuf.c b/components/esp_ringbuf/ringbuf.c index 0f9ab40bdc44..76db83601f9f 100644 --- a/components/esp_ringbuf/ringbuf.c +++ b/components/esp_ringbuf/ringbuf.c @@ -24,7 +24,8 @@ #define rbBYTE_BUFFER_FLAG ( ( UBaseType_t ) 2 ) //The ring buffer is a byte buffer #define rbBUFFER_FULL_FLAG ( ( UBaseType_t ) 4 ) //The ring buffer is currently full (write pointer == free pointer) #define rbBUFFER_STATIC_FLAG ( ( UBaseType_t ) 8 ) //The ring buffer is statically allocated -#define rbUSING_QUEUE_SET ( ( UBaseType_t ) 16 ) //The ring buffer has been added to a queue set +#define rbUSING_QUEUE_SET ( ( UBaseType_t ) 16) //The ring buffer has been added to a queue set +#define rbBUFFER_UNBLOCK_RX_FLAG ( ( UBaseType_t ) 32) //A request has been made to unblock any pending reads //Item flags #define rbITEM_FREE_FLAG ( ( UBaseType_t ) 1 ) //Item has been retrieved and returned by application, free to overwrite @@ -742,6 +743,7 @@ static size_t prvGetCurMaxSizeByteBuf(Ringbuffer_t *pxRingbuffer) return xFreeSize; } +<<<<<<< HEAD static BaseType_t prvSendAcquireGeneric(Ringbuffer_t *pxRingbuffer, const void *pvItem, void **ppvItem, @@ -808,6 +810,9 @@ static BaseType_t prvSendAcquireGeneric(Ringbuffer_t *pxRingbuffer, return xReturn; } +======= + +>>>>>>> 02eb8d68a2 ([Console] fix console deinit: add ability to unblock linenoise, to fix deadlock) static BaseType_t prvReceiveGeneric(Ringbuffer_t *pxRingbuffer, void **pvItem1, @@ -818,6 +823,7 @@ static BaseType_t prvReceiveGeneric(Ringbuffer_t *pxRingbuffer, TickType_t xTicksToWait) { BaseType_t xReturn = pdFALSE; +<<<<<<< HEAD BaseType_t xExitLoop = pdFALSE; BaseType_t xEntryTimeSet = pdFALSE; TimeOut_t xTimeOut; @@ -830,6 +836,26 @@ static BaseType_t prvReceiveGeneric(Ringbuffer_t *pxRingbuffer, #endif /*__clang_analyzer__ */ while (xExitLoop == pdFALSE) { +======= + BaseType_t xReturnSemaphore = pdFALSE; + TickType_t xTicksEnd = xTaskGetTickCount() + xTicksToWait; + TickType_t xTicksRemaining = xTicksToWait; + while (xTicksRemaining <= xTicksToWait) { //xTicksToWait will underflow once xTaskGetTickCount() > ticks_end + //Block until some bytes become available or timeout + if (xSemaphoreTake(rbGET_RX_SEM_HANDLE(pxRingbuffer), xTicksRemaining) != pdTRUE) { + xReturn = pdFALSE; //Timed out attempting to get semaphore + break; + } + + // has a request been made to unblock? + if (pxRingbuffer->uxRingbufferFlags & rbBUFFER_UNBLOCK_RX_FLAG) { + pxRingbuffer->uxRingbufferFlags &= ~rbBUFFER_UNBLOCK_RX_FLAG; // clear flag + xReturn = pdFALSE; + break; + } + + //Semaphore obtained, check if item can be retrieved +>>>>>>> 02eb8d68a2 ([Console] fix console deinit: add ability to unblock linenoise, to fix deadlock) portENTER_CRITICAL(&pxRingbuffer->mux); if (prvCheckItemAvail(pxRingbuffer) == pdTRUE) { //Item/data is available for retrieval @@ -1351,6 +1377,18 @@ void vRingbufferGetInfo(RingbufHandle_t xRingbuffer, portEXIT_CRITICAL(&pxRingbuffer->mux); } +void vRingbufferUnblockRx(RingbufHandle_t xRingbuffer) +{ + Ringbuffer_t *pxRingbuffer = (Ringbuffer_t *)xRingbuffer; + configASSERT(pxRingbuffer); + + // is the semaphore taken? + if (uxSemaphoreGetCount(rbGET_RX_SEM_HANDLE(pxRingbuffer)) == 0) { + pxRingbuffer->uxRingbufferFlags |= rbBUFFER_UNBLOCK_RX_FLAG; + xSemaphoreGive(rbGET_RX_SEM_HANDLE(pxRingbuffer)); + } +} + void xRingbufferPrintInfo(RingbufHandle_t xRingbuffer) { Ringbuffer_t *pxRingbuffer = (Ringbuffer_t *)xRingbuffer;