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

Remove recursion workaround in Nozzle Change #4736

Merged
merged 2 commits into from
Aug 8, 2024

Conversation

gudnimg
Copy link
Collaborator

@gudnimg gudnimg commented Jul 28, 2024

This workaround isn't needed today. At the time this workaround was added, lcd_show_multiscreen_message_yes_no_and_wait_P was calling lcd_update() via lcd_update_enable(true) which caused all sorts of problems like first layer calibration being inaccessible.

When testing this PR, I found one more place where lcd_update() was being called: 7b70f6a which has now been eliminated. This change is required to fully remove the workaround.

Steps to test:

  1. Power on printer
  2. Run Nozzle Change from Hardware setup menu
  3. Test: at no point does the LCD seem to freeze or become unresponsive.

Copy link

github-actions bot commented Jul 28, 2024

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG -36 0 247736 5653 6216 2539
MK3_MULTILANG -36 0 247050 5662 6902 2530

@gudnimg
Copy link
Collaborator Author

gudnimg commented Jul 31, 2024

Note: this PR is related to #4703

We should not be adding more “hacks” like this to add the unload filament dialog :)

@gudnimg gudnimg force-pushed the cleanup_recursion_hack branch from 83773d9 to 7b70f6a Compare July 31, 2024 12:29
@gudnimg gudnimg marked this pull request as ready for review July 31, 2024 12:31
@gudnimg
Copy link
Collaborator Author

gudnimg commented Jul 31, 2024

Opening this PR up for review. Nozzle change works well on my MK3S+. There may be regressions introduced by 7b70f6a but I think this is a good change.

@gudnimg gudnimg requested review from leptun and 3d-gussner July 31, 2024 12:32
gudnimg added 2 commits August 3, 2024 15:43
This fix is no longer needed today.

LCD knob clicks / and rotation, take care of updating lcd_draw_update.

The real bug was likely lcd_show_multiscreen_message_yes_no_and_wait_P calling lcd_update(), this is fixed now since.
The fixes a scenario where:

lcd_status_screen() calls lcd_commands() upon exiting
lcd_show_fullscreen_message_and_wait_P(_T(MSG_NOZZLE_CNG_READ_HELP));
and so not allowing the user to leave the screen since it will keep being rendered endlessly.

This change only affects lcd_show_fullscreen_message_and_wait_P
@gudnimg gudnimg force-pushed the cleanup_recursion_hack branch from 7b70f6a to 25a11bb Compare August 3, 2024 15:43
@3d-gussner 3d-gussner added this to the FW 3.14.1 milestone Aug 7, 2024
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 on MK404

  • MK3S nozzle change
  • REVO MK3S nozzle change

@3d-gussner 3d-gussner merged commit 428091b into prusa3d:MK3 Aug 8, 2024
4 checks passed
@gudnimg gudnimg deleted the cleanup_recursion_hack branch August 8, 2024 09:56
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.

2 participants