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.28k stars 19.24k forks source link

[Bug] ExtUI blocking call breaks M502 / ABL #17429

Closed minosg closed 4 years ago

minosg commented 4 years ago

Bug Description

Code snippets which are intended to be updating the user interface are blocking, affecting the flow of commands which should not rely on a display.

In my case I have been trying to compile marlin 2.0.4.4, or 2.0.5.2 from tags and eventually bugfix-2.0.x. The machine is an old printer with an BTT SKR mini e3 1.2 and to using MalyanLCD. It is set to use a genuine BLTOUCH 3.1 as a probe, but not for homing.

I noticed that with everything working properly, using AUTO_BED_LEVELING_LINEAR, AUTO_BED_LEVELING_3POINT and MESH_BED_LEVELING.

Then when attempting to activate AUTO_BED_LEVELING_BILINEAR, and AUTO_BED_LEVELING_UBL the machine would freeze when I was trying to issue an M502 to clear the previous settings, or perform the leveling routine.

Looking at the code I noticed the following snippet

#if ENABLED(EXTENSIBLE_UI)
  ExtUI::onMeshUpdate(meshCount, z_values[meshCount.x][meshCount.y]);
#endif

With EXTENSIBLE_UI being evaluated as

// Extensible UI serial touch screens. (See src/lcd/extensible_ui)
#if ANY(HAS_DGUS_LCD, MALYAN_LCD, TOUCH_UI_FTDI_EVE)
  #define IS_EXTUI
  #define EXTENSIBLE_UI
#endif

This to my understanding is intended to upload the z value on the user interface. But If the screen library has not properly implemented this feature it will block and result in undefined behavior, which users may attribute to incorrect configuration of their probe or platform.

My Configurations

configs.zip

Steps to Reproduce

  1. Enable BL Touch
  2. Enable MALYAN_LCD
  3. Enable AUTO_BED_LEVELING_UBL or AUTO_BED_LEVELING_BILINEAR
  4. (Optional) Enable DEBUG_LEVELING_FEATURE
  5. Compile build & upload

In order to trigger the crash:

Alternative method:

  1. Set z-offset if not already done. M851 Z-1.0 (replace with your value)
  2. Home all axis G28
  3. Start auto-level using G29

Expected behavior:

The Gcode commands which trigger a settings.reset() to complete

Actual behavior:

The printer freezes, and eventually time's out of the UART interface, until power cycled. The serial port is no longer properly evaluated, and appears as unrecognized device on a Windows host.

If attempting to do auto-leveling, the machine will home, head to the first point and freeze, when attempting to wipe the grid.

Additional Information

A quick workaround is to adjust the pre-processor logic to not include the logic when MALYAN_LCD is defined, without affecting other displays.

G29.cpp

bedlevel.cpp

#if ENABLED(EXTENSIBLE_UI) && DISABLED(MALYAN_LCD)
  ExtUI::onMeshUpdate(meshCount, z_values[meshCount.x][meshCount.y]);
#endif
thinkyhead commented 4 years ago

This might be related to a more general issue, which is that contributors to ExtUI (@marciot and others) don't always remember to update the ExtUI examples and other implementations when modifying the ExtUI API, so they fall out of sync. There are no official maintainers for these ExtUI implementations, so it's likely they will continue to get stale.

I could patch the onMeshUpdate methods pretty quickly, but someone with a deeper investment in ExtUI and its offshoots is going to have to take the reins or this kind of thing will just keep occurring.

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.