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

Start migrating to protobuf board configs #970

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Start migrating to protobuf board configs #970

wants to merge 14 commits into from

Conversation

bsstephan
Copy link
Contributor

@bsstephan bsstephan commented Apr 14, 2024 •
edited
Loading

This adds a post-build CMake command (thanks @FeralAI!) which uses gp2040ce-binary-tools to write a Protobuf board config into the .bin and .uf2 files generated by the normal build.

A Protobuf board config is a second reserved space at the end of the flash, essentially the exact same idea as the user config, just in an adjacent block of space. It is used to replace the Protobuf defaults with specified values (for now, as a demo, just the boardVersion version number) from the build, in order to convey a specific configuration with less C++ code in initUnset....

For the moment this doesn't undo anything material in initUnset..., but with this in place, the next step will be setting defaults in the config.proto file and overriding them with specific board config JSON files (when applicable) and removing the defines and C++ code accordingly. With time, then, all of the define-based config initialization code can go away, and we can stop doing per-board builds in GitHub. This doesn't interfere with initUnset... until then, as whatever is not specified gets initialized normally.

We won't need a .h file to understand board defaults anymore, because it'll be there, self-documented in the binary and viewable via gp2040ce-binary-tools. This should help diagnosis and maintenance, and hopefully we can get some vendors on board with this methodology (once all the porting is done) and make it easier for them to configure and ship properly-configured binaries without having to maintain code.

Worth noting that the board config is read by the firmware, but not modifyable, meaning the user config can be cleared to return to board defaults, same as before.

Again this is just step one of the process, but this is the step that lets us start strangling off the BoardConfig.h way of doing things.

NOTE: support for legacy (pre-protobuf) configs had to be removed because of competing expectations of what it meant for loading the config to "fail".

@bsstephan
Copy link
Contributor Author

Marked this as a draft because I still have some testing to do, and because I also first want to see how the board builds go in GitHub.

@bsstephan bsstephan marked this pull request as ready for review April 14, 2024 16:30
@bsstephan
Copy link
Contributor Author

bsstephan commented Apr 19, 2024 •
edited
Loading

Short summary of what this does, to encapsulate the details above:

  1. Removes the legacy pre-protobuf config code.
  2. Introduces JSON representations of the protobuf config as board configs --- boardVersion is meaningless to the running code and gets replaced in the user config, so this is just for a proof of concept so far.
  3. Patches the protobuf board config into the UF2 file created by the build.
  4. Changes the config loading at boot to: first load protobuf defaults, then load the board config from flash (written by the UF2), then load the user config from flash (which works as it always has)
    1. The old loader, as an aside, would try to load the user config, or load the protobuf defaults if that failed. That was a fallback process, this is more of an overlay process.
  5. Call initUnset... to fill in anything not handled by the board or user configs. As before this overrides defaults and leads to per-BoardConfig.h settings getting saved to the user config.

This does not:

  • Move away from UF2s; they are still built and the primary distributable.
  • Replace BoardConfig.h; that may happen in the future, if and only if board-config.json can cover everything we need.
  • Change the upgrade behavior; initUnset... still functions as before.

It does:

  • Introduce board-config.json in the middle of the config builder; so we can use protobuf features/definitions rather than having to also create duplicative C define directives.
  • Start defining more default values in config.proto; so that board configs only have to provide deltas from the norm, and furthering the amount of C defines we don't need anymore.
  • The above does mean we can start whittling down BoardConfig.h files as we find things they don't need to specify because they are now done via native protobuf.

@bsstephan
Copy link
Contributor Author

Future PR will include examples of what it looks like to whittle down the configs, as things are removed or replaced with protobuf representations, and will possibly/probably also include a tool or script or something to convert a whole .h to .json.

@arntsonl
Copy link
Contributor

Just a further note for anyone else reading this (including myself!), this is a step-us-in-the-right-direction PR change.

This is what is leading us down the road of using .json as configs, and then the UF2 generation will be somewhat seamless. So our builds right now are this:

  1. Include the boardconfig.h for the given "board name"
  2. Rebuild the entire C++ SDK
  3. Generate the uf2 from this setup

This will change us to:

  1. Build the entire C++ SDK
  2. Generate the uf2
  3. Transfer the .json configs into the uf2

Our build times will go down significantly and it will be much easier to build a custom config uf2 for non-technical users.

Going to wait on other folks before I merge this one, but code looks good!

@arntsonl
Copy link
Contributor

This will be a 0.8.0 feature as we will completely remove legacy config at this point.

@arntsonl arntsonl marked this pull request as draft May 3, 2024 14:14
@arntsonl
Copy link
Contributor

arntsonl commented May 3, 2024

Moving this to draft as we have a lot of things to do for protobuf

@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch from a8b1989 to b12dec5 Compare May 11, 2024 20:52
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch from b12dec5 to f57b206 Compare May 21, 2024 17:24
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch 2 times, most recently from 7c38545 to a205bae Compare June 11, 2024 21:34
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch 3 times, most recently from 5a6e16b to 60bffba Compare June 17, 2024 21:56
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch from 60bffba to 090b91b Compare July 21, 2024 13:49
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch from 090b91b to c06d605 Compare August 16, 2024 20:29
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch 5 times, most recently from 2c751a7 to 3542920 Compare October 1, 2024 12:53
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch from 3542920 to 2454252 Compare October 4, 2024 20:30
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch from 2454252 to 441031a Compare October 17, 2024 18:31
@bsstephan
super high level intent of the define removal
3cbade7
@bsstephan
remove support for legacy configs
14b42cc 
users should migrate to the previous release first, if they want to be
supported without redoing their config
@bsstephan
support loading the board config if the user config is not present
8b978db 
this also adds a sanity check for the now-expanded size of the storage
sections in the flash
@bsstephan
remove the build_info for compile-time pin defines
b1db8e2 
this is no longer relevant in the protobuf-as-board-config world
@bsstephan
break up the board/user config loaders and splice them together
d4372df 
this lets a board config patched into the binary to specify values that
are not in the user config, and have them be applied to the user config.
essentially, on board boot, we now:

1. pb_decode in loadBoardConfig
2. this loads the defaults
3. this then populates the structure with values from the board config
4. pb_decode_ex in loadUserConfig
5. this does NOT load the defaults
6. this then splices values from the user config into the structure

the BoardConfig.h files and initUnset... remain, but this will allow us
to move off of that as we start getting tooling in place for the
protobuf board configs
@bsstephan
DRY the config loader, and always start with initialized config
624a77a 
it was possible to never call pb_decode to initialize the config with
defaults (e.g. if both the board and user configs were empty), this
avoids that, in which case the calls are essentially identical
@bsstephan
start replacing some initUnset... with proto defaults
186ba99 
I think this is going to be a long road, but this is the kind of thing
that's possible even without a board config in place
@bsstephan
add a post-build command to patch board config into .bin/.uf2
026850d 
this starts the replacement of BoardConfig.h + migration code with
shipped protobuf configs. this should help cleanup some code, and one
day will hopefully replace the need for per-board builds entirely,
greatly speeding up the GitHub build/creation of board binaries
@bsstephan
remove some inits that just set the protobuf default
d240ae8
@bsstephan
only patch the UF2, and keep a backup of the old one
1431951 
also spit out the summary info just so people can see what happened
happened
@bsstephan
copy the .proto files into one location before running concatenate
a69389c 
something about the protoc compiler (maybe just on Python 3.12?) doesn't
like multiple -P path arguments, and until that's fixed, it's easier to
just copy the files into one spot, which works fine
@bsstephan
add more board-config.json placeholders for newer boards
0a37f1a
@bsstephan bsstephan force-pushed the start-migrating-to-protobuf-board-configs branch from 441031a to 0a37f1a Compare December 12, 2024 22:36
@bsstephan bsstephan marked this pull request as ready for review December 13, 2024 17:41
@bsstephan
add -P to summarize-gp2040ce invocation and fix a cmake idiom
9e17785
@bsstephan
compile .proto files with protoc so gp2040ce-binary-tools doesn't hav…
852cd32 
…e to

this hopefully gets around the weird bug with Windows stuff that seems
to be caused by gRPC and/or dynamically compiling the files as part of
gp2040ce-binary-tools invocations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bsstephan @arntsonl