Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] (cannot compile when using Y_MAX Endstop with dual steppers SKR1_4) #22484

Closed
AliSot2000 opened this issue Aug 1, 2021 · 15 comments
Closed

Comments

@AliSot2000
Copy link

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

Yes, and the problem still exists.

Bug Description

I'm trying to compile my custom firmware for the MPCNC.

I had the same issue with 2.0.8, then upgraded to 2.0.9 to see if it was fixed.

I have no extruders but want to have dual x and -y steppers. Also, I have the necessary endstop switches to have an endstop for the second steppers. Adding the items one by one showed, that enabeling #define Y_DUAL_ENDSTOPS is the culprit.

So far I've tried:

  • upgraded from 2.0.8 to 2.0.9
  • upgraded from 2.0.8 to the latest bugfix
  • commenting #define X_DUAL_STEPPER_DRIVERS -> Build Failed
  • commenting #define X_DUAL_ENDSTOPS -> Build Failed

I also tried to modify the pins.h file.
According to a recommendation on the Marlin Discord in support, I added the following lines:

  • #define Y_MAX P1_25 leading to Marlin\src\module\stepper.cpp:369:55: error: 'Y2_MAX' was not declared in this scope; did you mean 'Y_MAX'? ...
  • #define Y2_MAX P1_25 leading to Marlin\src\module/endstops.h:82:39: error: 'Y_MAX' was not declared in this scope; did you mean 'Y2_MAX'?
  • and:
#define Y_MAX P1_25
#define Y2_MAX P1_25

leading to:

Marlin\src\module\stepper.cpp: In static member function 'static void Stepper::pulse_phase_isr()':
Marlin\src\module\../inc/../HAL/./LPC1768/../shared/Marduino.h:42:26: warning: left shift count >= width of type [-Wshift-count-overflow]

Bug Timeline

2.0.8

Expected behavior

Compile.

Actual behavior

Created an error:

In file included from Marlin\src\inc/MarlinConfigPre.h:37,
                 from Marlin\src\inc/MarlinConfig.h:28,
                 from Marlin\src\MarlinCore.h:24,
                 from Marlin\src\MarlinCore.cpp:31:
Marlin\src\module/endstops.h:82:39: error: 'Y_MAX' was not declared in this scope
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                                       ^~~~~
Marlin\src\inc/../core/macros.h:558:26: note: in definition of macro 'THIRD'
  558 | #define THIRD(a,b,c,...) c
      |                          ^
Marlin\src\inc/../core/macros.h:204:29: note: in expansion of macro '___TERN'
  204 | #define __TERN(T,V...)      ___TERN(_CAT(_NO,T),V)  // Prepend '_NO' to get '_NOT_0' or '_NOT_1'
      |                             ^~~~~~~
Marlin\src\inc/../core/macros.h:203:29: note: in expansion of macro '__TERN'
  203 | #define _TERN(E,V...)       __TERN(_CAT(T_,E),V)    // Prepend 'T_' to get 'T_0' or 'T_1'
      |                             ^~~~~~
Marlin\src\inc/../core/macros.h:199:29: note: in expansion of macro '_TERN'
  199 | #define TERN(O,A,B)         _TERN(_ENA_1(O),B,A)    // OPTION ? 'A' : 'B'
      |                             ^~~~~
Marlin\src\module/endstops.h:82:19: note: in expansion of macro 'TERN'
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                   ^~~~
In file included from Marlin\src\gcode\calibrate\../../inc/MarlinConfigPre.h:37,
                 from Marlin\src\gcode\calibrate\../../inc/MarlinConfig.h:28,
                 from Marlin\src\gcode\calibrate\M666.cpp:23:
Marlin\src\gcode\calibrate\../../module/endstops.h:82:39: error: 'Y_MAX' was not declared in this scope
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                                       ^~~~~
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:558:26: note: in definition of macro 'THIRD'
  558 | #define THIRD(a,b,c,...) c
      |                          ^
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:204:29: note: in expansion of macro '___TERN'
  204 | #define __TERN(T,V...)      ___TERN(_CAT(_NO,T),V)  // Prepend '_NO' to get '_NOT_0' or '_NOT_1'
      |                             ^~~~~~~
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:203:29: note: in expansion of macro '__TERN'
  203 | #define _TERN(E,V...)       __TERN(_CAT(T_,E),V)    // Prepend 'T_' to get 'T_0' or 'T_1'
      |                             ^~~~~~
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:199:29: note: in expansion of macro '_TERN'
  199 | #define TERN(O,A,B)         _TERN(_ENA_1(O),B,A)    // OPTION ? 'A' : 'B'
      |                             ^~~~~
Marlin\src\gcode\calibrate\../../module/endstops.h:82:19: note: in expansion of macro 'TERN'
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                   ^~~~
In file included from Marlin\src\gcode\control\../../inc/MarlinConfigPre.h:37,
                 from Marlin\src\gcode\control\../../inc/MarlinConfig.h:28,
                 from Marlin\src\gcode\control\../gcode.h:306,
                 from Marlin\src\gcode\control\M120_M121.cpp:23:
Marlin\src\gcode\control\../../module/endstops.h:82:39: error: 'Y_MAX' was not declared in this scope
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                                       ^~~~~
Marlin\src\gcode\control\../../inc/../core/macros.h:558:26: note: in definition of macro 'THIRD'
  558 | #define THIRD(a,b,c,...) c
      |                          ^
Marlin\src\gcode\control\../../inc/../core/macros.h:204:29: note: in expansion of macro '___TERN'
  204 | #define __TERN(T,V...)      ___TERN(_CAT(_NO,T),V)  // Prepend '_NO' to get '_NOT_0' or '_NOT_1'
      |                             ^~~~~~~
Marlin\src\gcode\control\../../inc/../core/macros.h:203:29: note: in expansion of macro '__TERN'
  203 | #define _TERN(E,V...)       __TERN(_CAT(T_,E),V)    // Prepend 'T_' to get 'T_0' or 'T_1'
      |                             ^~~~~~
Marlin\src\gcode\control\../../inc/../core/macros.h:199:29: note: in expansion of macro '_TERN'
  199 | #define TERN(O,A,B)         _TERN(_ENA_1(O),B,A)    // OPTION ? 'A' : 'B'
      |                             ^~~~~
Marlin\src\gcode\control\../../module/endstops.h:82:19: note: in expansion of macro 'TERN'
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                   ^~~~
*** [.pio\build\LPC1768\src\src\MarlinCore.cpp.o] Error 1
In file included from Marlin\src\gcode\calibrate\../../inc/MarlinConfigPre.h:37,
                 from Marlin\src\gcode\calibrate\../../inc/MarlinConfig.h:28,
                 from Marlin\src\gcode\calibrate\G28.cpp:23:
Marlin\src\gcode\calibrate\../../module/endstops.h:82:39: error: 'Y_MAX' was not declared in this scope
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                                       ^~~~~
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:558:26: note: in definition of macro 'THIRD'
  558 | #define THIRD(a,b,c,...) c
      |                          ^
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:204:29: note: in expansion of macro '___TERN'
  204 | #define __TERN(T,V...)      ___TERN(_CAT(_NO,T),V)  // Prepend '_NO' to get '_NOT_0' or '_NOT_1'
      |                             ^~~~~~~
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:203:29: note: in expansion of macro '__TERN'
  203 | #define _TERN(E,V...)       __TERN(_CAT(T_,E),V)    // Prepend 'T_' to get 'T_0' or 'T_1'
      |                             ^~~~~~
Marlin\src\gcode\calibrate\../../inc/../core/macros.h:199:29: note: in expansion of macro '_TERN'
  199 | #define TERN(O,A,B)         _TERN(_ENA_1(O),B,A)    // OPTION ? 'A' : 'B'
      |                             ^~~~~
Marlin\src\gcode\calibrate\../../module/endstops.h:82:19: note: in expansion of macro 'TERN'
   82 |     , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
      |                   ^~~~
*** [.pio\build\LPC1768\src\src\gcode\calibrate\M666.cpp.o] Error 1
*** [.pio\build\LPC1768\src\src\gcode\control\M120_M121.cpp.o] Error 1
*** [.pio\build\LPC1768\src\src\gcode\calibrate\G28.cpp.o] Error 1

Steps to Reproduce

  1. Set Platformio to LPC1768
  2. Paste config and config_adv
  3. Build

Version of Marlin Firmware

Bugfix 2.0.9

Printer model

MPCNC

Electronics

BTT_SKR_V1_4, TMC2208

Add-ons

No response

Your Slicer

No response

Host Software

No response

Additional information & file uploads

def Y_DUAL_ENDSTOPS.txt
ndef Y_DUAL_ENDSTOPS.txt
configuration.zip

@ellensp
Copy link
Contributor

ellensp commented Aug 2, 2021

Take a look at the board, at the labels, do you see any X Max or Y Max endstop plugs? No you don't. because this board only has x_stop, y_stop, z_stop, e0_det and e1_det. If you wan't to use e0_det and e1_det you have to tell marlin to do so.

Ie In Configuration_adv.h you need

#define X_DUAL_STEPPER_DRIVERS
#if ENABLED(X_DUAL_STEPPER_DRIVERS)
  #define INVERT_X2_VS_X_DIR    // Enable if X2 direction signal is opposite to X
  #define X_DUAL_ENDSTOPS
  #if ENABLED(X_DUAL_ENDSTOPS)
    #define X2_USE_ENDSTOP _E0DIAG_
    #define X2_ENDSTOP_ADJUSTMENT  0
  #endif
#endif

#define Y_DUAL_STEPPER_DRIVERS
#if ENABLED(Y_DUAL_STEPPER_DRIVERS)
  #define INVERT_Y2_VS_Y_DIR   // Enable if Y2 direction signal is opposite to Y
  #define Y_DUAL_ENDSTOPS
  #if ENABLED(Y_DUAL_ENDSTOPS)
    #define Y2_USE_ENDSTOP _E1DIAG_
    #define Y2_ENDSTOP_ADJUSTMENT  0
  #endif
#endif

@AliSot2000
Copy link
Author

AliSot2000 commented Aug 2, 2021

Thank you for your help!

I would like to add, that I was confused by the pins.h file for the SKR board. Based on it I thought I didn't have to change the Endstop even though I considered it as a cause of the problem. I wrote it off due to the following:

//
// TMC StallGuard DIAG pins
//
#define X_DIAG_PIN                         P1_29  // X-STOP
#define Y_DIAG_PIN                         P1_28  // Y-STOP
#define Z_DIAG_PIN                         P1_27  // Z-STOP
#define E0_DIAG_PIN                        P1_26  // E0DET
#define E1_DIAG_PIN                        P1_25  // E1DET

As I neither have extruders nor TMC StallGuard, I assumed P1_26 and P1_25 were not assigned.

//
// Limit Switches
//
#ifdef X_STALL_SENSITIVITY
  #define X_STOP_PIN                  X_DIAG_PIN
  #if X_HOME_TO_MIN
    #define X_MAX_PIN                      P1_26  // E0DET
  #else
    #define X_MIN_PIN                      P1_26  // E0DET
  #endif
#elif ENABLED(X_DUAL_ENDSTOPS)
  #ifndef X_MIN_PIN
    #define X_MIN_PIN                      P1_29  // X-STOP
  #endif
  #ifndef X_MAX_PIN
    #define X_MAX_PIN                      P1_26  // E0DET
  #endif
#else
  #define X_STOP_PIN                       P1_29  // X-STOP
#endif

#ifdef Y_STALL_SENSITIVITY
  #define Y_STOP_PIN                  Y_DIAG_PIN
  #if Y_HOME_TO_MIN
    #define Y_MAX_PIN                      P1_25  // E1DET
  #else
    #define Y_MIN_PIN                      P1_25  // E1DET
  #endif
#elif ENABLED(Y_DUAL_ENDSTOPS)
  #ifndef Y_MIN_PIN
    #define Y_MIN_PIN                      P1_28  // Y-STOP
  #endif
  #ifndef Y_MAX_PIN
    #define Y_MAX_PIN                      P1_25  // E1DET
  #endif
#else
  #define Y_STOP_PIN                       P1_28  // Y-STOP
#endif

I assumed they would have been assigned as the X_MAX and Y_MAX pins due to the #elif ENABLED(Y_DUAL_ENDSTOPS) and #elif ENABLED(X_DUAL_ENDSTOPS).

Maybe this needs some clarification or this is a feature not yet completely implemented and will be in the future?

@ellensp
Copy link
Contributor

ellensp commented Aug 2, 2021

reopening for further research

@ellensp ellensp reopened this Aug 2, 2021
@ellensp
Copy link
Contributor

ellensp commented Aug 2, 2021

I think that is someones attempt at avoiding having to set E0DIAG and E1DIAG
But it doesn't work..

What I have found so far is there is a macro
_HAS_STOP(Y,MAX) which expands to (PIN_EXISTS(Y_MAX) && !IS_PROBE_PIN(Y,MAX) && !IS_X2_ENDSTOP(Y,MAX) && !IS_Y2_ENDSTOP(Y,MAX) && !IS_Z2_ENDSTOP(Y,MAX) && !IS_Z3_ENDSTOP(Y,MAX) && !IS_Z4_ENDSTOP(Y,MAX))

The test !IS_Y2_ENDSTOP(Y,MAX) returns 0 as it explicitly says that if Y2 endstop is set to Y_MAX, Y_MAX endstop does not exist.

@thinkyhead
Copy link
Member

That construct in some pins files that have DIAG pins was requested so that an unused endstop pin would have a pin name (the unused endstop at the other end of the axis). The curious can use "Line History" to find out more about those changes and take a look at the original conversations. Meanwhile, we should mitigate any side-effects from that.

@ellensp
Copy link
Contributor

ellensp commented Aug 2, 2021

@thinkyhead yes that was before someone added #elif ENABLED(Y_DUAL_ENDSTOPS) code to the pins file

@ellensp
Copy link
Contributor

ellensp commented Aug 2, 2021

issue stems from #define Y_HOME_DIR 1 with Y_DUAL_ENDSTOPS

In endstops.h

 #if HAS_Y_MIN || HAS_Y_MAX
    , Y_ENDSTOP = TERN(Y_HOME_TO_MAX, Y_MAX, Y_MIN)
  #endif 

so if homing to max, it presumes Y_MAX exists, but Y_MAX does not exists when Y_DUAL_ENDSTOPS is enable

@thinkyhead
Copy link
Member

thinkyhead commented Aug 2, 2021

That Y_MAX enumerated constant should refer to the end of the axis that Y homes to, regardless of which pins are used for the endstops. So, the logic to define HAS_Y_MIN, HAS_Y_MAX, and those enumerated constants needs to be modified to account for the possibility of dual endstops. Probably for X and Z as well.

@thinkyhead
Copy link
Member

Checking on the rest of the code, HAS_Y_MIN or HAS_Y_MAX should be set based on the first Y endstop, since there are also HAS_Y2_M(IN|AX) flags for the other endstop.

@thinkyhead
Copy link
Member

thinkyhead commented Aug 2, 2021

So, this might work in Conditionals_post.h

#if ALL(HAS_Y_AXIS, Y_HOME_TO_MIN, USE_YMIN_PLUG) && _HAS_STOP(Y,MIN)
  #define HAS_Y_MIN 1
#endif
#if ALL(HAS_Y_AXIS, Y_HOME_TO_MAX, USE_YMAX_PLUG) && _HAS_STOP(Y,MAX)
  #define HAS_Y_MAX 1
#endif
#if ALL(HAS_Z_AXIS, Z_HOME_TO_MIN, USE_ZMIN_PLUG) && _HAS_STOP(Z,MIN)
  #define HAS_Z_MIN 1
#endif
#if ALL(HAS_Z_AXIS, Z_HOME_TO_MAX, USE_ZMAX_PLUG) && _HAS_STOP(Z,MAX)
  #define HAS_Z_MAX 1
#endif
#if LINEAR_AXES >= 4 && ENABLED(USE_IMIN_PLUG) && _HAS_STOP(I,MIN)
  #define HAS_I_MIN 1
#endif
#if LINEAR_AXES >= 4 && ENABLED(USE_IMAX_PLUG) && _HAS_STOP(I,MAX)
  #define HAS_I_MAX 1
#endif
#if LINEAR_AXES >= 5 && ENABLED(USE_JMIN_PLUG) && _HAS_STOP(J,MIN)
  #define HAS_J_MIN 1
#endif
#if LINEAR_AXES >= 5 && ENABLED(USE_JMAX_PLUG) && _HAS_STOP(J,MAX)
  #define HAS_J_MAX 1
#endif
#if LINEAR_AXES >= 6 && ENABLED(USE_KMIN_PLUG) && _HAS_STOP(K,MIN)
  #define HAS_K_MIN 1
#endif
#if LINEAR_AXES >= 6 && ENABLED(USE_KMAX_PLUG) && _HAS_STOP(K,MAX)
  #define HAS_K_MAX 1
#endif
#if ENABLED(X_DUAL_ENDSTOPS) && PIN_EXISTS(X2_MIN)
  #define HAS_X2_MIN 1
#endif
#if ENABLED(X_DUAL_ENDSTOPS) && PIN_EXISTS(X2_MAX)
  #define HAS_X2_MAX 1
#endif
#if ENABLED(Y_DUAL_ENDSTOPS) && PIN_EXISTS(Y2_MIN)
  #define HAS_Y2_MIN 1
#endif
#if ENABLED(Y_DUAL_ENDSTOPS) && PIN_EXISTS(Y2_MAX)
  #define HAS_Y2_MAX 1
#endif
#if ENABLED(Z_MULTI_ENDSTOPS)
  #if PIN_EXISTS(Z2_MIN)
    #define HAS_Z2_MIN 1
  #endif
  #if PIN_EXISTS(Z2_MAX)
    #define HAS_Z2_MAX 1
  #endif
  #if NUM_Z_STEPPER_DRIVERS >= 3
    #if PIN_EXISTS(Z3_MIN)
      #define HAS_Z3_MIN 1
    #endif
    #if PIN_EXISTS(Z3_MAX)
      #define HAS_Z3_MAX 1
    #endif
  #endif
  #if NUM_Z_STEPPER_DRIVERS >= 4
    #if PIN_EXISTS(Z4_MIN)
      #define HAS_Z4_MIN 1
    #endif
    #if PIN_EXISTS(Z4_MAX)
      #define HAS_Z4_MAX 1
    #endif
  #endif
#endif

@thinkyhead
Copy link
Member

thinkyhead commented Aug 2, 2021

Then in endstops.h this becomes cleaner:

  // Extra Endstops for XYZ
  _ES_ITEM(HAS_X2_MIN, X2_MIN)
  _ES_ITEM(HAS_X2_MAX, X2_MAX)
  _ES_ITEM(HAS_Y2_MIN, Y2_MIN)
  _ES_ITEM(HAS_Y2_MAX, Y2_MAX)
  _ES_ITEM(HAS_Z2_MIN, Z2_MIN)
  _ES_ITEM(HAS_Z2_MAX, Z2_MAX)
  _ES_ITEM(HAS_Z3_MIN, Z3_MIN)
  _ES_ITEM(HAS_Z3_MAX, Z3_MAX)
  _ES_ITEM(HAS_Z4_MIN, Z4_MIN)
  _ES_ITEM(HAS_Z4_MAX, Z4_MAX)

@thinkyhead
Copy link
Member

thinkyhead commented Aug 2, 2021

As far as I can tell, there is no implementation of SPI_ENDSTOPS at this time to handle multi- TMC steppers on an axis.

@thinkyhead
Copy link
Member

I pushed a patch for endstop flags and it could use some extra confirmation. Please test the bugfix-2.0.x branch to see where it stands. If the problem has been resolved then we can close this issue. If the issue isn't resolved yet, then we should investigate further.

@AliSot2000
Copy link
Author

Hi
Sorry for the long wait, I was able to confirm, that the endstops work now correctly. Thank you for the quick help.

@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants