Conversation
AveryLor
left a comment
There was a problem hiding this comment.
Overall pretty good, having a super loop is actually what we had when we first ran firmware so this is a mostly valid way to do it.
However, one of the best things about our new firmware is concurrent programming (not parallel since we only have one thread lol)
Right now you are mostly fulfilling all the important requirements of the project in void loop(void){} which is completely fine.
If you want to much more accurately hit your timing requirements which are very tight in embedded design and high performance computing you start manually managing what tasks are run on a particular thread.
So instead of writing a super loop here's what I recommend, you should have 5 tasks
sensor_current_update sensor_steering_angle_update bms_monitoring_task lcd_update torque_update
You can do this like this for example
xTaskCreate(lcd_update, "lcd_update", 1024, NULL, TaskPriority_LCD, NULL)
This is the function prototype
BaseType_t xTaskCreate( TaskFunction_t pvTaskCode, const char * const pcName, const configSTACK_DEPTH_TYPE uxStackDepth, void *pvParameters, UBaseType_t uxPriority, TaskHandle_t *pxCreatedTask );
The idea here is that the FreeRTOS scheduler (a more deterministic scheduler than one you would find in a general OS) would manage which tasks are run at a particular point in time, with tasks being able to preempt based on interrupts / semaphore / mutex permissions.
Use Queues to communicate between these tasks, use mutexes / semaphores to manage resources / task permissions.
So by managing task priorities you can be much more deterministic with the timing requirements specified.
| void setup(void) | ||
| { | ||
| can_init(2, 3, 1000000); | ||
| lcd_init(5, 6, 7, 8); |
There was a problem hiding this comment.
I would define macros for all your pins, for example #define LCD_CS 8, but that's a very small thing, otherwise correct
| lcd_init(5, 6, 7, 8); | ||
| bms_init(5, 6, 7, 9); | ||
|
|
||
| pinMode(AIR_POS, OUTPUT); |
There was a problem hiding this comment.
For the precharge setup we cannot immediately close the relays when we go from LV -> TS
As it says in the top of the file
"When we go from LV -> TS, we cannot simply energize the relays (ask an
electrical lead why!). Instead we first close AIR-, then another relay called
the "Precharge Relay". After ~5 seconds we close AIR+ and open the pre-charge
relay"
The reason for this is because I believe the high voltage system specifically the inverter contains high capacitance. Empty capacitors act as shorts so a massive spike of current rushes from the battery into those empty capacitors and its so much to the point that arcing occurs and they literally weld shut the AIRs (accumulator isolation relays).
To get the delay you use vTaskDelay("put time in milliseconds here), but make sure you don't block the CPU
|
|
||
| float torques[4]; | ||
| float current = analogRead(CURRENT_GPIO); | ||
| float wheelspeeds = analogRead(WHEELSPEED_GPIO); |
There was a problem hiding this comment.
You cannot do an analog read on the wheelspeed pin, sorry I didn't realize this earlier, but it is a digital input (from the perspective of the MCU).
You can reference this in the project:
"Wheelspeeds are a bit tricky. The sensor is routed to a GPIO pin, and is
normally high. However, the wheels have a gear looking thing with 17 teeth --
everytime one of the teeth passes the sensor, the voltage at the GPIO pin goes
low. You can derive the RPM of the wheel from how often the voltage goes low"
| void setup(void) | ||
| { | ||
| can_init(2, 3, 1000000); | ||
| lcd_init(5, 6, 7, 8); |
There was a problem hiding this comment.
Both the LCD and BMS are using the same SPI pins / peripheral (5 and 6). If you have an interrupt configured (which you should for the wheelspeed), one device could try to talk while the other was trying to transmit information and that's how you result with a race condition.
A mutex would be helpful to use here
You can define a mutex like this: SemaphoreHandle_t spi_mutex;
| float steering_angle; | ||
| can_receive(&steering_angle, STEERING_CAN_ID); | ||
|
|
||
| calculate_torque_cmd(&torques, current, wheelspeeds, steering_angle); |
There was a problem hiding this comment.
You should send torque on CAN
Built my implementation of the intro project. Looking for feedback.