From 0ddfc127499513f0ef726f6c4c1dfdbae0a742b1 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 17 Feb 2021 06:00:52 +0000 Subject: [PATCH 1/3] increased can select timeout --- libraries/ms-common/src/x86/can_hw.c | 14 +++++++------- libraries/ms-common/src/x86/wait.c | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libraries/ms-common/src/x86/can_hw.c b/libraries/ms-common/src/x86/can_hw.c index c5b47bed8..1ff1267ce 100644 --- a/libraries/ms-common/src/x86/can_hw.c +++ b/libraries/ms-common/src/x86/can_hw.c @@ -22,8 +22,8 @@ #define CAN_HW_DEV_INTERFACE "vcan0" #define CAN_HW_MAX_FILTERS 14 #define CAN_HW_TX_FIFO_LEN 8 -// Check for thread exit once every 10ms -#define CAN_HW_THREAD_EXIT_PERIOD_US 10000 +// Check for thread exit once every 1s +#define CAN_HW_THREAD_EXIT_PERIOD_S 1 typedef struct CanHwEventHandler { CanHwEventHandlerCb callback; @@ -69,7 +69,7 @@ static void *prv_rx_thread(void *arg) { pthread_barrier_wait(&s_barrier); - struct timeval timeout = { .tv_usec = CAN_HW_THREAD_EXIT_PERIOD_US }; + struct timeval timeout = { .tv_sec = CAN_HW_THREAD_EXIT_PERIOD_S }; // Mutex is unlocked when the thread should exit while (pthread_mutex_trylock(&s_keep_alive) != 0) { @@ -80,6 +80,8 @@ static void *prv_rx_thread(void *arg) { select(s_socket_data.can_fd + 1, &input_fds, NULL, NULL, &timeout); + timeout.tv_sec = CAN_HW_THREAD_EXIT_PERIOD_S; + if (FD_ISSET(s_socket_data.can_fd, &input_fds)) { int bytes = read(s_socket_data.can_fd, &s_socket_data.rx_frame, sizeof(s_socket_data.rx_frame)); @@ -89,8 +91,6 @@ static void *prv_rx_thread(void *arg) { s_socket_data.handlers[CAN_HW_EVENT_TX_READY].context); } - // Wakes the main thread - x86_interrupt_wake(); // Limit how often we can receive messages to simulate bus speed usleep(s_socket_data.delay_us); } @@ -112,7 +112,7 @@ static void *prv_tx_thread(void *arg) { while (pthread_mutex_trylock(&s_keep_alive) != 0) { // Wait until the producer has created an item sem_wait(&s_tx_sem); - x86_interrupt_wake(); + fifo_pop(&s_socket_data.tx_fifo, &frame); int bytes = write(s_socket_data.can_fd, &frame, sizeof(frame)); @@ -145,6 +145,7 @@ StatusCode can_hw_init(const CanHwSettings *settings) { sem_post(&s_tx_sem); sem_destroy(&s_tx_sem); + pthread_join(s_tx_pthread_id, NULL); } @@ -255,7 +256,6 @@ StatusCode can_hw_transmit(uint32_t id, bool extended, const uint8_t *data, size } // Unblock TX thread sem_post(&s_tx_sem); - return STATUS_CODE_OK; } diff --git a/libraries/ms-common/src/x86/wait.c b/libraries/ms-common/src/x86/wait.c index 0755c93fb..c45cbecf8 100644 --- a/libraries/ms-common/src/x86/wait.c +++ b/libraries/ms-common/src/x86/wait.c @@ -3,8 +3,8 @@ #include void wait(void) { - sigset_t s_wait_sigset; + sigset_t wait_sigset; - sigemptyset(&s_wait_sigset); - sigsuspend(&s_wait_sigset); + sigemptyset(&wait_sigset); + sigsuspend(&wait_sigset); } From a065f7838af58214975f1e35164e9ce7c3ea1a73 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 18 Feb 2021 00:19:53 +0000 Subject: [PATCH 2/3] changed time to 100ms --- libraries/ms-common/src/x86/can_hw.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/ms-common/src/x86/can_hw.c b/libraries/ms-common/src/x86/can_hw.c index 1ff1267ce..aca2382c7 100644 --- a/libraries/ms-common/src/x86/can_hw.c +++ b/libraries/ms-common/src/x86/can_hw.c @@ -22,8 +22,8 @@ #define CAN_HW_DEV_INTERFACE "vcan0" #define CAN_HW_MAX_FILTERS 14 #define CAN_HW_TX_FIFO_LEN 8 -// Check for thread exit once every 1s -#define CAN_HW_THREAD_EXIT_PERIOD_S 1 +// Check for thread exit once every 100ms +#define CAN_HW_THREAD_EXIT_PERIOD_US 100000 typedef struct CanHwEventHandler { CanHwEventHandlerCb callback; @@ -69,7 +69,7 @@ static void *prv_rx_thread(void *arg) { pthread_barrier_wait(&s_barrier); - struct timeval timeout = { .tv_sec = CAN_HW_THREAD_EXIT_PERIOD_S }; + struct timeval timeout = { .tv_usec = CAN_HW_THREAD_EXIT_PERIOD_US }; // Mutex is unlocked when the thread should exit while (pthread_mutex_trylock(&s_keep_alive) != 0) { @@ -80,7 +80,7 @@ static void *prv_rx_thread(void *arg) { select(s_socket_data.can_fd + 1, &input_fds, NULL, NULL, &timeout); - timeout.tv_sec = CAN_HW_THREAD_EXIT_PERIOD_S; + timeout.tv_usec = CAN_HW_THREAD_EXIT_PERIOD_US; if (FD_ISSET(s_socket_data.can_fd, &input_fds)) { int bytes = From 6d87cd96027c9cb255f9e4605e1228fab99f6764 Mon Sep 17 00:00:00 2001 From: Ryan Dancy Date: Sun, 4 Apr 2021 10:51:03 -0400 Subject: [PATCH 3/3] Rewrite test_can_wake_works to avoid infinite looping There was a very odd race condition in test_can_wake_works where very occasionally it would loop forever. Not sure why, but it also wasn't testing what it was supposed to particularly effectively, so I rewrote it. Also added a couple of x86_interrupt_wake() calls in can_hw to be safe. --- libraries/ms-common/src/x86/can_hw.c | 2 ++ libraries/ms-common/test/test_wait.c | 17 ++++++++--------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/libraries/ms-common/src/x86/can_hw.c b/libraries/ms-common/src/x86/can_hw.c index 32b4e1b78..91babc01c 100644 --- a/libraries/ms-common/src/x86/can_hw.c +++ b/libraries/ms-common/src/x86/can_hw.c @@ -102,6 +102,7 @@ static void *prv_rx_thread(void *arg) { s_socket_data.handlers[CAN_HW_EVENT_MSG_RX].callback( s_socket_data.handlers[CAN_HW_EVENT_TX_READY].context); } + x86_interrupt_wake(); // just to make sure wait() wakes up to process the message // Limit how often we can receive messages to simulate bus speed usleep(s_socket_data.delay_us); @@ -300,6 +301,7 @@ StatusCode can_hw_transmit(uint32_t id, bool extended, const uint8_t *data, size if (filter_match && s_socket_data.handlers[CAN_HW_EVENT_MSG_RX].callback != NULL) { s_socket_data.handlers[CAN_HW_EVENT_MSG_RX].callback( s_socket_data.handlers[CAN_HW_EVENT_TX_READY].context); + x86_interrupt_wake(); } } diff --git a/libraries/ms-common/test/test_wait.c b/libraries/ms-common/test/test_wait.c index 74a6ce74b..2174811f9 100644 --- a/libraries/ms-common/test/test_wait.c +++ b/libraries/ms-common/test/test_wait.c @@ -208,6 +208,11 @@ void test_wait_works_raw_x86(void) { TEST_ASSERT_EQUAL(EXPECTED_TIMES_x86_CALLBACK_CALLED, s_num_times_x86_callback_called); } +// Test that wait() wakes when we receive a CAN message at a time such that we can to process it. +// This isn't perfect because x86 can_hw bypasses vcan in loopback mode, so it's not representative +// of receiving a real external CAN message, but it's the best we can do from a unit test. +// For this reason we don't count the number of wait cycles but just make sure that we wake up when +// the RX event has been raised. void test_can_wake_works(void) { uint8_t num_wait_cycles_timer = 0; @@ -217,17 +222,11 @@ void test_can_wake_works(void) { pthread_create(&can_send_thread, NULL, prv_can_tx, NULL); Event e = { 0 }; - while (!s_can_received) { + while (!status_ok(event_process(&e))) { wait(); - while (event_process(&e) != STATUS_CODE_OK) { - } - can_process_event(&e); - - num_wait_cycles_timer++; } + can_process_event(&e); + TEST_ASSERT_TRUE(s_can_received); pthread_join(can_send_thread, NULL); - - // we should only wait once - TEST_ASSERT_EQUAL(EXPECTED_x86_INTERRUPT_CYCLES, num_wait_cycles_timer); }