From 8282c73ac25a3b6e8895719eadab0eec92f1e1b3 Mon Sep 17 00:00:00 2001 From: Wangjialin Date: Thu, 3 Nov 2016 18:28:36 +0800 Subject: [PATCH] debug ring buffer error. --- components/driver/include/driver/uart.h | 2 +- components/driver/uart.c | 205 +++++++++++++++++------- components/freertos/ringbuf.c | 29 +++- 3 files changed, 177 insertions(+), 59 deletions(-) diff --git a/components/driver/include/driver/uart.h b/components/driver/include/driver/uart.h index 3ea77b2d02..445d71b685 100644 --- a/components/driver/include/driver/uart.h +++ b/components/driver/include/driver/uart.h @@ -530,7 +530,7 @@ int uart_read_char(uart_port_t uart_num, TickType_t ticks_to_wait); * @param uart_no UART_NUM_0, UART_NUM_1 or UART_NUM_2 * @param buf pointer to the buffer. * @param length data length - * @param ticks_to_wait: Timeout, count in RTOS ticks + * @param ticks_to_wait sTimeout, count in RTOS ticks * * @return * - (-1) Error diff --git a/components/driver/uart.c b/components/driver/uart.c index eeb2c64208..d6585405f2 100644 --- a/components/driver/uart.c +++ b/components/driver/uart.c @@ -37,8 +37,6 @@ const char* UART_TAG = "UART"; #define UART_EMPTY_THRESH_DEFAULT (10) #define UART_FULL_THRESH_DEFAULT (120) #define UART_TOUT_THRESH_DEFAULT (10) -#define UART_TX_TASK_DEPTH_DEFAULT (256*2+64) -#define UART_TX_TASK_PRIO_DEFAULT (10) #define UART_ENTER_CRITICAL_ISR(mux) portENTER_CRITICAL_ISR(mux) #define UART_EXIT_CRITICAL_ISR(mux) portEXIT_CRITICAL_ISR(mux) #define UART_ENTER_CRITICAL(mux) portENTER_CRITICAL(mux) @@ -60,7 +58,6 @@ typedef struct { RingbufHandle_t rx_ring_buf; int tx_buf_size; RingbufHandle_t tx_ring_buf; - TaskHandle_t tx_task_handle; bool buffer_full_flg; bool tx_waiting; int cur_remain; @@ -439,8 +436,17 @@ static void IRAM_ATTR uart_rx_intr_handler_default(void *param) uart_dev_t* uart_reg = UART[uart_num]; uint8_t buf_idx = 0; uint32_t uart_intr_status = UART[uart_num]->int_st.val; - static int rx_fifo_len = 0; + int rx_fifo_len = 0; uart_event_t uart_event; + + static uint8_t * tx_ptr; + static uart_event_t* tx_head; + static int tx_len_tot = 0; + static int brk_flg = 0; + static int tx_brk_len = 0; + static int wait_brk = 0; + + portBASE_TYPE HPTaskAwoken = 0; while(uart_intr_status != 0x0) { buf_idx = 0; @@ -450,14 +456,99 @@ static void IRAM_ATTR uart_rx_intr_handler_default(void *param) uart_reg->int_ena.txfifo_empty = 0; uart_reg->int_clr.txfifo_empty = 1; UART_EXIT_CRITICAL_ISR(&uart_spinlock[uart_num]); + if(wait_brk) { + return; + } if(p_uart->tx_waiting == true) { p_uart->tx_waiting = false; xSemaphoreGiveFromISR(p_uart->tx_fifo_sem, NULL); } + else { + int tx_fifo_rem = UART_FIFO_LEN - UART[uart_num]->status.txfifo_cnt; + bool en_tx_flg = false; + if(tx_len_tot == 0) { + size_t size; +// ets_printf("dbg1,tot=0,get 1st head\n"); +// xRingbufferPrintInfo(p_uart->tx_ring_buf); + tx_head = (uart_event_t*) xRingbufferReceiveFromISR(p_uart->tx_ring_buf, &size); +// xRingbufferPrintInfo(p_uart->tx_ring_buf); + if(tx_head) { //enable empty intr +// tx_ptr = (uint8_t*)tx_head + sizeof(uart_event_t); + tx_ptr = NULL; +// en_tx_flg = true; + tx_len_tot = tx_head->data.size; + if(tx_head->type == UART_DATA_BREAK) { + tx_len_tot = tx_head->data.size; + brk_flg = 1; + tx_brk_len = tx_head->data.brk_len; + } +// ets_printf("ret1,tot: %d\n", tx_len_tot); + vRingbufferReturnItemFromISR(p_uart->tx_ring_buf, tx_head, &HPTaskAwoken); +// xRingbufferPrintInfo(p_uart->tx_ring_buf); +// xRingbufferPrintInfo(p_uart->tx_ring_buf); + } + else { + return; + } + } + if(tx_ptr == NULL) { + size_t size; +// ets_printf("dbg2, tx ptr null, get 2nd tx ptr\n"); +// xRingbufferPrintInfo(p_uart->tx_ring_buf); + tx_ptr = (uint8_t*) xRingbufferReceiveFromISR(p_uart->tx_ring_buf, &size); + +// xRingbufferPrintInfo(p_uart->tx_ring_buf); + if(tx_ptr) { + tx_head = (void*) tx_ptr; +// ets_printf("get size: %d ; h size: %d\n", size, tx_len_tot); + en_tx_flg = true; + } else { + return; + } + } +// else + if(tx_len_tot > 0 && tx_ptr) { //tx + int send_len = tx_len_tot > tx_fifo_rem ? tx_fifo_rem : tx_len_tot; + for(buf_idx = 0; buf_idx < send_len; buf_idx++) { + WRITE_PERI_REG(UART_FIFO_AHB_REG(uart_num), *(tx_ptr++) & 0xff); + } + tx_len_tot -= send_len; +// ets_printf("tot: %d\n", tx_len_tot); + if(tx_len_tot == 0) { + if(brk_flg == 1) { + UART_ENTER_CRITICAL_ISR(&uart_spinlock[uart_num]); + uart_reg->int_ena.tx_brk_done = 0; + uart_reg->idle_conf.tx_brk_num = tx_brk_len; + uart_reg->conf0.txd_brk = 1; + uart_reg->int_clr.tx_brk_done = 1; + uart_reg->int_ena.tx_brk_done = 1; + UART_EXIT_CRITICAL_ISR(&uart_spinlock[uart_num]); + wait_brk = 1; + } else { + en_tx_flg = true; + } +// ets_printf("ret2\n"); + vRingbufferReturnItemFromISR(p_uart->tx_ring_buf, tx_head, &HPTaskAwoken); +// xRingbufferPrintInfo(p_uart->tx_ring_buf); +// xRingbufferPrintInfo(p_uart->tx_ring_buf); + tx_head = NULL; + tx_ptr = NULL; + } else { + en_tx_flg = true; + } + } + if(en_tx_flg) { + UART_ENTER_CRITICAL_ISR(&uart_spinlock[uart_num]); + uart_reg->int_clr.txfifo_empty = 1; + uart_reg->int_ena.txfifo_empty = 1; + UART_EXIT_CRITICAL_ISR(&uart_spinlock[uart_num]); + } + } } else if((uart_intr_status & UART_RXFIFO_TOUT_INT_ST_M) || (uart_intr_status & UART_RXFIFO_FULL_INT_ST_M)) { if(p_uart->buffer_full_flg == false) { //Get the buffer from the FIFO +// ESP_LOGE(UART_TAG, "FULL\n"); rx_fifo_len = uart_reg->status.rxfifo_cnt; p_uart->data_len = rx_fifo_len; memset(p_uart->data_buf, 0, sizeof(p_uart->data_buf)); @@ -506,12 +597,22 @@ static void IRAM_ATTR uart_rx_intr_handler_default(void *param) uart_reg->int_clr.frm_err = 1; uart_event.type = UART_PARITY_ERR; } else if(uart_intr_status & UART_TX_BRK_DONE_INT_ST_M) { +// ESP_LOGE(UART_TAG, "UART TX BRK DONE\n"); + ets_printf("tx brk done\n"); UART_ENTER_CRITICAL_ISR(&uart_spinlock[uart_num]); uart_reg->conf0.txd_brk = 0; uart_reg->int_ena.tx_brk_done = 0; uart_reg->int_clr.tx_brk_done = 1; + if(brk_flg == 1) { + uart_reg->int_ena.txfifo_empty = 1; + } UART_EXIT_CRITICAL_ISR(&uart_spinlock[uart_num]); - xSemaphoreGiveFromISR(p_uart->tx_brk_sem, &HPTaskAwoken); + if(brk_flg == 1) { + brk_flg = 0; + wait_brk = 0; + } else { + xSemaphoreGiveFromISR(p_uart->tx_brk_sem, &HPTaskAwoken); + } } else if(uart_intr_status & UART_TX_BRK_IDLE_DONE_INT_ST_M) { UART_ENTER_CRITICAL_ISR(&uart_spinlock[uart_num]); uart_reg->int_ena.tx_brk_idle_done = 0; @@ -638,26 +739,26 @@ static int uart_tx_all(uart_port_t uart_num, const char* src, size_t size, bool return original_size; } -static void uart_tx_task(void* arg) -{ - uart_obj_t* p_uart = (uart_obj_t*) arg; - size_t size; - uart_event_t evt; - for(;;) { - char* data = (char*) xRingbufferReceive(p_uart->tx_ring_buf, &size, portMAX_DELAY); - if(data == NULL) { - continue; - } - memcpy(&evt, data, sizeof(evt)); - if(evt.type == UART_DATA) { - uart_tx_all(p_uart->uart_num, (const char*) data + sizeof(uart_event_t), evt.data.size, 0, 0); - } else if(evt.type == UART_DATA_BREAK) { - uart_tx_all(p_uart->uart_num, (const char*) data + sizeof(uart_event_t), evt.data.size, 1, evt.data.brk_len); - } - vRingbufferReturnItem(p_uart->tx_ring_buf, data); - } - vTaskDelete(NULL); -} +//static void uart_tx_task(void* arg) +//{ +// uart_obj_t* p_uart = (uart_obj_t*) arg; +// size_t size; +// uart_event_t evt; +// for(;;) { +// char* data = (char*) xRingbufferReceive(p_uart->tx_ring_buf, &size, portMAX_DELAY); +// if(data == NULL) { +// continue; +// } +// memcpy(&evt, data, sizeof(evt)); +// if(evt.type == UART_DATA) { +// uart_tx_all(p_uart->uart_num, (const char*) data + sizeof(uart_event_t), evt.data.size, 0, 0); +// } else if(evt.type == UART_DATA_BREAK) { +// uart_tx_all(p_uart->uart_num, (const char*) data + sizeof(uart_event_t), evt.data.size, 1, evt.data.brk_len); +// } +// vRingbufferReturnItem(p_uart->tx_ring_buf, data); +// } +// vTaskDelete(NULL); +//} int uart_tx_all_chars(uart_port_t uart_num, const char* src, size_t size) { @@ -666,19 +767,18 @@ int uart_tx_all_chars(uart_port_t uart_num, const char* src, size_t size) UART_CHECK(src, "buffer null"); if(p_uart_obj[uart_num]->tx_buf_size > 0) { if(xRingbufferGetMaxItemSize(p_uart_obj[uart_num]->tx_ring_buf) > (size + sizeof(uart_event_t))) { - uart_event_t *evt = (uart_event_t*) malloc(sizeof(uart_event_t) + size); - if(evt == NULL) { - ESP_LOGE(UART_TAG, "UART EVT MALLOC ERROR\n"); - return -1; - } + uart_event_t evt; xSemaphoreTake(p_uart_obj[uart_num]->tx_buffer_mutex, (portTickType)portMAX_DELAY); - evt->type = UART_DATA; - evt->data.size = size; - memcpy(evt->data.data, src, size); - xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void*) evt, sizeof(uart_event_t) + size, portMAX_DELAY); - free(evt); - evt = NULL; + evt.type = UART_DATA; + evt.data.size = size; + ets_printf("-----1st send-----\n"); + xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void*) &evt, sizeof(uart_event_t), portMAX_DELAY); + xRingbufferPrintInfo(p_uart_obj[uart_num]->tx_ring_buf); + ets_printf("====2nd send====\n"); + xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void*) src, size, portMAX_DELAY); + xRingbufferPrintInfo(p_uart_obj[uart_num]->tx_ring_buf); xSemaphoreGive(p_uart_obj[uart_num]->tx_buffer_mutex); + uart_enable_tx_intr(uart_num, 1, UART_EMPTY_THRESH_DEFAULT); return size; } else { ESP_LOGW(UART_TAG, "UART TX BUFFER TOO SMALL[0], SEND DIRECTLY\n"); @@ -698,19 +798,15 @@ int uart_tx_all_chars_with_break(uart_port_t uart_num, const char* src, size_t s UART_CHECK((brk_len > 0 && brk_len < 256), "break_num error"); if(p_uart_obj[uart_num]->tx_buf_size > 0) { if(xRingbufferGetMaxItemSize(p_uart_obj[uart_num]->tx_ring_buf) > (size)) { - uart_event_t *evt = (uart_event_t*) malloc(sizeof(uart_event_t) + size); - if(evt == NULL) { - return -1; - } + uart_event_t evt; xSemaphoreTake(p_uart_obj[uart_num]->tx_buffer_mutex, (portTickType)portMAX_DELAY); - evt->type = UART_DATA_BREAK; - evt->data.size = size; - evt->data.brk_len = brk_len; - memcpy(evt->data.data, src, size); - xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void*) evt, sizeof(uart_event_t) + size, portMAX_DELAY); - free(evt); - evt = NULL; + evt.type = UART_DATA_BREAK; + evt.data.size = size; + evt.data.brk_len = brk_len; + xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void*) &evt, sizeof(uart_event_t), portMAX_DELAY); + xRingbufferSend(p_uart_obj[uart_num]->tx_ring_buf, (void*) src, size, portMAX_DELAY); xSemaphoreGive(p_uart_obj[uart_num]->tx_buffer_mutex); + uart_enable_tx_intr(uart_num, 1, UART_EMPTY_THRESH_DEFAULT); return size; } else { ESP_LOGW(UART_TAG, "UART TX BUFFER TOO SMALL[1], SEND DIRECTLY\n"); @@ -782,8 +878,10 @@ int uart_read_bytes(uart_port_t uart_num, uint8_t* buf, uint32_t length, TickTyp p_uart_obj[uart_num]->head_ptr = data; p_uart_obj[uart_num]->rd_ptr = data; p_uart_obj[uart_num]->cur_remain = size; +// ets_printf("dbg0\n"); } else { xSemaphoreGive(p_uart_obj[uart_num]->rx_mux); +// ets_printf("dbg1\n"); return copy_len; } } @@ -792,17 +890,20 @@ int uart_read_bytes(uart_port_t uart_num, uint8_t* buf, uint32_t length, TickTyp } else { len_tmp = p_uart_obj[uart_num]->cur_remain; } +// ets_printf("dbga\n"); memcpy(buf + copy_len, p_uart_obj[uart_num]->rd_ptr, len_tmp); p_uart_obj[uart_num]->rd_ptr += len_tmp; p_uart_obj[uart_num]->cur_remain -= len_tmp; copy_len += len_tmp; length -= len_tmp; +// ets_printf("dbgb\n"); if(p_uart_obj[uart_num]->cur_remain == 0) { vRingbufferReturnItem(p_uart_obj[uart_num]->rx_ring_buf, p_uart_obj[uart_num]->head_ptr); p_uart_obj[uart_num]->head_ptr = NULL; p_uart_obj[uart_num]->rd_ptr = NULL; if(p_uart_obj[uart_num]->buffer_full_flg) { BaseType_t res = xRingbufferSend(p_uart_obj[uart_num]->rx_ring_buf, p_uart_obj[uart_num]->data_buf, p_uart_obj[uart_num]->data_len, 1); +// ets_printf("dbg2\n"); if(res == pdTRUE) { p_uart_obj[uart_num]->buffer_full_flg = false; uart_enable_rx_intr(p_uart_obj[uart_num]->uart_num); @@ -810,6 +911,7 @@ int uart_read_bytes(uart_port_t uart_num, uint8_t* buf, uint32_t length, TickTyp } } } +// ets_printf("dbg3\n"); xSemaphoreGive(p_uart_obj[uart_num]->rx_mux); return copy_len; } @@ -944,14 +1046,11 @@ esp_err_t uart_driver_install(uart_port_t uart_num, int rx_buffer_size, int tx_b p_uart_obj[uart_num]->rx_buf_type = rx_buf_type; p_uart_obj[uart_num]->rx_ring_buf = xRingbufferCreate(rx_buffer_size, rx_buf_type); if(tx_buffer_size > 0) { - p_uart_obj[uart_num]->tx_ring_buf = xRingbufferCreate(tx_buffer_size, RINGBUF_TYPE_NOSPLIT); + p_uart_obj[uart_num]->tx_ring_buf = xRingbufferCreate(tx_buffer_size, RINGBUF_TYPE_NOSPLIT);//RINGBUF_TYPE_BYTEBUF);//RINGBUF_TYPE_NOSPLIT); p_uart_obj[uart_num]->tx_buf_size = tx_buffer_size; - xTaskCreate(uart_tx_task, "uart_tx_task", UART_TX_TASK_DEPTH_DEFAULT, (void*)p_uart_obj[uart_num], UART_TX_TASK_PRIO_DEFAULT, &p_uart_obj[uart_num]->tx_task_handle); - } else { p_uart_obj[uart_num]->tx_ring_buf = NULL; p_uart_obj[uart_num]->tx_buf_size = 0; - p_uart_obj[uart_num]->tx_task_handle = NULL; } } else { ESP_LOGE(UART_TAG, "UART driver already installed\n"); @@ -986,10 +1085,6 @@ esp_err_t uart_driver_delete(uart_port_t uart_num) uart_disable_tx_intr(uart_num); uart_isr_register(uart_num, p_uart_obj[uart_num]->intr_num, NULL, NULL); - if(p_uart_obj[uart_num]->tx_task_handle) { - vTaskDelete(p_uart_obj[uart_num]->tx_task_handle); - p_uart_obj[uart_num]->tx_task_handle = NULL; - } if(p_uart_obj[uart_num]->tx_fifo_sem) { vSemaphoreDelete(p_uart_obj[uart_num]->tx_fifo_sem); p_uart_obj[uart_num]->tx_fifo_sem = NULL; diff --git a/components/freertos/ringbuf.c b/components/freertos/ringbuf.c index ce5504596a..560eb5fdd9 100644 --- a/components/freertos/ringbuf.c +++ b/components/freertos/ringbuf.c @@ -77,9 +77,13 @@ static int ringbufferFreeMem(ringbuf_t *rb) { int free_size = rb->free_ptr-rb->write_ptr; if (free_size <= 0) free_size += rb->size; + //If we free the last dummy item in the buffer, free_ptr will point to rb->data + //In this case, after we write the last some bytes, the buffer might wrap around if we don't have room for a header anymore. +// if (free_size == 0 && rb->read_ptr == rb->write_ptr) free_size += rb->size; //Reserve one byte. If we do not do this and the entire buffer is filled, we get a situation - //where read_ptr == free_ptr, messing up the next calculation. - return free_size-1; + //where write_ptr == free_ptr, messing up the next calculation. +// return free_size == 0 ? 0 : free_size - 1; + return free_size - 1; } @@ -334,6 +338,10 @@ static uint8_t *getItemFromRingbufByteBuf(ringbuf_t *rb, size_t *length, int wan //can be increase. //This function by itself is not threadsafe, always call from within a muxed section. static void returnItemToRingbufDefault(ringbuf_t *rb, void *item) { + ets_printf("in returnItemToRingbufDefault\n"); + xRingbufferPrintInfo(rb); + + uint8_t *data=(uint8_t*)item; configASSERT(((int)rb->free_ptr&3)==0); configASSERT(data >= rb->data); @@ -350,9 +358,16 @@ static void returnItemToRingbufDefault(ringbuf_t *rb, void *item) { hdr=(buf_entry_hdr_t *)rb->free_ptr; //basically forward free_ptr until we run into either a block that is still in use or the write pointer. while (((hdr->flags & iflag_free) || (hdr->flags & iflag_dummydata)) && rb->free_ptr != rb->write_ptr) { + if (hdr->flags & iflag_dummydata) { + ets_printf("hrd len: %d; flg: 0x%02x\n",hdr->len,hdr->flags); //Rest is dummy data. Reset to start of ringbuffer. rb->free_ptr=rb->data; + //If the read_ptr is pointing to this dummy item, + //we should also move the read pointer to data, in case we overwrite the read hdr. +// if(rb->read_ptr == (uint8_t*)hdr) { +// rb->read_ptr = rb->data; +// } } else { //Skip past item size_t len=(hdr->len+3)&~3; @@ -363,8 +378,10 @@ static void returnItemToRingbufDefault(ringbuf_t *rb, void *item) { if ((rb->data+rb->size)-rb->free_ptr < sizeof(buf_entry_hdr_t)) { rb->free_ptr=rb->data; } + if(rb->free_ptr == rb->read_ptr) break; //Next header hdr=(buf_entry_hdr_t *)rb->free_ptr; + } } @@ -386,6 +403,12 @@ void xRingbufferPrintInfo(RingbufHandle_t ringbuf) configASSERT(rb); ets_printf("Rb size %d free %d rptr %d freeptr %d wptr %d\n", rb->size, ringbufferFreeMem(rb), rb->read_ptr-rb->data, rb->free_ptr-rb->data, rb->write_ptr-rb->data); + buf_entry_hdr_t *hdr=(buf_entry_hdr_t *)rb->read_ptr; + if(rb->write_ptr == rb->read_ptr) { + ets_printf("write que read\n"); + } else { + ets_printf("hdr len: %d; flg: 0x%08x\n", hdr->len, hdr->flags); + } } @@ -493,7 +516,7 @@ BaseType_t xRingbufferSend(RingbufHandle_t ringbuf, void *data, size_t dataSize, ticks_to_wait = ticks_end - xTaskGetTickCount(); } } while (ringbufferFreeMem(rb) < needed_size && ticks_to_wait>=0); - + //Lock the mux in order to make sure no one else is messing with the ringbuffer and do the copy. portENTER_CRITICAL(&rb->mux); //Another thread may have been able to sneak its write first. Check again now we locked the ringbuff, and retry