-
Notifications
You must be signed in to change notification settings - Fork 0
feat: use cpp version of common drivers #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| build/ | ||
| .vscode | ||
| .settings | ||
| .cproject | ||
| .project | ||
| .launch | ||
| .cproject | ||
| .cache |
| 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 | ||
| */ | ||
| void device_init(SPI_HandleTypeDef *hspi1, SPI_HandleTypeDef *hspi2, SPI_HandleTypeDef *hspi3, CRC_HandleTypeDef *hcrc); | ||
| void device_disable_flash(); | ||
|
|
||
| #ifdef __cplusplus | ||
| } | ||
| #endif | ||
|
|
||
| #endif | ||
| 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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...
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair enough, that's fine with me There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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*) ©, sizeof(copy)); | ||
| } | ||
| } | ||
|
|
||
| void disable_flash() { | ||
| if (save_to_flash) { | ||
| save_to_flash = false; | ||
| vTaskNotifyGiveFromISR(flash_task_handle, NULL); | ||
| } | ||
| } | ||
|
Comment on lines
+162
to
+167
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be good to look into using |
||
| } | ||
| }; | ||
|
|
||
| // 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(); | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really? 💀