Skip to content
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

[WIP] Reduce ROM by define some helper functions in pages/ #625

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

howard0su
Copy link
Contributor

No description provided.

@howard0su
Copy link
Contributor Author

I did some analyze on the .text size of each C files, no rodata (no string counted). here is the result. Please check if anything catch your eyes. I am surprised on the size of model.c.

objs/devo7e/model.o Total | 8406
objs/devo7e/mixer_setup.o Total | 5346
objs/devo7e/telemetry.o Total | 3402
objs/devo7e/mixer.o Total | 3157
objs/devo7e/protocol.o Total | 2938
objs/devo7e/tx_configure.o Total | 2793
objs/devo7e/gui.o Total | 2528
objs/devo7e/usb_core.o Total | 2046
objs/devo7e/display.o Total | 2033
objs/devo7e/lcd_gfx.o Total | 2029
objs/devo7e/model_page.o Total | 1892
objs/devo7e/keyboard.o Total | 1889
objs/devo7e/tx.o Total | 1850
objs/devo7e/main_page.o Total | 1842
objs/devo7e/curves_page.o Total | 1760
objs/devo7e/mixer_curves.o Total | 1731
objs/devo7e/petit_fat.o Total | 1658
objs/devo7e/dialogs.o Total | 1628
objs/devo7e/inputs.o Total | 1586
objs/devo7e/model_config.o Total | 1518
objs/devo7e/telemtest_page.o Total | 1491
objs/devo7e/model_loadsave.o Total | 1467
objs/devo7e/pages.o Total | 1458
objs/devo7e/mixer_standard.o Total | 1338
objs/devo7e/mixer_limits.o Total | 1307
objs/devo7e/mixer_page.o Total | 1220
objs/devo7e/main_config.o Total | 1208
objs/devo7e/scrollable.o Total | 1194
objs/devo7e/curves.o Total | 1186
objs/devo7e/timer_page.o Total | 1168
objs/devo7e/trim_page.o Total | 1160
objs/devo7e/common_standard.o Total | 1124
objs/devo7e/timer.o Total | 1121
objs/devo7e/chantest_page.o Total | 1118
objs/devo7e/toggle_select.o Total | 1080
objs/devo7e/textsel.o Total | 1058
objs/devo7e/bargraph.o Total | 1048

@TheRealMoeder
Copy link
Contributor

According to the checks actually Rom size increase in some transmitters?

@howard0su
Copy link
Contributor Author

Need covert the code to use the new function.

@PhracturedBlue
Copy link
Contributor

the model.c has always been a problem. I've taken several stabs at optimizing it, but I never was able to get it as small as I'd like.
My last idea was to use DevationUploader to do all the work and to store the files on Tx as binaries. We'd compile them to match the actual memory layout during upload, and download/decompile/recompile/upload during a dfu update.
It would remove basically all 8k of code from model.c, but have the downside that Deviation-Uploader is required to work with files (I'd add a gui to work as a file explorer). It also requires being able to run the compile/decompile stages from Java on the host machine, so I was looking at a C->Javascript translator that could reproduce the binary structure. I got close to getting the latter part working, but it is a very complex system, and would be error prone. Anyhow, I gave up on that path. I'll be interested to see what you come up with.
I'd be open to a different file-format if it would actually help significantly, but it must remain human readable/editable (and we'd need to be able to provide a converter)

@PhracturedBlue
Copy link
Contributor

Also, I have:

utils/get_function_size.pl -rom devo8

Which may be helpful in identifying memory usage

@howard0su
Copy link
Contributor Author

howard0su commented Jan 21, 2019 via email

@hexfet
Copy link
Contributor

hexfet commented Jan 21, 2019

I'll throw model file versioning in as another concern. I think an external compile step might make it easier. One method I looked at is to have the ini read code apply any transformations necessary to convert an older model file into the latest Model definition. That didn't give a good solution for the cyclic AIL/ELE reversal because model.c reads each config value independently so correction has to wait till all the parameters are loaded. At that point correcting the cyclics requires scanning through all the mixers. The additional code needed to support previous model file versions gets large. An external model file compile step would keep the compatibility code out of the firmware.

It does make managing the model files more complex for users. We'd also need a way to generate readable model files from the model data on the tx so users could save changes made on the tx.

@howard0su
Copy link
Contributor Author

howard0su commented Jan 22, 2019

Any specific reason to swap order of CreateLabel parameters v.s. CreateLabelBox? the strCallback and label description is swapped?

         const char *(*strCallback)(guiObject_t *, const void *),
         const struct LabelDesc *desc,

         const struct LabelDesc *desc,
         const char *(*strCallback)(guiObject_t *, const void *),

@PhracturedBlue
Copy link
Contributor

CreateLabel was one of the very 1st functions in the GUI and we didn't have the data parameter. When it was added, it was tacked on the end.. When CreateLabelBox was added, we had a convention (cb and data go together at the end). So CreateLabelBox is 'right' and CreateLabel is like that because there wasn't any reason to change every call as a beutification project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants