From cbe82746514ca10bafc678c298382f440c3bd6d1 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 10 Oct 2019 14:20:20 +0200 Subject: [PATCH 1/3] drivers/can: fix skipped function calls when assertions disabled When CONFIG_FREERTOS_ASSERT_DISABLE is set, the function calls wrapped inside the assertion macros would be removed from the code. Closes https://github.com/espressif/esp-idf/issues/4143 --- components/driver/can.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/components/driver/can.c b/components/driver/can.c index 8d374f32f..6442cf37e 100644 --- a/components/driver/can.c +++ b/components/driver/can.c @@ -452,13 +452,17 @@ static void can_intr_handler_tx(can_status_reg_t *status, int *alert_req) //Update TX message count p_can_obj->tx_msg_count--; - configASSERT(p_can_obj->tx_msg_count >= 0); //Sanity check + assert(p_can_obj->tx_msg_count >= 0); //Sanity check //Check if there are more frames to transmit if (p_can_obj->tx_msg_count > 0 && p_can_obj->tx_queue != NULL) { can_frame_t frame; - configASSERT(xQueueReceiveFromISR(p_can_obj->tx_queue, &frame, NULL) == pdTRUE); - can_set_tx_buffer_and_transmit(&frame); + int res = xQueueReceiveFromISR(p_can_obj->tx_queue, &frame, NULL); + if (res == pdTRUE) { + can_set_tx_buffer_and_transmit(&frame); + } else { + assert(false && "failed to get a frame from TX queue"); + } } else { //No more frames to transmit CAN_RESET_FLAG(p_can_obj->control_flags, CTRL_FLAG_TX_BUFF_OCCUPIED); @@ -674,7 +678,8 @@ esp_err_t can_driver_install(const can_general_config_t *g_config, const can_tim } periph_module_reset(PERIPH_CAN_MODULE); periph_module_enable(PERIPH_CAN_MODULE); //Enable APB CLK to CAN peripheral - configASSERT(can_enter_reset_mode() == ESP_OK); //Must enter reset mode to write to config registers + esp_err_t err = can_exit_reset_mode(); //Must enter reset mode to write to config registers + assert(err == ESP_OK); can_config_pelican(); //Use PeliCAN addresses /* Note: REC is allowed to increase even in reset mode. Listen only mode will freeze REC. The desired mode will be set when can_start() is called. */ @@ -729,7 +734,8 @@ esp_err_t can_driver_uninstall() //Check state CAN_CHECK_FROM_CRIT(p_can_obj != NULL, ESP_ERR_INVALID_STATE); CAN_CHECK_FROM_CRIT(p_can_obj->control_flags & (CTRL_FLAG_STOPPED | CTRL_FLAG_BUS_OFF), ESP_ERR_INVALID_STATE); - configASSERT(can_enter_reset_mode() == ESP_OK); //Enter reset mode to stop any CAN bus activity + esp_err_t err = can_exit_reset_mode(); //Enter reset mode to stop any CAN bus activity + assert(err == ESP_OK); //Clear registers by reading (void) can_get_interrupt_reason(); (void) can_get_arbitration_lost_capture(); @@ -767,7 +773,8 @@ esp_err_t can_start() //Reset RX queue, and RX message count xQueueReset(p_can_obj->rx_queue); p_can_obj->rx_msg_count = 0; - configASSERT(can_enter_reset_mode() == ESP_OK); //Should already be in bus-off mode, set again to make sure + esp_err_t err = can_exit_reset_mode(); //Should already be in bus-off mode, set again to make sure + assert(err == ESP_OK); //Currently in listen only mode, need to set to mode specified by configuration can_mode_t mode; @@ -780,7 +787,8 @@ esp_err_t can_start() } can_config_mode(mode); //Set mode (void) can_get_interrupt_reason(); //Clear interrupt register - configASSERT(can_exit_reset_mode() == ESP_OK); + err = can_exit_reset_mode(); + assert(err == ESP_OK); CAN_RESET_FLAG(p_can_obj->control_flags, CTRL_FLAG_STOPPED); CAN_EXIT_CRITICAL(); @@ -795,7 +803,8 @@ esp_err_t can_stop() CAN_CHECK_FROM_CRIT(!(p_can_obj->control_flags & (CTRL_FLAG_STOPPED | CTRL_FLAG_BUS_OFF)), ESP_ERR_INVALID_STATE); //Clear interrupts and reset flags - configASSERT(can_enter_reset_mode() == ESP_OK); + esp_err_t err = can_exit_reset_mode(); + assert(err == ESP_OK); (void) can_get_interrupt_reason(); //Read interrupt register to clear interrupts can_config_mode(CAN_MODE_LISTEN_ONLY); //Set to listen only mode to freeze REC CAN_RESET_FLAG(p_can_obj->control_flags, CTRL_FLAG_TX_BUFF_OCCUPIED); @@ -846,11 +855,13 @@ esp_err_t can_transmit(const can_message_t *message, TickType_t ticks_to_wait) CAN_ENTER_CRITICAL(); if (p_can_obj->control_flags & (CTRL_FLAG_STOPPED | CTRL_FLAG_BUS_OFF)) { //TX queue was reset (due to stop/bus_off), remove copied frame from queue to prevent transmission - configASSERT(xQueueReceive(p_can_obj->tx_queue, &tx_frame, 0) == pdTRUE); + int res = xQueueReceive(p_can_obj->tx_queue, &tx_frame, 0); + assert(res == pdTRUE); ret = ESP_ERR_INVALID_STATE; } else if ((p_can_obj->tx_msg_count == 0) && !(p_can_obj->control_flags & CTRL_FLAG_TX_BUFF_OCCUPIED)) { //TX buffer was freed during copy, manually trigger transmission - configASSERT(xQueueReceive(p_can_obj->tx_queue, &tx_frame, 0) == pdTRUE); + int res = xQueueReceive(p_can_obj->tx_queue, &tx_frame, 0); + assert(res == pdTRUE); can_set_tx_buffer_and_transmit(&tx_frame); p_can_obj->tx_msg_count++; CAN_SET_FLAG(p_can_obj->control_flags, CTRL_FLAG_TX_BUFF_OCCUPIED); @@ -941,7 +952,8 @@ esp_err_t can_initiate_recovery() CAN_SET_FLAG(p_can_obj->control_flags, CTRL_FLAG_RECOVERING); //Trigger start of recovery process - configASSERT(can_exit_reset_mode() == ESP_OK); + esp_err_t err = can_exit_reset_mode(); + assert(err == ESP_OK); CAN_EXIT_CRITICAL(); return ESP_OK; From 896ba6a83e8ebaaf0abbfbd4bc95bbc820cf8b74 Mon Sep 17 00:00:00 2001 From: Darian Leung Date: Tue, 22 Oct 2019 18:05:19 +0800 Subject: [PATCH 2/3] can: Fix reset mode entry and exit calls This commit fixes multiple bugs caused by incorrect calls to can_enter_reset_mode() and can_exit_reset_mode. Example test scripts also updated to match new runners. --- components/driver/can.c | 11 ++++++----- .../can/can_alert_and_recovery/example_test.py | 7 ++++--- examples/peripherals/can/can_network/example_test.py | 4 ++-- .../peripherals/can/can_self_test/example_test.py | 7 ++++--- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/components/driver/can.c b/components/driver/can.c index 6442cf37e..a97d23fc6 100644 --- a/components/driver/can.c +++ b/components/driver/can.c @@ -376,7 +376,8 @@ static void can_intr_handler_err_warn(can_status_reg_t *status, BaseType_t *task can_alert_handler(CAN_ALERT_ABOVE_ERR_WARN, alert_req); } else if (p_can_obj->control_flags & CTRL_FLAG_RECOVERING) { //Bus recovery complete. - can_enter_reset_mode(); + esp_err_t err = can_enter_reset_mode(); + assert(err == ESP_OK); //Reset and set flags to the equivalent of the stopped state CAN_RESET_FLAG(p_can_obj->control_flags, CTRL_FLAG_RECOVERING | CTRL_FLAG_ERR_WARN | CTRL_FLAG_ERR_PASSIVE | CTRL_FLAG_BUS_OFF | @@ -678,7 +679,7 @@ esp_err_t can_driver_install(const can_general_config_t *g_config, const can_tim } periph_module_reset(PERIPH_CAN_MODULE); periph_module_enable(PERIPH_CAN_MODULE); //Enable APB CLK to CAN peripheral - esp_err_t err = can_exit_reset_mode(); //Must enter reset mode to write to config registers + esp_err_t err = can_enter_reset_mode(); //Must enter reset mode to write to config registers assert(err == ESP_OK); can_config_pelican(); //Use PeliCAN addresses /* Note: REC is allowed to increase even in reset mode. Listen only mode @@ -734,7 +735,7 @@ esp_err_t can_driver_uninstall() //Check state CAN_CHECK_FROM_CRIT(p_can_obj != NULL, ESP_ERR_INVALID_STATE); CAN_CHECK_FROM_CRIT(p_can_obj->control_flags & (CTRL_FLAG_STOPPED | CTRL_FLAG_BUS_OFF), ESP_ERR_INVALID_STATE); - esp_err_t err = can_exit_reset_mode(); //Enter reset mode to stop any CAN bus activity + esp_err_t err = can_enter_reset_mode(); //Enter reset mode to stop any CAN bus activity assert(err == ESP_OK); //Clear registers by reading (void) can_get_interrupt_reason(); @@ -773,7 +774,7 @@ esp_err_t can_start() //Reset RX queue, and RX message count xQueueReset(p_can_obj->rx_queue); p_can_obj->rx_msg_count = 0; - esp_err_t err = can_exit_reset_mode(); //Should already be in bus-off mode, set again to make sure + esp_err_t err = can_enter_reset_mode(); //Should already be in bus-off mode, set again to make sure assert(err == ESP_OK); //Currently in listen only mode, need to set to mode specified by configuration @@ -803,7 +804,7 @@ esp_err_t can_stop() CAN_CHECK_FROM_CRIT(!(p_can_obj->control_flags & (CTRL_FLAG_STOPPED | CTRL_FLAG_BUS_OFF)), ESP_ERR_INVALID_STATE); //Clear interrupts and reset flags - esp_err_t err = can_exit_reset_mode(); + esp_err_t err = can_enter_reset_mode(); assert(err == ESP_OK); (void) can_get_interrupt_reason(); //Read interrupt register to clear interrupts can_config_mode(CAN_MODE_LISTEN_ONLY); //Set to listen only mode to freeze REC diff --git a/examples/peripherals/can/can_alert_and_recovery/example_test.py b/examples/peripherals/can/can_alert_and_recovery/example_test.py index e94de8ff6..fe6b792a5 100644 --- a/examples/peripherals/can/can_alert_and_recovery/example_test.py +++ b/examples/peripherals/can/can_alert_and_recovery/example_test.py @@ -16,10 +16,11 @@ import IDF STR_EXPECT = ("CAN Alert and Recovery: Driver installed", "CAN Alert and Recovery: Driver uninstalled") EXPECT_TIMEOUT = 20 -@IDF.idf_example_test(env_tag='Example_CAN') + +@IDF.idf_example_test(env_tag='Example_CAN1') def test_can_alert_and_recovery_example(env, extra_data): - #Get device under test, flash and start example. "dut4" must be defined in EnvConfig - dut = env.get_dut('dut4', 'examples/peripherals/can/can_alert_and_recovery') + # Get device under test, flash and start example. "dut4" must be defined in EnvConfig + dut = env.get_dut('dut1', 'examples/peripherals/can/can_alert_and_recovery') dut.start_app() for string in STR_EXPECT: diff --git a/examples/peripherals/can/can_network/example_test.py b/examples/peripherals/can/can_network/example_test.py index 517ec5fe6..e56c37e78 100644 --- a/examples/peripherals/can/can_network/example_test.py +++ b/examples/peripherals/can/can_network/example_test.py @@ -17,7 +17,7 @@ import IDF #Define tuple of strings to expect for each DUT. master_expect = ("CAN Master: Driver installed", "CAN Master: Driver uninstalled") slave_expect = ("CAN Slave: Driver installed", "CAN Slave: Driver uninstalled") -listen_only_expect = ("CAN Listen Only: Driver installed", "Listen Only: Driver uninstalled") +listen_only_expect = ("CAN Listen Only: Driver installed", "CAN Listen Only: Driver uninstalled") def dut_thread_callback(**kwargs): #Parse keyword arguments @@ -34,7 +34,7 @@ def dut_thread_callback(**kwargs): #Mark thread has run to completion without any exceptions result[0] = True -@IDF.idf_example_test(env_tag='Example_CAN') +@IDF.idf_example_test(env_tag='Example_CAN2') def test_can_network_example(env, extra_data): #Get device under test. "dut1", "dut2", and "dut3" must be properly defined in EnvConfig diff --git a/examples/peripherals/can/can_self_test/example_test.py b/examples/peripherals/can/can_self_test/example_test.py index 370cdd951..28ade8515 100644 --- a/examples/peripherals/can/can_self_test/example_test.py +++ b/examples/peripherals/can/can_self_test/example_test.py @@ -16,10 +16,11 @@ import IDF STR_EXPECT = ("CAN Self Test: Driver installed", "CAN Self Test: Driver uninstalled") EXPECT_TIMEOUT = 20 -@IDF.idf_example_test(env_tag='Example_CAN') + +@IDF.idf_example_test(env_tag='Example_CAN1') def test_can_self_test_example(env, extra_data): - #Get device under test, flash and start example. "dut4" must be defined in EnvConfig - dut = env.get_dut('dut4', 'examples/peripherals/can/can_self_test') + # Get device under test, flash and start example. "dut4" must be defined in EnvConfig + dut = env.get_dut('dut1', 'examples/peripherals/can/can_self_test') dut.start_app() for string in STR_EXPECT: From 2b5f488d88a56b319b57ef1e7295dfaa2e36b368 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Fri, 25 Oct 2019 10:27:16 +1100 Subject: [PATCH 3/3] ci: Temporarily ignore CAN example test failures Due to internal CI issue. --- examples/peripherals/can/can_alert_and_recovery/example_test.py | 2 +- examples/peripherals/can/can_network/example_test.py | 2 +- examples/peripherals/can/can_self_test/example_test.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/peripherals/can/can_alert_and_recovery/example_test.py b/examples/peripherals/can/can_alert_and_recovery/example_test.py index fe6b792a5..dfd693cf6 100644 --- a/examples/peripherals/can/can_alert_and_recovery/example_test.py +++ b/examples/peripherals/can/can_alert_and_recovery/example_test.py @@ -17,7 +17,7 @@ STR_EXPECT = ("CAN Alert and Recovery: Driver installed", "CAN Alert and Recover EXPECT_TIMEOUT = 20 -@IDF.idf_example_test(env_tag='Example_CAN1') +@IDF.idf_example_test(env_tag='Example_CAN1', ignore=True) def test_can_alert_and_recovery_example(env, extra_data): # Get device under test, flash and start example. "dut4" must be defined in EnvConfig dut = env.get_dut('dut1', 'examples/peripherals/can/can_alert_and_recovery') diff --git a/examples/peripherals/can/can_network/example_test.py b/examples/peripherals/can/can_network/example_test.py index e56c37e78..41ebae096 100644 --- a/examples/peripherals/can/can_network/example_test.py +++ b/examples/peripherals/can/can_network/example_test.py @@ -34,7 +34,7 @@ def dut_thread_callback(**kwargs): #Mark thread has run to completion without any exceptions result[0] = True -@IDF.idf_example_test(env_tag='Example_CAN2') +@IDF.idf_example_test(env_tag='Example_CAN2', ignore=True) def test_can_network_example(env, extra_data): #Get device under test. "dut1", "dut2", and "dut3" must be properly defined in EnvConfig diff --git a/examples/peripherals/can/can_self_test/example_test.py b/examples/peripherals/can/can_self_test/example_test.py index 28ade8515..13f1cb004 100644 --- a/examples/peripherals/can/can_self_test/example_test.py +++ b/examples/peripherals/can/can_self_test/example_test.py @@ -17,7 +17,7 @@ STR_EXPECT = ("CAN Self Test: Driver installed", "CAN Self Test: Driver uninstal EXPECT_TIMEOUT = 20 -@IDF.idf_example_test(env_tag='Example_CAN1') +@IDF.idf_example_test(env_tag='Example_CAN1', ignore=True) def test_can_self_test_example(env, extra_data): # Get device under test, flash and start example. "dut4" must be defined in EnvConfig dut = env.get_dut('dut1', 'examples/peripherals/can/can_self_test')