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

add servo support to MAG_MOUNTED_PROBE #27551

Open
wants to merge 3 commits into
base: bugfix-2.1.x
Choose a base branch
from

Conversation

ellensp
Copy link
Contributor

@ellensp ellensp commented Nov 29, 2024

Description

User built machine with a MAG_MOUNTED_PROBE on a servo arm, a non supported configuration.
I added basic support for it.

Requirements

MAG_MOUNTED_PROBE
NUM_SERVOS
Z_SAFE_HOMING (a requirement)
new
MAG_MOUNTED_PROBE_SERVO_NR
MAG_MOUNTED_PROBE_SERVO_ANGLES

Benefits

A MAG_MOUNTED_PROBE on a servo arm has a chance of working.

User requesting this has provided videos that it work as intended

IMG_7736.mov

@ellensp ellensp marked this pull request as ready for review November 29, 2024 12:07
@gjdodd
Copy link
Contributor

gjdodd commented Jan 8, 2025

I should have looked here first, just spent the afternoon implementing this myself. is there any chance of this branch been updated to latest, and also would it help get this one through if I helped test it out.

@ellensp ellensp force-pushed the add-servo-support-to-magnetic-probe branch from 7d519a6 to 4a2f753 Compare January 8, 2025 16:56
@ellensp
Copy link
Contributor Author

ellensp commented Jan 8, 2025

@gjdodd updated to current bugfix 2.1.x

@gjdodd
Copy link
Contributor

gjdodd commented Jan 9, 2025

@ellensp, Thankyou for updating the branch. There is one small issue, currently enabling the MAG_MOUNTED_PROBE_SERVO_NR, will cause a compliation error, it needs the NUM_SERVOS block in "Conditionals-4-adv.h" updating to automatically increment the servo count. Like in the code below, once this is in place it will compile. Just going to start testing as I have switched everything over on my machine now, so hopefully it all goes well.

image

@ellensp
Copy link
Contributor Author

ellensp commented Jan 9, 2025

I was unaware of that particular check, I just always set NUM_SERVOS...

Added

@gjdodd
Copy link
Contributor

gjdodd commented Jan 9, 2025

found one more slight issue, if you enable the deactivate servos after move option, it will fail to compile, line 983 in SanityCheck.h needs !defined(MAG_MOUNTED_PROBE_SERVO_NR) adding to the if clause.

So far everything else is looking good

@gjdodd
Copy link
Contributor

gjdodd commented Jan 9, 2025

would you prefer me to create PR's into your branch instead of comments if I find anything else

@gjdodd
Copy link
Contributor

gjdodd commented Jan 9, 2025

I have spotted an issue in the deploy/stow code, it relates to the new PROBE_TRIGGERED_WHEN_STOWED_TEST work by the looks of it, as the probe finishes the homing cycle it is running through the stow code so quick that the condition for PROBE_TRIGGERED == deploy does not work correctly as the condition for checking the probe state is not what would be expected, I am just looking at what to do to fix it, might be we need a stow delay similar to the servo delay or something with a config to enable it or not, or the other option is to ignore the trigger state when stowing and just continue to try and stow.

What are your thoughts @ellensp

@gjdodd
Copy link
Contributor

gjdodd commented Jan 9, 2025

Scratch that, I have now seen what the issue is, needed to look at the printer a bit closer, at the end of the zero it is not moving the z away from the probe and so it is staying triggered so the condition check does not match and so the stow code never runs. Just looking at what to do with it now.

@gjdodd
Copy link
Contributor

gjdodd commented Jan 10, 2025

@ellensp I have created a PR on your repo to fix the stowing issues with not moving z before stowing which triggers errors with miss matched endpoint and deploy/stow flag.

It also includes a couple of extra configs to stop the nozzle crashing into the servo deployed arm after ABL etc

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.

2 participants