MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.22k stars 19.22k forks source link

[BUG] LEVEL_BED_CORNERS UI quits early without input #17883

Closed Knifa closed 4 years ago

Knifa commented 4 years ago

Bug Description

When using LEVEL_BED_CORNERS on bugfix-2.0.x (852a8d6685ec1137eb65e78fa748cae41fbd36b), the UI quits out early without pressing Done.

My Configurations

config.zip

Steps to Reproduce

  1. Go to Motion > Level Bed Corners
  2. Wait 5-10s. Don't do anything.

Expected behavior: [What you expect to happen]

I should be able to proceed through the corner steps indefinitely, as on 2.0.5.3.

Actual behavior: [What actually happens]

The UI exits suddenly without pressing Done.

Additional Information

This is on an Ender 3 with an SKR Mini E3 v1.2. It works OK on 2.0.5.3!

swilkens commented 4 years ago

I can confirm this, I see the same behaviour.

Knifa commented 4 years ago

A new macro was added around deferring the status screen ten days ago --- maybe it's not working as expected? Will try the commit before this one. https://github.com/MarlinFirmware/Marlin/commit/c536b8de629807b489f054051bb120457f112a11 is the commit that added it. https://github.com/MarlinFirmware/Marlin/blame/bugfix-2.0.x/Marlin/src/lcd/ultralcd.h#L511

FORCE_INLINE static void defer_status_screen(const bool defer=true) {
    TERN(LCD_TIMEOUT_TO_STATUS, defer_return_to_status = defer, UNUSED(defer));
}

previously just used #if. The TERN macro is so hard to follow! I can't make heads or tails of it.

Knifa commented 4 years ago

Yes, reverting this bit of code to how it was in the prior commit i.e.:

    FORCE_INLINE static void defer_status_screen(const bool defer=true) {
      #if LCD_TIMEOUT_TO_STATUS
        defer_return_to_status = defer;
      #else
        UNUSED(defer);
      #endif
    }

solves the issue.

Something is not right with TERN --- perhaps it is because LCD_TIMEOUT_TO_STATUS has a value rather than just being empty? I can't understand the macro chain at all so I'm stuck on how to fix it as intended.

I had a play around with the macros on Compile Explorer and it seems to evaluate the expression regardless as to whether or not LCD_TIMEOUT_TO_STATUS is present. Might be a Compile Explorer issue though.

Knifa commented 4 years ago

OK --- with more fanangling in Compiler Explorer, it seems that the macro only works if the define has no value, which LCD_TIMEOUT_TO_STATUS does.

Does this even need checked? LCD_TIMEOUT_TO_STATUS is always set in Conditionals_post.h so it should always be around.

swilkens commented 4 years ago

previously just used #if. The TERN macro is so hard to follow! I can't make heads or tails of it.

I would agree, the code readability didn't improve with this change. Additionally, it seems some expansion is causing unexpected consequences like this one.

Knifa commented 4 years ago

Fixed in https://github.com/MarlinFirmware/Marlin/commit/7c26a54d3f2434c4d578f58af011a76dd298d4e3.

swilkens commented 4 years ago

I can confirm the fix works, bed corner menu now properly remains

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.