Hi Markus :). I have already completed this task.

Utils was used to move some general stuff, but we had no time to review the architecture again, as we were trying to fix some things after GSoC2020 and discovered problems. Feel free to propose or add improvements. We can discuss anytime.

In T1216#17821, @eppisai wrote:

Hi Markus :). I have already completed this task.

@eppisai if you work on this please claim the task.
Have you commited a PR already?

No, I wanted to check on real board before PR,so was waiting. Had completed it last month. Will create P.R for this and Keyboard today.

In T1216#17827, @sebastian wrote:

In T1216#17821, @eppisai wrote:

Hi Markus :). I have already completed this task.

@eppisai if you work on this please claim the task.
Have you commited a PR already?

I get your point, my P.R for task wasn't made,so saying task was complete would be wrong on my part. sorry.

@sebastian , @BAndiT1983 I am really sorry, I called this task complete (i thought it was), I have not been confident enought to raise proper pull request, that I should have done way earlier, and I would be been having this discussion earlier, instead of jumping to another task. Please help me devise the right approach in this regard.

My understanding and Approach

1. Painter - Only Responsible for populating framebuffer.

2. Utilities - Group of different functions used to assist object(s), hence we are using namespace, instead of class (since we don't need encapsulation or abstraction)

Since, current font that is selected ,setting the font (or changing) and calculating text width does not involve manipulating framebuffer directly, so they should not be placed in painter.

What I thought is a good approach, setfont() , textwidth(), current font selected, height of current font all involves FontSettings.
so instead of placing it in utils, we have an class of CharacterFormat.h (or FontSettings.h or TextSettings.h), which involves setting the fontstyle, selecting fontsize, and other Character formatting related methods and attributes that we might add or need in firmware, in future.

Advantage -

1. like all normal User interfaces, if we'll give option to change fontsize in settings of remote to user, then CharacterFormat.h class might be the right way.
2. Since current font and current font height is only variables that is needed in other objects of the firmware (and these variables do not belong to any class yet), and since they are current font size and style belong more to character formatting, so placing these variables (or settings) in painter would not align with Single Responsibility Principle(SRP)
3. We can have a class for global variables in firmware, that involves variables that are needed in various objects of the firmware, (unlike global settings because it stores variables involved in data abstraction of axiom remote not just firmware), but again these variables are being manipulated by other objects and currently we don't have a class or namespace for global variables, because we didn't need that (since variables are organized, as they always belong to some object according to their usage or properties)

• Font manager could help, but why is current font needed elsewhere than painter? Who uses it and for which reason?
  • Are you sure that current font is not belonging to some class? Was under impression that it belonged to painter before.

• Font size changes won't be done by user, don't see a necessity there

• We would like to avoid global variables, as they are very error-prone as everybody can rewrite values from anywhere and break application logic, but we have CentralDB which is a hybrid solution, so we can react to value changes

1. current font, and current font height, changes according to SetFont(). In painter we need current font height, while drawing text on screen, only. But the thing is, If we look at Numeric Value Screen , Image Button, Push Button, Toggle Button, Parameter List Screen, they also require the current font's height, and when whenever we set/change the font, the current font and hence current font height changes. So, I thought the sole purpose of current font and and its dimension (height) is not just drawing text on screen, but also in calculation of other objects of firmware, infact current font and its dimension(s) is more needed outside painter, same as GetTextWidth() .

2. I don't think providing option to change font size might be needed either ,(no other camera GUI's have this I checked). Was just saying in terms of advantages Font Manager might offer, if we ever need something like that.

3. I couldn't agree more. Global variables are not needed ("Abstractions should not depend upon details. Details should depend upon abstractions." - SOLID). But as you said, all the attributes pair's in CebtralDb (or global variables) involves observing changes by subject, which then dispatches those changes to observer that registered for those changes. So, placing current font dimension or font selected would not belong here, I thought.

Hence, I proposed and want to discuss the possiblity of FontManager.h, Since yesterday, I saw the logic of why and how things are organized the way they are in AXIOM Remote Firmware, the complexity of structure, the design (previous remote version was more functional oriented, and this is object oriented, hence I have wondered how to cameup with structure so organized) but reading SOILD principle gave me some thinking capacity in this direction.

I see there might be some confusion about the SetFont() usage. The purpose is not to allow a user to select the general size of fonts used in the remote (like operating systems let you set larger fonts that are easier to read for all software). The purpose is to allow drawing headers in a larger font than normal menu text for example.

Sorry, my English is rather bad, I was saying that for FontManager object, not SetFont() method. But as I have realized and after discussion I am sure, this feature was and is something, that is not needed, in axiom remote. But rest of the points still stand for FontManager .

In T1216#17838, @sebastian wrote:

I see there might be some confusion about the SetFont() usage. The purpose is not to allow a user to select the general size of fonts used in the remote (like operating systems let you set larger fonts that are easier to read for all software). The purpose is to allow drawing headers in a larger font than normal menu text for example.

I see there is a branch already, please look through it if anything is still to be cleaned up/improved, then please create a PR so code review and merge can follow.