Skip to content

sys/flash_utils: helpers to store data in flash#18148

Merged
bors[bot] merged 9 commits intoRIOT-OS:masterfrom
maribu:sys/flash_utils
Feb 27, 2023
Merged

sys/flash_utils: helpers to store data in flash#18148
bors[bot] merged 9 commits intoRIOT-OS:masterfrom
maribu:sys/flash_utils

Conversation

@maribu
Copy link
Member

@maribu maribu commented May 30, 2022

Contribution description

This helpers that allow storing, accessing, and working with data in flash that works for both classical Harvard architectures (which do not map flash also into the address space) as well as modern Harvard architectures and von-Neumann architectures.

With this, examples/default again runs on the Arduino Uno / Nano. Since this board is still the "entry kit" for many people to embedded hardware, it would be nice to support it with our default example.

Testing procedure

examples/default should run and work on ATmega boards (especially ATmega328P and ATmega32U4 based boards) as well on all other boards now.

Issues/PRs references

None

@maribu maribu added the Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR label May 30, 2022
@maribu maribu requested a review from kYc0o as a code owner May 30, 2022 19:25
@github-actions github-actions bot added Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: SAUL Area: Sensor/Actuator Uber Layer Area: sys Area: System Platform: AVR Platform: This PR/issue effects AVR-based platforms labels May 30, 2022
@maribu maribu changed the title [PoS] sys/flash_utils: helpers to store data in flash [PoC] sys/flash_utils: helpers to store data in flash May 30, 2022
benpicco
benpicco reviewed May 31, 2022
View reviewed changes
kaspar030
kaspar030 requested changes May 31, 2022
View reviewed changes
Copy link
Contributor

@kaspar030 kaspar030 left a comment

Choose a reason for hiding this comment

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

I don't think this is worth the maintenance overhead. It makes code more unique, less readable, less idiomatic, less understandable, for all platforms. All to save RAM on low-end boards like ATmega328P and ATmega32U4.

There was an understanding that RIOT does support Atmegas somewhat, but won't compromise too much for that support. This PR crosses that line.

There are many reasons:

  • _flash et all, where do we use that? Are we gonna request new PRs to use that and flash_utils where appropriate? I guess we do agree that those at least decrease readability and increase the uniqueness of our codebase, which is rarely a good thing. And this forces a specific coding style just for the benefit of an obsolete architecture...

  • the mapped functions (strlen, strcmp, ...) are difficult enough to get right with the combination of multiple libcs, multiple compilers, multiple intrinsics (libgcc, compiler-rt), multiple optimization levels. Adding another layer on top is opening a can of worms.

  • macros. they seem to creep in lately. but they suck. XFAs just doesn't work without, but are pretty contained, there's no other way, and they (potentially) provide huge benefits. In rare cases, macros increase readability, at a cost. There's "standard macros" (ARRAY_SIZE) which almost go by as idiomatic C. But wrapping puts() and printf()? please no! PRIsflash is also way too special for my taste.

@maribu maribu force-pushed the sys/flash_utils branch from c676e7b to 6f91e43 Compare June 1, 2022 21:00
@github-actions github-actions bot added Area: BLE Area: Bluetooth Low Energy support Area: doc Area: Documentation Area: examples Area: Example Applications Area: pkg Area: External package ports Area: tests Area: tests and testing framework labels Jun 1, 2022
@benpicco benpicco mentioned this pull request Jun 2, 2022
Merged
@maribu maribu force-pushed the sys/flash_utils branch from 6f91e43 to 2a45961 Compare June 28, 2022 12:25
@maribu maribu changed the title [PoC] sys/flash_utils: helpers to store data in flash sys/flash_utils: helpers to store data in flash Jun 28, 2022
@github-actions github-actions bot removed Area: BLE Area: Bluetooth Low Energy support Area: pkg Area: External package ports Area: doc Area: Documentation Area: examples Area: Example Applications labels Jun 28, 2022
@maribu maribu added CI: full build disable CI build filter CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Feb 23, 2023
kfessel
kfessel reviewed Feb 23, 2023
View reviewed changes
kfessel
kfessel reviewed Feb 23, 2023
View reviewed changes
@maribu maribu removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 23, 2023
@maribu
Copy link
Member Author

maribu commented Feb 23, 2023

🎉 This now passes a full Murdock run :)

@benpicco
Copy link
Contributor

benpicco commented Feb 24, 2023

This needs a rebase

maribu
maribu commented Feb 27, 2023
View reviewed changes
miri64
miri64 reviewed Feb 27, 2023
View reviewed changes
@maribu
sys/flash_utils: add helpers for placing variables in flash
4e3c077 
This adds a layer of convenience abstraction over classical Harvard
architectures (like most AVRs) that do not map the flash memory into
the data address space and modern Harvard architectures or von-Neumann
architectures that do so. The motivation is to safe a lot of RAM for
AVR by storing constant strings into flash.
@maribu
pkg/cryptoauthlib: Fix compilation with -Wformat-nonliteral
75f17b4
@maribu
cpu/avr8_common: Wrap stdio.h
c406f7e 
This allows automatically moving format strings to flash, provided that
code previously compiled fine with `-Wformat-nonliteral` (which in RIOT
is the case due to `-Wformat=2`).
@maribu
sys/shell: use flash_utils
2825f5f
@maribu
sys/phydat: use flash_utils
29cfeb7
@maribu
drivers/saul: use flash_utils
cdcec5b
@maribu
examples,tests: Update Makefile.cis
7e58bea
@benpicco
Copy link
Contributor

benpicco commented Feb 27, 2023

Bors merge

@bors
Copy link
Contributor

bors bot commented Feb 27, 2023

Build succeeded:

@maribu
Copy link
Member Author

maribu commented Feb 27, 2023

🎉 💃 🕺 🎊

@bors
Copy link
Contributor

bors bot commented Feb 28, 2023

try

Merge conflict.

@maribu
Copy link
Member Author

maribu commented Feb 28, 2023

bors wtf?

@jia200x jia200x mentioned this pull request Apr 26, 2023
Merged
@Teufelchen1 Teufelchen1 mentioned this pull request Apr 2, 2024
Merged
@leandrolanzieri leandrolanzieri mentioned this pull request Jan 13, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 left review comments

@kfessel kfessel kfessel left review comments

@benpicco benpicco benpicco approved these changes

@kaspar030 kaspar030 kaspar030 approved these changes

@fabian18 fabian18 fabian18 left review comments

@kYc0o kYc0o Awaiting requested review from kYc0o kYc0o is a code owner

@smlng smlng Awaiting requested review from smlng

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@nandojve nandojve Awaiting requested review from nandojve nandojve is a code owner

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

Assignees

No one assigned

Labels

Area: build system Area: Build system Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: examples Area: Example Applications Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports Area: SAUL Area: Sensor/Actuator Uber Layer Area: sys Area: System Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: AVR Platform: This PR/issue effects AVR-based platforms Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care.

Projects

None yet

Milestone

Release 2023.04

Development

Successfully merging this pull request may close these issues.

8 participants

@maribu @kfessel @benpicco @riot-ci @kaspar030 @miri64 @fabian18 @MrKevinWeiss