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

Add Eject Option To Filament Load Message #4644

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

sarusani
Copy link
Contributor

@sarusani sarusani commented Mar 22, 2024

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

Closes #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 😸

Copy link

github-actions bot commented Mar 22, 2024

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

@sarusani sarusani force-pushed the AddEjectToFilamentLoadMSG branch from bc66635 to 70db3b4 Compare March 22, 2024 10:41
@gudnimg
Copy link
Collaborator

gudnimg commented Mar 22, 2024

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
Copy link
Collaborator

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
Copy link
Collaborator

gudnimg commented Mar 23, 2024

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
Copy link
Collaborator

gudnimg commented Mar 23, 2024

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 3d-gussner added this to the FW 3.14.1 milestone Apr 2, 2024
@3d-gussner
Copy link
Collaborator

@gudnimg Any idea if that can be optimized?

@3d-gussner
Copy link
Collaborator

@sarusani Please rebase the PR and solve the conflicts.

@sarusani sarusani force-pushed the AddEjectToFilamentLoadMSG branch from 70db3b4 to bd935db Compare April 9, 2024 13:51
@3d-gussner 3d-gussner requested review from 3d-gussner and gudnimg April 9, 2024 16:50
Copy link
Collaborator

@3d-gussner 3d-gussner left a comment

Choose a reason for hiding this comment

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

Tested with MK404 ✔️

Tue Apr  9 18:50:31 2024

@3d-gussner
Copy link
Collaborator

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

@3d-gussner 3d-gussner requested a review from DRracer April 11, 2024 05:09
@3d-gussner 3d-gussner merged commit 361ce65 into prusa3d:MK3 Apr 11, 2024
4 checks passed
@sarusani sarusani deleted the AddEjectToFilamentLoadMSG branch July 5, 2024 14:32
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.

[ENHANCEMENT]Filament load needs to be 3 state
3 participants