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

refactor: FLASH size optimisations for B&W. #4827

Merged
merged 6 commits into from
Mar 31, 2024

Conversation

philmoz
Copy link
Collaborator

@philmoz philmoz commented Mar 31, 2024

Some optimisations to reduce FLASH size, predominantly for B&W radios.
Also include a small RAM saving.

Changes:

  • Move _flex_inputs array to FLASH and reduce size of static string used for YAML conversion.
  • Move large inline functions, which are used multiple times, from header files to code files.
  • Remove white space from edges of icons.
  • Helper function to simplify some popup menus.
  • Move 128x64 menuTabGeneral and menuTabModel out of header file so only one instance is created (matches 212x64 code)
  • Pack data structures and change static array elements to smaller types.
  • Replace custom code for converting integers to strings with sprintf calls (YAML writer).

Savings:
RAM reduction for all radios - 264 bytes
FLASH reduction (bytes):

  • X9E - 2648
  • X7S - 4708
  • T20 V2 - 4648
  • TX12 Mk2 - 5160
  • MT12 - 5320

@pfeerick pfeerick added compilation Related to compiling the firmware and firmware options house keeping 🧹 Cleanup of code and house keeping labels Mar 31, 2024
@pfeerick
Copy link
Member

bw128 - USB popup menu now has four entries... number 4 is "select mode" 🤪

@philmoz
Copy link
Collaborator Author

philmoz commented Mar 31, 2024

bw128 - USB popup menu now has four entries... number 4 is "select mode" 🤪

Thanks for picking that up. Fixed now.

for (uint8_t i = 0; i < keysGetMaxTrims(); i++) {
#if defined(SURFACE_RADIO)
static coord_t x[] = {TRIM_RH_X, TRIM_LH_X, TRIM_RV_X, TRIM_LV_X, TRIM_LV_X};
static uint8_t x[] = {TRIM_RH_X, TRIM_LH_X, TRIM_RV_X, TRIM_LV_X, TRIM_LV_X};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the issue in coord_t type definition ? Need to try it by I feel it should be uint8_t for max(LCD_W, LCD_H) < 256

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coord_t can be outside screen bounds (negative and greater than width/height) so it can't be an unsigned value and 8 bits isn't enough.

I tried changing the type for coord_t to int16_t; but this increased code size by ~1K 😕

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, not sure why it should allow off screen values tho, but thats probably opening another can of worm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the trims are in the right place and all on the MT12... if I were to nitpick, there seems to be a (not related to this PR as it's present in main code also) issue with the trim 'knob' reaching the end at more like 80% travel... whereas on colorlcd the knob stops moving exactly at end of trim travel. This is without any extended trims, etc... new blank model.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@3djc Let me know if you need/want time to look into the coord_t/uint8_t change further... otherwise I think this is ready to merge.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I were to nitpick, there seems to be a (not related to this PR as it's present in main code also) issue with the trim 'knob' reaching the end at more like 80% travel... whereas on colorlcd the knob stops moving exactly at end of trim travel.

Fixed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on MT12 and Pocket. Thanks :)

@pfeerick
Copy link
Member

np. just saw it affected 212 also but good thing it was common code so both should be fixed now. Nothing else jumped out on X9D+2019 or Pocket (or T20 which was the first one). Flashing MT12 now as the final guinea pig...

@pfeerick
Copy link
Member

pfeerick commented Mar 31, 2024

MT12 seems fine/unchanged to me... and yes, PXX2 removal can be dropped after this PR... UA is tight, but still has at least 1KB flash remaining with PXX2 restored. edit: dammit... opened wrong PR to set to draft! lol

@pfeerick pfeerick added this to the 2.10 milestone Mar 31, 2024
@pfeerick pfeerick marked this pull request as draft March 31, 2024 09:31
@pfeerick pfeerick marked this pull request as ready for review March 31, 2024 09:32
@pfeerick pfeerick changed the title chore: FLASH size optimisations for B&W. refactor: FLASH size optimisations for B&W. Mar 31, 2024
@pfeerick pfeerick merged commit 6c5eada into EdgeTX:main Mar 31, 2024
44 checks passed
ThomasKuehne pushed a commit to ThomasKuehne/edgetx that referenced this pull request Apr 5, 2024
@philmoz philmoz deleted the size-reduction branch June 25, 2024 02:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compilation Related to compiling the firmware and firmware options house keeping 🧹 Cleanup of code and house keeping
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants