Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
build/
.vscode
.settings
.cproject
.project
.launch
.cproject
.cache
7 changes: 6 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ set(CMAKE_C_STANDARD 11)
set(CMAKE_C_STANDARD_REQUIRED ON)
set(CMAKE_C_EXTENSIONS ON)

set(CMAKE_CXX_STANDARD 20)
set(CMAKE_CXX_STANDARD_REQUIRED ON)
set(CMAKE_CXX_EXTENSIONS ON)

# Define the build type
if(NOT CMAKE_BUILD_TYPE)
Expand All @@ -29,7 +32,7 @@ project(${CMAKE_PROJECT_NAME})
message("Build type: " ${CMAKE_BUILD_TYPE})

# Enable CMake support for ASM and C languages
enable_language(C ASM)
enable_language(C CXX ASM)

# Create an executable object type
add_executable(${CMAKE_PROJECT_NAME})
Expand All @@ -46,6 +49,7 @@ target_link_directories(${CMAKE_PROJECT_NAME} PRIVATE
# Add sources to executable
target_sources(${CMAKE_PROJECT_NAME} PRIVATE
# Add user sources here
./Core/Src/device-init.cpp
)

# Add include paths
Expand All @@ -56,6 +60,7 @@ target_include_directories(${CMAKE_PROJECT_NAME} PRIVATE
# Add project symbols (macros)
target_compile_definitions(${CMAKE_PROJECT_NAME} PRIVATE
# Add user defined symbols
USE_CDC_DEBUG
)

# Remove wrong libob.a library dependency when using cpp files
Expand Down
24 changes: 24 additions & 0 deletions Core/Inc/device-init.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
*
*/

#ifndef DEVICE_INIT_H_
#define DEVICE_INIT_H_

#ifdef __cplusplus
extern "C" {
#endif

#include "hal.h"

/**
* @brief device init

@ncorrea210 ncorrea210 Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really? 💀

*/
void device_init(SPI_HandleTypeDef *hspi1, SPI_HandleTypeDef *hspi2, SPI_HandleTypeDef *hspi3, CRC_HandleTypeDef *hcrc);
void device_disable_flash();

#ifdef __cplusplus
}
#endif

#endif
188 changes: 188 additions & 0 deletions Core/Src/device-init.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
#include "device-init.h"

#include "main.h"
#include "FreeRTOS.h"
#include "semphr.h"

#include "log.h"

#include "sensor.h"
#include "spi.h"
#include "bmi088.h"
#include "bmp581.h"
#include "gd5f1gq5xe.h"
#include "stm32f411xe.h"
#include "stm32f4xx_hal_crc.h"

#include <array>
#include <optional>

namespace {
inline uint32_t checksum(CRC_HandleTypeDef &hcrc, const uint8_t *data, size_t length) {
return HAL_CRC_Calculate(&hcrc, (uint32_t *)data, length);
}
}

enum Sensors { BMP581, BMI088, NUM_SENSORS };
class TEST_CLASS {
private:
CRC_HandleTypeDef &hcrc;

// Peripherals
Common::SPI accel;
Common::SPI gyro;
Common::SPI bmp;
Common::SPI flash;

// Sensors and Flash
Common::GD5F1GQ5XE flash_dev;
Common::BMP581 bmp581_dev;
Common::BMI088 bmi088_dev;
std::array<Common::Sensor*, NUM_SENSORS> sensors;

// Runtime State
SemaphoreHandle_t packet_mutex = xSemaphoreCreateMutex();
Common::Packet packet = Common::Packet();
volatile bool save_to_flash = false;
lfs_file_t packet_file;

int32_t fs_size = 0;
uint32_t boot_count = 0;
uint32_t file_size = 0;

// Task attributes
TaskHandle_t sensor_task_handle = NULL;
TaskHandle_t flash_task_handle = NULL;
// Tasks
static void sensor_task(void *argument) {
auto *self = static_cast<TEST_CLASS*>(argument);
self->sensor_loop();
}

static void flash_task(void *argument) {
auto *self = static_cast<TEST_CLASS*>(argument);
self->flash_loop();
}
public:
TEST_CLASS(SPI_HandleTypeDef &hspi1, SPI_HandleTypeDef &hspi2, SPI_HandleTypeDef &hspi3, CRC_HandleTypeDef &hcrc)
: hcrc(hcrc),
accel(&hspi1, IMU2_ACC_CS_GPIO_Port, IMU2_ACC_CS_Pin),
gyro(&hspi1, IMU2_GYRO_CS_GPIO_Port, IMU2_GYRO_CS_Pin),
bmp(&hspi2, BAR1_CS_GPIO_Port, BAR1_CS_Pin),
flash(&hspi3, FLASH_CS_GPIO_Port, FLASH_CS_Pin),

flash_dev(flash),
bmp581_dev(bmp),
bmi088_dev(accel, gyro),
sensors{&bmp581_dev, &bmi088_dev}
{
// Initialize sensors
for (auto& sensor : sensors) {
bool ready = 0;
for (int c = 0; c < 10; ++c) {
ready = sensor->init();
if (ready) break;
Delay(20000);
}
if (!ready) sensor = nullptr;
}

// Initialize flash
for (int c = 0; c < 3; ++c) {
if (flash_dev.init()) {
fs_size = flash_dev.mount();
if (fs_size >= 0) {
save_to_flash = true;
boot_count = flash_dev.bootcount(false);
file_size = flash_dev.open(&packet_file, "packets");
}
break;
}
Delay(5000);
}
Common::LOG("Flash: %u (size), %u (boot), %u (packet_size)\r\n", fs_size, boot_count, file_size);
}
Comment on lines +27 to +104

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't really like this method of setting things up. I'd rather see tasks completely isolated rather than share a class for initialization. This is kinda the setup I have in mind:
In platform:
sensor task and init high level is written here. Shouldn't really have device specific code, can assume FreeRTOS and STM32 is used though.
In Hephaestus:
Any device specific functions needed for the task. This might be where we do stuff with specific pins for example, you use an LED for the sensor loop.

In the actual device_init function we would then have FlashTask::Init() which would handle the scheduling of the task. The idea here is to keep device-init.cpp clean, and to keep applications compartmentalized.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets get the cpp-migration merged first for this and common-drivers, then work on the task abstraction. Mahir is gonna yell at me if I stuff more abstractions in...

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough, that's fine with me

@dmanslick dmanslick Aug 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with most of Nathan's proposal here, I also think tasks should be isolated units. However, I disagree with moving the Sensor Task into common. The sensors that each board is polling will be different, so a common Sensor Task will basically become a become a god class/object, not ideal.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does that look though? If we made an individual sensor task for every sensor, we'd have a lot of boilerplate. Additionally we want to read all sensors at the same time to ensure the timestamp is accurate for everything. I think we can make a truly general sensor task by making it take the list of sensors on init and having a couple functions that are defined for each device? Some of those might be things like NotifyFlashTask and GetTimestamp.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current sensor task just does this

        for (auto *sensor : sensors)
          if (sensor != nullptr) sensor->read(packet);

It takes a list of the sensors it wants and just loop over it. It doesn't really care about what sensors are available, just what sensors is currently in the array

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course it also does a bit of other stuff like semaphores, checksums, and notifying the flash task. Anyways, I did want more discussion on task abstractions which is why I wanted to punt this to another PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ncorrea210 You misunderstood what I wrote. I was not saying to have a task for each sensor. Doesn't matter anyways since the way I was thinking the task would've been implemented was a bit foolish anyways, what Ivan just wrote makes sense so I can see how it can be in common,


void sensor_loop() {
// Simple counter for task notification
uint32_t counter = 0;
// Run this task 200 times per second
TickType_t last_wake_up = xTaskGetTickCount();
int hertz = 200;

for (;;) {
if (xSemaphoreTake(packet_mutex, portMAX_DELAY) == pdTRUE) {
// Read from all sensors
for (auto *sensor : sensors)
if (sensor != nullptr) sensor->read(packet);

// This is originally for camera, but we will use it for flash for now
packet.status = (flash_task_handle != NULL);
// TODO: change this to microseconds at a later time
packet.time_us = xTaskGetTickCount() * portTICK_PERIOD_MS;
packet.checksum = checksum(hcrc, (const uint8_t *) &packet + sizeof(short),
sizeof(packet) - 6);
HAL_GPIO_TogglePin(LED_GPIO_Port, LED_Pin);
xSemaphoreGive(packet_mutex);

// Tell flash to save data every other packet
if ((++counter) >= 2 && flash_task_handle != nullptr) {
counter = 0;
if (save_to_flash) xTaskNotifyGive(flash_task_handle);
}
}
// Go to sleep little task...
vTaskDelayUntil(&last_wake_up, configTICK_RATE_HZ / hertz);
}
}

void flash_loop() {
auto copy = Common::Packet();
for (;;) {
// Wait until notified
ulTaskNotifyTake(pdTRUE, portMAX_DELAY);
// Someone notified us to stop task collection, shut the poor flash down :(
if (!save_to_flash) {
flash_dev.close(&packet_file);
flash_dev.unmount();
vTaskDelete(NULL);
return;
}
// Make a local copy, so we don't block
if (xSemaphoreTake(packet_mutex, portMAX_DELAY) == pdTRUE) {
copy = packet;
xSemaphoreGive(packet_mutex);
}

// Save to flash
flash_dev.append(&packet_file, (uint8_t*) &copy, sizeof(copy));
}
}

void disable_flash() {
if (save_to_flash) {
save_to_flash = false;
vTaskNotifyGiveFromISR(flash_task_handle, NULL);
}
}
Comment on lines +162 to +167

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this function guaranteed to only be called in an ISR context? It can be dangerous to use FromISR functions in a non-ISR context. See this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is guaranteed to only be called from ISR, similarly non-ISR functions should not be called in ISR-context so it goes both ways.


void start_tasks() {
// Idle task has priority of 0
xTaskCreate(flash_task, "flash task", 512, this, 1, &flash_task_handle);
xTaskCreate(sensor_task, "sensor task", 256, this, 2, &sensor_task_handle);
Comment on lines +171 to +172

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to look into using xTaskCreateStatic so there aren't any heap allocations. Probably not super important though.

}
};

// TODO: need to discuss how to better do this
static TEST_CLASS* device_ptr = nullptr;
extern "C" {
void device_init(SPI_HandleTypeDef *hspi1, SPI_HandleTypeDef *hspi2, SPI_HandleTypeDef *hspi3, CRC_HandleTypeDef *hcrc) {
static TEST_CLASS device = TEST_CLASS(*hspi1, *hspi2, *hspi3, *hcrc);
device_ptr = &device;
device.start_tasks();
}

void device_disable_flash() {
if (device_ptr != nullptr) device_ptr->disable_flash();
}
}
Loading