-
-
Notifications
You must be signed in to change notification settings - Fork 343
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
Conversation
bw128 - USB popup menu now has four entries... number 4 is "select mode" 🤪 |
Thanks for picking that up. Fixed now. |
radio/src/gui/128x64/view_main.cpp
Outdated
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}; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 😕
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 :)
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... |
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 |
Some optimisations to reduce FLASH size, predominantly for B&W radios.
Also include a small RAM saving.
Changes:
Savings:
RAM reduction for all radios - 264 bytes
FLASH reduction (bytes):