-
-
Notifications
You must be signed in to change notification settings - Fork 691
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.
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
Remove unsupported settings if a device doesn't support these settings / WIP PoC #1776
base: dev
Are you sure you want to change the base?
Conversation
…ed feature enabled only
Ah; yes this is on the todo for 2.23. This design breaks the all settings shall always have same id's and space on all devices. Longer term plan (after oled updates) is to end up with 3 sets of data that is pre-processed in python:
Since then we can tag out at build time if something is used or not; and then omit it entirely. As the bigger waste of space than this is the translation strings that are not used on the TS80P. In terms of the pr, the index list doesnt end up in flash, those are just squashed to constant numbers during compile time; and we use them to ensure that in all the places we reference a setting its at the same offset. In general its flash thats limited rather than rom; ergo why I dont trim or mess with the settings structure. If I have an end goal its really to discontinue most of whats in |
TL;DR: It's stalled but ongoing. Sorry that I keep it open so long without any related activity. But to whom it may concern: if no one minds, I still would like to try & to test a couple of approaches addressing the issue in the future as soon as I will get enough of time to dig into this more deep & thoroughly. Thanks. |
What kind of change does this PR introduce?
Remove "Profile Support" related data fully for unsupported hardware.
What is the current behavior?
"Profile Support" related data populates internal structs which takes precious RAM & ROM.
What is the new behavior (if this is a feature change)?
"Profile Support" related data removed & a bit more space available for more useful features.
Other information:
TL; DR: over-using of RAM & ROM for unused options should be addressed, in one way or another. Here is my rant...
Hello. I ended up with this as always by accident. I worked on adding one tiny additional boolean option into settings. As always, it doesn't fit for
TS80P
:(I mean, without any managing code for a setting itself, just proper initialization in structs & "registration" in menu making a binary run out of size. Not knowing what else to optimize, I decided to make some kind of PoC. Yes, I know, it's horrible, awful, terrible, hacky, and so on and so forth. But the concept itself seems working: removing settings which are just physically not on a device allows to save not only flash but RAM as well.
As a proof for this proof-of-concept, here is all the successful builds with this PoC/WIP patch on top of discip/off-icon branch.
Tossing settings-related structs filled with unused data between RAM & ROM is getting a luxury on
TS80P
. However, I do realize that the way how this patch looks now it's barely possible to see it ending up in upstream. But I like to be a team player, I really do. Plus, the last thing I would want is to support some set of patches separately.So, what are the technical realistic options?
configure.h
their own pair ofSettingsOptions
/SettingsItemIndex
(painful to debug in a case of problems)SettingsOptions
/SettingsItemIndex
(half)dynamically (just like assertions insideTranslation.LANG.cpp
through python script but not sure on that as well)Oh, and I have a couple of questions regarding those structs..:
SettingsOptions
is just a list of settings, physically values are located insidesystemSettingsType
struct? So, basically planar (as opposite to nested) allocation for settings stored on the flash is the following:SettingsItemIndex
mainly needed for translation purposes? Basically, it says thatSettingsItemIndex::SleepTemperature
holds long & short descriptions for sleeping temperature, right? But couldn't this struct has the same order of elements or maybe even share the same storage for indexes betweenSettingsItemIndex
&SettingsOptions
since the lists are identical and just representing IDs of settings elements?Oh, and I know that recent builds from
gui-dispatches
reducing binary size forTS80P
with very impressive numbers. But with this model we eventually going to the moment when every byte will be matter. And I just really like to make my hardware keep up & working as long as possible. Oh, and speaking about this and that "off icon" update. I like clear & obvious icons, notifications & messages in interfaces myself but if it will come to the choice between "eyecandy" interface and functionality I personally always prefer the latter. So, if adding "off icon" into source code takes so much space that now some builds are not fit, then maybe is it possible to make this change optional (i.e. on by default but possible to off at compile time to save space)?As always, thanks for any feedback a lot. For the past few weeks I already learned so much!
/cc @Ralim & @discip