Restructuring: ui_driver.c

[db4ple]

I would suggest to split ui_driver.c into several parts.
Rationale: It is way to big, several only loosely related activities are in there. Being so large, if will in future with more collaborators happen more often that they will work on the same file. While not being catastrophic, merge can be more painful.

Proposal:
One related to loading and saving configuration values. (Roughly 800 lines of code)
One related to anything truely UI (buttons, knobs, touchscreen) (many lines of code)
One related to support the RF and AUDIO (configuration of Si570, Code, filter activation etc.) (at least 2000 - 3000 lines of code).

I started marking functions for move to new rf/audio related files using TODO: comments. Work is not complete and it is not too urgent, but 10k+ lines in a single file is quite a lot to deal with.

[df8oe]

Hello Danilo,

I agree with the proposal you have done. It is loically intended and all coders will understand and identiy function by filename.

vy 73
Andreas

[s53dz]

It is a very good idea since this file is frequently visited. It could be of a great help to have it split. For safety as also for clarity.

73, Bojan S53DZ

[jkramarz]

Hi there,

Please bear in mind that this issue needs horrific amount of work.
Here is the call graph for void ui_driver_toggle_tx(void) function:

And almost every other looks just like that.

I've already started working on this issue few weeks ago to examine difficulties - with this amount of shared global state, and tight coupling it needs not only spliting but also designing proper architectural design. Furthermore, it makes 50% of project's code. There're also some dragons, like #95, which we'd encounter refactoring this code.

The bottom line is: this project needs refactoring of ui_driver.c, but it needs a plan and some architectural changes.

What do you think about changing direct function calls to some kind of a priority list of functions to call after each event, on which each module can register on load? Yes, of course - it'll take some (but not much!) memory and processing time, but hardware is cheaper than our time.

If you'd consider in this approach, I'm open to discuss it further.
If you're interested in, I can provide some proof-of-concept code (the pubsub system itself) in next week and changes to the project itself ~ at the end of March.

[db4ple]

Well, it is clearly the case that ui_driver is a beast.
I am currently exploring it and try to understand and simplify it step by step. Simplfication here does not involve major changes in call graphs, I just used opportunities for relatively simple deduplications and to introduce some kind of proper datastructures here and there etc. Once this is done, we should discuss architecture and go for the big restructuring. Problem I see with this is, this restructuring does not provide visible benefit to the end user for quite a while unless done either very quickly or very incrementally.

Anyway, I am completely aligned with what you say. What I suggested here is intended as a prelude to this.

One more thing: before we start doing restructuring on this level, I would like to have some kind of test and measurement system for certain performance critera in place, to ensure quick feedback regarding performance impacts after changes.

Why: The audio interrupt handler right now creates a load of at least 30% (with the "right" settings even more) Many of the interesting applications such as live decoding of digital modes will need even more performance.
If we convert this to a better architecture but loose to much performance, it will create frustration among the "non-programming" users for good reason. On the other hand, we have a 168 Mhz 32 bit machine, this is it is not like an Arduino. I am pretty sure, that with proper care, we gain performance from an improved architectural approach.

I am not so sure about how much "dynamic" loading/unloading we should have, except in some places such as the signal processing flow routing. In most cases, an semidynamic approach which is resolved at compile time is preferable for performance and memory consumption reasons.
The resource we have the smallest amount right now is ram. We are using about 60% of it now, more signal processing will eat more of this I guess, display buffering as well.

Simply having a separate display driver / UI thread which is interleaved with the "normal" application for instance would free a lot of performance (right now, lot of waiting for the SPI bytes to be transmitted), this would work very well with an event driven approach.

If we go into this direction, I would suggest to look into using an existing RTOS to provide the primitives for threads, interprocess communication and synchronization. I have been developing embedded operating systems many years, so I know a bit what this takes, I don't want to do that, there is plenty available. Especially FreeRTOS seems to be a candidate here since it only does the basic stuff which would allow us to use a lot of the existing hardware drivers from ST.

If your idea will meet these goals, I don't know. I am open for discussion, I am quite interested to see your proposal. But we should also listen to the others to not offend them.

[jkramarz]

One question, out of pure curiosity: how did you measured audio interrupt handler load? I can't recall any instrumentation in the code itself.

[db4ple]

That is no secret: I used the Power LED (on/off at start/stop of ISR) plus oscilloscope/logic analyzer.
The two LEDs pins (especially the power led, since no other use) are easily accessible.
The normal clock does not work, since it is interrupt based.

[s53dz]

The reason I supported the proposal thrown by db4ple was simply based on what was said in the proposal itself.

The last *_master *_version .27.0 of the file was 11.985 lines long, the last *_devel *_version is 9.702 lines long.

This alone is a good reason to split it in some reasonable units. But I think by doing this we should not increase the need to use more RAM, MC processing power. So by my opinion it should be done step by step. Polishing first. As it was said: RAM, flash, 165MHz are the ceiling merit for the time.

73, Bojan S53DZ

[db4ple]

Flash we have plenty. RAM not so much, and CPU cycles are wasted a lot, but to get these back, tghere is a need for some architectural change. But refactoring the architecture of unstable and/or unreadable software is an adventure I don't need for my well being.

[jkramarz]

There's a lot of commented out, work-in-progress or not used anywhere functions.
Eg. [ui_driver.c:8656] void UiSideToneRef(void) with no references in the project.
What do you think about starting from eliminating dead code?

[df8oe]

Ufff... I saw your work of the last night and now I know what I will do today :)

I do not want to comment each ull request so I deceided to nswer only HERE.

1. License
   I cannot change license because the project has already license and its:
   https://creativecommons.org/licenses/by-nc-sa/3.0/
   The text in splashscreen is grown and only a "hello" to users - not a license description.

2. Restructuring of Code
   We need a restructuring of code - but very careful! If we do make too much changes in a short time issues can be dig deep into the new code which re detected a few days later and so you must go back and dig into all changes - gorrible. We have had this 3 weeks ago and I do not want to have this much often.... So I will try to check every commit and maybe I test it a few hours before merging the next. You will see - of course I will do my very best to do everything ASAP ;)

3. Freeing pins
   There are more possibilities for freeing pins. The oldest one s now possible: using LCD in SPI instead of parallel mode. Speed of LCD now is nearly he same using SPI. During normaloperation there is no difference that blocks you from using SPI. I recommend not to do too big hardware modifications so I think we should use SPI mode for freeing pins.

4. Removing dead code
   In the past I have not removed dead code intensively but I think its now time to start this. We can ga back to versions where probably important parts are lost and get them back. So we should start with this!

5. PA9 usage
   As I implemented touchscreen function I just changed GPIO structure because of I do not want to break Clint's and Chris' debug. But now I decide that we should rewrite or remove this debig structure. It is very easy to debug using STLINK-V2 and we dont need printfs - that's my opinion. We should shortly iscuss this and then decide.

I am not sure I have answered all what happens here the last 1 hours but I hope. Now I will start reviewing and merging your work. Many thanks for all!

[jkramarz]

Just for the records: #98, #99, #100 mentioned here.