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

Fix bad movement in gcode_T when switching extruders #3829

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

thinkyhead
Copy link
Member

@thinkyhead thinkyhead commented May 23, 2016

Background: When you switch extruders with the "T" command, the machine will attempt to move the selected extruder to the position of the previously-selected extruder. The machine still attempts to make this movement even with software endstops enabled because gcode_T is overriding current_position directly, but it doesn't follow up fully as G92 does, by updating the software endstops.

This PR makes two changes:

  • In gcode_T when switching the extruder, apply the offsets to position_shift and call update_software_endstops() for X and Y.
  • For update_software_endstops[Z_AXIS] apply any negative offsets from zprobe_zoffset and/or home_offset[Z_AXIS] to sw_endstop_min[Z_AXIS]. This is instead of applying these offsets in clamp_to_software_endstops. (Some question as to why the zprobe_zoffset constraint is needed outside of probing…?)
  • Since zprobe_zoffset now applies to sw_endstop_min[Z_AXIS] generally, we now call update_software_endstops(Z_AXIS) when changing zprobe_zoffset.

References: #3823, #3792

@@ -555,6 +555,7 @@ void Config_ResetDefault() {

#if ENABLED(AUTO_BED_LEVELING_FEATURE)
zprobe_zoffset = Z_PROBE_OFFSET_FROM_EXTRUDER;
update_software_endstops(Z_AXIS);
#endif
Copy link
Member

@AnHardt AnHardt May 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to be problematic. Seems to be the same as in M851. We probably should not apply this immediately to the software endstops. We don't move the nozzle nor shift a coordinate system, just change the value without using it. So the correction for this should apply during the next homing and/or probing ( L 3624 https://github.com/MarlinFirmware/Marlin/pull/3829/files#diff-1cb08de130a6ece2d1b5b9c37bcfef48R3634)?
I guess it needs a dual stage strategy, holding back the new zprobe_zoffset until rehomed/probed and not to apply it accidentally by a G92 or Tool-change with a dual z axis system.

Should reloading the config and M851 reset the axis_is_homed flag for z, or for all axes, or at all?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the zprobe_zoffset matters to the sw_endstop_min[Z_AXIS] then the call to update_software_endstops(Z_AXIS) is necessary to apply it here. Only the sw_endstop_min[Z_AXIS] will be affected by this call, as nothing else will have changed.

@AnHardt
Copy link
Member

AnHardt commented May 25, 2016

Let's go one step back and take a look on the whole picture.
The home_offsets are ought to adjust the endstops/homeswitches, to meet nozzle and bed at a special point. (for example [0,0,0] is middle of bed.) Then zprobe_zoffset is the same when we use the probe for homing.
So home_offset[Z_AXIS] is used for homing with one of the z-axis-endstops/homeswitches.
zprobe_zoffset is used for homing with the probe and as a correction while probing.

When we are homing with the endstop and probing with the probe things become un-obvious. Likely we have to change from home_offset[Z_AXIS] to zprobe_zoffset while or after probing. Likely we have to subtract home_offset[Z_AXIS] and add zprobe_zoffset for the very firs probe and have to switch back to home_offset[Z_AXIS] when homed an other time.

I guess currently it works most of the time because most people use the probe for homing when they have one and leave the home_offsets at zero. Even if they begin to play with home_offset[Z_AXIS] the worst thing what can happen, is they have to find a new zprobe_zoffset what is not the 'real' distance between nozzle and trigger-point anymore.

@thinkyhead
Copy link
Member Author

@AnHardt I'm still trying to find a use-case or common instance where the extended Z floor makes sense. As you mention, the home_offset and position_shift together shift the software endstops everywhere, and for the case of 0,0 in the middle of the bed, these offsets are sufficient. The min software endstops end up being at something like { -100, -100, 0 } and all is well.

But when does the nozzle want to go "below the bed"? Or, to consider the other possibility, when is the Z position set "artificially" — as in a probe function — and then moved using a function that calls clamp_to_software_endstops? Maybe the flowcharts made by @esenapaj can be helpful here!

@AnHardt
Copy link
Member

AnHardt commented May 27, 2016

@thinkyhead
I'm not the one who defends the current endstop code. I guess i'm about its hardest critic.

1 Software endstops are complete nonsense before homing. Because we do not know where we are. When we boot we set current_position to [0,0,0]. If our z-min-pos is 0, z-min endstop is enabled, we home with a probe what needs Z_RAISE_BEFORE_PROBING and we booted with the z-axis to high - we are not able to lower the nozzle because the software endstops would prevent that.
Hardware endstop make sens before homing - especially then.
2 During homing the software endstops should still be off. At the moment a axis is homed we know where we are and can enable the software endstops. As soon as we try to home a second time we should switch the software endstops off again. We could have an thinner buildplate where we have to go deeper. Currently we simply bypass the software endstops in homeaxis().
Hardware endstops should be off during homing a axis. Logically the are now home-switches. The difference is, endstops should stop any further action, home-switches are measuring the position. A probe is always a home-switch.
3 During a normal print sowtware endstops should be on and prevent from moves outside MIN_POS/MAX_POS. The adequate action is to at least stop the print - no silent clipping.
4 While probing the software endstops have to be off. One or more points could be deeper than the homing point.
The hardware z-min endstops has to to be off during probing. The probe acts as a measuring device and should not shift the z-min-home point. (ideally we get results very cloth to Z_MIN_POS - means corrected by the Z_PROBE_OFFSET_FROM_EXTRUDER)
5 Printing with z-correction should work like (3). The correction has to be applied after the software endstop test.
A z-min hardware endstop eventually has to be off to be not triggered in a corrected move at z-min-level.
6 If we want to find a new/better home_offset or Z_PROBE_OFFSET_FROM_EXTRUDER we eventually have to go blow our former homing point. In that case we need ether a special procedure, or a possibility to switch the software endstop(s) off/on manually.

To your questions:
A z-min-software endstop does not need to allow for any moves below Z_MIN_POS but has to be deactivated or bypassed at some times.

Maybe the flowcharts made by @esenapaj can be helpful here!

No. They describe the current chaos for the different probes. Not a reasonable plan for endstops - especially not for the software endstops.

Can you think of any reasonable justification to selectively apply these offsets to sw_endstop_min[Z_AXIS], or does this also seem obsolete to you?

No, i can't. If it then does not work other parts of the code are broken.

@jbrazio jbrazio added this to the 1.1.0 milestone May 30, 2016
@thinkyhead
Copy link
Member Author

Ok, I will drop the extra (weird) offsets being applied to the Z sw endstop, give this a test and see what happens. I will have my printer set up again tomorrow.

@thinkyhead
Copy link
Member Author

So far I see no problems in testing.

Thinking about the zprobe_zoffset thing…. If anything, I would think that you'd want to take the positive zprobe_zoffset and subtract it from the sw_min_endstop[Z_AXIS], because when you have a positive offset, you actually do want to go below zero when probing.

In retrospect, adding (again) the negative part of home_offset[Z_AXIS] in the clamp function continues to make no sense in my mind, since it's already applied to sw_min_endstop[Z_AXIS].

@thinkyhead thinkyhead force-pushed the rc_fix_T_command branch 3 times, most recently from 320f516 to fd6fb7a Compare June 1, 2016 02:14
@thinkyhead thinkyhead merged commit f331763 into MarlinFirmware:RCBugFix Jun 1, 2016
@thinkyhead thinkyhead deleted the rc_fix_T_command branch June 1, 2016 02:36
@thinkyhead thinkyhead mentioned this pull request Jul 8, 2016
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.

3 participants