Port Speed Controller Firmware to PlatformIO, Clean up, and Correct Driver Implementations #23

a2k-hanlon · 2022-02-25T10:22:22Z

Towards the effort to debug the faulty accelerator pedal behaviour, this PR introduces several improvements to the Speed Controller (SCN) firmware that runs on one of the STM32 Nucleo boards on Daybreak's dashboard.

The following improvements were made:

Transitioned the Speed Controller firmware from Kiel uVision to PlatformIO for ease of build and debug
Refactored of all of the hardware register accesses to use the register bit and bitfield definitions provided by the CMSIS device header for readability
Corrected several mistakes and oversights in the hardware drivers, including one to the encoder timer initialization (see line comments pointing out these corrections)
Added a baud rate parameter to the virtual COM driver (serial communication using a UART) with higher baud rates
Added missing include guards in some of the headers
Removed duplicate function documentation comments from the header files and refactored all these comments to a consistent doxygen format
Cleaned up some of the preprocessor flags
Changed all helper functions and global variables to static type
Added a CI build workflow for the Speed Controller Firmware

a2k-hanlon · 2022-02-25T10:30:10Z

Speed_Controller/src/clock_init/clock_init.c

+    /* SYSCLK, HCLK, PCLK2 and PCLK1 configuration ---------------------------*/
+    /* Enable HSE in BYPASS mode (no crystal, oscillator bypassed by an external clock signal) */
+    /* On a Nucleo board, this external clock signal comes from the built-in ST Link */
+    RCC->CR |= (uint32_t)(RCC_CR_HSEBYP | RCC_CR_HSEON);


The RCC_CR_HSEBYP bit was not being set in the old default clock configuration generated by Kiel

a2k-hanlon · 2022-02-25T10:32:48Z

Speed_Controller/src/Timer/Timer.c

+	TIM2->CR1 &= ~TIM_CR1_CKD; 	// Set the timer to use default clock division
+	TIM2->CR1 |= TIM_CR1_OPM; 	// Set the timer to use one pulse mode
+
+	TIM2->ARR = 10 * period - 1;// Set autoreload value to set the period


Adjusted this to assume and use a reset value of 0 instead of 1 for the timer counter. The hardware always uses a 0 reset.

a2k-hanlon · 2022-02-25T10:35:51Z

Speed_Controller/src/Timer/Timer.c


-/** Interrupt handler for Timer 2.
+static volatile int8_t timeoutFlag = FALSE;


This needs to be volatile since it is set from an interrupt service routine. It has also been made static, and a getter and setter pair have been added for this flag to be used by main.c.

a2k-hanlon · 2022-02-25T10:37:14Z

Speed_Controller/src/Timer/Timer.c

+/**
+ * Get the timer timeout flag to see if the timer period has elapsed
+ * 
+ * @returns The state of the timeout flag
+ */
+uint8_t GetTimeoutFlag(void)
+{
+	return timeoutFlag;
 }

 /**
- * Stops the counter on the timer
+ * Clear the timer timeout flag 
 */
-void StopTimer(void){	
-	TIM2->CR1 &= ~(0x1UL);		//disable TIM2
-
+void ClearTimeoutFlag(void)
+{
+	timeoutFlag = FALSE;


New getter and setter functions for timeoutFlag. Now it can only be read and cleared by external code. Only the TIM2 ISR sets the flag.

a2k-hanlon · 2022-02-25T10:38:46Z

Speed_Controller/src/Timer/Timer.c

-
-	TIM2->ARR = 10*period;		//set autoreload to reset every period
-
-	TIM2->CR1 &= (0x11UL << 8);	//set the timer to use default clock division


Missing ~ here, this has been corrected

a2k-hanlon · 2022-02-25T10:39:58Z

Speed_Controller/src/ADC/ADC.c

+	SendInt(ADC_reading);
+#endif /* ADC_DEBUG */
+
+#if ADC_REVERSE_READING


New preprocessor flags!

a2k-hanlon · 2022-02-25T10:52:46Z

Speed_Controller/src/Encoder/encoder.c

+	// Enable capture/compare channels 1 and 2, other channels disabled and all polarities default
+	TIM1->CCER = (TIM_CCER_CC2E | TIM_CCER_CC1E);
+
+	TIM1->SMCR |= 0x3 << TIM_SMCR_SMS_Pos; // Set timer to encoder mode 3 (clocked by both TI1FP1 and TI2FP2)


There was an easy and rather significant mistake here - the STM32F1 reference manual specifies a value of '011' for the SMCR register SMS bitfield for encoder mode 3 (where edges of both encoder signals increment/decrement the timer counter). This is the mode we want. The '011' is binary (ie. = 0x3); however, it was entered in the code as hex 0x011 (= 0b0001_0001) instead.

Not only did this put the timer in encoder mode 1 instead of 3, it also changed the TS bitfield unintentionally. The effect of doing this for TIM1 is to enable the trigger output (TRGO signal) of TIM2 as a trigger source. By default it triggers a reset of the TIM1 counter (which tracks the encoder position). This is not good, because TIM2 is used to provide a countdown for the maximum time interval between speed control CAN messages. However, I think by default the TIM2 TRGO is configured to only assert upon software setting the UG bit in TIM2's register, which the software definitely isn't doing. Needs a bit more investigation.

a2k-hanlon · 2022-02-25T10:53:52Z

Speed_Controller/src/main.c

-	GPIOC->CRL |= (0x4 << 24); //C6
-
-	//CAN receive setup
+	VirtualComInit(BAUD_115200);


New baud 115200 instead of 9600

a2k-hanlon · 2022-02-25T10:54:49Z

Speed_Controller/src/main.c

+		if (GetTimeoutFlag() == TRUE)
+		{
+#if PRINT_DEBUG
+			SendString(", timout ");
+#endif /* PRINT_DEBUG */
+
+#if SEND_CAN_MSG
+			CANSend(&CAN_drive);
+#endif /* SEND_CAN_MSG */
+
+			ClearTimeoutFlag();


Here's where the new timeout flag getter and setter are used

a2k-hanlon · 2022-02-25T10:55:58Z

Speed_Controller/src/main.c

+static void sendMotorCommand(float curr, float vel);
+static void sendReverseToggle(void);


Moved to bottom of file and made static

a2k-hanlon · 2022-02-25T11:01:47Z

Speed_Controller/src/Timer/Timer.c

+
+	DBGMCU->CR |= DBGMCU_CR_DBG_TIM2_STOP; // Halt TIM2 upon debug halt
+
+	TIM2->CR1 |= TIM_CR1_CEN; 	// Enable TIM2


Enabling the timer should be the very last step of the initialization. This was corrected.

a2k-hanlon · 2022-02-26T08:25:12Z

Peripherals/CAN/CAN.c

-	CAN1->FM1R |= 0x1C << 8;              // Assign all filters to CAN1
-	CAN1->FMR  |=   0x1UL;                // Set to filter initialization mode


CAN1->FM1R |= 0x1C << 8 is supposed to be a write to CAN1->FMR. Although it was done wrong, fortunately the FM1R register is locked until the next operation CAN1->FMR |= 0x1UL; (enter filter initialization mode). So, it just did nothing rather than what it was supposed to do. This has been corrected.

a2k-hanlon · 2022-02-26T08:28:19Z

Peripherals/CAN/CAN.h

+int CANSetFilter(uint16_t id);
+int CANSetFilters(uint16_t *ids, uint8_t num);


Added completion status return values

a2k-hanlon added 11 commits February 24, 2022 23:49

Port Speed Controller firmware to PlatformIO

c2dcb60

Add CI build for Speed Controller

de7d021

Clean up virtual com driver and add baud selection

4c996dd

Clean up RTC driver

766519b

Clean up timer driver in Speed Controller

79e10c3

Clean up encoder driver in Speed Controller

2f4a444

Correct encoder timer config in Speed Controller

5c010ab

Clean up ADC driver in Speed Controller

b271f02

Clean up Speed Controller main.c

0ff2e0e

Start clean up of CAN driver

f581957

Add Peripherals folder to trigger for Speed Controller CI

4ac61e8

a2k-hanlon requested review from mihirnimgade and NicolasRic February 25, 2022 10:22

a2k-hanlon commented Feb 25, 2022

View reviewed changes

Complete clean up of CAN driver

620def9

a2k-hanlon commented Feb 26, 2022

View reviewed changes

Fix accidentally renamed setpoint variables

5ccb7c5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Port Speed Controller Firmware to PlatformIO, Clean up, and Correct Driver Implementations #23

Port Speed Controller Firmware to PlatformIO, Clean up, and Correct Driver Implementations #23

a2k-hanlon commented Feb 25, 2022 •

edited

Loading

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 25, 2022

a2k-hanlon Feb 26, 2022

a2k-hanlon Feb 26, 2022


		/** Interrupt handler for Timer 2.
		static volatile int8_t timeoutFlag = FALSE;


		TIM2->ARR = 10*period; //set autoreload to reset every period

		TIM2->CR1 &= (0x11UL << 8); //set the timer to use default clock division

		static void sendMotorCommand(float curr, float vel);
		static void sendReverseToggle(void);


		DBGMCU->CR \|= DBGMCU_CR_DBG_TIM2_STOP; // Halt TIM2 upon debug halt

		TIM2->CR1 \|= TIM_CR1_CEN; // Enable TIM2

		CAN1->FM1R \|= 0x1C << 8; // Assign all filters to CAN1
		CAN1->FMR \|= 0x1UL; // Set to filter initialization mode

		int CANSetFilter(uint16_t id);
		int CANSetFilters(uint16_t *ids, uint8_t num);

Port Speed Controller Firmware to PlatformIO, Clean up, and Correct Driver Implementations #23

Are you sure you want to change the base?

Port Speed Controller Firmware to PlatformIO, Clean up, and Correct Driver Implementations #23

Conversation

a2k-hanlon commented Feb 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

a2k-hanlon commented Feb 25, 2022 •

edited

Loading