From 853ef585fda047a1d122284475e86d787bec3eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Boris=20Labb=C3=A9?= Date: Thu, 19 Sep 2024 15:27:57 +0200 Subject: [PATCH 1/2] GH-47: Add fail safe to tx queue This should prevent a faulty frame to block all the tx queue by itself. Forwarded: https://github.com/SiliconLabs/UnifySDK/pull/47 Bug-SiliconLabs: UIC-3335 Bug-Github: https://github.com/SiliconLabs/UnifySDK/pull/47 --- .../zwave/zwave_tx/src/zwave_tx.cpp | 10 +++ .../zwave/zwave_tx/test/zwave_tx_test.c | 85 +++++++++++++++++-- 2 files changed, 89 insertions(+), 6 deletions(-) diff --git a/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp b/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp index 4c5fafffb4..99c48faed2 100644 --- a/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp +++ b/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp @@ -141,6 +141,16 @@ sl_status_t sl_log_error(LOG_TAG, "Tx Queue rejected new frame request."); tx_queue.log(true); + // Add a failsafe in case the queue is getting blocked + // This prevent one faulty device to block the whole queue + // The subsequent frames will most likely discarded since they timed out by now + if (tx_queue.size() == ZWAVE_TX_QUEUE_BUFFER_SIZE) { + sl_log_info( + LOG_TAG, + "Abort transmission of first frame in queue due to full Tx Queue."); + zwave_tx_process_abort_transmission( + tx_queue.first_in_queue()->zwave_tx_session_id); + } return SL_STATUS_FAIL; } diff --git a/applications/zpc/components/zwave/zwave_tx/test/zwave_tx_test.c b/applications/zpc/components/zwave/zwave_tx/test/zwave_tx_test.c index b64b6ac8a0..02397a6539 100644 --- a/applications/zpc/components/zwave/zwave_tx/test/zwave_tx_test.c +++ b/applications/zpc/components/zwave/zwave_tx/test/zwave_tx_test.c @@ -2064,6 +2064,8 @@ void test_full_zwave_tx_queue() &test_tx_session_id_2)); contiki_test_helper_run(0); + + for (int i = 1; i < ZWAVE_TX_QUEUE_BUFFER_SIZE; ++i) { TEST_ASSERT_EQUAL(SL_STATUS_OK, zwave_tx_send_data(&test_connection_1, @@ -2074,9 +2076,14 @@ void test_full_zwave_tx_queue() (void *)&my_user_pointer, &test_tx_session_id)); } + zwave_tx_session_id_t second_message_id = test_tx_session_id; TEST_ASSERT_EQUAL(ZWAVE_TX_QUEUE_BUFFER_SIZE, zwave_tx_get_queue_size()); + zwave_controller_transport_abort_send_data_ExpectAndReturn( + test_tx_session_id_2, + SL_STATUS_FAIL); + // Now there is no more queue space: TEST_ASSERT_EQUAL(SL_STATUS_FAIL, zwave_tx_send_data(&test_connection_1, @@ -2087,14 +2094,11 @@ void test_full_zwave_tx_queue() (void *)&my_user_pointer, &test_tx_session_id)); - // Get one free slot - on_zwave_transport_send_data_complete(TRANSMIT_COMPLETE_OK, - NULL, - test_tx_session_id_2); - + // Process events contiki_test_helper_run(0); TEST_ASSERT_EQUAL(1, send_done_count); - TEST_ASSERT_EQUAL(TRANSMIT_COMPLETE_OK, send_done_status); + TEST_ASSERT_EQUAL(TRANSMIT_COMPLETE_FAIL, send_done_status); + // Now queueing should work again: TEST_ASSERT_EQUAL(SL_STATUS_OK, @@ -2106,6 +2110,9 @@ void test_full_zwave_tx_queue() (void *)&my_user_pointer, &test_tx_session_id)); + zwave_controller_transport_abort_send_data_ExpectAndReturn( + second_message_id, + SL_STATUS_FAIL); // And now we are full again: TEST_ASSERT_EQUAL(SL_STATUS_FAIL, zwave_tx_send_data(&test_connection_1, @@ -2117,6 +2124,72 @@ void test_full_zwave_tx_queue() &test_tx_session_id)); } +void test_full_zwave_tx_queue_with_timeouts() +{ + // We do not care about interactions with the transports for this test. + zwave_controller_transport_send_data_IgnoreAndReturn(SL_STATUS_OK); + + // Queue a first element: + TEST_ASSERT_EQUAL(SL_STATUS_OK, + zwave_tx_send_data(&test_connection_2, + sizeof(test_expected_frame_data_2), + test_expected_frame_data_2, + &test_tx_options_2, + send_data_callback, + (void *)&my_user_pointer, + &test_tx_session_id_2)); + contiki_test_helper_run(0); + + + + for (int i = 1; i < ZWAVE_TX_QUEUE_BUFFER_SIZE; ++i) { + TEST_ASSERT_EQUAL(SL_STATUS_OK, + zwave_tx_send_data(&test_connection_2, + sizeof(test_expected_frame_data_2), + test_expected_frame_data_2, + &test_tx_options_2, + send_data_callback, + (void *)&my_user_pointer, + &test_tx_session_id)); + } + //zwave_tx_session_id_t second_message_id = test_tx_session_id; + + TEST_ASSERT_EQUAL(ZWAVE_TX_QUEUE_BUFFER_SIZE, zwave_tx_get_queue_size()); + + zwave_controller_transport_abort_send_data_ExpectAndReturn( + test_tx_session_id_2, + SL_STATUS_FAIL); + + // Now there is no more queue space: + TEST_ASSERT_EQUAL(SL_STATUS_FAIL, + zwave_tx_send_data(&test_connection_1, + sizeof(test_expected_frame_data_1), + test_expected_frame_data_1, + &test_tx_options_1, + send_data_callback, + (void *)&my_user_pointer, + &test_tx_session_id)); + + // Process events + contiki_test_helper_run(1000); + TEST_ASSERT_EQUAL(ZWAVE_TX_QUEUE_BUFFER_SIZE, send_done_count); + TEST_ASSERT_EQUAL(TRANSMIT_COMPLETE_FAIL, send_done_status); + + + // Now queueing should work again: + for (int i = 1; i < ZWAVE_TX_QUEUE_BUFFER_SIZE; ++i) { + TEST_ASSERT_EQUAL(SL_STATUS_OK, + zwave_tx_send_data(&test_connection_2, + sizeof(test_expected_frame_data_2), + test_expected_frame_data_2, + &test_tx_options_2, + send_data_callback, + (void *)&my_user_pointer, + &test_tx_session_id)); + } +} + + void test_additional_back_off() { zwave_node_id_t chatty_node_id = 23; From 9745901697999e5c852f30b92440c6b58d405357 Mon Sep 17 00:00:00 2001 From: silabs-borisl <144136056+silabs-borisl@users.noreply.github.com> Date: Thu, 19 Sep 2024 16:46:45 +0200 Subject: [PATCH 2/2] Update applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp Co-authored-by: Phil Coval --- applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp b/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp index 99c48faed2..a72afbba2f 100644 --- a/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp +++ b/applications/zpc/components/zwave/zwave_tx/src/zwave_tx.cpp @@ -144,7 +144,7 @@ sl_status_t // Add a failsafe in case the queue is getting blocked // This prevent one faulty device to block the whole queue // The subsequent frames will most likely discarded since they timed out by now - if (tx_queue.size() == ZWAVE_TX_QUEUE_BUFFER_SIZE) { + if (tx_queue.size() >= ZWAVE_TX_QUEUE_BUFFER_SIZE) { sl_log_info( LOG_TAG, "Abort transmission of first frame in queue due to full Tx Queue.");