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.

Sign up for GitHub

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

Jump to bottom

Cleanup and consolidate probe conditionals for clarity #3672

Merged
merged 1 commit into from
May 8, 2016
Merged

Cleanup and consolidate probe conditionals for clarity #3672

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions Marlin/Conditionals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@
#define HAS_Z_MAX (PIN_EXISTS(Z_MAX))
#define HAS_Z2_MIN (PIN_EXISTS(Z2_MIN))
#define HAS_Z2_MAX (PIN_EXISTS(Z2_MAX))
#define HAS_Z_PROBE (PIN_EXISTS(Z_MIN_PROBE))
#define HAS_Z_MIN_PROBE_PIN (PIN_EXISTS(Z_MIN_PROBE))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why Z_MIN in HAS_Z_MIN_PROBE_PIN ?

Copy link
Member Author

@thinkyhead thinkyhead May 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It reflects the naming of the pin. If the pin was called PROBE_PIN then by convention, this would be HAS_PROBE (but you can see why that's confusing, given the naming of some other conditionals, like HAS_ANY_PROBE and HAS_Z_PROBE.) It's because of this confusion that I even added _PIN to the end in contravention.

Maybe we should consider BED_PROBE_PIN as the pin name. But BED almost always refers to heaters/thermistors and the bed, not things that probe the bed… Naming things can be tricky.

Copy link
Member Author

@thinkyhead thinkyhead May 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It's really Z_MIN_PROBE_ENDSTOP that I most want to rename. It's too damn vague.

Copy link
Contributor

@Blue-Marlin Blue-Marlin May 5, 2016 •
edited by thinkyhead
Loading

Choose a reason for hiding this comment

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

Indeed we have removed the possibility to have z-max, x-max, x-min, y-max, y-min probes/servo-endstops. So z-min is the only remaining one. So PROBE seems to be enough to describe.

A possible concept for the use and pin configuration of a probe could be

// If you have a probe, select one of the next three options.
Z_MIN_PROBE_ENDSTOP ->                USE_PROBE_AT_PROBE_PIN_FOR_HOMING_AND_PROBING
DISABLE_Z_MIN_PROBE_ENDSTOP ->        USE_PROBE_AT_PROBE_PIN_FOR_PROBING_ONLY
Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN -> USE_PROBE_AT_Z_MIN_PIN_FOR_HOMING_AND_PROBING
// If you don't have a probe, a z-min endstop, connected to a Z_MIN_PIN will be used to home towards z-min. No probing is possible.

The names are a bit long, but hopefully self explaining.

Copy link
Member Author

Choose a reason for hiding this comment

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

we have removed the possibility to have z-max, x-max, x-min, y-max, y-min probes/servo-endstops

Looks like nobody told homeaxis() that fact…

#if HAS_SERVO_ENDSTOPS
  // Engage an X or Y Servo endstop if enabled
  if (_Z_SERVO_TEST && servo_endstop_id[axis] >= 0) {
    servo[servo_endstop_id[axis]].move(servo_endstop_angle[axis][0]);
    if (_Z_PROBE_SUBTEST) endstops.z_probe_enabled = true;
  }
#endif

#define HAS_SOLENOID_1 (PIN_EXISTS(SOL1))
#define HAS_SOLENOID_2 (PIN_EXISTS(SOL2))
#define HAS_SOLENOID_3 (PIN_EXISTS(SOL3))
Expand Down Expand Up @@ -742,16 +742,11 @@
#endif
#endif

#if ( (HAS_Z_MIN && ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)) || HAS_Z_PROBE ) \
&& ( \
ENABLED(FIX_MOUNTED_PROBE) \
|| ENABLED(MECHANICAL_PROBE) \
|| HAS_Z_ENDSTOP_SERVO \
|| ENABLED(Z_PROBE_ALLEN_KEY) \
|| ENABLED(Z_PROBE_SLED) \
)
#define HAS_Z_MIN_PROBE
#endif
#define PROBE_SELECTED (ENABLED(FIX_MOUNTED_PROBE) || ENABLED(MECHANICAL_PROBE) || HAS_Z_ENDSTOP_SERVO || ENABLED(Z_PROBE_SLED))

Copy link
Contributor

Choose a reason for hiding this comment

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

Z_PROBE_ALLEN_KEY vanished. ???

Copy link
Member Author

Choose a reason for hiding this comment

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

Z_PROBE_ALLEN_KEY => MECHANICAL_PROBE.

#define PROBE_PIN_CONFIGURED (HAS_Z_MIN_PROBE_PIN || (HAS_Z_MIN && ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)))

#define HAS_BED_PROBE (PROBE_SELECTED && PROBE_PIN_CONFIGURED)

/**
* Delta radius/rod trimmers
Expand Down
8 changes: 4 additions & 4 deletions Marlin/Marlin_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1681,7 +1681,7 @@ static void setup_for_endstop_move() {
refresh_cmd_timeout();
}

#if ENABLED(HAS_Z_MIN_PROBE)
#if HAS_BED_PROBE

static void deploy_z_probe() {

Expand Down Expand Up @@ -1878,7 +1878,7 @@ static void setup_for_endstop_move() {

endstops.enable_z_probe(false);
}
#endif // HAS_Z_MIN_PROBE
#endif // HAS_BED_PROBE

enum ProbeAction {
ProbeStay = 0,
Expand Down Expand Up @@ -3579,7 +3579,7 @@ inline void gcode_G28() {
}
#endif
enqueue_and_echo_commands_P(PSTR(Z_PROBE_END_SCRIPT));
#if ENABLED(HAS_Z_MIN_PROBE)
#if HAS_BED_PROBE
endstops.enable_z_probe(false);
#endif
stepper.synchronize();
Expand Down Expand Up @@ -3942,7 +3942,7 @@ inline void gcode_M42() {
* Z_MIN_PROBE_PIN, but here for clarity.
*/
#if ENABLED(Z_MIN_PROBE_ENDSTOP)
#if !HAS_Z_PROBE
#if !HAS_Z_MIN_PROBE_PIN
#error You must define Z_MIN_PROBE_PIN to enable Z probe repeatability calculation.
#endif
#elif !HAS_Z_MIN
Expand Down
97 changes: 66 additions & 31 deletions Marlin/SanityCheck.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,50 +204,55 @@
* Probes
*/

/**
* A probe needs a pin
*/
#if (!((HAS_Z_MIN && ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)) || HAS_Z_PROBE )) && ( ENABLED(FIX_MOUNTED_PROBE) || defined(Z_ENDSTOP_SERVO_NR) || ENABLED(MECHANICAL_PROBE) || ENABLED(Z_PROBE_SLED))
#error A probe needs a pin! [Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN || HAS_Z_PROBE]
#endif
#if PROBE_SELECTED

#if ((HAS_Z_MIN && ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)) && HAS_Z_PROBE) && ( ENABLED(FIX_MOUNTED_PROBE) || defined(Z_ENDSTOP_SERVO_NR) || ENABLED(MECHANICAL_PROBE) || ENABLED(Z_PROBE_SLED))
#error A probe should not be connected to more than one pin! [Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN || HAS_Z_PROBE]
#endif
/**
* A probe needs a pin
*/
#if !PROBE_PIN_CONFIGURED
#error A probe needs a pin! Use Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN or Z_MIN_PROBE_PIN.
#endif

/**
* Require one kind of probe
*/
#if ENABLED(AUTO_BED_LEVELING_FEATURE) && !( ENABLED(FIX_MOUNTED_PROBE) || defined(Z_ENDSTOP_SERVO_NR) || ENABLED(MECHANICAL_PROBE) || ENABLED(Z_PROBE_SLED))
#error For AUTO_BED_LEVELING_FEATURE define one kind of probe! [Servo | MECHANICAL_PROBE | Z_PROBE_SLED | FIX_MOUNTED_PROBE]
#endif
/**
* Z_MIN_PIN and Z_MIN_PROBE_PIN can't co-exist when Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN
*/
#if HAS_Z_MIN && HAS_Z_MIN_PROBE_PIN && ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
#error A probe cannot have more than one pin! Use Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN or Z_MIN_PROBE_PIN.
#endif

// To do: Fail with more than one probe defined
/**
* Make sure the plug is enabled if it's used
*/
#if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN) && DISABLED(USE_ZMIN_PLUG)
#error You must enable USE_ZMIN_PLUG if any probe or endstop is connected to the ZMIN plug.
#endif

/**
* Auto Bed Leveling
*/
#if ENABLED(AUTO_BED_LEVELING_FEATURE)
/**
* Only allow one probe option to be defined
*/
#if (ENABLED(FIX_MOUNTED_PROBE) && (ENABLED(MECHANICAL_PROBE) || HAS_Z_ENDSTOP_SERVO || ENABLED(Z_PROBE_SLED))) \
|| (ENABLED(MECHANICAL_PROBE) && (HAS_Z_ENDSTOP_SERVO || ENABLED(Z_PROBE_SLED))) \
|| (HAS_Z_ENDSTOP_SERVO && ENABLED(Z_PROBE_SLED))
#error Please define only one type of probe: Z Servo, MECHANICAL_PROBE, Z_PROBE_SLED, or FIX_MOUNTED_PROBE.
#endif

Copy link
Contributor

Choose a reason for hiding this comment

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

Allen Key?

Copy link
Member Author

@thinkyhead thinkyhead May 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Z_PROBE_ALLEN_KEY automagically sets MECHANICAL_PROBE.

/**
* Require a Z min pin
* Don't allow nonsense probe-pin settings
*/
#if !PIN_EXISTS(Z_MIN)
#if !PIN_EXISTS(Z_MIN_PROBE) || (DISABLED(Z_MIN_PROBE_ENDSTOP) || ENABLED(DISABLE_Z_MIN_PROBE_ENDSTOP)) // It's possible for someone to set a pin for the Z probe, but not enable it.
#if ENABLED(Z_MIN_PROBE_REPEATABILITY_TEST)
#error You must have a Z_MIN or Z_PROBE endstop to enable Z_MIN_PROBE_REPEATABILITY_TEST.
#else
#error AUTO_BED_LEVELING_FEATURE requires a Z_MIN or Z_PROBE endstop. Z_MIN_PIN or Z_MIN_PROBE_PIN must point to a valid hardware pin.
#endif
#endif
#if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN) && ENABLED(Z_MIN_PROBE_ENDSTOP)
#error You can't enable both Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN and Z_MIN_PROBE_ENDSTOP.
#elif ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN) && ENABLED(DISABLE_Z_MIN_PROBE_ENDSTOP)
#error Don't enable DISABLE_Z_MIN_PROBE_ENDSTOP with Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN.
#elif ENABLED(DISABLE_Z_MIN_PROBE_ENDSTOP) && DISABLED(Z_MIN_PROBE_ENDSTOP)
#error DISABLE_Z_MIN_PROBE_ENDSTOP requires Z_MIN_PROBE_ENDSTOP to be set.
#endif

/**
* Require a Z probe pin if Z_MIN_PROBE_ENDSTOP is enabled.
*/
#if ENABLED(Z_MIN_PROBE_ENDSTOP)
#if !PIN_EXISTS(Z_MIN_PROBE)
#error You must have a Z_MIN_PROBE_PIN defined in your pins_XXXX.h file if you enable Z_MIN_PROBE_ENDSTOP.
#if !HAS_Z_MIN_PROBE_PIN
#error Z_MIN_PROBE_ENDSTOP requires a Z_MIN_PROBE_PIN in your board's pins_XXXX.h file.
#endif
// Forcing Servo definitions can break some hall effect sensor setups. Leaving these here for further comment.
//#ifndef NUM_SERVOS
Expand All @@ -263,6 +268,36 @@
// #error You must have SERVO_ENDSTOP_ANGLES defined for Z Extend and Retract to use Z_MIN_PROBE_ENDSTOP.
//#endif
#endif

#else

/**
* Require some kind of probe for bed leveling
*/
#if ENABLED(AUTO_BED_LEVELING_FEATURE)
#error AUTO_BED_LEVELING_FEATURE requires a probe! Define a Z Servo, MECHANICAL_PROBE, Z_PROBE_SLED, or FIX_MOUNTED_PROBE.
#endif

#endif

/**
* Auto Bed Leveling
*/
#if ENABLED(AUTO_BED_LEVELING_FEATURE)

/**
* Require a Z min pin
*/
#if !PIN_EXISTS(Z_MIN)
#if !PIN_EXISTS(Z_MIN_PROBE) || (DISABLED(Z_MIN_PROBE_ENDSTOP) || ENABLED(DISABLE_Z_MIN_PROBE_ENDSTOP)) // It's possible for someone to set a pin for the Z probe, but not enable it.
#if ENABLED(Z_MIN_PROBE_REPEATABILITY_TEST)
#error You must have a Z_MIN or Z_PROBE endstop to enable Z_MIN_PROBE_REPEATABILITY_TEST.
#else
#error AUTO_BED_LEVELING_FEATURE requires a Z_MIN or Z_PROBE endstop. Z_MIN_PIN or Z_MIN_PROBE_PIN must point to a valid hardware pin.
#endif
#endif
#endif

/**
* Check if Probe_Offset * Grid Points is greater than Probing Range
*/
Expand Down
10 changes: 5 additions & 5 deletions Marlin/endstops.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Endstops::Endstops() {
#endif
);
enable(true);
#if ENABLED(HAS_Z_MIN_PROBE)
#if HAS_BED_PROBE
enable_z_probe(false);
#endif
} // Endstops::Endstops
Expand Down Expand Up @@ -108,7 +108,7 @@ void Endstops::init() {
#endif
#endif

#if HAS_Z_PROBE && ENABLED(Z_MIN_PROBE_ENDSTOP) // Check for Z_MIN_PROBE_ENDSTOP so we don't pull a pin high unless it's to be used.
#if HAS_Z_MIN_PROBE_PIN && ENABLED(Z_MIN_PROBE_ENDSTOP) // Check for Z_MIN_PROBE_ENDSTOP so we don't pull a pin high unless it's to be used.
SET_INPUT(Z_MIN_PROBE_PIN);
#if ENABLED(ENDSTOPPULLUP_ZMIN_PROBE)
WRITE(Z_MIN_PROBE_PIN,HIGH);
Expand Down Expand Up @@ -195,7 +195,7 @@ void Endstops::M119() {
SERIAL_PROTOCOLPGM(MSG_Z2_MAX);
SERIAL_PROTOCOLLN(((READ(Z2_MAX_PIN)^Z2_MAX_ENDSTOP_INVERTING) ? MSG_ENDSTOP_HIT : MSG_ENDSTOP_OPEN));
#endif
#if HAS_Z_PROBE
#if HAS_Z_MIN_PROBE_PIN
SERIAL_PROTOCOLPGM(MSG_Z_PROBE);
SERIAL_PROTOCOLLN(((READ(Z_MIN_PROBE_PIN)^Z_MIN_PROBE_ENDSTOP_INVERTING) ? MSG_ENDSTOP_HIT : MSG_ENDSTOP_OPEN));
#endif
Expand Down Expand Up @@ -317,7 +317,7 @@ void Endstops::update() {

#else // !Z_DUAL_ENDSTOPS

#if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN) && ENABLED(HAS_Z_MIN_PROBE)
#if HAS_BED_PROBE && ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
if (z_probe_enabled) UPDATE_ENDSTOP(Z, MIN);
#else
UPDATE_ENDSTOP(Z, MIN);
Expand All @@ -327,7 +327,7 @@ void Endstops::update() {

#endif // HAS_Z_MIN

#if ENABLED(Z_MIN_PROBE_ENDSTOP) && DISABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN) && ENABLED(HAS_Z_MIN_PROBE)
#if HAS_BED_PROBE && ENABLED(Z_MIN_PROBE_ENDSTOP) && DISABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
if (z_probe_enabled) {
UPDATE_ENDSTOP(Z, MIN_PROBE);
if (TEST_ENDSTOP(Z_MIN_PROBE)) SBI(endstop_hit_bits, Z_MIN_PROBE);
Expand Down
2 changes: 1 addition & 1 deletion Marlin/endstops.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class Endstops {
FORCE_INLINE void hit_on_purpose() { endstop_hit_bits = 0; }

// Enable / disable endstop z-probe checking
#if ENABLED(HAS_Z_MIN_PROBE)
#if HAS_BED_PROBE
volatile bool z_probe_enabled = false;
FORCE_INLINE void enable_z_probe(bool onoff=true) { z_probe_enabled = onoff; }
#endif
Expand Down
2 changes: 1 addition & 1 deletion Marlin/stepper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ void Stepper::isr() {
if (current_block != NULL) {

// Update endstops state, if enabled
#if ENABLED(HAS_Z_MIN_PROBE)
#if HAS_BED_PROBE
if (endstops.enabled || endstops.z_probe_enabled) endstops.update();
#else
if (endstops.enabled) endstops.update();
Expand Down