protoloft / klipper_z_calibration

Klipper plugin for self-calibrating z-offset
GNU General Public License v3.0
1.05k stars 151 forks source link

Configuration is too complex #39

Closed rlaferla closed 2 years ago

rlaferla commented 2 years ago

I was hoping this plugin would solve Z offset once and for all for my Voron Trident. I installed it but then I started to configure it and there a tons of parameters (around 17 values) that need to be set. I think this is a good project but it needs more work. It could extract the z_safe_home values for the position of the nozzle. It could use the Afterburner or Stealthburner distance between nozzle and probe as a default or as comments in the cfg. Perhaps, parse the bed mesh to determine the reference index. There must be ways of cutting down the sheer number of these parameters.


probe_nozzle_x:199.0
probe_nozzle_y:348.0
#   The X and Y coordinates (in mm) for clicking the nozzle on the
#   Z endstop.
probe_switch_x:199.0
probe_switch_y:325.0
#   The X and Y coordinates (in mm) for clicking the probe's switch
#   on the Z endstop.
probe_bed_x: default from relative_reference_index of bed_mesh
probe_bed_y: default from relative_reference_index of bed_mesh
#   The X and Y coordinates (in mm) for probing on the print surface
#   (e.g. the center point) These coordinates will be adapted by the
#   probe's X and Y offsets. The default is the relative_reference_index
#   of the configured bed_mesh. It will raise an error if there is no
#   probe_bed site and no bed_mesh with a relative_reference_index
#   configured.
switch_offset:
#   The trigger point offset of the used mag-probe switch.
#   This needs to be fined out manually. More on this later
#   in this section..
max_deviation: 1.0
#   The maximum allowed deviation of the calculated offset.
#   If the offset exceeds this value, it will stop!
#   The default is 1.0 mm.
samples: default from "probe:samples" section
#   The number of times to probe each point. The probed z-values
#   will be averaged. The default is from the probe's configuration.
samples_tolerance: default from "probe:samples_tolerance" section
#   The maximum Z distance (in mm) that a sample may differ from other
#   samples. The default is from the probe's configuration.
samples_tolerance_retries: default from "probe:samples_tolerance_retries" section
#   The number of times to retry if a sample is found that exceeds
#   samples_tolerance. The default is from the probe's configuration.
samples_result: default from "probe:samples_result" section
#   The calculation method when sampling more than once - either
#   "median" or "average". The default is from the probe's configuration.
clearance: 2 * z_offset from the "probe:z_offset" section
#   The distance in mm to move up before moving to the next
#   position. The default is two times the z_offset from the probe's
#   configuration.
position_min: default from "stepper_z:position_min" section.
#   Minimum valid distance (in mm) used for probing move. The
#   default is from the Z rail configuration.
speed: 50
#   The moving speed in X and Y. The default is 50 mm/s.
lift_speed: default from "probe:lift_speed" section
#   Speed (in mm/s) of the Z axis when lifting the probe between
#   samples and clearance moves. The default is from the probe's
#   configuration.
probing_speed: default from "stepper_z:homing_speed" section.
#   The fast probing speed (in mm/s) used, when probing_first_fast
#   is activated. The default is from the Z rail configuration.
probing_second_speed: default from "stepper_z:second_homing_speed" section.
#   The slower speed (in mm/s) for probing the recorded samples.
#   The default is second_homing_speed of the Z rail configuration.
probing_retract_dist: default from "stepper_z:homing_retract_dist" section.
#   Distance to backoff (in mm) before probing the next sample.
#   The default is homing_retract_dist from the Z rail configuration.```
TitusLabs commented 2 years ago

Most of the properties are predefined with the values taken from all the other objects in Klipper. Someone could argue, that this is not a best practice too. All defaults are documented. Even the bed site defaults to the relative reference index of the bed mesh - if configured. And generally, this is a Klipper extension - not a Voron extension.

Do you have any concrete improvements?

rlaferla commented 2 years ago

I am providing constructive feedback to improve the plugin. That is why I wrote "I think this is a good project". If it is a general Klipper plugin, that's great but somehow I misinterpreted the description. You may want to update the description.

image
rlaferla commented 2 years ago

As for more concrete improvements, perhaps printer specific .cfg templates? Perhaps based on popular sizes with values for different sizes in comments? e.g. # 350mm use x, 250mm use , 300mm use

TitusLabs commented 2 years ago

Thanks and don't get me wrong - I know that the documentation is complex :see_no_evil: and every improvement is welcome!

Oh, but this is a good point. I will change the description..

I don't think templates are necessary. Where possible, default values are provided and I cannot provide any predefined positions in the configuration. But maybe I can add a minimal configuration section with all mandatory properties :thinking:

I am not sure about the example configurations though. These are only examples and maybe they make it worse. They are all about configuring a Voron using a mag-probe. For the z calibration, there is only a configuration section and a macro... By deleting them all, I would refer to the Klicky configuration/macros (as I already did in the documentation) and the all in one config from zellneralex...

TitusLabs commented 2 years ago

I update the configuration section and added a minimal configuration example. The example scripts are removed completely without replacement. There is also a PR pending which reduces the x and y position properties to one x,y coordinate property.

If there are no more suggestions, I will close this ticket...