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

Implement DELTA_GET_CALIBRATION and DELTA_SET_CALIBRATION #4820

Closed
wants to merge 10 commits into from

Conversation

Fabi0San
Copy link

@Fabi0San Fabi0San commented Oct 20, 2021

Simple implementation of commands to get and set delta calibration parameters.
The main point of adding this support is just so Klipper can be used with external delta calibration solutions such as Delta Micro Calibrator which I authored.

With this merged I'll be able to add Klipper support which has been long requested: Fabi0San/DuCalibrator#1

Signed-off-by: Fabio Santos [email protected]

@Fabi0San
Copy link
Author

sigh, this back compat with step_distance is such a pain... I need some time.

@pandel
Copy link

pandel commented Oct 21, 2021

Hi!

I wanted to try your PR, but get the following error with current Klipper version (5c10001)

I just downloaded both files and replaced the original ones.

Internal error on command:"DELTA_GET_CALIBRATION"
Traceback (most recent call last):
  File "/home/pi/klipper/klippy/gcode.py", line 182, in _process_commands
    handler(gcmd)
  File "/home/pi/klipper/klippy/gcode.py", line 120, in <lambda>
    func = lambda params: origfunc(self._get_extended_params(params))
  File "/home/pi/klipper/klippy/extras/delta_calibrate.py", line 303, in cmd_DELTA_GET_CALIBRATION
    (",".join(("%.6f" % (n)) for n in delta_params.stepdists))))
TypeError: not all arguments converted during string formatting
Transition to shutdown state: Internal error on command:"DELTA_GET_CALIBRATION"

Regards,
Holger

@Fabi0San
Copy link
Author

Thanks for looking Holger, I took a beating from the tests and tried to move things around and ended up breaking something. I'm taking a look and will update here once it works.

@Fabi0San
Copy link
Author

@pandel, it should be ok now. It was a silly mistake over operator precedence (%+).

@pandel
Copy link

pandel commented Oct 21, 2021

Thanks for the quick fix! I'll give it a try asap.

@pandel
Copy link

pandel commented Oct 22, 2021

@Fabi0San
It works now :-). And it is definitely better to move my questions, so see: Fabi0San/DuCalibrator#21

@Fabi0San
Copy link
Author

Thank you Holger, those are very good questions and suggestions for the plugin, but unrelated to this Klipper PR.
In the interest of keeping the noise out of this repo, we should either take it offline or make it a bug/suggestion on DuCalibrator repository.

I'll get back to you shortly.

Thanks,
Fabio.

@KevinOConnor
Copy link
Collaborator

Thanks. As high-level feedback, I'm not sure this PR would meet our guidelines that new commands have a "high-impact" ( https://www.klipper3d.org/CONTRIBUTING.html ). Said another way, I'm not sure I could identify a target user audience of at least 100 people and it's not clear to me the advantage they would have using the new commands over the existing delta calibration tools.

I could be wrong of course. FYI, a good place to discuss this PR, get testers, and engage a reviewer is on the Klipper Discourse server ( https://www.klipper3d.org/Contact.html ).

-Kevin

@KevinOConnor KevinOConnor added the pending feedback Topic is pending feedback from submitter label Jan 10, 2022
@github-actions github-actions bot added the inactive Not currently being worked on label Jan 31, 2022
@github-actions
Copy link

It looks like this GitHub Pull Request has become inactive. If there are any further updates, you can add a comment here or open a new ticket.

Best regards,
~ Your friendly GitIssueBot

PS: I'm just an automated script, not a human being.

@github-actions github-actions bot closed this Jan 31, 2022
@Fabi0San
Copy link
Author

Hello @KevinOConnor, thanks for looking, sorry it took so long to respond, work has kept me busy.

Generally speaking, I think that motion control FW such as Smoothieware, Marlin, Klipper gains by being hyper focused on just running GCODE really well. I think we should leave the other specific scenarios such as screens, calibration, slicing, network connectivity, etc to external solutions. Especially now that our ecosystem has grown so much and great solution stacks exist with various plugins for every crazy scenario.

In my specific case, I have been seduced by the speed of Deltas early on and ended up discovering its intrinsic strong calibration dependency and error magnification nature when it was too hard to back out.
I'd argue that very few Deltas out there can deliver dimensional accuracy when printing away from the center, and most are now leaning on mesh leveling too hard.

The calibration processes that exist today take too few calibration points (minimum to resolve linear regression), and that does not account for the noise of probles, belts, pulleys etc. Those unaccounted errors get magnified because Delta.

My approach was to use thousands of calibration points on a very laborious process which I assumed to be once in a lifetime event. Long story short, turns out geometries change, either by my constant modding of various printer parts, expansion of materials with room temperature, or belt stretch by relative humidity, or because I put my printer on trunk of my car and drive like a maniac once a week to bring it to my favorite maker space show and tell.

So I automated my Obsessive Compulsive Delta Calibration process into an Octoprint Plugin which is installed on at least 120 instances of Octoprint:
https://github.com/Fabi0San/DuCalibrator
https://plugins.octoprint.org/plugins/DuCalibrator/

And I have used it with Smoothie, Marlin, and Now Klipper, which all needed a few adjusts to play well with external plugins for all delta geometry parameters:
MarlinFirmware/Marlin#18423
MarlinFirmware/Marlin#18367
MarlinFirmware/Marlin#17299

So that is my pitch:

  • Calibration doesn't have to be part of motion control FW
  • 120+ users(I can rally more at the Delta forums and Discord)
  • Existing calibration methods take too few data points to wash out probing/measuring errors,
  • Geometries move.

I hope you'll reconsider even if for the first point alone.
Thanks for all the good work!

@KevinOConnor
Copy link
Collaborator

I understand. As high-level feedback, this sounds like something to discuss on Klipper Discourse ( https://www.klipper3d.org/Contact.html ) to gather ongoing feedback from users and developers.

Separately, Klipper also has an API Server that allows great control of Klipper functionality from external programs ( https://github.com/Klipper3d/klipper/blob/master/docs/API_Server.md ).

-Kevin

@github-actions github-actions bot locked and limited conversation to collaborators Mar 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
inactive Not currently being worked on pending feedback Topic is pending feedback from submitter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants