prusa3d / Prusa-Firmware

Firmware for Original Prusa i3 3D printer by PrusaResearch
GNU General Public License v3.0
1.99k stars 1.05k forks source link

Add Eject Option To Filament Load Message #4644

Closed sarusani closed 2 months ago

sarusani commented 3 months ago

Add a third option ("eject") to the "Filament extruding & with correct color?" message.

Closes https://github.com/prusa3d/Prusa-Firmware/issues/4643

The third_choice text was rendered incorrectly. (Only showed the first letter of the text because the position was hardcoded.) I made sure the third_choice text will be offset to the left, but not overlap with the second_choice text.

This uses quite a lot of mem, any help in optimizing is appreciated 😸

github-actions[bot] commented 3 months ago

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG 228 0 247412 5653 6540 2539
MK3_MULTILANG 228 0 246690 5662 7262 2530
gudnimg commented 3 months ago

I wonder if we can save memory by converting the following arguments into a single struct and adding third_col parameter. And that way force the caller to specify the column. 🤔

void lcd_show_choices_prompt_P(uint8_t selected, const char *first_choice, const char *second_choice, uint8_t second_col, const char *third_choice)

3d-gussner commented 3 months ago

I wonder if we can save memory by converting the following arguments into a single struct and adding third_col parameter. And that way force the caller to specify the column. 🤔

void lcd_show_choices_prompt_P(uint8_t selected, const char *first_choice, const char *second_choice, uint8_t second_col, const char *third_choice)

The biggest cost +196 bytes is https://github.com/prusa3d/Prusa-Firmware/pull/4644/files#diff-5fbfdca43c192573e31d8e78d255c510fb6e213b118cf38173277ec2a2163880R2261-R2269

The last part only costs additional +32 bytes https://github.com/prusa3d/Prusa-Firmware/pull/4644/files#diff-5fbfdca43c192573e31d8e78d255c510fb6e213b118cf38173277ec2a2163880R3047-R3053

gudnimg commented 3 months ago

The biggest cost +196 bytes is https://github.com/prusa3d/Prusa-Firmware/pull/4644/files#diff-5fbfdca43c192573e31d8e78d255c510fb6e213b118cf38173277ec2a2163880R2261-R2269

In that case I wonder if some of the function calls are being inlined. You can try adding __attribute__((noinline)) to the definition and see if that helps. None of these are time critical.

gudnimg commented 3 months ago

I looked a bit into this flash usage, and I suspect this is due to the compiler not able to optimise the code like it could before. third_choice was always set to nullptr before, but now you give it a value which complicates things. I've pretty much ruled out inlining.

3d-gussner commented 3 months ago

@gudnimg Any idea if that can be optimized?

3d-gussner commented 2 months ago

@sarusani Please rebase the PR and solve the conflicts.

3d-gussner commented 2 months ago

@gudnimg Do you think we can save any bytes here? If not please approve.