Open
Conversation
…ight-FirmwareSource into fix_mem_alloc_issue
|
Board and firmware folder for this pull request: |
elral
changed the title
There was a problem hiding this comment.
Pull request overview
This PR addresses #374 by switching the device-buffer allocator to operate in bytes (via a uint8_t buffer) on 32-bit targets like RP2040, adding alignment-aware allocations, and updating device modules to allocate from the device buffer via new macros and null-checks.
Changes:
- Reworked
allocateMemto allocate from an aligneduint8_tdevice buffer with size/alignment parameters and simplified overflow checking. - Updated multiple modules’
setupArray()/ internal buffers to allocate usingMF_ALLOC_TYPE/MF_ALLOC_BYTESand handle allocation failures. - Changed
MFStepper::attach()to returnbooland added failure propagation to the Stepper registration path.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| src/MF_Stepper/Stepper.cpp | Switches stepper array allocation to new allocator macros and checks attach failure. |
| src/MF_Stepper/MFStepper.h | Changes attach API to return bool. |
| src/MF_Stepper/MFStepper.cpp | Allocates AccelStepper from device buffer and returns success/failure. |
| src/MF_Servo/Servos.cpp | Switches servo array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_Segment/LedSegment.cpp | Switches segment array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_Segment/LedControl_dual.cpp | Migrates internal buffers to MF_ALLOC_BYTES and adds null checks. |
| src/MF_OutputShifter/OutputShifter.cpp | Switches output shifter array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_OutputShifter/MFOutputShifter.cpp | Migrates _lastState buffer allocation to MF_ALLOC_BYTES. |
| src/MF_Output/Output.cpp | Switches output array allocation to MF_ALLOC_TYPE and adds null check (plus formatting). |
| src/MF_LCDDisplay/LCDDisplay.cpp | Switches LCD array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_InputShifter/MFInputShifter.cpp | Migrates _lastState buffer allocation to MF_ALLOC_BYTES. |
| src/MF_InputShifter/InputShifter.cpp | Switches input shifter array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_Encoder/Encoder.cpp | Switches encoder array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_DigInMux/DigInMux.cpp | Switches dig-in mux array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_CustomDevice/CustomDevice.cpp | Switches custom device array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_Button/Button.cpp | Switches button array allocation to MF_ALLOC_TYPE and adds null check. |
| src/MF_Analog/MFAnalog.cpp | Whitespace-only change. |
| src/MF_Analog/Analog.cpp | Switches analog array allocation to MF_ALLOC_TYPE and adds null check. |
| src/allocateMem.h | Introduces MF_ALLOC_BYTES / MF_ALLOC_TYPE macros and updates allocator API signatures. |
| src/allocateMem.cpp | Implements aligned byte-buffer allocator and updates available-memory reporting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Board and firmware folder for this pull request: |
|
Board and firmware folder for this pull request: |
|
Board and firmware folder for this pull request: |
|
Board and firmware folder for this pull request: |
|
Board and firmware folder for this pull request: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of changes
For Picos the device buffer is initialized with
std::size_twhich is 32bit, but only 8bit are used per memory location.This PR changes the device buffer to
uint8_talso for the Pico. But as also classes or 16bit arrays get allocated in the device buffer, the return parameter is nowvoid*and the calling function has to cast the return value to the correct size.This means that every
device.cppandMFdevice.cpphas to be changed.The check if the required memory can be allocated in the device buffer is simplified.
Further more it`s now checked if a stepper class can allocate the required memory and returns the result.
For the AVR's there is no change in allocating memory in the device buffer even the same changes in
device.cppapply. It's only differentiated inallocateMem.cpp/hto keep the code readable.This PR is tested with each device on a Mega and Pico.
Fixes #374