IronOS icon indicating copy to clipboard operation
IronOS copied to clipboard

Remove unsupported settings if a device doesn't support these settings / WIP PoC

Open ia opened this issue 11 months ago • 3 comments

  • Please check if the PR fulfills these requirements
  • [x] The changes have been tested locally
  • [x] There are no breaking changes
  • 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?

  • add exceptions for every may-be-not-presented hardware setting (i.e., Hall Sensor, BLE, DC, etc.) in that mixture of python scripts & c/cpp headers (probably not an option)
  • holding for every device through configure.h their own pair of SettingsOptions / SettingsItemIndex (painful to debug in a case of problems)
  • generate the pair of SettingsOptions / SettingsItemIndex (half)dynamically (just like assertions inside Translation.LANG.cpp through python script but not sure on that as well)
  • ... something else which I'm probably missing?

Oh, and I have a couple of questions regarding those structs..:

  • is my understanding correct that while SettingsOptions is just a list of settings, physically values are located inside systemSettingsType struct? So, basically planar (as opposite to nested) allocation for settings stored on the flash is the following:
uint16_t versionMarker;
uint16_t value with length of settings
uint16_t value of soldering temp setting
uint16_t value of sleeping temp
uint16_t value of sleeping time
...
uint16_t value of Profile Cooldown Speed
uint16_t value with length of settings
uint32_t value with padding of FFFFFFFF
  • is my understanding correct that SettingsItemIndex mainly needed for translation purposes? Basically, it says that SettingsItemIndex::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 between SettingsItemIndex & 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 for TS80P 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

ia avatar Aug 04 '23 04:08 ia