From 2406c0dd5fda6f9406891dbc6eab70816d3e8b03 Mon Sep 17 00:00:00 2001 From: LeonardoBizzoni Date: Thu, 12 Mar 2026 20:24:43 +0100 Subject: [PATCH] explicit enum variant values + comments --- .../Core/Inc/firmware/fmw_core.h | 14 ++- .../Core/Inc/firmware/fmw_messages.h | 90 +++++++++++-------- pioneer_controller/Core/Src/main.c | 43 ++++++--- 3 files changed, 90 insertions(+), 57 deletions(-) diff --git a/pioneer_controller/Core/Inc/firmware/fmw_core.h b/pioneer_controller/Core/Inc/firmware/fmw_core.h index 16baa05..87a1fc8 100644 --- a/pioneer_controller/Core/Inc/firmware/fmw_core.h +++ b/pioneer_controller/Core/Inc/firmware/fmw_core.h @@ -3,10 +3,10 @@ typedef uint8_t FMW_Mode; enum { - FMW_Mode_None, - FMW_Mode_Config, - FMW_Mode_Run, - FMW_Mode_COUNT, + FMW_Mode_None = 0, + FMW_Mode_Config = 1, + FMW_Mode_Run = 2, + FMW_Mode_COUNT = 3, }; typedef struct FMW_Encoder { @@ -119,6 +119,12 @@ FMW_Message fmw_message_from_uart_error(const UART_HandleTypeDef *huart); #define FMW_METERS_FROM_TICKS(Ticks, WheelCircumference, TicksPerRevolution) \ ((Ticks * WheelCircumference) / TicksPerRevolution) +// NOTE(lb): The variadic arguments are the fields of FMW_Hook, so you can just type +// `.callback = your_function_pointer, .args = your_pointer_to_args`. +// Calling this macro with the condition argument is GCC extension. +// I don't use `assert` because i never figured out how to make it trigger +// a debug breakpoint inside the STM32 ide debugger (and also because having a +// on-failure callback is handy). #define FMW_ASSERT(Cond, ...) \ do { \ FMW_Hook hook = { __VA_ARGS__ }; \ diff --git a/pioneer_controller/Core/Inc/firmware/fmw_messages.h b/pioneer_controller/Core/Inc/firmware/fmw_messages.h index 2a833d8..295ec40 100644 --- a/pioneer_controller/Core/Inc/firmware/fmw_messages.h +++ b/pioneer_controller/Core/Inc/firmware/fmw_messages.h @@ -10,54 +10,66 @@ typedef union { float values[3]; } FMW_PidConstants; -#define FMW_MESSAGE_TYPE_VARIANTS(X) \ - X(FMW_MessageType_None) \ - X(FMW_MessageType_Response) \ - X(FMW_MessageType_ModeChange_Config) \ - X(FMW_MessageType_ModeChange_Run) \ - X(FMW_MessageType_Config_Robot) \ - X(FMW_MessageType_Config_PID) \ - X(FMW_MessageType_Config_LED) \ - X(FMW_MessageType_Run_GetStatus) \ - X(FMW_MessageType_Run_SetVelocity) \ - X(FMW_MessageType_COUNT) +// NOTE(lb): The expansion of this macro relies on another macro called "X" (hence the name X-macro). +// X isn't define yet but, for it to work with this macro expansion, it must be a function-like macro +// with exactly 2 arguemnts: 1) the enumation variant name. 2) the corresponding numerical id. +#define FMW_MESSAGE_TYPE_VARIANTS() \ + X(FMW_MessageType_None, 0) \ + X(FMW_MessageType_Response, 1) \ + X(FMW_MessageType_ModeChange_Config, 2) \ + X(FMW_MessageType_ModeChange_Run, 3) \ + X(FMW_MessageType_Config_Robot, 4) \ + X(FMW_MessageType_Config_PID, 5) \ + X(FMW_MessageType_Config_LED, 6) \ + X(FMW_MessageType_Run_GetStatus, 7) \ + X(FMW_MessageType_Run_SetVelocity, 8) \ + X(FMW_MessageType_COUNT, 9) +// NOTE(lb): Here i want to take all of the variants that are generated from the macro +// expension of FMW_MESSAGE_TYPE_VARIANTS and use them to populate the enum definition. +// For it to be a valid enum definition you need to take every `X(VariantName, Id)` +// and turn them into `VariantName = Id,`. +// It's important to undefine `X` after use to avoid redefinition and, more importantly, +// to use it in another X-macro expansion by accident. +// This seems pointless but when paired with `#VariantName` you can automatically +// turn all of the symbolic variant names into strings and now printing the variant +// can be done via an array lookup `[VariantName] = #VariantName` instead of a runtime check. typedef uint8_t FMW_MessageType; enum { -#define X(Variant) Variant, - FMW_MESSAGE_TYPE_VARIANTS(X) +#define X(Variant, Id) Variant = Id, + FMW_MESSAGE_TYPE_VARIANTS() #undef X }; -#define FMW_RESULT_VARIANTS(X) \ - X(FMW_Result_Ok) \ - X(FMW_Result_Error_InvalidArguments) \ - X(FMW_Result_Error_FaultPinTriggered) \ - X(FMW_Result_Error_UART_Crc) \ - X(FMW_Result_Error_UART_NegativeTimeout) \ - X(FMW_Result_Error_UART_ReceiveTimeoutElapsed) \ - X(FMW_Result_Error_UART_Parity) \ - X(FMW_Result_Error_UART_Frame) \ - X(FMW_Result_Error_UART_Noise) \ - X(FMW_Result_Error_UART_Overrun) \ - X(FMW_Result_Error_Encoder_InvalidTimer) \ - X(FMW_Result_Error_Encoder_NonPositiveTicksPerRevolution) \ - X(FMW_Result_Error_Encoder_NonPositiveWheelCircumference) \ - X(FMW_Result_Error_Encoder_GetTick) \ - X(FMW_Result_Error_Buzzer_Timer) \ - X(FMW_Result_Error_MessageHandler_InvalidState) \ - X(FMW_Result_Error_MessageHandler_Config_NonPositiveBaseline) \ - X(FMW_Result_Error_MessageHandler_Config_NonPositiveWheelCircumference) \ - X(FMW_Result_Error_MessageHandler_Config_NonPositiveTicksPerRevolution) \ - X(FMW_Result_Error_MessageHandler_Config_NonPositiveLEDUpdatePeriod) \ - X(FMW_Result_Error_Command_NotRecognized) \ - X(FMW_Result_Error_Command_NotAvailable) \ - X(FMW_Result_COUNT) +#define FMW_RESULT_VARIANTS() \ + X(FMW_Result_Ok, 0) \ + X(FMW_Result_Error_InvalidArguments, 1) \ + X(FMW_Result_Error_FaultPinTriggered, 2) \ + X(FMW_Result_Error_UART_Crc, 3) \ + X(FMW_Result_Error_UART_NegativeTimeout, 4) \ + X(FMW_Result_Error_UART_ReceiveTimeoutElapsed, 5) \ + X(FMW_Result_Error_UART_Parity, 6) \ + X(FMW_Result_Error_UART_Frame, 7) \ + X(FMW_Result_Error_UART_Noise, 8) \ + X(FMW_Result_Error_UART_Overrun, 9) \ + X(FMW_Result_Error_Encoder_InvalidTimer, 10) \ + X(FMW_Result_Error_Encoder_NonPositiveTicksPerRevolution, 11) \ + X(FMW_Result_Error_Encoder_NonPositiveWheelCircumference, 12) \ + X(FMW_Result_Error_Encoder_GetTick, 13) \ + X(FMW_Result_Error_Buzzer_Timer, 14) \ + X(FMW_Result_Error_MessageHandler_InvalidState, 15) \ + X(FMW_Result_Error_MessageHandler_Config_NonPositiveBaseline, 16) \ + X(FMW_Result_Error_MessageHandler_Config_NonPositiveWheelCircumference, 17) \ + X(FMW_Result_Error_MessageHandler_Config_NonPositiveTicksPerRevolution, 18) \ + X(FMW_Result_Error_MessageHandler_Config_NonPositiveLEDUpdatePeriod, 19) \ + X(FMW_Result_Error_Command_NotRecognized, 20) \ + X(FMW_Result_Error_Command_NotAvailable, 21) \ + X(FMW_Result_COUNT, 22) typedef uint8_t FMW_Result; enum { -#define X(Variant) Variant, - FMW_RESULT_VARIANTS(X) +#define X(Variant, Id) Variant = Id, + FMW_RESULT_VARIANTS() #undef X }; diff --git a/pioneer_controller/Core/Src/main.c b/pioneer_controller/Core/Src/main.c index b901b77..45104e1 100644 --- a/pioneer_controller/Core/Src/main.c +++ b/pioneer_controller/Core/Src/main.c @@ -38,7 +38,7 @@ FMW_Result message_handler(FMW_Message *msg, CRC_HandleTypeDef *hcrc) /* Private define ------------------------------------------------------------*/ /* USER CODE BEGIN PD */ -#define UART_MESSANGER_HANDLE (&huart3) +#define UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL (&huart3) /* USER CODE END PD */ /* Private macro -------------------------------------------------------------*/ @@ -797,7 +797,7 @@ static void MX_GPIO_Init(void) /* USER CODE BEGIN 4 */ void start(void) { // Enables UART RX interrupt - HAL_UART_Receive_DMA(UART_MESSANGER_HANDLE, (uint8_t*)&uart_message_buffer, sizeof uart_message_buffer); + HAL_UART_Receive_DMA(UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL, (uint8_t*)&uart_message_buffer, sizeof uart_message_buffer); for (;;) { switch (current_mode) { @@ -904,6 +904,9 @@ FMW_Result message_handler(FMW_Message *msg, CRC_HandleTypeDef *hcrc) { pid_cross.setpoint = odometry.setpoint_left - odometry.setpoint_right; } // fallthrough case FMW_MessageType_Run_GetStatus: { + // NOTE(lb): SetVelocity continues here as well because the previous case doesn't end with a `break`. + // order matters. + int32_t current_ticks_left = ticks_left + fmw_encoder_count_get(&encoders.left); int32_t current_ticks_right = ticks_right + fmw_encoder_count_get(&encoders.right); ticks_left = ticks_right = 0; @@ -920,8 +923,10 @@ FMW_Result message_handler(FMW_Message *msg, CRC_HandleTypeDef *hcrc) { msg.response.ticks_left = current_ticks_left; msg.response.ticks_right = current_ticks_right; msg.header.crc = HAL_CRC_Calculate(hcrc, (uint32_t*)&msg, sizeof msg); - (void)HAL_UART_Transmit(UART_MESSANGER_HANDLE, (uint8_t*)&msg, sizeof msg, HAL_MAX_DELAY); + (void)HAL_UART_Transmit(UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL, (uint8_t*)&msg, sizeof msg, HAL_MAX_DELAY); return FMW_Result_Ok; + // NOTE(lb): GetStatus&SetVelocity have to respond with the same special message format. + // so they don't continue down to `msg_contains_error`. } break; case FMW_MessageType_ModeChange_Config: { fmw_motors_stop(motors.values, ARRLENGTH(motors.values)); @@ -952,11 +957,13 @@ FMW_Result message_handler(FMW_Message *msg, CRC_HandleTypeDef *hcrc) { } break; } + // NOTE(lb): control flow naturally converges here. + // the symbol is used to jump here directly in case of error. msg_contains_error:; FMW_Message response = {0}; response.header.type = FMW_MessageType_Response; response.response.result = result; - HAL_StatusTypeDef send_res = fmw_message_uart_send(UART_MESSANGER_HANDLE, hcrc, &response, 1); + HAL_StatusTypeDef send_res = fmw_message_uart_send(UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL, hcrc, &response, 1); FMW_ASSERT(send_res == HAL_OK); return result; } @@ -988,27 +995,35 @@ void HAL_TIM_PeriodElapsedCallback(TIM_HandleTypeDef *htim) { } void HAL_UART_RxCpltCallback(UART_HandleTypeDef *huart) { - if (huart != UART_MESSANGER_HANDLE) { return; } + if (huart != UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL) { return; } (void)message_handler((FMW_Message*)&uart_message_buffer, &hcrc); - // NOTE(lb): listen for the next message. - HAL_UART_Receive_DMA(UART_MESSANGER_HANDLE, (uint8_t*)&uart_message_buffer, sizeof uart_message_buffer); + + // NOTE(lb): listen for the next message "recursively". + HAL_UART_Receive_DMA(UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL, (uint8_t*)&uart_message_buffer, sizeof uart_message_buffer); } void HAL_UART_ErrorCallback(UART_HandleTypeDef *huart) { - if (huart != UART_MESSANGER_HANDLE) { return; } + if (huart != UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL) { return; } + // NOTE(lb): i don't know how to determine if the error that cause the jump here + // was during a receive or a send of a message over UART, so i'm just + // going to stop the motors and abort the receive just in case. fmw_motors_stop(motors.values, ARRLENGTH(motors.values)); - FMW_Message response = fmw_message_from_uart_error(huart); + HAL_UART_AbortReceive(huart); + FMW_Message response = fmw_message_from_uart_error(huart); retry:; - HAL_StatusTypeDef res = fmw_message_uart_send(UART_MESSANGER_HANDLE, &hcrc, &response, HAL_MAX_DELAY); - if (res != HAL_OK) { // NOTE(lb): send help + HAL_StatusTypeDef res = fmw_message_uart_send(UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL, &hcrc, &response, HAL_MAX_DELAY); + if (res != HAL_OK) { + // NOTE(lb): keep trying to send the error message even on failure until it succeeds + // while also being extra annoying. fmw_buzzers_set(&buzzer, 1, true); goto retry; } + fmw_buzzers_set(&buzzer, 1, false); - HAL_UART_AbortReceive(huart); - HAL_UART_Receive_DMA(UART_MESSANGER_HANDLE, (uint8_t*)&uart_message_buffer, sizeof uart_message_buffer); + // NOTE(lb): go back to normal message receive after the error has been notified + HAL_UART_Receive_DMA(UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL, (uint8_t*)&uart_message_buffer, sizeof uart_message_buffer); } void HAL_GPIO_EXTI_Callback(uint16_t GPIO_Pin) { @@ -1039,7 +1054,7 @@ void HAL_GPIO_EXTI_Callback(uint16_t GPIO_Pin) { response.response.result = FMW_Result_Error_FaultPinTriggered; retry:; - HAL_StatusTypeDef res = fmw_message_uart_send(UART_MESSANGER_HANDLE, &hcrc, &response, HAL_MAX_DELAY); + HAL_StatusTypeDef res = fmw_message_uart_send(UART_HANDLE_PTR_USED_FOR_MESSAGE_EXCHANGE_PROTOCOL, &hcrc, &response, HAL_MAX_DELAY); if (res != HAL_OK) { // NOTE(lb): send help fmw_buzzers_set(&buzzer, 1, true); goto retry; -- 2.52.0