Filament RUNOUT software "runout.cpp" and "runout.h" conflicts with "printer_operation.cpp"

[tvonderhaar]

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

Yes, and the problem still exists.

Bug Description

I have a Pyramid A1.1 with this basic configuration:
1)#define MKS_ROBIN_TFT35
2)#define TFT_LVGL_UI //This forces #define MKS_WIFI_MODULE

If I define:
#define FILAMENT_RUNOUT_SENSOR

Then two different sets of software monitor and react to filament runout:
first set: "runout.cpp", "runout.h".
second set: "printer_operation.cpp" "pins_MKS_ROBIN_NANO.h"

I'm pretty sure this is not correct. I can turn off the first set by not defining "FILAMENT_RUNOUT_SENSOR", but then "pins_MKS_ROBIN_NANO.h" has to be modifies as neither configuration file "configuration.h" or "configuration_adv.h" allow properly configuring for my printer. I had to perform these changes to "pins_MKS_ROBIN_NANO.h":

#if HAS_TFT_LVGL_UI
  #define MT_DET_1_PIN                         PA4   // LVGL UI FILAMENT RUNOUT1 PIN
//#define MT_DET_2_PIN                        PE6   //TLV; LVGL UI FILAMENT RUNOUT2 PIN
                                                                         //TLV; Must not be defined if only
               							        //TLV; one filament sensor exist
//#define MT_DET_PIN_INVERTING      false   //TLV; LVGL UI filament RUNOUT PIN STATE
  #define MT_DET_PIN_INVERTING       true    //TLV; Pyramid A1.1 LVGL UI filament
                                                                        //TLV; RUNOUT PIN STATE

  #define WIFI_IO0_PIN                        PC13  // MKS ESP WIFI IO0 PIN
  #define WIFI_IO1_PIN                        PC7    // MKS ESP WIFI IO1 PIN
  #define WIFI_RESET_PIN                     PA5   // MKS ESP WIFI RESET PIN
#else

The code "printer_operation.cpp" will fail if you don't properly set the "RUNOUT PIN STATE". It will also fail if you don't comment out "MT_DET_2_PIN" if you don't have a second filament runout sensor. Neither of these variables are identified in the configuration files and of course if set there they will probably be reset when the compiler scans "pins_MKS_ROBIN_NANO.h".

Another bug is with the "runout" code implemented by defining "FILAMENT_RUNOUT_SENSOR". Both approaches are run monitoring routines from the main program  loop "loop()" or idle() loop. See "MarlinCore.cpp" where "loop()" calls "idle()" and "printer_state_polling()". The function "idle() which calls "runout.run()" . 

The monitor code "runout.run()" will do nothing unless the variable "filament_ran_out" is false. Once an initial runout occurs "filament_ran_out" is set to true and the monitor code will never execute again until "filament_ran_out" is set to false. This should be accomplished after the runout script runs and the user pushes the resume button by calling the function "rounout.reset()".  This never occurs and then only the "printer_state_polling()" function operates as expected. This might be fooling people into thinking the runout routine is working if they had a "MT_DET_PIN_INVERTING" set to false, (I need to have it set to true for my sensor), which might mask the error of defining of a second filament sensor pin that does not exist (see "filament_check()".) The second pin "MT_DET_2_PIN" is currently defined in error by default in "pins_MKS_ROBIN_NANO.h".

I will include the relevant files. I have added some debugging serial output that helped me detect these errors. Just uncomment "DEBUG_FILAMNENT_CHECK" and "FILAMENT_RUNOUT_SENSOR_DEBUG" defines in "printer_operation.cpp" and "runout.h" to use them. Some of the original debug outputs improperly attempt to print ASCII characters, (ex."0"+[single digit var]), using the number output routine.

If the resolution to these problems is to get rid of "printer_state_polling()" and fix the "runout.*" software, please implement the park routine for we who have the new color TFT and have to define "TFT_LVGL_UI". I have a WiFi module and it also requires the "TFT_LVGL_UI" option. Please don't break the WiFi functionality if you remove "printer_state_polling()". WiFi periodic processing is called via the function call "wifi_looping()" in "printer_state_polling()". Remember to move "wifi_looping()" to "loop()" or "idle()";

Bug Timeline

New. Since update from V1 to V2 a few weeks ago starting with V2.009 and exist with v2.0091..

Expected behavior

I expect the printer to pause and park when the filament runs out. After installing the new filament and pushing the resume button the printer should continue where it paused.

Actual behavior

first:
The printer reacted opposite to what was expected. With the filament out of sensor it ran, with the filament in the sensor the printer paused.

second: after setting MT_DET_PIN_INVERTING true
Printer just parked, no matter whether the filament was in or out. After pressing resume the printer moved to original pause location, and then went right back to resume. I could not print at all.

Third: after commenting out #define MT_DET_2_PIN PE6
Printer worked as expected, but observed via debug serial output that "runout.run()" was never called again.

Steps to Reproduce

Refer to previous comments.

Version of Marlin Firmware

V2.0091

Printer model

Pyramid A1.1, see on Amazon.com

Electronics

Stock MKS Robin Nano V1.2 with color 3.5" TFT and touch screen.

Add-ons

3D-Touch, WiFi module

Bed Leveling

ABL Bilinear mesh

Your Slicer

Cura

Host Software

Pronterface

Additional information & file uploads

Marlin V2.0091 fixes mods.zip

[thinkyhead]

Marlin 2.0.9.1 doesn't have a "printer_operation.cpp" file. Are the attached files for bugfix-2.0.x or 2.0.9.1?

Nevermind. I forgot I turned off Spotlight Indexing for my git repository folders.

[thinkyhead]

Please provide changes relevant to bugfix-2.0.x … The pins file now looks like this:

//
// Misc. Functions
//
#if HAS_TFT_LVGL_UI
  #define MT_DET_1_PIN                      PA4
  #define MT_DET_2_PIN                      PE6
  #define MT_DET_PIN_STATE                  LOW

  #define WIFI_IO0_PIN                      PC13
  #define WIFI_IO1_PIN                      PC7
  #define WIFI_RESET_PIN                    PA5
#else
  //#define POWER_LOSS_PIN                  PA2   // PW_DET
  //#define PS_ON_PIN                       PB2   // PW_OFF
  #define FIL_RUNOUT_PIN                    PA4
  #define FIL_RUNOUT2_PIN                   PE6
#endif

Have we confirmed that all machines are using HIGH to indicate that the filament is NOT PRESENT?

        if (READ(MT_DET_1_PIN) == MT_DET_PIN_STATE)
          runout_mks.runout_status = RUNOUT_STATUS;

[thinkyhead]

I see the problem. MKS UI isn't taking account of FILAMENT_RUNOUT_SENSOR, NUM_RUNOUT_SENSORS, etc., but is doing its own thing, and I would guess not very well. It would be better if the filament sensor associated with TFT_LVGL_UI would just integrate with Marlin's existing runout handling, which provides parking, heater timeout, and other basic amenities.

Please try #22595 with FILAMENT_RUNOUT_SENSOR enabled and configured. Hopefully it will work better than the basic handling provided in MKS UI.

[tvonderhaar]

I am sorry about the bad formatting in the previous messages. I'm just becoming familiar with this report system. Here is a copy of my last comment content hopefully easier to read now:

I took the bugfix-V2.0.x version of "pins_MKS_ROBIN_NANO.h" and modified it so it should work with my Pyramid A1.1. I attached it to this comment. I hope that is what you wanted. This is my the first time using this forum. Should I respond to your E-Mail in the future or here? I have no way of knowing if all Pyramid A1.1's use the same active high on filament not present signal. I just purchase the machine May 29th, 2021.

I agree that who ever wrote the MKS UI ignored the built in filament run out code or maybe visa-versa. The MKS version does a nice pause with a park right up front. If the other is more robust it needs true integration. I mean you can just rip out the MKS code and uncomment the other filament run out code. Pyramid A1.1 and other "TFT_LVGL_UI" option users will lose WiFi functionality. WiFi periodic processing needs to be called via the function "wifi_looping()" by "loop()" or "idle()". Also, the "runout.reset()" must be properly called after resuming to get filament monitoring started again. If "TFT_LVGL_UI" is not defined, does the current code call "runout.reset()" after a resume()? I just studied enough of the code to deal with my configuration and maybe this taken into consideration in other defined UIs. Thanks for responding.

Tom V

https://github.com/MarlinFirmware/Marlin/files/7011742/pins_MKS_ROBIN_NANO.zip

[tvonderhaar]

By the way, I have tried to use the current sensor software, but as mentioned above the function "runout.reset()" is never called upon pushing the resume button. This needs to be addressed in the integration. Nothing in the "TFT_LVGL_UI" enabled user interface does this reset and as a result the filament sensor run out monitoring function "runout.run()" will never run again until a "m412 r" command is manually executed,(I used Ponterface remote control program to issue the reset), forcing a reset. Also, I discovered from this forum that "M600" can't be called with "TFT_LVGL_UI" set as the 3.5" color TFT touch screen is not supported by the current "M600" implementation. Only the user configured call to "M25" seems to be supported. This just pauses the printer and waits for the resume button to be pushed. No park, no heater timeout or any other "M600" functions are performed(Note: Advanced pause is also not supported.) The M25 pause,(currently required with the "TFT_LVGL_UI" option ), is very confusing to the operator as they may not know why the printer has paused. Also, changing filament while poised above an incomplete model seems destined for disaster.
Tom V

[thinkyhead]

If you have suggestions specific to #22595 please post them on that PR.

[tvonderhaar]

Done.

[github-actions]

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.