-
Notifications
You must be signed in to change notification settings - Fork 0
Javier/dma to flash #18
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: master
Are you sure you want to change the base?
Conversation
This commit adds the initial, complete C++ driver for the NAU7802 sensor, placing it in the PeripheralDriversSubmodule. - Includes a reusable I2C_Wrapper to abstract STM32 HAL functions. - Implements core logic: init, reset, setGain, and readSensor. - Adds register definitions, example usage files, and a README.
Renamed Files: Changed I2C_Wrapper source and header files to i2c_wrapper (lowercase) for consistent naming conventions. Driver Update: Updated NAU7802 constructor to accept the I2C wrapper pointer and HAL_Delay function directly. Main Test Update: Modified main_read_test.cpp to implement the new initialization logic, set default gain to 1x, and added placeholders for future calibration. Cleanup: Removed the redundant ExampleUsage.cpp and the binary datasheet file (.pdf). Registers: Minor updates to register definitions in NAU7802_regs.hpp.
- Add MX66L1G45GMI_Interface.cpp: DMA-based SPI interface implementation
- Update MX66L1G45GMI.cpp/.hpp to support abstracted interface and DMA
- Integrate DMATransfer.hpp for efficient SPI transfers
- Refactor driver to use function pointers for SPI, CS, and delay
- Improve blocking/wait logic for DMA safety
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.
Pull request overview
Adds a DMA-backed SPI interface layer for the MX66L1G45GMI flash driver and refactors the core driver to use injected function pointers for IO (SPI read/write, chip-select, delay), along with new usage documentation.
Changes:
- Added a new interface implementation (
MX66L1G45GMI_Interface.cpp) intended to bridge the abstract driver to STM32 SPI/DMA. - Refactored the core driver (
MX66L1G45GMI.cpp/.hpp) to use an injectedMX66_Config(function pointers) instead of direct HAL calls. - Added a new driver README and updated gitignore rules related to DMA notes / Makefile.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| MX66L1G45GMI/README.md | Documents the layered architecture and initialization/usage guidance for the new injected/DMA approach. |
| MX66L1G45GMI/MX66L1G45GMI_Interface.cpp | Provides the concrete STM32 HAL + DMA-based implementation of the injected IO hooks. |
| MX66L1G45GMI/MX66L1G45GMI.cpp | Refactors the driver internals to call injected function pointers for SPI/CS/delay. |
| MX66L1G45GMI/Inc/MX66L1G45GMI.hpp | Introduces the MX66_Config struct and MX66_Init() API for dependency injection. |
| DMA/.gitignore | Ignores local DMA notes file(s). |
| .gitignore | Adds an ignore rule for Makefile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Use DMA Tool | ||
| DMAControl::Transfer(&hspi1, 0, nullptr, data, len); | ||
| // BLOCKING WAIT | ||
| while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); |
Copilot
AI
Jan 31, 2026
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.
Same issue as the write path: DMAControl::Transfer uses HAL_SPI_TransmitReceive_DMA(...) and will fail when txData is nullptr. For RX-only, the helper should call HAL_SPI_Receive_DMA (or provide a dummy TX buffer), and the returned status should be checked.
| // Use DMA Tool | |
| DMAControl::Transfer(&hspi1, 0, nullptr, data, len); | |
| // BLOCKING WAIT | |
| while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); | |
| // For small reads, use polling to avoid DMA overhead | |
| if (len <= 32) { | |
| HAL_SPI_Receive(&hspi1, data, len, 100); | |
| } else { | |
| // Use DMA for larger reads | |
| HAL_StatusTypeDef status = HAL_SPI_Receive_DMA(&hspi1, data); | |
| if (status != HAL_OK) { | |
| // DMA did not start; return without blocking indefinitely | |
| return; | |
| } | |
| // BLOCKING WAIT | |
| while (HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); | |
| } |
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.
Pull request overview
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| MX_DMA_Init(); // <--- DMA MUST be initialized BEFORE SPI | ||
| MX_SPI1_Init(); | ||
|
|
||
| // Inject the hardware dependencies into the driver |
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.
We want these to be inside the taks or driver, otherwise main gets crowded
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.
its fro lines 87-97
| HAL_GPIO_WritePin(SPI_FLASH_CS_GPIO_Port, SPI_FLASH_CS_Pin, high ? GPIO_PIN_SET : GPIO_PIN_RESET); | ||
| } | ||
|
|
||
| // Delay Implementation |
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.
have a note that this uses HAL delay, what is the point of this if we are renaming it only? And why is there two delays and explain why you used each one at that point and why
|
|
||
| // Delay Implementation | ||
| void IMPL_Delay(uint32_t ms) { | ||
| // Change this line to switch between HAL_Delay, osDelay, vTaskDelay, etc |
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.
We dont want the programmer changing source code if they HAVE to make a change. This is fine since usually programmer wont have to switch between the delay functions. Do they? What is the point of using any other delay funcition?
MX66L1G45GMI: Add DMA-backed SPI interface and refactor driver