Allow Z_STOP_PIN override on SKR 1.4

[Glought]

Description

This is for SKR 1.4 and SKR 1.4 Turbo.
This simple change will allow the Z_STOP_PIN to be definable in the config file.
This is useful for people that don't want to rewire their plug to use the z-stop connector .

For example the BLtouch wouldn't work right using the Z_MIN_PROBE_PIN (doesn't deploy on z home) but works fine using Z_STOP_PIN so instead of physically changing connectors we can just change the Z_STOP_PIN to the probe port pin (P0_10).

If the Z_STOP_PIN is changed to P0_10 it will make Z_MIN_PROBE_PIN P1_27

Benefits

• Allows the Z_STOP_PIN to be definable in the config file.
• The user wouldn't need to physically rewire the BLTOUCH to use the z end stop connector on the board just simply redefine the Z_STOP_PIN.

[codiac2600]

Whatever this is It broke my ability to home Z. I haven’t tried my printer with a BL Touch but every single printer with a Pinda won’t home. I’ve even tried using the 2.0.4.4 pins file and nothing. Stop pushing untested things into stable.

[Glought]

Whatever this is It broke my ability to home Z. I haven’t tried my printer with a BL Touch but every single printer with a Pinda won’t home. I’ve even tried using the 2.0.4.4 pins file and nothing.

Must be something else causing your issue if 2.0.4.4 pin file isn't work for you. This change allows people to override the Z_STOP_PIN other wise its the same pin as it was before this change.

If your not overriding Z_STOP_Pin it should be its default pin P1_27 and Z_MIN_PROBE_PIN should be pin P0_10

Stop pushing untested things into stable.

I have tested this before making this pull request I have my bltough z stop wires plugged into the SKR 1.4 probe connector it wasn't homing or anything when using the Z_MIN_PROBE_PIN so instead of cutting off the JST connector and putting a new one on it was just easier to just override the Z_STOP_PIN and my bltouch homes and works fine.

[codiac2600]

Did you test with any other probe? No.

It’s broken numerous users ability to probe using the probe pin. Worked perfectly fine in 2.0.4.4. Nothing else is wrong.

Every release it’s like this. Broken firmware that is untested by a legitimate team of testers. It’s getting ridiculous to ask that crap like this isn’t pushed into the main builds? Make a dev build and let users actually see if it works.

Be nice if things were actually tested though.

[codiac2600]

The reason the z min pin didn’t work is because you have to cut the diagnostic pin for it work. The skr 1.4 doesn’t have the jumpers like the 1.3.

Just ONLY enable the probe pins to work like they should. If you have a probe use the probe pin 1_10.

It should be if Tmc2208-2209 then endstop is P1_10. The nonsense in the pins file isn’t working. Also you don’t need to cut a new jst. Just flip the wires. It takes 3 seconds to push the lock pins and move the wires however you want. Instead you break probing for hundreds of users because people want to be lazy.

[Vertabreak]

everyone chill im working on a patch that will make everyone happy.

[Glought]

The reason the z min pin didn’t work is because you have to cut the diagnostic pin for it work. The skr 1.4 doesn’t have the jumpers like the 1.3.

Just ONLY enable the probe pins to work like they should. If you have a probe use the probe pin 1_10.

It should be if Tmc2208-2209 then endstop is P1_10. The nonsense in the pins file isn’t working. Also you don’t need to cut a new jst. Just flip the wires. It takes 3 seconds to push the lock pins and move the wires however you want. Instead you break probing for hundreds of users because people want to be lazy.

Dude i did cut the diagnostic pin on my tmc2209 for my z steppers when I installed it

This change is a simple if else.

 #ifndef Z_STOP_PIN < if not define aka not defined in the config file then it will be defined here
   #define Z_STOP_PIN     P1_27   // Z-STOP  
 #endif
#endif

//
// Z Probe (when not Z_MIN_PIN)
//
#ifndef Z_MIN_PROBE_PIN
 #if Z_STOP_PIN != P1_27 < If Z_STOP_PIN doesn't equal P1_27 then define Z_MIN_PROBE_PIN P1_27
   #define Z_MIN_PROBE_PIN P1_27
 #else
   #define Z_MIN_PROBE_PIN P0_10 < as long as your not overriding Z_STOP_PIN then this is the Z_MIN_PROBE_PIN aka the default pin for Z_MIN_PROBE_PIN

 #endif
#endif 

And no I don't have any other probe and i don't see how this change could effect them unless Z_STOP_PIN is overridden then both Z_STOP_PIN and Z_MIN_PROBE_PIN should be their default pins.

[codiac2600]

Your simple if else broke using the dedicated probe pin. The BTT website has a big diagram how to wire the BL touch. It won’t work with your setup now.

Also you can’t say this is tested if you don’t try other probes. Forcing everyone to use the wrong way to use the z min pin isn’t how this should have been handled. Use the correct probe pin no need to be lazy about it and break everyone’s build.

[codiac2600]

I literally can use the probe pin with 2.0.4.4 pins or the new pull by Vert. Tested on 3 printers. Numerous users couldn’t use their probe with whatever was pushed to what is supposed to be a stable branch.

[Glought]

well technically thinkyhead did change my code when he merged it here is my code when i did this pull request

https://github.com/Glought/Marlin/blob/c7c17f714095a14f0e4572f8cefa520b368d7652/Marlin/src/pins/lpc1768/pins_BTT_SKR_V1_4.h
Could you try my original code to see it if it works for you out of curiosity.

[codiac2600]

Says 404 when I click the link.

[Glought]

sorry i fixed the link

[thisiskeithb]

You can see the changes easier here: b57e662

[codiac2600]

sorry i fixed the link

That caused the nozzle to crash into the bed.

[Vertabreak]

the patch has been posted and should make everyone happy.

[Glought]

I'm going to try 2.0.5 to see if it cause me any issues

[thisiskeithb]

I'm going to try 2.0.5 to see if it cause me any issues

Move your Z up a bit and use your hand to test a BLTouch/metal for a PINDA when homing Z so you don't accidentally damage your bed.

[Glought]

I know i raise the z up high so i have time to react if it doesn't work.

[thisiskeithb]

I just wanted to call it out since BLTouch and Pinda probes act differently with these changes now.

[codiac2600]

Why can't we make this work:

#define Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN

uses the Z min endstop pin

#define Z_MIN_PROBE_PIN P0_10

This quits making Z min pin work and only enables P1_10 as the endstop pin???

I mean isn't that what it's for? This would solve every issue.

Because as of right now that doesn't work whatsoever.

[Glought]

I just wanted to call it out since BLTouch and Pinda probes act differently with these changes now.

My bltouch works fine I really honestly don't see how my change could cause issues with the Pinda probes unless Z_STOP_PIN is overridden then it should be default like the change never existed.

Why can't we make this work:
#define Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN
uses the Z min endstop pin
#define Z_MIN_PROBE_PIN P1_10
This quits making Z min pin work and only enables P1_10 as the endstop pin???
I mean isn't that what it's for? This would solve every issue.

I just tested that (also its pin P0_10 not P1_10(that's the X_SERIAL_RX_PIN and X_SERIAL_TX_PIN pins) it causes my bltouch to error out it deploys then stows then deploys then error.

[codiac2600]

Hi everyone. I've been taking longer breaks from the project because it takes too much effort to read the long threads and get out the useful parts. Could someone sum up the problem and proposed solution for me in a single post so I can attend to some other issues? Thanks!

Z_STOP_PIN override doesn't work as expected.

Works fine with a BL Touch but when you try and use an inductive probe by enabling the probe pin in configuration.h you get 2 independent endstops as if it has 2 z stops. This causes the nozzle to now crash into the bed because the probe only triggers 1 of the now 2 endstops.

[Glought]

I do have Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN enabled when overriding Z_STOP_PIN, is it enabled when you tested?

From my config file:

/**
 * Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN
 *
 * Enable this option for a probe connected to the Z Min endstop pin.
 */
#define Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN
#define Z_STOP_PIN P0_10

[Glought]

When Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN is enabled bltouch works and homes fine:

Send: M119
Recv: Reporting endstop status
Recv: x_min: open
Recv: y_min: open
Recv: z_min: TRIGGERED
Recv: ok

When Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN is disabled bltouch doesn't work and just stows and deploys till error:

Send: M119 
Recv: Reporting endstop status
Recv: x_min: open
Recv: y_min: open
Recv: z_min: open
Recv: z_probe: TRIGGERED
Recv: ok

[thinkyhead]

Ok, so it's been a lifetime since this PR under current circumstances, so let me review again what the original code was in the pins file, and the changes I made to accommodate sensorless homing, and then take account of the unintended consequence you're describing. I'll mix these together into a stew pot and push a patch asap. Sorry for the inconvenience.

[thinkyhead]

So… This change is kind of baroque, but on first glance I think it will give the desired result:

Too much, I reckon…

//
// Z Probe (when not Z_MIN_PIN)
//
#ifndef Z_MIN_PROBE_PIN
  #if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
    #if Z_STOP_PIN == P1_27
      #define Z_MIN_PROBE_PIN P1_27
    #else
      #define Z_MIN_PROBE_PIN P0_10
    #endif
  #else
    #if Z_STOP_PIN == P0_10
      #define Z_MIN_PROBE_PIN P1_27
    #else
      #define Z_MIN_PROBE_PIN P0_10
    #endif
  #endif
#endif

Or, really it seems all that is needed is this, since you might want to also override Z_STOP_PIN in your configs without any side effect on the probe pin:

#ifndef Z_MIN_PROBE_PIN
  #if ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
    #define Z_MIN_PROBE_PIN P1_27
  #else
    #define Z_MIN_PROBE_PIN P0_10
  #endif
#endif

[codiac2600]

That also didn't work. Doesn't compile.

[thinkyhead]

n/p we'll get there.

[thinkyhead]

Yep, at this point I think the correct code for the probe is this:

#ifndef Z_MIN_PROBE_PIN
  #define Z_MIN_PROBE_PIN  P0_10
#endif

[codiac2600]

That works only if you #define Z_STOP_PIN P0_10 as well. Because if you don't it thinks you have an endstop in Z min AND a probe in Probe pin.

[thinkyhead]

By "it thinks" are you referring to a sanity check that fails to account for that combination?

[codiac2600]

I’m the furthest from a programmer but in visual talk what happens is it’ll have 2 endstops for Z. Probe and Z min.

So when you try and probe you have the actual probe at P0_10 and then it’ll show Z min at P1_27.

When it goes to probe it crashes into the bed because even though the probe triggered the z min pin didn’t and crash.

[thinkyhead]

Please ZIP your configurations and drop them on your next reply so I can see what code path is being compiled and where this confusion is arising.

[thinkyhead]

Just so we're all on the same page (and for the benefit of future readers), the default behavior is that Marlin will use Z_MIN_PROBE_PIN for the probe and Z_MIN_PIN for the Z (min) endstop, homing Z to the endstop.

But if you enable Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN Marlin will:

• Use Z_MIN_PIN (as marked on the board) to detect the probe
• Use the probe for homing Z, since "there is no Z endstop"

So, let's see… If you want to home with the probe but also use a different pin with this board, you would have to reassign Z_STOP_PIN to the pin the probe will be connected to, enable Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN, and… that should be all. At least, Marlin shouldn't be looking for a Z_PROBE_PIN once that's done. It will only care about the "endstop" (actually the probe) pin.

[codiac2600]

That’s what I was doing in 2.0.3-2.0.4.4.

The logic change didn’t allow for that to happen in 2.0.5 and broke that ability. For many users on Facebook that caused crashing nozzles into the bed because it was assigning both a z min at pin 1_27 and the probe pin at 0_10.

The majority of boards you’d just use Z min for the endstop. With the newer tmc chips that have sensorless homing and no jumper you can’t use z min because it becomes a diagnostic pin

With a small subset of boards they are adding a probe pin so you don’t need to worry about the cutting pins and making the probe work.

Going forward is there a need to allow marlin to swap what it thinks the Z min is and not have this issue going forward?

If we declare Z_MIN_PROBE_PIN perhaps marlin should logically ignore Z min and only enable a probe endstop?

[thinkyhead]

The logic change didn’t allow for that to happen in 2.0.5 and broke that ability.

Right, so I'm referring now to that change being reverted. (It's been reverted, hurrah.)

With the newer tmc chips that have sensorless homing and no jumper

Yeah. This is the thing that has thrown us for a loop. I want to find an elegant and automated way to make choosing both pin and probing behavior, and with this issue I'm thinking it might be time to expand beyond the aging paradigm of just "probe is Z min and homes" vs "probe is custom and doesn't home" for the probe….

I think we at least have the DIAG assignment to endstop issue sorted out. As far as what to do with probe pin assignment, going forward, well, let's talk use-cases. The typical ones are considered already. So which kinds of use-cases can you think of where we need more flexibility, and how many ways can we envision combining endstops and probes?

[thinkyhead]

…is there a need to allow marlin to swap what it thinks the Z min is and not have this issue…

Yes. I'll just aim to pave that path first. The main thing is it should be clear what to expect.

[thinkyhead]

If we declare Z_MIN_PROBE_PIN perhaps marlin should logically ignore Z min and only enable a probe endstop?

This is so close to being covered by the case where you don't enable Z_MIN_PROBE_USES_Z_MIN_PIN. Marlin does use the Z_MIN_PROBE_PIN in that case, but it also gives you a Z endstop Z_MIN_PIN and assumes endstop homing.

We currently have a conditional called HOMING_Z_WITH_PROBE that means:

HAS_BED_PROBE && Z_HOME_DIR < 0 && ENABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)

We could perhaps make it that if you enable HOMING_Z_WITH_PROBE earlier, it changes the behavior so that Marlin uses the Z_MIN_PROBE_PIN instead of Z_MIN_PIN but otherwise treats the probe as the exclusive Z switch.

[djessup]

@thinkyhead

But if you enable Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN Marlin will:

Use Z_MIN_PIN (as marked on the board) to detect the probe
Use the probe for homing Z, since "there is no Z endstop"

I'm not sure this is correct, due to this block:

  #if DISABLED(Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN)
    #define HAS_CUSTOM_PROBE_PIN 1
  #endif
  #if Z_HOME_DIR < 0 && !HAS_CUSTOM_PROBE_PIN
    #define HOMING_Z_WITH_PROBE 1
  #endif

If Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN is enabled then HAS_CUSTOM_PROBE_PIN will be true, and so HOMING_Z_WITH_PROBE will be false. Later in the homing code this is used to decide whether to deploy the servo probe during homing.

If the intention is to define HOMING_Z_WITH_PROBE in Configuration.h or similar then this should be documented somewhere - if it is apologies but I have never seen it.

Derp. This won't work since it'll get re-set during Conditionals_LCD.h. It seems with a board that has a custom Z-pin for probing there is no way to use it for homing without either hacking the Z_STOP_PIN, or Conditionals_LCD.h. This seems like a shame, as it just makes upgrades more complex for what seems like a reasonable and simple scenario.

I'm struggling to understand the logic of the && !HAS_CUSTOM_PROBE_PIN part? Why would you want to exclude the option of z-homing with a custom pin?

[thinkyhead]

If Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN is enabled then HAS_CUSTOM_PROBE_PIN will be true…

Read the code again. If Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN is disabled then HAS_CUSTOM_PROBE_PIN will be true.

[thinkyhead]

Why would you want to exclude the option of z-homing with a custom pin?

When you're using the probe for Z homing it is replacing the min endstop, so that plug is going to be free for the probe to be plugged into it and so you should use it.

Boards that come with a pin specifically for a probe typically also come with a Z endstop.

[djessup]

Read the code again. If Z_MIN_PROBE_USES_Z_MIN_ENDSTOP_PIN is disabled then HAS_CUSTOM_PROBE_PIN will be true.

Sorry, typo - anyway you've answered my question which was the rationale behind this logic -

When you're using the probe for Z homing it is replacing the min endstop, so that plug is going to be free for the probe to be plugged into it and so you should use it.

Thanks, I can understand where you're coming from now. From the perspective of a SKR1.4 user this seems counter intuitive which is why I was confused. The board provides a "native" socket for BLTouch, so users naturally expect that this is the most-correct plug to use. Clearly BTT and Marlin are not totally aligned here.

Personally I still think it would be nice to support probe-based z-homing in a situation like this. There are use-cases for re-using the normal z-endstop (e.g. for a z-max switch?) while using the custom probe for homing, but at least now I can understand the assumptions made.

[djessup]

@thinkyhead How would you feel about having a config flag along the lines of CUSTOM_PROBE_PIN_HOMING and adjusting the logic to this?

  #if Z_HOME_DIR < 0 && (!HAS_CUSTOM_PROBE_PIN || ENABLED(CUSTOM_PROBE_PIN_HOMING))
    #define HOMING_Z_WITH_PROBE 1
  #endif

The flag would be disabled by default for backwards compatibility, but would allow configurations like SKR's to work without resorting to hacking the pin definitions or conditionals logic? Happy to put together a PR if so.

[thisiskeithb]

@thinkyhead: Let me know if you guys need an SKR 1.4 for testing. This is a becoming a popular board, especially in the Prusa community, so resolving the dedicated probe pin vs. z min pin issue would be nice without having to modify the pins file.

[djessup]

I've raised a simple PR which should resolve this in a way we can have our cake and eat it too. Hope it meets with approval.
#17359