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

[BUG] Unconditionnal definition of `SUICIDE_PIN` for `BOARD_RAMPS_CREALITY`, leading to issue with `PSU_CONTROL` #27142

Closed TheRaf974 closed 3 months ago

TheRaf974 commented 3 months ago

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

Yes, and the problem still exists.

Bug Description

I have a CR-10 S5 with a Creality mainboard v2.2. It runs on Marlin 2.1.2.3 with BOARD_RAMPS_CREALITY

I was setting up PSU_CONTROL to turn off my printer on M81 using a static relay located before my power supply unit.

To use PSU_CONTROL, a PS_ON_PIN is required. The default pin 40, in pins_RAMPS CREALITY.h isn't accessible in my board. The CR2020 must have a specific board with pin 40 exposed, which is not my case.

#ifndef PS_ON_PIN
  #define PS_ON_PIN                           40  // Used by CR2020 Industrial series
#endif

So, I added #define PS_ON_PIN EXP4_PIN in my Configuration.h, based on pins_RAMPS CREALITY.h. Comments suggest that PS_ON_PIN could be use on pin 12, which seems much more logical than pin 40.

#define EXP1_PIN                              65  // A11 - Used by CR2020 Industrial series for case
#define EXP2_PIN                              66  // A12
#define EXP3_PIN                              11  // SERVO0_PIN
#define EXP4_PIN                              12  // PS_ON_PIN

At this time, PSU_CONTROL comportement was inversed (M80 turned off and M81 turned on) no matter what was the value of PSU_ACTIVE_STATE. After digging a bit, I found that in M80_M81.cpp for M81() (turn off), the usage of suicide() is prioritary to powerManager.power_off_soon():

  #if HAS_SUICIDE
    suicide();
  #elif ENABLED(PSU_CONTROL)
    powerManager.power_off_soon();
  #endif

For M80() (turn on), this time after powerManager.power_on(), it turn low the SUICIDE_PIN:

powerManager.power_on();

    /**
     * If you have a switch on suicide pin, this is useful
     * if you want to start another print with suicide feature after
     * a print without suicide...
     */
    #if HAS_SUICIDE
      OUT_WRITE(SUICIDE_PIN, !SUICIDE_PIN_STATE);
    #endif

PSU_CONTROL comportement was inversed because :

  1. SUICIDE_PIN is unconditionnally defined to pin 12 (the one I use)
  2. the logic of suicide is the opposite of the one I was searching for
  3. suicide was prioritary to PSU_CONTROL

This unconfitionnal definition sounds problematic to me, as it seems to be specific to one printer among the tens of creality's printer.

#define SUICIDE_PIN                           12  // Used by CR2020 Industrial series
#ifndef SUICIDE_PIN_STATE
  #define SUICIDE_PIN_STATE                 HIGH
#endif

I solved my problem by replacing this part with this

#ifndef SUICIDE_PIN
  #define SUICIDE_PIN                           12  // Used by CR2020 Industrial series
  #ifndef SUICIDE_PIN_STATE
    #define SUICIDE_PIN_STATE                 HIGH
  #endif
#endif

and #define SUICIDE_PIN -1 in my Configuration.h (I'll make a PR for this, looks like a good fix to me)

Nevertheless, I'm not convinced that one specific printer using a modified version of a more standard board should drive the default definition of the standard one.

At least, I think that PS_ON_PIN and SUICIDE_PIN should be modified. Pin 40 (PS_ON_PIN) isn't exposed on the standard creality mainboard, so this is irrelevant. Pin 12 (SUICIDE_PIN) is a part of a group of 4 pins planned as extensions pins.

If a default PS_ON_PIN is really required, then its sounds more logic to me to use pin 12, as comments suggested.

PS: After some research, the board used in the CR2020 seems to be the Creality mainboard v3.1, which seems to be discontinued. Maybe, this board should have its own pins.h file.

Bug Timeline

No response

Expected behavior

No response

Actual behavior

No response

Steps to Reproduce

No response

Version of Marlin Firmware

2.1.2.3

Printer model

Creality CR-10 S5

Electronics

Power Supply Control with a static relay

LCD/Controller

No response

Other add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

Additional information & file uploads

Configuration.zip

TheRaf974 commented 3 months ago

I also have an interrogation about the suicide feature, what is it exactly ? I can't find any documentation or discussion about it. It seems to be a legacy of Marlin 1. Should it be mapped to a pin connected to the reset pin of a the microcontroller to force reset in case of a critical failure ?

thinkyhead commented 3 months ago

Some boards (ZM3) and add-ons (MKS_PWC) provide a "suicide pin" feature, similar to a "dead-man's switch." The pin must remain in a particular state, or else the machine does a rapid shutdown. The "CR2020 Industrial series" must have this also. These pins definitions came directly from Creality, and often vendor code changes are not tailored to consider use in our more generic codebase. I am adding a wrapper so that configs will need to specify that they are actually a "CR2020" and only then will these power / suicide pins be applied.

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