-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/MDMA #538
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: development
Are you sure you want to change the base?
Feat/MDMA #538
Conversation
… be problematic with resource management.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…-LIB into feat/Promises
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ansfers, still lacks decoupling from instance logic
…t sends it to the destination buffer
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
This PR introduces MDMA (Master Direct Memory Access) peripheral management for efficient data transfers using linked lists, along with a Promise-based asynchronous programming utility and supporting data structures (Pool and Stack). The implementation integrates into the HALAL layer and provides an MdmaPacket class for packet serialization/deserialization.
Key Changes:
- New MDMA class managing 8 hardware channels with linked-list mode transfers and queue-based request handling
- Promise utility for asynchronous operations with chaining, combinators (all/any), and memory pooling
- Pool and Stack container classes for fixed-size memory management
- MdmaPacket template class for MDMA-accelerated packet building and parsing
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
Src/HALAL/Models/MDMA/MDMA.cpp |
Core MDMA implementation with transfer management, interrupt handlers, and channel initialization |
Inc/HALAL/Models/MDMA/MDMA.hpp |
MDMA class interface with LinkedListNode helper and API declarations |
Inc/HALAL/Models/Packets/MdmaPacket.hpp |
Template-based packet class using MDMA for data transfers with Promise integration |
Inc/HALAL/Utils/Promise.hpp |
Promise implementation with atomic state management, chaining, and combinator methods |
Inc/C++Utilities/Pool.hpp |
Memory pool with fixed-size allocation, specialized optimization for pools ≤32 elements |
Inc/C++Utilities/Stack.hpp |
Fixed-size stack implementation |
Inc/C++Utilities/CppUtils.hpp |
Updated to include Stack and Pool utilities |
Src/ST-LIB.cpp |
Added MDMA::update() and Promise::update() to main update loop |
Src/HALAL/HALAL.cpp |
Added MDMA::start() initialization |
Inc/HALAL/HALAL.hpp |
Added includes for Promise and MDMA, contains commented-out duplicate include |
CMakeLists.txt |
Added stm32h7xx_hal_mdma.c driver source |
.vscode/settings.json |
Updated file associations for C++ standard library headers |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if (HAL_MDMA_GenerateSWRequest(&instance.handle) != HAL_OK) | ||
| { | ||
| instance.handle.State = HAL_MDMA_STATE_BUSY; |
Copilot
AI
Dec 14, 2025
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.
On error, the state is being set to HAL_MDMA_STATE_BUSY, which is incorrect. When HAL_MDMA_GenerateSWRequest fails, the state should be set to HAL_MDMA_STATE_READY (or another appropriate non-busy state) to allow the instance to be reused, and instance_free_map should be set back to true.
| instance.handle.State = HAL_MDMA_STATE_BUSY; | |
| instance.handle.State = HAL_MDMA_STATE_READY; | |
| instance_free_map[instance.id] = true; |
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.
Im setting it to busy, cause in case the transfer fails its better to stop the transfers than to continue with them with an error. Idk if this is a good view or not.
| void MDMA::update() | ||
| { | ||
| if(transfer_queue.empty()) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| for(size_t i = 0; i < instances.size(); i++) | ||
| { | ||
| if(transfer_queue.empty()) | ||
| { | ||
| return; | ||
| } | ||
| if(instance_free_map[i]) | ||
| { | ||
| Instance& instance = get_instance(i); | ||
| auto transfer = transfer_queue.top(); | ||
| instance.done = transfer.second; | ||
| prepare_transfer(instance, transfer.first); | ||
| transfer_queue.pop(); | ||
| } | ||
| } |
Copilot
AI
Dec 14, 2025
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.
Race condition in update() function. The function checks instance_free_map[i] and then calls prepare_transfer which sets it to false. However, if an MDMA interrupt fires between these operations and the TransferCompleteCallback sets instance_free_map[i] back to true, the state could become inconsistent. Consider using atomic operations or disabling interrupts during this critical section.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… be selected correctly
Removed the NODES_MAX definition and related comments.
New class to manage the MDMA peripheral. It works on linked list mode. The mdma recieves the first linked list node and will store it in a queue until the next stlib::update where each of the 8 mdma channels will process a request. The nodes need to be stored on a non-cached buffer or shit will go very wrong
To create a linked list node all you have to do is create a instance of the MDMA::LinkedListNode(args, buffer + offset,type_size);