IMU | LSM6DSV320X SPI DMA Driver by MasterUser43 · Pull Request #28 · SunDevilRocketry/driver

MasterUser43 · 2026-05-01T18:33:09Z

Description

This PR introduces the initial driver implementation for the LSM6DSV320X high-g IMU, specifically targeting the STM32H733 (A0010, Rev 3 PCB).
The driver is currently implemented as imu_t.c/h to allow for a focused code review of the new DMA-based architecture and SFLP integration before it is merged/optimized with the existing old IMU.

Summary of functions:
Asynchronous DMA Path: (HAL_SPI_TransmitReceive_DMA) for FIFO burst reads to minimize CPU overhead during flight.
SFLP Integration: Support for the Sensor Fusion Low Power (SFLP) engine, providing game rotation vectors (quaternions), gravity vectors, and gyroscope bias estimates directly from the IMU.
Dynamic Range Handling: Includes logic for transitioning between the Low-G (up to ±16g) and High-G (up to ±320g) @ runtime.

Documentation is, frankly intense, so bare patience. Recommended to have associated datasheet open as citations are made heavily. I expect readability issues, but want to ensure nothing critical as at foot [Mainly if imu has unnecessary functions or is goes beyond scope etc.]

Issue Link

Associated issue IMU R3

Child PRs:
PR#1 Library Import
External lsm6dsv320x-pid library (STMicroelectronics).
https://github.com/SunDevilRocketry/lib/tree/feat/add-lsm6dsv320x-submodule

PR#2: DMA linker
Modifying linker script
https://github.com/SunDevilRocketry/Flight-Computer-Firmware/tree/feat/add-DMA-linker

Testing [NOT APPLICABLE]

Passes existing unit tests
Unit tests modified (link the test changes as a child PR)
Integration test performed

Attach any test artifacts here, if relevant.

Other

Leave any additional notes here

Reviewer Checklist

Standards

Follows FCF Architectural Standards
Follows SDR Coding Standards
Code complexity/function Size is minimized
Code is testable
Code is readable and commented properly
License terms are respected

Error Handling

Potentially unsafe functions return a status code
Error returns properly handled

Memory

Stack allocated memory is scoped correctly
Heap allocated memory is avoided
Globally allocated memory is minimized except when necessary
Pointers are used correctly
Concurrency has been considered

Performance

Rate limiters are respected
Busy waiting is avoided
"Delay" calls are not used in performance sensitive code

ETSells · 2026-05-23T19:14:36Z

Ah sorry I just saw this! I've pinged myself as a reviewer and will take a look when I get a minute.

Changing your target branch to reflect our move to an unstable trunk system.

ETSells · 2026-05-23T19:15:21Z

Also, I've added @266-750Balloons as this introduces DMA, which I'm less familiar with than CPU-bound interrupts.

ETSells

dude this is actually crazy...

lgtm honestly, ignore my CR comments below, I'd take this as-is...

ETSells · 2026-05-23T21:26:51Z

+  * @param  len:    Number of data bytes (address byte excluded).
+  * @retval 0 on success, 1 on HAL error or oversized request.
+  */
+static int32_t platform_spi_write


cr: Please use a status type in your final version (you can reuse the old IMU status enum if you want, else if this stays separate you'll need a new one.) <-- ignore; all external interfaces follow the status return convention which is good enough for me

ETSells · 2026-05-23T21:28:59Z

+memcpy( &tx[1], buf, len );
+
+HAL_GPIO_WritePin( IMU_CS_GPIO_PORT, IMU_CS_PIN, GPIO_PIN_RESET );
+__NOP(); __NOP();


Interesting that these are needed here -- we've never had to do NOPs since the procedure call overhead is usually sufficient delay between writing CS low.

ETSells · 2026-05-23T21:29:57Z

+
+(void)handle;
+
+if ( len >= SPI_WRITE_BUF_MAX ) {


nit: max implies that the upper bound is inclusive.

ETSells · 2026-05-23T21:30:58Z

+__NOP(); __NOP();
+HAL_GPIO_WritePin( IMU_CS_GPIO_PORT, IMU_CS_PIN, GPIO_PIN_SET );
+
+return ( hal_status == HAL_OK ) ? 0 : 1;


nit: we don't usually like inline logic like this since its harder to parse through

ETSells · 2026-05-23T21:31:54Z

+    uint16_t       len
+    )
+{
+#define SPI_WRITE_BUF_MAX  ( 64U )


Can we define this scoped to the header file for visibility? Either that or I guess leave like an implNote in your doxygen header.

ETSells · 2026-05-23T21:40:03Z

+ Initializations
+--------------------------------------------------------------------------*/
+imu_sflp_enabled  = false;
+imu_dma_ready     = false;


doesn't look like we have DMA r/w callbacks available but i'm glad we've started laying the groundwork for them!

ETSells · 2026-05-23T21:41:50Z

+        xl_bdr = LSM6DSV320X_XL_BATCHED_AT_7680Hz;
+        g_bdr  = LSM6DSV320X_GY_BATCHED_AT_7680Hz;
+        break;
+    case IMU_ODR_1920HZ: /* fall-through to default */


for defensive programming, consider using a debug assert here that ODR == 1920, that way in debug mode we catch that an improper param was passed.

the construct was just added in mod/error_sdr/error_sdr.h:debug_assert

ETSells · 2026-05-23T21:42:56Z

+pid_status |= lsm6dsv320x_gy_data_rate_set( &imu_ctx, g_odr  );
+
+/*
+ * Full-scale and ODR configuration - two separate accelerometer chains.


just a heads up that in the future we'll want this to be runtime configurable! i'll probably ask you to write the config api for that when we're ready, but we'd want high-G mode during boost and switch to low-G in coast and beyond.

ETSells · 2026-05-23T21:45:36Z

+  *         high-G to low-G: clears XL_HG_REGOUT_EN, restores low-G batching.
+  *         DS14623 Rev3 §6.1.2, Tables 70, 149, 150; COUNTER_BDR_REG1 (0Bh).
+  */
+IMU_STATUS imu_set_accel_fs


oh dang, ignore me earlier, you already did it!!

ETSells · 2026-05-23T21:47:46Z

+/* Assert CS and launch DMA - CS de-asserted in imu_process_async_cb() */
+HAL_GPIO_WritePin( IMU_CS_GPIO_PORT, IMU_CS_PIN, GPIO_PIN_RESET );
+__NOP(); __NOP();
+hal_status = HAL_SPI_TransmitReceive_DMA( &IMU_SPI,


ohhhh dang! you've got the async interface too?!

ETSells · 2026-05-23T21:53:32Z

Also -- because of the divergence between this version and the old IMU, we should consider keeping these separate with maybe a guard at the top of imu.h that points to imu_t.h if we're not using the legacy IMU. Don't worry about merging to the old platform unless you feel its the smarter design choice.

Because this is kept separate from the old IMU, we can merge as-is once the PID driver is imported somewhere (either driver or lib)

ETSells · 2026-06-20T18:41:41Z

This is in my review queue, but it might take me a second to get there, just as a heads up.

ETSells

Significant build errors present. Please resolve these before requesting re-review.

I have added this driver to the build system via your FCF child PR. Please pull that up and test by invoking

cd app/rev3
make -jX # X = number of parallel processes you want for compilation

Build error log attached: build_err_log.txt

ETSells · 2026-06-21T20:51:54Z

+*       https://opensource.org/license/bsd-3-clause          
+*
+*******************************************************************************/
+#if !defined( A0010 )


Consider:

#ifndef( A0002_REV2 ) #error "Unsupported hardware platform #endif

rather than wrapping the whole thing in a preprocessor directive.

ETSells · 2026-06-21T20:52:30Z

+} IMU_RAW;
+
+/* Processed IMU aata */
+typedef struct _STATE_ESTIMATION {


note for myself: beware of conflicts

ETSells · 2026-06-21T20:53:32Z

Assumption: I'm assuming this has been copied over with no functional changes from the previous iteration. If I'm wrong, then please fix.

ETSells · 2026-06-21T20:53:50Z

Assumption: I'm assuming this has been copied over with no functional changes from the previous iteration. If I'm wrong, then please fix.

… error recovery

…nt/rename changes

MasterUser43 · 2026-06-24T15:56:46Z

Still have debug_assert to fix + maybe do more tinkering to avoid deprecated silly warnings. Will verify if effort is worth it or not (of warnings)

MasterUser43 linked an issue May 1, 2026 that may be closed by this pull request

IMU: Initial R3 Driver Implementation #7

Open

ETSells self-requested a review May 23, 2026 19:14

ETSells changed the base branch from development/appa to main May 23, 2026 19:14

ETSells requested a review from 266-750Balloons May 23, 2026 19:14

ETSells mentioned this pull request May 23, 2026

feat: Add assert for debug builds that won't affect release stability SunDevilRocketry/mod#127

Merged

19 tasks

ETSells approved these changes May 23, 2026

View reviewed changes

MasterUser43 force-pushed the appa/driver/imu branch from 547fdbc to 7682028 Compare June 17, 2026 01:49

github-advanced-security AI found potential problems Jun 17, 2026

View reviewed changes

Comment thread imu/imu_t.c Fixed

MasterUser43 changed the title ~~Files to be reviewed (not optimized or merged w/ other imu)~~ IMU | LSM6DSV320X SPI DMA Driver Jun 20, 2026

This was referenced Jun 20, 2026

Add lsm6dsv320x-pid submodule to lib root (pair w/ IMU r3) SunDevilRocketry/lib#22

Open

Added .dma_buffer to bypass DTCM SunDevilRocketry/Flight-Computer-Firmware#291

Open

MasterUser43 requested a review from ETSells June 20, 2026 00:50

ETSells requested changes Jun 21, 2026

View reviewed changes

ETSells mentioned this pull request Jun 21, 2026

Sensor: Design sensor data struct for Rev 3 and implement sensor_dump SunDevilRocketry/mod#125

Open

MasterUser43 added 6 commits June 24, 2026 07:32

Files to be reviewed (not optimized or merged w/ other imu)

1e57c65

integrate official SFLP conversions, H7 DMA memory placement, and bus…

c298794

… error recovery

+ comment changes

abf9f42

implement unified compile-time driver facade b/c simple enough. Comme…

37ff733

…nt/rename changes

3 liner for compiling

88a056d

Added files associated w/ previous commits

41ed939

MasterUser43 force-pushed the appa/driver/imu branch from f64060f to 41ed939 Compare June 24, 2026 14:33

Resolve SFLP compilation errors and polish comments

47c74f1

Uh oh!

Conversation

MasterUser43 commented May 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issue Link

Testing [NOT APPLICABLE]

Other

Reviewer Checklist

Standards

Error Handling

Memory

Performance

Uh oh!

ETSells commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ETSells commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ETSells left a comment

Choose a reason for hiding this comment

Uh oh!

ETSells May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ETSells commented May 23, 2026

Uh oh!

Uh oh!

ETSells commented Jun 20, 2026

Uh oh!

ETSells left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MasterUser43 commented Jun 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MasterUser43 commented May 1, 2026 •

edited

Loading

ETSells commented May 23, 2026 •

edited

Loading

ETSells commented May 23, 2026 •

edited

Loading

ETSells May 23, 2026 •

edited

Loading

MasterUser43 commented Jun 24, 2026 •

edited

Loading