Implement printSymbolDeg() helper function as method for OLED class

[ia]

• Please check if the PR fulfills these requirements

• The changes have been tested locally
• There are no breaking changes

• What kind of change does this PR introduce?
  Implement helper function for repetitive blocks of code.

• What is the current behavior?
  The same snippet used a few times in code base to print degree symbol & temperature symbol.

• What is the new behavior (if this is a feature change)?
  Helper function to make snippet reusable in more convenient way.

• Other information:
  I have a couple of questions though.

Is my intuitive speculation correct that this block:

      if (getSettingValue(SettingsOptions::TemperatureInF)) {
        OLED::drawSymbol(0);
      } else {
        OLED::drawSymbol(1);
      }

is a full equivalent to this:

      if (getSettingValue(SettingsOptions::TemperatureInF)) {
        OLED::print(LargeSymbolDegF, FontStyle::LARGE);
      } else {
        OLED::print(LargeSymbolDegC, FontStyle::LARGE);
      }

I tried to figure it out myself but I couldn't dig out how exactly (according to which table of decoding chars?) charCode 0 turns into *F with large font inside drawChar. Could you, please, elaborate a bit on that? The point here is that if my assumption is correct, then the other part of code block could be put inside the helper function as well. Thanks.

And what you would say about putting all the functions from all files inside utils/ dir into one pair of files (like Utils.cpp / Utils.h ) instead? By brief calculations the resulting Utils.cpp should be about something ~500 lines of code which should be relatively manageable than having file-per-function approach. Just a question / suggestion from the side.

[ia]

I tried to figure it out myself but I couldn't dig out how exactly (according to which table of decoding chars?) charCode 0 turns into *F with large font inside drawChar. Could you, please, elaborate a bit on that?

Oh, wait... I think I figured it out myself. Just would like to get some confirmation here...

So, drawSymbol(0/1) goes to drawChar with EXTRAS font style, hence only short case block in the beginning of switch performed. And after switch basic pointers' math kicks in and either first 24 bytes or second 24 bytes from ExtraFontChars used which turns by OLED driver either into *F or *C on display in LARGE (since it's 12 & 16 in switch for EXTRA). Right?

Wow. What a trickery. But does OLED::drawSymbol(0) have any important win from the point of performance than OLED::print(LargeSymbolDegF, FontStyle::LARGE) ? Just curious. It can be "encapsulated" as is into printSymbolDeg() anyway.

[ia]

It can be "encapsulated" as is into printSymbolDeg() anyway.

Done. Just check this out and let me know what you think.

P.S. Haven't tested on the iron itself though.

[ia]

JFYI: I think I have at least one more commit to go so don't rush on merge yet.

[ia]

JFYI: I think I have at least one more commit to go so don't rush on merge yet.

Done. Let me know what you think. But I still have only two questions though:

1. What do you think about putting all the functions from all files inside utils/ dir into one pair of files (like Utils.cpp / Utils.h ) instead? If you agree, I could try to implement it as a separate PR (sorry for repeat, I bring it again only to be clear since I'm very curious about your opinion & approach to organize files).

2. It seems there is some data type incoherence across code base between int32_t, uint32_t & uint16_t for variables representing temperatures. Since memory is a huge factor here - once you confirm - I could try to make another one PR by making all the variables as uint16_t:

• this should be more than enough to keep the values of maximum temperatures possible on supported hardware anyway;
• this should save some RAM.

What do you think?

[Ralim]

JFYI: I think I have at least one more commit to go so don't rush on merge yet.

Done. Let me know what you think. But I still have only two questions though:

1. What do you think about putting all the functions from all files inside utils/ [dir](https://github.com/Ralim/IronOS/tree/dev/source/Core/Threads/OperatingModes/utils) into one pair of files (like Utils.cpp / Utils.h ) instead? If you agree, I could try to implement it as a separate PR (sorry for repeat, I bring it again only to be clear since I'm very curious about your opinion & approach to organize files).

Please dont. Much like Adam savage's thoughts on miscellanious drawers, I find "Utils.x" is where functions go to die rot. I prefer to have the files approximately 1:1 to functional to use to prevent undocumented dependencies. Additionally more files makes the build somewhat faster then a heavy handed mega file.
Also modern screens are wider than taller, so more "wide" functions > long files.

in my opinion of course 🙇🏼

2. It seems there is some data type incoherence across code base between [`int32_t`](https://github.com/Ralim/IronOS/blob/d95af7d1a0ddd5a93a0827d08294c5504c9e9a94/source/Core/Drivers/TipThermoModel.cpp#L74), [`uint32_t`](https://github.com/Ralim/IronOS/blob/d95af7d1a0ddd5a93a0827d08294c5504c9e9a94/source/Core/Drivers/TipThermoModel.cpp#L87) & [`uint16_t`](https://github.com/Ralim/IronOS/blob/d95af7d1a0ddd5a93a0827d08294c5504c9e9a94/source/Core/BSP/MHP30/BSP.cpp#L215) for variables representing temperatures. Since memory is a huge factor here - once you confirm - I could try to make another one PR by making all the variables as **`uint16_t`**:


* this _should be_ more than enough to keep the values of maximum temperatures possible on supported hardware anyway;

* this _should_ save some RAM.

What do you think?

I'm all for this.
However, could I ask to maybe avoid large refactoring for a ~week so that I can get the gui-dispatches branch refactor done as it touches ~= all oled drawing code 😁

[ia]

Please dont.

Sure, ok, no problem. Just asked.

Much like Adam savage's thoughts on miscellanious drawers,

Well, software is not hardware so not all approaches may be valid between these two worlds ;) ...

I find "Utils.x" is where functions go to die rot. I prefer to have the files approximately 1:1 to functional to use to prevent undocumented dependencies. Additionally more files makes the build somewhat faster then a heavy handed mega file. Also modern screens are wider than taller, so more "wide" functions > long files.

... BUT I see your valid points on that now! Thanks for your time explaining your vision. Ok... ok... just one (UPD: two) more question and I will stop to bother you on that: how about keeping everything as is except moving functions related to output on the screen under OLED::? My suggested logic here is that: "if there is some function printing something on the screen, then look for it in OLED.cpp". In particular, obvious candidates would be drawPowerSourceIcon.cpp : gui_drawBatteryIcon(), DrawTipTemperature.cpp : gui_drawTipTemp(), printSleepCountdown.cpp : printCountdownUntilSleep(), PrintVoltage.cpp : printVoltage(), and ShowWarning.cpp : warnUser()...
... but while I was typing it, I came with another idea... how about this:

• make new GUI class;
• put under GUI class "high level" functions/methods which have to display something by using multiple calls to different OLED:: methods at once;
• keep OLED class with its low-level methods (draw/print) but moving such methods as OLED::debugNumber & OLED::printSymbolDeg inside GUI:: (since these are not low-level OLED:: functions but high-level GUI:: related functions).

In this way there will be the following object-oriented logic:

• low-level OLED-related functions kept in OLED.cpp
• high-level GUI-related functions put in GUI.cpp (i.e. when you need to print something on the screen calling different OLED::func as building blocks to construct fancy GUI output for something).

Hence, the "architecture" of this approach intuitively will be looking something like:

• GUI::
  • OLED::
    • low-level display buffer stuff

Just give a thought for it.

DISCLAIMER! I'm just thinking out loud about my very humble knowledge of software architecture approaches and project/file organization, that's it. As always, curious about your opinion on my suggestions.

---

I'm all for this.

So, would you agree that 640K uint16_t for any temperatures related variables should be enough, right? Just want to be fully clear on that since you know better if there may be some nuances.

However, could I ask to maybe avoid large refactoring for a ~week so that I can get the gui-dispatches branch refactor done as it touches ~= all oled drawing code

Aha!.. Well, thanks a lot for this warning since I have an idea for some another one very messy pull-request addressing end-of-line types... TL; DR: every second file in the repo has DOS EOL/CRLF/\r\n while every first file has UNIX/Linux EOL/LF/\n, regardless current .gitattributes.

It's not a big issue but it's getting a bit irritating so I would like to work on that as well but since it affects huge amount of files - not their content per se but metadata only (luckily), still it probably will require a huge update of files. For the record: OS religion has nothing to do with that (I use all modern OSes on different hardware) but since the reference environment to build & CI is gnulinux inside docker, then it should be LF EOLs only (in my opinion) for the content in the github remote repo, but for local copies obviously it should allow to use any EOLs (stripping "foreign" ones on push). Started to work on that a couple days ago. BTW Subversion has very nice repo property "eol:native" which once being set for a repo, it does exactly that but with git/github it seems there are some nuances and already committed files must be updated forcefully.

So... where we were... oh, right. So, the bottom line is once I see gui-dispatches merged into dev, I can start to mess with data types and other things but not before, is that correct?

P.S. Thanks a lot for fixing those yellow stalled github jobs BTW!

[Ralim]

So, would you agree that 640K uint16_t for any temperatures related variables should be enough, right? Just want to be fully clear on that since you know better if there may be some nuances.

Yes, but inside calculations we often have to use uint32_t to avoid under/overflow. So cant blanket replace things.

So... where we were... oh, right. So, the bottom line is once I see gui-dispatches merged into dev, I can start to mess with data types and other things but not before, is that correct?

Ask me when its done, its part of a 3/4 set series I'm trying to get rolling, so will depend how the rest of the follow ups are going too. (see below)

---

So as part of / spinout from #1718 I have planned a multi-step process to get to my goal of code generated layout implementations:

1. Move as much of the GUI as possible to immediate mode dispatch (aka mega switch cases)
2. Refactor current OLED into just a raw drawing api, pull out all smart logic to drawing and widget's files
3. Create python or rust app that runs during build that converts pre-defined screen layouts into render lists for tooling in (2)
4. Extract interface from OLED class to allow for multiple implementations

So you seem to have been roughly working towards the same concept where I'm heading in your OLED refactoring question 😁 I'm just getting there in a slightly different order as I'm not certain of the interfaces for steps 2 and onwards yet, so trying to shake it down and build one at a time.

[ia]

Forgot to thank you for taking your time to provide & explain all of these technical details & future plans regarding OLED/refactoring. I do appreciate it a lot! Thanks. 🙏