From 22a30e274004317cf07e3a999ada47e690813c5b Mon Sep 17 00:00:00 2001 From: Konstantin Kondrashov Date: Fri, 17 May 2019 21:27:05 +0800 Subject: [PATCH 1/3] i2c: Fix ticks_to_wait when 0 or time expired Closes: https://github.com/espressif/esp-idf/issues/3301 Closes: IDFGH-964 --- components/driver/i2c.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/components/driver/i2c.c b/components/driver/i2c.c index 2b3a48e63..ded57c3a9 100644 --- a/components/driver/i2c.c +++ b/components/driver/i2c.c @@ -1388,13 +1388,18 @@ int i2c_slave_write_buffer(i2c_port_t i2c_num, uint8_t* data, int size, TickType portBASE_TYPE res; int cnt = 0; - portTickType ticks_end = xTaskGetTickCount() + ticks_to_wait; + portTickType ticks_start = xTaskGetTickCount(); res = xSemaphoreTake(p_i2c->slv_tx_mux, ticks_to_wait); if (res == pdFALSE) { return 0; } - ticks_to_wait = ticks_end - xTaskGetTickCount(); + TickType_t ticks_end = xTaskGetTickCount(); + if (ticks_end - ticks_start > ticks_to_wait) { + ticks_to_wait = 0; + } else { + ticks_to_wait = ticks_to_wait - (ticks_end - ticks_start); + } res = xRingbufferSend(p_i2c->tx_ring_buf, data, size, ticks_to_wait); if (res == pdFALSE) { cnt = 0; @@ -1429,18 +1434,28 @@ int i2c_slave_read_buffer(i2c_port_t i2c_num, uint8_t* data, size_t max_size, Ti i2c_obj_t* p_i2c = p_i2c_obj[i2c_num]; portBASE_TYPE res; - portTickType ticks_end = xTaskGetTickCount() + ticks_to_wait; + portTickType ticks_start = xTaskGetTickCount(); res = xSemaphoreTake(p_i2c->slv_rx_mux, ticks_to_wait); if (res == pdFALSE) { return 0; } - ticks_to_wait = ticks_end - xTaskGetTickCount(); + TickType_t ticks_end = xTaskGetTickCount(); + if (ticks_end - ticks_start > ticks_to_wait) { + ticks_to_wait = 0; + } else { + ticks_to_wait = ticks_to_wait - (ticks_end - ticks_start); + } int cnt = i2c_slave_read(i2c_num, data, max_size, ticks_to_wait); if (cnt > 0) { I2C_ENTER_CRITICAL(&i2c_spinlock[i2c_num]); I2C[i2c_num]->int_ena.rx_fifo_full = 1; I2C_EXIT_CRITICAL(&i2c_spinlock[i2c_num]); - ticks_to_wait = ticks_end - xTaskGetTickCount(); + ticks_end = xTaskGetTickCount(); + if (ticks_end - ticks_start > ticks_to_wait) { + ticks_to_wait = 0; + } else { + ticks_to_wait = ticks_to_wait - (ticks_end - ticks_start); + } if (cnt < max_size && ticks_to_wait > 0) { cnt += i2c_slave_read(i2c_num, data + cnt, max_size - cnt, ticks_to_wait); } From 355f209dba8394c9efbd0cad55712efd6ba6b32c Mon Sep 17 00:00:00 2001 From: Konstantin Kondrashov Date: Fri, 17 May 2019 21:29:45 +0800 Subject: [PATCH 2/3] uart: Fix ticks_to_wait when 0 or expired Closes: https://github.com/espressif/esp-idf/issues/3301 Closes: IDFGH-964 --- components/driver/uart.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/components/driver/uart.c b/components/driver/uart.c index 411279e22..806998820 100644 --- a/components/driver/uart.c +++ b/components/driver/uart.c @@ -1040,20 +1040,25 @@ esp_err_t uart_wait_tx_done(uart_port_t uart_num, TickType_t ticks_to_wait) UART_CHECK((uart_num < UART_NUM_MAX), "uart_num error", ESP_FAIL); UART_CHECK((p_uart_obj[uart_num]), "uart driver error", ESP_FAIL); BaseType_t res; - portTickType ticks_end = xTaskGetTickCount() + ticks_to_wait; + portTickType ticks_start = xTaskGetTickCount(); //Take tx_mux res = xSemaphoreTake(p_uart_obj[uart_num]->tx_mux, (portTickType)ticks_to_wait); if(res == pdFALSE) { return ESP_ERR_TIMEOUT; } - ticks_to_wait = ticks_end - xTaskGetTickCount(); xSemaphoreTake(p_uart_obj[uart_num]->tx_done_sem, 0); - ticks_to_wait = ticks_end - xTaskGetTickCount(); if(UART[uart_num]->status.txfifo_cnt == 0) { xSemaphoreGive(p_uart_obj[uart_num]->tx_mux); return ESP_OK; } uart_enable_intr_mask(uart_num, UART_TX_DONE_INT_ENA_M); + + TickType_t ticks_end = xTaskGetTickCount(); + if (ticks_end - ticks_start > ticks_to_wait) { + ticks_to_wait = 0; + } else { + ticks_to_wait = ticks_to_wait - (ticks_end - ticks_start); + } //take 2nd tx_done_sem, wait given from ISR res = xSemaphoreTake(p_uart_obj[uart_num]->tx_done_sem, (portTickType)ticks_to_wait); if(res == pdFALSE) { From ed22949847801969f69ef9ca6bc60392e8edecb2 Mon Sep 17 00:00:00 2001 From: Konstantin Kondrashov Date: Tue, 4 Jun 2019 12:17:55 +0800 Subject: [PATCH 3/3] driver: Add uart and i2c UTs to check ticks_to_wait in some functions --- components/driver/test/test_i2c.c | 64 ++++++++++++++++++++++++++++++ components/driver/test/test_uart.c | 50 +++++++++++++++++++++++ 2 files changed, 114 insertions(+) diff --git a/components/driver/test/test_i2c.c b/components/driver/test/test_i2c.c index 48aea2a4b..96c236dd9 100644 --- a/components/driver/test/test_i2c.c +++ b/components/driver/test/test_i2c.c @@ -506,3 +506,67 @@ static void i2c_slave_repeat_read() } TEST_CASE_MULTIPLE_DEVICES("I2C repeat write test", "[i2c][test_env=UT_T2_I2C][timeout=150]", i2c_master_repeat_write, i2c_slave_repeat_read); + +static volatile bool exit_flag; +static bool test_read_func; + +static void test_task(void *pvParameters) +{ + xSemaphoreHandle *sema = (xSemaphoreHandle *) pvParameters; + + uint8_t *data = (uint8_t *) malloc(DATA_LENGTH); + i2c_config_t conf_slave = i2c_slave_init(); + TEST_ESP_OK(i2c_param_config( I2C_SLAVE_NUM, &conf_slave)); + TEST_ESP_OK(i2c_driver_install(I2C_SLAVE_NUM, I2C_MODE_SLAVE, + I2C_SLAVE_RX_BUF_LEN, + I2C_SLAVE_TX_BUF_LEN, 0)); + while (exit_flag == false) { + if (test_read_func) { + i2c_slave_read_buffer(I2C_SLAVE_NUM, data, DATA_LENGTH, 0); + } else { + i2c_slave_write_buffer(I2C_SLAVE_NUM, data, DATA_LENGTH, 0); + } + } + + free(data); + xSemaphoreGive(*sema); + vTaskDelete(NULL); +} + +TEST_CASE("test i2c_slave_read_buffer is not blocked when ticks_to_wait=0", "[i2c]") +{ + xSemaphoreHandle exit_sema = xSemaphoreCreateBinary(); + exit_flag = false; + + test_read_func = true; + xTaskCreate(test_task, "tsk1", 2048, &exit_sema, 5, NULL); + + printf("Waiting for 5 sec\n"); + vTaskDelay(5000 / portTICK_PERIOD_MS); + exit_flag = true; + if (xSemaphoreTake(exit_sema, 1000 / portTICK_PERIOD_MS) == pdTRUE) { + vSemaphoreDelete(exit_sema); + } else { + TEST_FAIL_MESSAGE("i2c_slave_read_buffer is blocked"); + } + TEST_ESP_OK(i2c_driver_delete(I2C_SLAVE_NUM)); +} + +TEST_CASE("test i2c_slave_write_buffer is not blocked when ticks_to_wait=0", "[i2c]") +{ + xSemaphoreHandle exit_sema = xSemaphoreCreateBinary(); + exit_flag = false; + + test_read_func = false; + xTaskCreate(test_task, "tsk1", 2048, &exit_sema, 5, NULL); + + printf("Waiting for 5 sec\n"); + vTaskDelay(5000 / portTICK_PERIOD_MS); + exit_flag = true; + if (xSemaphoreTake(exit_sema, 1000 / portTICK_PERIOD_MS) == pdTRUE) { + vSemaphoreDelete(exit_sema); + } else { + TEST_FAIL_MESSAGE("i2c_slave_write_buffer is blocked"); + } + TEST_ESP_OK(i2c_driver_delete(I2C_SLAVE_NUM)); +} diff --git a/components/driver/test/test_uart.c b/components/driver/test/test_uart.c index 55a41f43b..8ef579c80 100644 --- a/components/driver/test/test_uart.c +++ b/components/driver/test/test_uart.c @@ -118,6 +118,56 @@ static void uart_config(uint32_t baud_rate, bool use_ref_tick) uart_driver_install(UART_NUM1, BUF_SIZE * 2, BUF_SIZE * 2, 20, NULL, 0); } +static volatile bool exit_flag; + +static void test_task(void *pvParameters) +{ + xSemaphoreHandle *sema = (xSemaphoreHandle *) pvParameters; + char* data = (char *) malloc(256); + + while (exit_flag == false) { + uart_tx_chars(UART_NUM1, data, 256); + // The uart_wait_tx_done() function does not block anything if ticks_to_wait = 0. + uart_wait_tx_done(UART_NUM1, 0); + } + + free(data); + xSemaphoreGive(*sema); + vTaskDelete(NULL); +} + +static void test_task2(void *pvParameters) +{ + while (exit_flag == false) { + // This task obstruct a setting tx_done_sem semaphore in the UART interrupt. + // It leads to waiting the ticks_to_wait time in uart_wait_tx_done() function. + uart_disable_intr_mask(UART_NUM1, UART_TX_DONE_INT_ENA_M); + } + vTaskDelete(NULL); +} + +TEST_CASE("test uart_wait_tx_done is not blocked when ticks_to_wait=0", "[uart]") +{ + uart_config(UART_BAUD_11520, false); + + xSemaphoreHandle exit_sema = xSemaphoreCreateBinary(); + exit_flag = false; + + xTaskCreate(test_task, "tsk1", 2048, &exit_sema, 5, NULL); + xTaskCreate(test_task2, "tsk2", 2048, NULL, 5, NULL); + + printf("Waiting for 5 sec\n"); + vTaskDelay(5000 / portTICK_PERIOD_MS); + exit_flag = true; + + if (xSemaphoreTake(exit_sema, 1000 / portTICK_PERIOD_MS) == pdTRUE) { + vSemaphoreDelete(exit_sema); + } else { + TEST_FAIL_MESSAGE("uart_wait_tx_done is blocked"); + } + TEST_ESP_OK(uart_driver_delete(UART_NUM1)); +} + TEST_CASE("test uart get baud-rate", "[uart]") { uint32_t baud_rate1 = 0;