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

[BUG] 2.1.2.2 with Input Shaping locks up after flash #26811

Closed
1 task done
lsellens opened this issue Feb 23, 2024 · 9 comments
Closed
1 task done

[BUG] 2.1.2.2 with Input Shaping locks up after flash #26811

lsellens opened this issue Feb 23, 2024 · 9 comments
Labels
Bug: Confirmed ! C: Motion Fix Included A fix is included in the description

Comments

@lsellens
Copy link
Contributor

Did you test the latest bugfix-2.1.x code?

No, but I will test it now!

Bug Description

I have a very old board It's a mks smelzi v1.0 it runs 2.1.2.1 just fine but when trying to flash 2.1.2.2 it just gets stuck on the marlin screen. I am able to flash back to 2.1.2.1 from there. I know this board and printer is old but it works great for me and would like to keep it alive and running with the newest features. I'm at a loss on how to troubleshoot this and could use some advice on how to proceed? I don't believe its an issue with flash space because 2.1.2.2 is slightly smaller. Here is exactly what I'm flashing https://github.com/lsellens/Marlin/commits/MKS_SMELZI_V1.0-2.1.x-new/

Bug Timeline

new issue with 2.1.2.2

Expected behavior

No response

Actual behavior

No response

Steps to Reproduce

No response

Version of Marlin Firmware

2.1.2.2

Printer model

alunar m518

Electronics

mostly I've just added a custom board I made that cuts the 12v line to the board unless its needed for movement/cooling

LCD/Controller

MKS_MINI_12864

Other add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

OctoPrint

Don't forget to include

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Marlin-configs.zip

@ellensp
Copy link
Contributor

ellensp commented Feb 23, 2024

BOARD_MKS_SMELZI_10 is not even in stock marlin code...

@ellensp
Copy link
Contributor

ellensp commented Feb 24, 2024

#define INPUT_SHAPING_X // lsellens
#define INPUT_SHAPING_Y // lsellens

Input shaping is broken on 2.1.2.2, disable it

@enisrat
Copy link

enisrat commented Feb 24, 2024

I am also on 2.1.2.2. Compiled Marlin with its default Configuration.h and just enabling INPUT_SHAPING_X,Y does not increase the RAM usage on mega2560.

I fiddled around some with the compiler and it looks like the compiler optimized out ShapingQueue::times.
With the following patch it would increase RAM usage from 2482 to 4426:

--- a/Marlin/src/module/stepper.h
+++ b/Marlin/src/module/stepper.h
@@ -422,7 +422,7 @@ constexpr ena_mask_t enable_overlap[] = {
       }
       #if ENABLED(INPUT_SHAPING_X)
         static shaping_time_t peek_x() { return peek_x_val; }
-        static bool dequeue_x() {
+        static bool __attribute__((optimize("O0"))) dequeue_x() {

No idea why, but at least after the time spent I know the issue and not use this branch.

@lsellens
Copy link
Contributor Author

BOARD_MKS_SMELZI_10 is not even in stock marlin code...

Yes, marlin doesn't have support for my board it is a melzi clone. Only a few minor differences. I added support here. lsellens@7af853e

#define INPUT_SHAPING_X // lsellens #define INPUT_SHAPING_Y // lsellens

Input shaping is broken on 2.1.2.2, disable it

Disabling input shaping did indeed fix my issue. Hopefully it gets fixed soon. Thank you.

@ellensp
Copy link
Contributor

ellensp commented Feb 26, 2024

you should add it to official Marlin, just add some ascii art of all the connectors pins, so the board is basically documented in the pins file

@classicrocker883
Copy link
Contributor

I am also on 2.1.2.2. Compiled Marlin with its default Configuration.h and just enabling INPUT_SHAPING_X,Y does not increase the RAM usage on mega2560.

I fiddled around some with the compiler and it looks like the compiler optimized out ShapingQueue::times. With the following patch it would increase RAM usage from 2482 to 4426:

--- a/Marlin/src/module/stepper.h
+++ b/Marlin/src/module/stepper.h
@@ -422,7 +422,7 @@ constexpr ena_mask_t enable_overlap[] = {
       }
       #if ENABLED(INPUT_SHAPING_X)
         static shaping_time_t peek_x() { return peek_x_val; }
-        static bool dequeue_x() {
+        static bool __attribute__((optimize("O0"))) dequeue_x() {

No idea why, but at least after the time spent I know the issue and not use this branch.

instead this change should be replaced like so:

static bool __O0 dequeue_x() {
and should Y_AXIS also have no optimization?:
static bool __O0 dequeue_y() {

@thisiskeithb thisiskeithb changed the title [BUG] 2.1.2.2 locks up after flash [BUG] 2.1.2.2 with Input Shaping locks up after flash Mar 1, 2024
@enisrat
Copy link

enisrat commented Mar 3, 2024

I am also on 2.1.2.2. Compiled Marlin with its default Configuration.h and just enabling INPUT_SHAPING_X,Y does not increase the RAM usage on mega2560.
I fiddled around some with the compiler and it looks like the compiler optimized out ShapingQueue::times. With the following patch it would increase RAM usage from 2482 to 4426:

--- a/Marlin/src/module/stepper.h
+++ b/Marlin/src/module/stepper.h
@@ -422,7 +422,7 @@ constexpr ena_mask_t enable_overlap[] = {
       }
       #if ENABLED(INPUT_SHAPING_X)
         static shaping_time_t peek_x() { return peek_x_val; }
-        static bool dequeue_x() {
+        static bool __attribute__((optimize("O0"))) dequeue_x() {

No idea why, but at least after the time spent I know the issue and not use this branch.

instead this change should be replaced like so:

static bool __O0 dequeue_x() { and should Y_AXIS also have no optimization?: static bool __O0 dequeue_y() {

This was not a proper fix, just to show something is wrong with the code and ShapingQueue::times gets optimized out.

I tested again, on bugfix-2.1.x the problem does not exist anymore.
But release 2.1.2.2 is broken in that regard.

@tombrazier
Copy link
Contributor

#26936 resolves the issue with input shaping not increasing RAM usage. It will also remove a guaranteed infinite loop inside the stepper ISR which, I take it, the compiler was clever enough to spot and, therefore, optimise out a lot of the ISR.

The reason this specific bug behaviour does not occur in bugfix appears to be that the variable in question (step_needed in function Stepper::shaping_isr()) has changed type from xy_bool_t to AxisFlags.

@thisiskeithb
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Confirmed ! C: Motion Fix Included A fix is included in the description
Projects
None yet
Development

No branches or pull requests

6 participants