-
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); | |
| } |
MX66L1G45GMI/README.md
Outdated
|
|
||
| - Template-based utility | ||
| - Automated Cache Coherency (D-Cache cleaning/invalidating for G4/H7) | ||
| - Protocol-specific DMA calls (`HAL_SPI_Transmit_DMA`, etc.) |
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.
The README claims the DMA tool uses protocol-specific calls like HAL_SPI_Transmit_DMA, but the current DMAControl::Transfer implementation uses HAL_SPI_TransmitReceive_DMA for SPI. Either update the documentation to match the implementation, or update the DMA helper to select Transmit_DMA / Receive_DMA / TransmitReceive_DMA based on which buffers are provided.
| - Protocol-specific DMA calls (`HAL_SPI_Transmit_DMA`, etc.) | |
| - Protocol-specific DMA calls (e.g., `HAL_SPI_TransmitReceive_DMA` for SPI) |
| void SPI_Write(uint8_t *data, uint16_t len) { | ||
| if(io_driver.Write) io_driver.Write(data, len); | ||
| } |
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.
These wrapper functions become silent no-ops when MX66_Init() hasn’t been called (function pointers are null). That makes failures hard to diagnose and is a breaking behavior change vs the prior direct-HAL implementation. Consider providing default HAL-backed implementations in io_driver, or emitting a clear error/assert when the driver is used before initialization.
| HAL_SPI_Transmit(&hspi1, (uint8_t*)data, len, 100); | ||
| } else { | ||
| // Use DMA Tool for data | ||
| DMAControl::Transfer(&hspi1, 0, (uint8_t*)data, nullptr, len); |
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.
DMAControl::Transfer currently routes SPI to HAL_SPI_TransmitReceive_DMA(...), which returns HAL_ERROR if either TX or RX pointer is null. Here rxData is nullptr, so the DMA transfer will not run. Update the DMA helper to call HAL_SPI_Transmit_DMA for TX-only transfers (or provide a valid RX buffer), and check/handle the returned status.
| DMAControl::Transfer(&hspi1, 0, (uint8_t*)data, nullptr, len); | ||
|
|
||
| // BLOCKING WAIT: Protects the stack buffer 'tData' inside the driver | ||
| 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.
Unbounded busy-spin: while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); can hang forever if the SPI enters an error state or the DMA never starts. Add a timeout and handle HAL_SPI_STATE_ERROR (and consider yielding/delaying in RTOS builds).
MX66L1G45GMI/README.md
Outdated
| - Connects the Abstract Driver to the actual STM32 hardware | ||
| - Implements `IMPL_Write`, `IMPL_Read`, `IMPL_SetCS`, and `IMPL_Delay` | ||
| - Uses a hybrid strategy: Polling for small commands (≤ 32 bytes) and DMA for large data payloads | ||
| - note: Delay implementation on `MX66L1G45GMI_Interface.cpp` is subgect to change via |
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.
This bullet is incomplete (ends with "via") and contains a typo ("subgect" → "subject"). Clarify what mechanism is intended for changing the delay implementation (e.g., "...is subject to change via editing IMPL_Delay").
| - note: Delay implementation on `MX66L1G45GMI_Interface.cpp` is subgect to change via | |
| - Note: Delay implementation in `MX66L1G45GMI_Interface.cpp` is subject to change via editing `IMPL_Delay` below. |
| // 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.
Unbounded busy-spin: while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); can hang forever if the SPI enters an error state or the DMA never starts. Add a timeout and handle HAL_SPI_STATE_ERROR (and consider yielding/delaying in RTOS builds).
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.
| // Use DMA Tool | ||
| DMAControl::Transfer(&hspi1, 0, nullptr, data, len); | ||
| // BLOCKING WAIT | ||
| for (int TIMEOUT = 1000000; TIMEOUT > 0; TIMEOUT--) { | ||
| if(HAL_SPI_GetState(&hspi1) == HAL_SPI_STATE_READY) break; | ||
| MX66_Delay(1); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| //while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); | ||
|
|
||
|
|
||
| /* | ||
| Unbounded busy-spin: while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); can hang | ||
| forever if the SPI enters an error state or the DMA never starts. Add a timeout and handle HAL_SPI_STATE_ERROR | ||
| (and consider yielding/delaying in RTOS builds). | ||
| */ |
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.
IMPL_Read calls DMAControl::Transfer(&hspi1, ..., nullptr, data, len). DMAControl::Transfer for SPI uses HAL_SPI_TransmitReceive_DMA (DMA/DMATransfer.hpp:39), which expects a valid TX buffer; txData=nullptr will fail and no clocks will be generated for SPI reads. Use HAL_SPI_Receive_DMA (or update DMAControl::Transfer to use Receive_DMA when txData==nullptr), or pass a dummy TX buffer filled with 0xFF/0x00.
| // Use DMA Tool | |
| DMAControl::Transfer(&hspi1, 0, nullptr, data, len); | |
| // BLOCKING WAIT | |
| for (int TIMEOUT = 1000000; TIMEOUT > 0; TIMEOUT--) { | |
| if(HAL_SPI_GetState(&hspi1) == HAL_SPI_STATE_READY) break; | |
| MX66_Delay(1); | |
| } | |
| } | |
| //while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); | |
| /* | |
| Unbounded busy-spin: while(HAL_SPI_GetState(&hspi1) != HAL_SPI_STATE_READY); can hang | |
| forever if the SPI enters an error state or the DMA never starts. Add a timeout and handle HAL_SPI_STATE_ERROR | |
| (and consider yielding/delaying in RTOS builds). | |
| */ | |
| // Start SPI receive using DMA | |
| if (HAL_SPI_Receive_DMA(&hspi1, data, len) != HAL_OK) { | |
| // Optional: handle error (e.g., logging) if needed | |
| return; | |
| } | |
| // BLOCKING WAIT with timeout to avoid unbounded busy-spin | |
| for (int TIMEOUT = 1000000; TIMEOUT > 0; TIMEOUT--) { | |
| if (HAL_SPI_GetState(&hspi1) == HAL_SPI_STATE_READY) { | |
| break; | |
| } | |
| MX66_Delay(1); | |
| } |
| // Delay Implementation | ||
| void IMPL_Delay(uint32_t ms) { | ||
| // Change this line to switch between HAL_Delay, osDelay, vTaskDelay, etc | ||
| osDelay(ms); |
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.
README’s IMPL_Delay example uses osDelay(ms), but the current MX66L1G45GMI_Interface.cpp implementation uses HAL_Delay(ms). Update the README snippet (or the code) so the documented default matches the actual implementation.
| osDelay(ms); | |
| HAL_Delay(ms); |
MX66L1G45GMI: Add DMA-backed SPI interface and refactor driver