Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Support for Storage Operations #2901

Merged
merged 18 commits into from
Mar 21, 2024
Merged

Add Support for Storage Operations #2901

merged 18 commits into from
Mar 21, 2024

Conversation

Ellerbach
Copy link
Member

@Ellerbach Ellerbach commented Mar 12, 2024 •
edited by josesimoes
Loading

Description

  • Implement Support for Storage Operation.
  • Add implementation for ESP32 using internal storage.
  • Add implementation for ChibiOS (STM32) using SPIFFs.

Motivation and Context

How Has This Been Tested?

  • Tested on a real ESP32.
  • Tested on a ORGPAL3 target.

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

@Ellerbach Ellerbach requested a review from josesimoes March 12, 2024 14:07
@Ellerbach Ellerbach marked this pull request as ready for review March 13, 2024 15:19
@networkfusion
Copy link
Member

Is their any reason why this a Hardware Abstraction rather than a Platform Abstraction?

josesimoes
josesimoes reviewed Mar 14, 2024
View reviewed changes
Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Still some work required, but overall, that's it.

CMakeLists.txt Outdated Show resolved Hide resolved
src/HAL/Include/nanoHAL_StorageOperation.h Outdated Show resolved Hide resolved
src/CLR/Debugger/Debugger.cpp Outdated Show resolved Hide resolved
@Ellerbach
Copy link
Member Author

Is their any reason why this a Hardware Abstraction rather than a Platform Abstraction?

I mimic the Configuration Manager which was HAL. I guess it does not have much impact.

@josesimoes
Update declaration to match managed class
4e4c80f
@josesimoes
HAL_StorageOperation now returns execution result on return type
0163672 
- Update code accordingly.
@josesimoes
Add sanity check for malloc
aa39e72
@josesimoes
Add platform error to enum
a5275b9 
- To pass generic information about platform related errors.
@josesimoes
Minor changes and tweaks in code
394f788 
- Add enums where used.
- Replace for loop with memcpy call.
- Add verbose comments to make it super easy to port to other platforms.
@josesimoes josesimoes force-pushed the add-support-storageoperation branch from a65c4a1 to 394f788 Compare March 20, 2024 22:46
@josesimoes
Fix comment
ebc5159
@josesimoes
Rename feature to HAS_ACCESSIBLE_STORAGE
115d0cc 
- Update code and CMake files accordingly.
@Ellerbach
Copy link
Member Author

Good to go! Thanks for the improvements on my initial code!

@josesimoes
Copy link
Member

I'm adding implementation for this to ORGPAL target before merging.

@josesimoes
Copy link
Member

Need to remove the command from nanoBooter. It really adds a lot of overhead and bloats the size of nanoBooter! 😯
Like the majority of the other commands it will be only available in the CLR not booter and that's OK.

@josesimoes
Remove command from Monitor command set
0f8a476 
- Adding it completely bloats nanoBooter image size.
@josesimoes
Remove duplicated include
36321dc
@josesimoes
Remove include from booter on all targets
63075e2
@josesimoes
Add implementation to ChibiOS using SPIFFs
828521e 
- Enabled it for ORGPAL3, which is the only target that supports this.
@josesimoes josesimoes enabled auto-merge (squash) March 21, 2024 15:57
@josesimoes josesimoes changed the title Add Support for Storage Operation Add Support for Storage Operations Mar 21, 2024
@josesimoes josesimoes merged commit e79c5fa into main Mar 21, 2024
23 checks passed
@josesimoes josesimoes deleted the add-support-storageoperation branch March 21, 2024 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@networkfusion networkfusion networkfusion left review comments

@josesimoes josesimoes josesimoes approved these changes

Assignees
No one assigned
Labels
Area: Config-and-Build Type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Ellerbach @networkfusion @josesimoes @nfbot