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.16k stars 19.21k forks source link

Filament Runout Sensor not working #4791

Closed gatho222 closed 8 years ago

gatho222 commented 8 years ago

Tried to enable the filament runout sensor on my homebrew 3D printer (Marlin 1.1.0-RC7, ATMega2560, Ramps 1.4, RepRapDiscount Full Graphic Smart LCD-Controller) but the firmware simply ignored my settings in Configuration.h. I was able to track down and solve the problem. In Configuration_adv.h there are further definitions which block the feature for all display types except Ultipanel. The corresponing lines of code read:

// Add support for experimental filament exchange support M600; requires display

if ENABLED(ULTIPANEL)

[...]

I changed the second line to [...]

if ENABLED(REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER)

[...]

and now the fil-runout sensor works. The comments in Configuration.h don't tell anything about a builtin limitation to Ultipanel. Also the comment says that Servo Pin 2 is used. This is wrong since the fil runout sensor uses SERVO3_PIN on Output D4 by default.

gatho222 commented 8 years ago

... on I/O Pin D4

thinkyhead commented 8 years ago

All display types with buttons/knobs actually enable ULTIPANEL as well. I'll check to make sure that's being defined for your specific display, and in advance of Configuration_adv.h

thinkyhead commented 8 years ago

Indeed, if all is well with your Arduino IDE, you should not have had to modify Configuration_adv.h to remove the dependency on ULTIPANEL.

MarlinConfig.h:

. . .
#include "Configuration.h"
#include "Conditionals_LCD.h"
#include "Configuration_adv.h"
. . .

Conditionals_LCD.h:

  #if ENABLED(REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER)
    #define DOGLCD
    #define U8GLIB_ST7920
    #define REPRAP_DISCOUNT_SMART_CONTROLLER
  #endif

  #if ENABLED(ULTIMAKERCONTROLLER)              \
   || ENABLED(REPRAP_DISCOUNT_SMART_CONTROLLER) \
   || ENABLED(G3D_PANEL)                        \
   || ENABLED(RIGIDBOT_PANEL)                   \
   || ENABLED(REPRAPWORLD_KEYPAD)
    #define ULTIPANEL
    #define NEWPANEL
  #endif
thinkyhead commented 8 years ago

P.S. Are you conflating FILAMENT_CHANGE_FEATURE and FILAMENT_RUNOUT_SENSOR here? It's the FILAMENT_CHANGE_FEATURE that enables M600 and which depends on ULTIPANEL, not FILAMENT_RUNOUT_SENSOR. I'm not seeing any connection between them in the code.

thinkyhead commented 8 years ago

By the way, welcome to Github.

gatho222 commented 8 years ago

I made no changes to MarlinConfig.h and Conditionals_LCD.h. Just checked the files and the lines you mentioned are all there. I completely agree that the root cause is the Filament Change Feature M600 (which in turn is triggered by the fil runout sensor). I rechecked the original Configuration_adv.h from the archive and found that even if ULTIPANEL is enabled, the Filament Change Feature is still disabled by default (I obviously uncommented this line, too). I reverted the dependency to ULTIPANEL and left FILAMENT_CHANGE_FEATURE uncommented. This configuration also works for me. So the problem was fil runout sensor using M600, but M600 disabled by default in Configuration_adv.h. A comment in Configuration.h at fil runout sensor would be helpful, that M600 is an experimental feature and has to be enabled separately in Configuration_adv.h. Please also change the comment with the wrong Pins.

Original Configurations_adv.h:

[...]
// Add support for experimental filament exchange support M600; requires display
#if ENABLED(ULTIPANEL)
  // #define FILAMENT_CHANGE_FEATURE             // Enable filament exchange menu and M600 g-code (used for runout sensor too)
  #if ENABLED(FILAMENT_CHANGE_FEATURE)
[...]

Thanks a lot for your help. Great Work!

thinkyhead commented 8 years ago

Please also change the comment with the wrong Pins.

Already in the queue.

thinkyhead commented 8 years ago

I see that the default handler for Filament Runout Sensor is "M600" but that's more of a suggestion than a rule. Obviously, if one wants to invoke "M600" then the filament change feature needs to also be enabled. I can add a comment there as a reminder.

I can't account for the failure of ULTIPANEL to exist. It is clearly set by Conditionals_LCD.h in the case of your display. This would seem to indicate a bug in the toolchain. But just to be sure, please make a ZIP of your Marlin folder and drop it here so we can look it over and see what's going on. I tested building with REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER earlier, adding an error flag, and it did get invoked…

[...]
 // Add support for experimental filament exchange support M600; requires display
 #if ENABLED(ULTIPANEL)
+  #error "This error is invoked!"
   // #define FILAMENT_CHANGE_FEATURE             // Enable filament exchange menu and M600 g-code (used for runout sensor too)
   #if ENABLED(FILAMENT_CHANGE_FEATURE)
[...]

What version of Arduino IDE are you using?

gatho222 commented 8 years ago

I can't account for the failure of ULTIPANEL to exist.

Correct. Works as expected. The fact that ULTIPANEL can be enabled directly in Configuration.h led me on the wrong track, since I had it disabled there, not knowing it was automatically enabled in Conditionals_LCD.h by my display settings. The problem clearly was caused by the comment before #define FILAMENT_CHANGE_FEATURE.

Perhaps a solution would be to enable FILAMENT_CHANGE_FEATURE in Configuration.h, Filament Runout Sensor section, directly behind the FILAMENT_RUNOUT_SCRIPT definition:

`//===========================================================================
//========================= Filament Runout Sensor ==========================
//===========================================================================
//#define FILAMENT_RUNOUT_SENSOR // Uncomment for defining a filament runout sensor such as a mechanical or opto endstop to check the existence of filament
                                 // In RAMPS uses SERVO3_PIN mapped to I/O Pin D4. Can be changed in pins file. For other boards pin definition should be made.
                                 // It is assumed that when logic high = filament available
                                 //                    when logic  low = filament ran out
#define FILAMENT_RUNOUT_SENSOR
#if ENABLED(FILAMENT_RUNOUT_SENSOR)
    const bool FIL_RUNOUT_INVERTING = false; // set to true to invert the logic of the sensor.
    //#define ENDSTOPPULLUP_FIL_RUNOUT // Uncomment to use internal pullup for filament runout pins if the sensor is defined.
  #define FILAMENT_RUNOUT_SCRIPT "M600"
  #define FILAMENT_CHANGE_FEATURE             // Enable filament exchange menu and M600 g-code when using M600 as runout script
#endif

This would leave M600 disabled as long as Filament Runout Sensor ist disabled, but enabling it together with the sensor.

What version of Arduino IDE are you using?

I'm using ArdurinoIDE 1.6.10 Hourly Build 2016/07/01 3:25. I doubt this caused the problem though. The firmware is working fine just with FILAMENT_CHANGE_FEATURE enabled and the rest of the code in Configuration_adv.h left original

Thanks again for your time.

thinkyhead commented 8 years ago

The problem clearly was caused by the comment before #define FILAMENT_CHANGE_FEATURE.

Indeed. You have to uncomment a define to enable a feature.

This would leave M600 disabled as long as Filament Runout Sensor ist disabled, but enabling it together with the sensor.

We prefer to have settings exist in only one place. So we can place a comment there that users should enable FILAMENT_CHANGE_FEATURE in Configuration_adv.h but we're not going to automatically enable it as a consequence of enabling FILAMENT_RUNOUT_SENSOR.

I doubt this caused the problem though.

No, as you explained, failing to actually enable FILAMENT_CHANGE_FEATURE caused the problem.

Sorry for the confusion.

github-actions[bot] commented 2 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.