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

crashdet_cancel() doesnt cleanup all variables when USB printing #4207

Merged
merged 3 commits into from
Dec 1, 2023

Conversation

gudnimg
Copy link
Collaborator

@gudnimg gudnimg commented May 14, 2023

This function doesn't look correct to me 🤔 I would think that this should behave similarly as when stopping the print via the LCD. This PR is my proposed improvement, un-tested at the moment.

Changed UnconditionalStop() to not close the SD card file if we're using Octoprint. Then there shouldnt be any file open.

Some of the variables which were not reset:
isPrintPaused
pause_time
saved_start_position
saved_printing_type

It seems like the bed heater could also be left enabled?

Fixes #4324

Change in memory:
Flash: -28 bytes
SRAM: 0 bytes

@gudnimg gudnimg added this to the FW 3.14.0 milestone May 14, 2023
@gudnimg gudnimg changed the title crashdet_cancel() doesnt cleanup all variables when using USB printing crashdet_cancel() doesnt cleanup all variables when USB printing May 14, 2023
@gudnimg gudnimg force-pushed the crashdet_cancel-fixup branch from 9593f50 to 9e78b13 Compare July 14, 2023 14:45
@wavexx
Copy link
Collaborator

wavexx commented Jul 22, 2023

I generally agree here, I think this has always been overlooked recently

wavexx
wavexx previously requested changes Jul 22, 2023
Copy link
Collaborator

@wavexx wavexx left a comment

Choose a reason for hiding this comment

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

I agree here, I think this has always been overlooked

Maybe we can also call directly print_stop() when handling the CRASHDET_CANCEL command in Marlin_main.cpp:4035.

Firmware/Marlin_main.cpp Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Aug 13, 2023

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG -84 0 246674 5656 7278 2536
MK3_MULTILANG -82 0 245958 5663 7994 2529

@gudnimg
Copy link
Collaborator Author

gudnimg commented Aug 13, 2023

I pushed another commit which optimizes setting the crash detection on or off.

@gudnimg gudnimg changed the title crashdet_cancel() doesnt cleanup all variables when USB printing 🚧 crashdet_cancel() doesnt cleanup all variables when USB printing Sep 17, 2023
@gudnimg
Copy link
Collaborator Author

gudnimg commented Sep 17, 2023

Adding 🚧 to the title as this PR is not tested

@gudnimg gudnimg force-pushed the crashdet_cancel-fixup branch from 365ca48 to 5b4983a Compare November 3, 2023 16:53
I would think that this should behave similarly as when stopping
the print via the LCD.

Changed UnconditionalSto()p to not close the SD card file
if we're using Octoprint. Then there shouldnt be any file open.

Some of the variables which were not reset:

isPrintPaused
pause_time
saved_start_position
saved_printing_type

Bed heater may be left on?

Change in memory:
Flash: -28 bytes
SRAM: 0 bytes
Change in memory:
Flash: -6 bytes
SRAM: 0 bytes
Add function crashdet_use_eeprom_setting

Change in memory:
Flash: -52 bytes
SRAM: 0 bytes
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
Started USB print

  • Triggered crash recovers
  • Triggered crash stop, stops the print correctly on printer and host
  • Crash det. [OFF] does work as it ignores the crashes

Approved only change I would like to see is that Recover crash default is YES

Firmware/Marlin_main.cpp Show resolved Hide resolved
@3d-gussner 3d-gussner changed the title 🚧 crashdet_cancel() doesnt cleanup all variables when USB printing crashdet_cancel() doesnt cleanup all variables when USB printing Nov 30, 2023
@3d-gussner 3d-gussner self-requested a review December 1, 2023 16:39
@3d-gussner 3d-gussner merged commit 4dfc484 into prusa3d:MK3 Dec 1, 2023
5 checks passed
@gudnimg gudnimg deleted the crashdet_cancel-fixup branch September 1, 2024 12:02
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.

[BUG] Mk3S+ with MMU3 not re-heating after safety timer
3 participants