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

Simplified endstop configuration #3065

Merged
merged 5 commits into from
Apr 4, 2016

Conversation

thinkyhead
Copy link
Member

One issue we constantly see is users failing to enable DISABLE_MAX_ENDSTOPS or DISABLE_MIN_ENDSTOPS, causing them to scratch their heads and complain in the issue queue. Instead of adding more stern comments to the configuration files, this solves the problem by adding individual flags to specify which endstops are connected. Sanity checks have been added to warn users about these deprecated configuration options.

As an adjunct to this, Z2 endstop configuration is simplified. Instead of setting a literal PIN number, #undeffing the endstop flag, and setting the inverting, instead you just specify which axis to use.

  • Add individual endstop flags to specify which endstops exist
  • Drop DISABLE_M(IN|AX)_ENDSTOPS and use individual endstop flags instead
  • Drop DISABLE_[XYZ]_ENDSTOPS
  • Reduce the configuration of the Z2 endstop to one setting

This PR is posted here as a conceptual example, not necessarily bound for the next RC release, but something to discuss and play around with.

I've tried to get the endstops correct in the example configurations, but a few of these configurations had an improper setting for DISABLE_MAX_ENDSTOPS. Please confirm whether these appear correct for each machine.

I also want to ensure that these tests are equivalent for this purpose:

-     #if DISABLED(DISABLE_MAX_ENDSTOPS)
+    #if DISABLED(USE_ENDSTOP_X_MAX) && DISABLED(USE_ENDSTOP_Y_MAX) && DISABLED(USE_ENDSTOP_Z_MAX)

#define ENDSTOPPULLUP_XMAX
#define ENDSTOPPULLUP_YMAX
#define ENDSTOPPULLUP_ZMAX
#endif
#if DISABLED(DISABLE_MIN_ENDSTOPS)
#if DISABLED(HAS_ENDSTOP_X_MIN) && DISABLED(HAS_ENDSTOP_Y_MIN) && DISABLED(HAS_ENDSTOP_Z_MIN)
#define ENDSTOPPULLUP_XMIN
#define ENDSTOPPULLUP_YMIN
#define ENDSTOPPULLUP_ZMIN
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has to be handled per endstop - not all together.

#if DISABLED(HAS_ENDSTOP_X_MAX)
  #define ENDSTOPPULLUP_XMAX
#endif
...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

@Blue-Marlin
Copy link
Contributor

Because of

  #define USE_X_MIN (PIN_EXISTS(X_MIN))
  #define USE_X_MAX (PIN_EXISTS(X_MAX))
  #define USE_Y_MIN (PIN_EXISTS(Y_MIN))
  #define USE_Y_MAX (PIN_EXISTS(Y_MAX))
  #define USE_Z_MIN (PIN_EXISTS(Z_MIN))
  #define USE_Z_MAX (PIN_EXISTS(Z_MAX))
  #define USE_Z2_MIN (PIN_EXISTS(Z2_MIN))
  #define USE_Z2_MAX (PIN_EXISTS(Z2_MAX))

in Conditionals.h and their use in stepper.cpp all this will not have the desired effect.

@Blue-Marlin
Copy link
Contributor

Maybe you want to adopt something from here (5734d0e)

@thinkyhead
Copy link
Member Author

Because of [code] in Conditionals.h and their use in stepper.cpp all this will not have the desired effect.

Actually, I don't think this PR changes the behavior of stepper.cpp w/r/t endstop testing. (But maybe that should also be changed.) As far as I can gather, the code in stepper.cpp has always monitored all the existing pins, even though nothing may be connected to them. I believe this is the reason why the pullups are set when endstops are claimed to not exist. So the stepper code will simply see those switches as never-triggered endstops.

@thinkyhead thinkyhead force-pushed the rc_doc_tweaks branch 4 times, most recently from dc1efad to 1d9b754 Compare March 3, 2016 08:10
@Blue-Marlin
Copy link
Contributor

Thanks. Looks very nice now.

@thinkyhead
Copy link
Member Author

@Blue-Marlin I gave a little more thought to your question about how the endstops are polled, and a couple of things dawned:

  • The stepper ISR just always just gets the state of the existing hardware pins
  • While we have a few settings which re-map the functions of those pins, we don't have a general way of remapping their functions. I could potentially re-work this to allow setting which port corresponds to which endstop function.

@thinkyhead
Copy link
Member Author

A clever technique is used in Simplify configuration of Z2 endstops to choose which of the six endstop connectors (standard fare on all boards) is used for the Z2 endstop. The same technique could be used to set which endstop connector you are using for a particular function, perhaps simplifying the way that z probe and z endstop relationships are worked out.

(Always looking for new ways to reduce the number of endstop, homing, probing, and probe options!)

Then configuration might be done by assigning the named connector to the function (instead of assigning the function to the connector)…

#define FUNC_ENDSTOP_MIN_X _XMIN_
#define FUNC_ENDSTOP_MAX_X _NONE_
#define FUNC_ENDSTOP_MIN_Y _YMIN_
#define FUNC_ENDSTOP_MAX_Y _NONE_
#define FUNC_ENDSTOP_MIN_Z _ZMIN_
#define FUNC_ENDSTOP_MAX_Z _NONE_
#define FUNC_INDUCTIVE_SENSOR _XMAX_ // Set to _NONE_ for no inductive sensor
#define FUNC_PROBE_SWITCH _ZMAX_ // Set to _NONE_ for no servo probe

@thinkyhead thinkyhead mentioned this pull request Mar 23, 2016
@thinkyhead thinkyhead force-pushed the rc_doc_tweaks branch 3 times, most recently from 8f453af to b79b38b Compare March 24, 2016 09:47
@thinkyhead
Copy link
Member Author

I've changed the name of the individual endstop flags to indicate that we are referring to the plug connectors and not to "endstops" per se.

@thinkyhead thinkyhead merged commit c045ec8 into MarlinFirmware:RCBugFix Apr 4, 2016
@thinkyhead thinkyhead deleted the rc_doc_tweaks branch April 15, 2016 04:46
@jbrazio jbrazio modified the milestone: 1.1.0 Jul 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Improvement T: Design Concept Technical ideas about ways and methods.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants