Skip to content

Comments

[2.0.x] Poll all endstops, even when stationary#11123

Merged
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-2.0.xfrom
thinkyhead:bf2_another_endstops_patch
Jun 27, 2018
Merged

[2.0.x] Poll all endstops, even when stationary#11123
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-2.0.xfrom
thinkyhead:bf2_another_endstops_patch

Conversation

@thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented Jun 27, 2018

Endstop fixes suggested by @ejtagle. Split off from #11098.

When ENDSTOP_NOISE_FILTER is enabled, the endstops are always polled. But, there was a problem there, as when the motors do not move, the endstop switches reading was skipped. So, i removed that condition, but just for the endstop line read. This seems to solve #11056 and several homing issues.

@thinkyhead
Copy link
Member Author

This PR seems okay as far as I've examined it. Are the core kinematic issues currently just with the sensorless homing? The main thing to watch out for with sensorless homing is that the endstop triggered states are extremely short-lived.

@ejtagle
Copy link
Contributor

ejtagle commented Jun 27, 2018

Yes, i do suspect exactly the same with sensorless homing. In fact, without using ENDSTOP_INTERRUPT am not even sure if it is possible to read them. And NOISE_FILTER i am absolutely sure it does not work with sensorless homing (At some point I even considered just copying endstop state bypassing filtering for those SENSORLESS inputs...

@ejtagle
Copy link
Contributor

ejtagle commented Jun 27, 2018

@smoki3 ... Are you using ENDSTOPS_INTERRUPT feature ?

@smoki3
Copy link
Contributor

smoki3 commented Jun 27, 2018

No it's disabled. But what I nocticed: on my actual configuration I interchanged the diag1 pins from the x and y axis. Nonetheless it worked before, even with the interchanged pins.
Unfortunately I now at vacation and I can not test if it's working with the correct pin setup.
So maybe it's already fixed when I change the pin configuration.

@thinkyhead
Copy link
Member Author

thinkyhead commented Jun 27, 2018

Previously the endstops (when polled) were checked frequently enough (on every stepper ISR) to catch sensorless homing signals. That is no longer the case, so ENDSTOP_INTERRUPTS is probably now required. Users may be SOL if the endstop pins are not interruptible, or will have to find free interruptible pins.

@ejtagle
Copy link
Contributor

ejtagle commented Jun 27, 2018

@thinkyhead : At some point i asked myself how short the DIAG1 pulse would be (I have no TMC2130 drivers to test... ), I've read the whole datasheet but no mention is done on pulse width.

What i do suspect is that the signal is HI while pulses are being received to spin the motor. After all, it is using an Stall detector, so if motors try to move, stall can be detected, and otherwise not.

So, as limits only trigger when motor moves, even without ENDSTOPS_INTERRUPTS , it still should work...

@teemuatlut
Copy link
Member

Measured as 436µs, 1316µs, 683µs and 1300µs.
The logic is that any time the stallGuard measurement is 0, the stallGuard status bit is set, and this can then also be routed to a diag pin. So there is no fixed pulse width and the width of the pulse would depend on how fast we can react to the pulse.

@ejtagle
Copy link
Contributor

ejtagle commented Jun 27, 2018

@teemuatlut : So, essentially, this supports the idea that the pulse will be long enough (because the motor is still being stepped to try to make it move) to trigger stopping the motor, so everything should be fine 👍
Obviously incompatible with NOISE_FILTER , but nevertheless, not a big problem as DIAG1 has probably a push-pull driver, so no noise should be present ...

@thinkyhead thinkyhead force-pushed the bf2_another_endstops_patch branch from ae1981c to e05560d Compare June 27, 2018 05:25
@thinkyhead
Copy link
Member Author

thinkyhead commented Jun 27, 2018

Added a sanity-check…

#error SENSORLESS_HOMING is incompatible with ENDSTOP_NOISE_FILTER.

@ejtagle
Copy link
Contributor

ejtagle commented Jun 27, 2018

@thinkyhead Probably yes. Sensorless homing for Z axis is mostly impossible, except for deltas. So, better to have a good Z endstop without noise ... ;)

@thinkyhead thinkyhead merged commit 053438a into MarlinFirmware:bugfix-2.0.x Jun 27, 2018
@thinkyhead thinkyhead deleted the bf2_another_endstops_patch branch June 27, 2018 08:19
@thinkyhead thinkyhead changed the title Poll all endstops, even when stationary [2.0.x] Poll all endstops, even when stationary Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants