Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
9.32k stars 5.28k forks source link

bed_mesh: support for Z in BED_MESH_OFFSET #6485

Closed Arksine closed 7 months ago

Arksine commented 7 months ago

This adds a "Z" parameter to BED_MESH_OFFSET that is used to account for changes in gcode offset on the Z axis when calculating fade. Without this, changing a tool that is offset on Z will cause jumps in the fade calculation. This likely makes fade unusable for multi-tool printers.

This addition does not add any additional adjustment on the z-axis, so it cannot be used to replace SET_GCODE_OFFSET, and instead must be used to complement it. Like X and Y, the Z offset should be relative to the primary extruder. If a secondary extruder's nozzle is higher than the primary extruder by .2mm, then BED_MESH_OFFSET Z=.2 is the correct setting. Presumably this would come after SET_GCODE_OFFSET Z=-.2. I believe this is consistent across all axes, but if it isn't I am open to changing the behavior.

This will need to be tested by some individuals with multiple tools, as I am unable to do so.

KevinOConnor commented 7 months ago

Thanks. Let me know when you'd like me to merge.

Separately, have you considered naming the parameter something other than Z to avoid confusion - maybe something like ZFADE=... or something else.

-Kevin

Arksine commented 7 months ago

Separately, have you considered naming the parameter something other than Z to avoid confusion - maybe something like ZFADE=... or something else.

That's a good idea, I just made the changes. I'll squash when its ready to go. I hope to get some feedback from users with multiple tools, they can chime in on the name of the parameter as well if they would like. ZFADE seems appropriate to me.

fiferboy commented 7 months ago

Just to chime in as a user with a multi-toolhead printer - I was mistaken in my understanding of how BED_MESH_OFFSET was to be used. For some reason I thought that it would allow the automatic compensation of differing nozzle positions in X and Y throughout the kinematics when it is clear from its name that it does not do this.

That being said, I think this is a useful change and I agree that ZFADE as a name should help avoid confusion (among users like me who didn't really think about the actual purpose of this command). I'm not sure how I would even go about testing this because even though I know it is the correct approach we are talking about usually fractions of a mm in nozzle offset in any direction, so while the correction is welcome I feel like the difference is going to be subtle in real-world tests without a contrived scenario. Any difference I see in nozzle squish on the first layer (probably the easiest way to see the effect of this adjustment) I am more likely to attribute to it being so hard to reliably get the Z offset between nozzles in the first place (at least, for me).

Completely off topic from the purpose of this PR - I wish there was a way to compensate for Z position differences in a toolhead and still allow global baby-stepping/live Z adjustment. Currently using SET_GCODE_OFFSET at each toolhead change makes baby-stepping for a different filament or build plate (or just questionable repeatability on initial Z probe) overwritten after the first toolhead. I suppose with what we currently have we would have to track the difference in GCODE offset between when a tool is started and when a tool change is requested and apply that additional amount every time there is a subsequent tool change.

Arksine commented 7 months ago

Thanks for the feedback fiferboy. I suspect that a good test will require a setup where there is substantial difference in nozzle height between tools. This addition was requested, so presumably it was causing an issue for a number of users out there.

The change is relatively simple and I'm confident it will work as intended, so if there is no other feedback I'm ok with merging now.

KevinOConnor commented 7 months ago

Thanks.

-Kevin