From 1f732bb0fbb3753d30200247b4ede4876104e5eb Mon Sep 17 00:00:00 2001 From: Samuel Sadok Date: Fri, 9 Mar 2018 20:23:03 -0800 Subject: [PATCH] fix stack overflow --- Firmware/Board/v3.3/Inc/FreeRTOSConfig.h | 2 ++ Firmware/MotorControl/communication.cpp | 28 +++++++++++------------- Firmware/MotorControl/encoder.hpp | 19 ++++++++-------- Firmware/MotorControl/main.cpp | 1 + Firmware/MotorControl/motor.hpp | 3 ++- Firmware/MotorControl/protocol.hpp | 16 ++++++-------- 6 files changed, 35 insertions(+), 34 deletions(-) diff --git a/Firmware/Board/v3.3/Inc/FreeRTOSConfig.h b/Firmware/Board/v3.3/Inc/FreeRTOSConfig.h index 3a55d727..2459c3e9 100644 --- a/Firmware/Board/v3.3/Inc/FreeRTOSConfig.h +++ b/Firmware/Board/v3.3/Inc/FreeRTOSConfig.h @@ -108,6 +108,7 @@ #define configUSE_MUTEXES 1 #define configQUEUE_REGISTRY_SIZE 8 #define configUSE_PORT_OPTIMISED_TASK_SELECTION 1 +#define configCHECK_FOR_STACK_OVERFLOW 1 /* Co-routine definitions. */ #define configUSE_CO_ROUTINES 0 @@ -123,6 +124,7 @@ to exclude the API function. */ #define INCLUDE_vTaskDelayUntil 1 #define INCLUDE_vTaskDelay 1 #define INCLUDE_xTaskGetSchedulerState 1 +#define INCLUDE_uxTaskGetStackHighWaterMark 1 /* Cortex-M specific definitions. */ #ifdef __NVIC_PRIO_BITS diff --git a/Firmware/MotorControl/communication.cpp b/Firmware/MotorControl/communication.cpp index 0e243911..4129e6bb 100644 --- a/Firmware/MotorControl/communication.cpp +++ b/Firmware/MotorControl/communication.cpp @@ -129,19 +129,6 @@ StreamToPacketConverter UART4_stream_sink(uart4_channel); #endif -class test_class { -public: - uint32_t property1; - float property2; - - float set_both(uint32_t arg1, float arg2) { - printf("set_both called with %u and %.3f\n", (unsigned int)arg1, arg2); - property1 = arg1; - property2 = arg2; - return arg1 + arg2; - } -}; - /* Private function prototypes -----------------------------------------------*/ /* Function implementations --------------------------------------------------*/ @@ -149,7 +136,7 @@ void init_communication(void) { printf("hi!\r\n"); // Start command handling thread - osThreadDef(task_cmd_parse, communication_task, osPriorityNormal, 0, 4*512); + osThreadDef(task_cmd_parse, communication_task, osPriorityNormal, 0, 5000 /* in 32-bit words */); // TODO: fix stack issues thread_cmd_parse = osThreadCreate(osThread(task_cmd_parse), NULL); // Start USB interrupt handler thread @@ -157,6 +144,9 @@ void init_communication(void) { thread_usb_pump = osThreadCreate(osThread(task_usb_pump), NULL); } + +uint32_t comm_stack_info = 0; // for debugging only + // Helper class because the protocol library doesn't yet // support non-member functions // TODO: make this go away @@ -167,9 +157,13 @@ public: void NVIC_SystemReset_helper() { NVIC_SystemReset(); } } static_functions; -static auto make_obj_tree() { +// When adding new functions/variables to the protocol, be careful not to +// blow the communication stack. You can check comm_stack_info to see +// how much headroom you have. +static inline auto make_obj_tree() { return make_protocol_member_list( make_protocol_ro_property("vbus_voltage", &vbus_voltage), + make_protocol_ro_property("comm_stack_info", &comm_stack_info), make_protocol_ro_property("UUID_0", (const uint32_t*)(ID_UNIQUE_ADDRESS + 0*4)), make_protocol_ro_property("UUID_1", (const uint32_t*)(ID_UNIQUE_ADDRESS + 1*4)), make_protocol_ro_property("UUID_2", (const uint32_t*)(ID_UNIQUE_ADDRESS + 2*4)), @@ -200,9 +194,13 @@ size_t n_endpoints_ = 0; void communication_task(void * ctx) { (void) ctx; // unused parameter + // TODO: this is supposed to use the move constructor, but currently + // the compiler uses the copy-constructor instead. Thus the make_obj_tree + // ends up with a stupid stack size of around 8000 bytes. Fix this. auto tree_ptr = new (tree_buffer) tree_type(make_obj_tree()); auto endpoint_provider = EndpointProvider_from_MemberList(*tree_ptr); set_application_endpoints(&endpoint_provider); + comm_stack_info = uxTaskGetStackHighWaterMark(nullptr); #if !defined(UART_PROTOCOL_NONE) //DMA open loop continous circular buffer diff --git a/Firmware/MotorControl/encoder.hpp b/Firmware/MotorControl/encoder.hpp index 1082594e..cc1701a0 100644 --- a/Firmware/MotorControl/encoder.hpp +++ b/Firmware/MotorControl/encoder.hpp @@ -54,6 +54,15 @@ public: // Communication protocol definitions auto make_protocol_definitions() { return make_protocol_member_list( + make_protocol_property("error", &error_), + make_protocol_ro_property("is_calibrated", &is_calibrated_), + make_protocol_ro_property("index_found", const_cast(&index_found_)), + make_protocol_property("state", &state_), + make_protocol_property("phase", &phase_), + make_protocol_property("pll_pos", &pll_pos_), + make_protocol_property("pll_vel", &pll_vel_), + make_protocol_property("pll_kp", &pll_kp_), + make_protocol_property("pll_ki", &pll_ki_), make_protocol_object("config", make_protocol_property("use_index", &config_.use_index), make_protocol_property("hand_calibrated", &config_.hand_calibrated), @@ -61,15 +70,7 @@ public: make_protocol_property("cpr", &config_.cpr), make_protocol_property("offset", &config_.offset), make_protocol_property("calib_range", &config_.calib_range) - ), - make_protocol_property("error", &error_), - make_protocol_ro_property("index_found", const_cast(&index_found_)), - make_protocol_property("state", &state_), - make_protocol_property("phase", &phase_), - make_protocol_property("pll_pos", &pll_pos_), - make_protocol_property("pll_vel", &pll_vel_), - make_protocol_property("pll_kp", &pll_kp_), - make_protocol_property("pll_ki", &pll_ki_) + ) ); } }; diff --git a/Firmware/MotorControl/main.cpp b/Firmware/MotorControl/main.cpp index 1c6b63bb..076d8dac 100644 --- a/Firmware/MotorControl/main.cpp +++ b/Firmware/MotorControl/main.cpp @@ -42,6 +42,7 @@ void erase_configuration(void) { extern "C" { int odrive_main(void); +void vApplicationStackOverflowHook(void) { for(;;); } } int odrive_main(void) { diff --git a/Firmware/MotorControl/motor.hpp b/Firmware/MotorControl/motor.hpp index f3f71907..caa133e7 100644 --- a/Firmware/MotorControl/motor.hpp +++ b/Firmware/MotorControl/motor.hpp @@ -36,7 +36,7 @@ typedef struct { // example: vel_gain is [V/(count/s)] instead of [A/(count/s)] // example: current_lim and calibration_current will instead determine the maximum voltage applied to the motor. typedef struct { - bool hand_calibrated = true; // can be set to true to indicate that all values here are valid + bool hand_calibrated = false; // can be set to true to indicate that all values here are valid int32_t pole_pairs = 7; // This value is correct for N5065 motors and Turnigy SK3 series. float calibration_current = 10.0f; // [A] float resistance_calib_max_voltage = 1.0f; // [V] - You may need to increase this if this voltage isn't sufficient to drive calibration_current through the motor. @@ -142,6 +142,7 @@ public: auto make_protocol_definitions() { return make_protocol_member_list( make_protocol_property("error", &error_), + make_protocol_ro_property("is_calibrated", &is_calibrated_), make_protocol_ro_property("current_meas_phB", ¤t_meas_.phB), make_protocol_ro_property("current_meas_phC", ¤t_meas_.phC), make_protocol_property("DC_calib_phB", &DC_calib_.phB), diff --git a/Firmware/MotorControl/protocol.hpp b/Firmware/MotorControl/protocol.hpp index b3606211..bd7f362a 100644 --- a/Firmware/MotorControl/protocol.hpp +++ b/Firmware/MotorControl/protocol.hpp @@ -496,7 +496,7 @@ ProtocolObject make_protocol_object(const char * name, TMembers&&.. } template -class ProtocolProperty : Endpoint { +class ProtocolProperty : public Endpoint { public: static constexpr const char * json_modifier = get_default_json_modifier(); static constexpr size_t endpoint_count = 1; @@ -505,26 +505,24 @@ public: : name_(name), property_(property) {} -// ProtocolProperty(const ProtocolProperty&) = delete; - +/* TODO: find out why the move constructor is not used when it could be + ProtocolProperty(const ProtocolProperty&) = delete; // @brief Move constructor ProtocolProperty(ProtocolProperty&& other) : Endpoint(std::move(other)), name_(std::move(other.name_)), property_(other.property_) {} - - //constexpr ProtocolProperty& operator=(const ProtocolProperty& other) = delete; - /*constexpr ProtocolProperty& operator=(const ProtocolProperty& other) { + constexpr ProtocolProperty& operator=(const ProtocolProperty& other) = delete; + constexpr ProtocolProperty& operator=(const ProtocolProperty& other) { //Endpoint(std::move(other)), //name_(std::move(other.name_)), //property_(other.property_) name_ = other.name_; property_ = other.property_; return *this; - }*/ - - /*ProtocolProperty& operator=(ProtocolProperty&& other) + } + ProtocolProperty& operator=(ProtocolProperty&& other) : name_(other.name_), property_(other.property_) {} ProtocolProperty& operator=(const ProtocolProperty& other)