py/mpconfig.h: Define templates for "feature levels". #7655
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7655 +/- ##
==========================================
- Coverage 98.24% 98.24% -0.01%
==========================================
Files 154 154
Lines 20043 20042 -1
==========================================
- Hits 19692 19691 -1
Misses 351 351
Continue to review full report at Codecov.
|
py/mpconfig.h
Outdated
| #define _MICROPY_CONFIG_ROM_LEVEL_COMMON (MICROPY_CONFIG_ROM_LEVEL >= 2) | ||
| #define _MICROPY_CONFIG_ROM_LEVEL_CONVENIENCE (MICROPY_CONFIG_ROM_LEVEL >= 3) | ||
| #define _MICROPY_CONFIG_ROM_LEVEL_ALL (MICROPY_CONFIG_ROM_LEVEL >= 4) | ||
| #define _MICROPY_CONFIG_ROM_LEVEL_EVERYTHING (MICROPY_CONFIG_ROM_LEVEL >= 5) |
There was a problem hiding this comment.
'all' and 'everything' are quite similar; use 'coverage' for the latter?
Wrt 'rom level' naming: since it's a set of feature levels, perhaps just name it like that (MICROPY_FEATURE_LEVEL) as well?
I can imagine that instead of the single-line description of which level has what, there'd be more elaborate comments. In that case the explanation of which one does what is going to be further away from where it gets defined. And now you have to repeat the numbers. How about moving those comments here, together with their respective definition? I.e.
// Only enable core features (constrained flash, e.g. STM32L072)
#define _MICROPY_CONFIG_ROM_LEVEL_CORE (MICROPY_CONFIG_ROM_LEVEL >= 1)
Perhaps overkill, but people might want to adopt this principle and define their own levels which are somewhere in between existing ones, which can be done by leaving gaps here i.e. 10, 20, 30, ...
There was a problem hiding this comment.
'all' and 'everything' are quite similar; use 'coverage' for the latter?
Yep, I think you're right.
Wrt 'rom level' naming: since it's a set of feature levels, perhaps just name it like that (MICROPY_FEATURE_LEVEL) as well?
I did want to also add RAM levels too (e.g. rp2 is ram but not flash constrained), but I think given that ROM generally maps to features, this makes sense. My reasoning for calling it ROM was to emphasize that we're "saving ROM", not "cutting features" (although I agree that's a fairly abstract distinction).
Perhaps overkill, but people might want to adopt this principle and define their own levels which are somewhere in between existing ones, which can be done by leaving gaps here i.e. 10, 20, 30, ...
I guess so... perhaps this could be useful for board-specific overrides. I wanted to avoid having to decide "is this as 27 or 26 level feature discussions" in the future.
On that though, I think it's worth putting #defines for the levels (somewhere before mpconfigport.h gets imported) so that the numbers themselves are not meaningful. (i.e. minimal/mpconfigport.h should set it to "MINIMAL" not "0").
2fb8bca to
fa5f21a
Compare
|
Updated the branch:
I think it's still worth separating full from everything. Full would be a board with unlimited flash, but there might still be features that we might not enable that only m 8000 ake sense for coverage (extra debugging, etc). Can always remove it if we don't end up using it. Maybe when we add the RAM levels, it'll make sense that the "everything" level is combined. |
|
If this goes towards addressing #5564 then it's a big step forward. |
ef34ec5 to
f0b1cf0
Compare
Defines the "core" level as the current default feature set, and "minimal" to turn off everything. Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Update minimal port to use the new "minimal" rom level config (this is a no-op change the binary is the same size and contains the exact same symbols). Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
f0b1cf0 to
169fc6f
Compare
|
Updated anything enabled via level=core that was dependent on a parent option also enabled via level=core to just be unconditionally enabled by default. i.e. |
Enable bus device on hallowing M0
This is a proposal/RFC for a larger piece of work (related to #7538) to simplify enabling/disabling features.
My goals are:
This PR just makes a step towards this and defines the "core" level as the current default feature set, and a "minimal" level to turn off everything (and updates just the minimal port to use it). It highlights a few things that are enabled on the minimal port that perhaps shouldn't be (~2kiB).
I've added a couple of placeholder "levels" as candidate suggestions for where the other ports will roughly land.
This is a no-op change (the x86/arm minimal binary is the same size and contains the exact same symbols). All other ports unchanged.