diff --git a/libraries/ms-common/src/x86/can_hw.c b/libraries/ms-common/src/x86/can_hw.c index 0be2e2228..91babc01c 100644 --- a/libraries/ms-common/src/x86/can_hw.c +++ b/libraries/ms-common/src/x86/can_hw.c @@ -27,8 +27,8 @@ #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 100ms +#define CAN_HW_THREAD_EXIT_PERIOD_US 100000 typedef struct CanHwEventHandler { CanHwEventHandlerCb callback; @@ -91,6 +91,8 @@ static void *prv_rx_thread(void *arg) { select(s_socket_data.can_fd + 1, &input_fds, NULL, NULL, &timeout); + timeout.tv_usec = CAN_HW_THREAD_EXIT_PERIOD_US; + 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)); @@ -100,9 +102,8 @@ 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 - // Wakes the main thread - x86_interrupt_wake(); // Limit how often we can receive messages to simulate bus speed usleep(s_socket_data.delay_us); } @@ -124,7 +125,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)); @@ -157,6 +158,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); } @@ -299,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/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); } 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); }