search

Duet3D / RepRapFirmware

OO C++ RepRap Firmware
GNU General Public License v3.0
937 stars 534 forks source link

Using M400 in deployprobe.g causes machine to try to move outside limits #978

Open dc42 opened 4 months ago

dc42 commented 4 months ago

If M400 is used in file deployprobe.g then commands such as G30 K0 P0 X15 Y31 Z-99999 and G29 S0 do not work properly and subsequent movements may be executing incorrectly, e.g. trying to move outside machine limits. Reported at https://forum.duet3d.com/topic/35469/3-5-0rc4-g29-crashes-nozzle-into-bed-after-first-probe-point. Reproduced using command G30 K0 P0 X15 Y31 Z-99999 on a E3D tool changer.

Workaround: if using M400 withing deployprobe.g or retractprobe.g then add parameter S1 to the M400 command.

dc42 commented 4 months ago

Likely to be two issues here: (a) When moves are generated internally, there may be cases in which some of the axes being moved are not flagged as being owned. This appears to be the case for the G30 K0 P0 X15 Y31 Z-99999 command. (b) In the case of G29 S0 I think the XYZ axes are flagged as owned at the start of the command, however using M400 without the S1 parameter will release them when the first point is probed; so they are not owned when they are moved subsequently.

dc42 commented 4 months ago

Proposed fix:

  1. Add an internal check in NewMoveAvailable or similar to catch any instances of trying to move axes that are not owned and abort the move (or more) in that case.
  2. Remedy any instances that the check catches, in the absence of M400 calls in deploy/retract files.
  3. To handle the special case of deploy/retract files, in the code for M400 check whether any of the bed probing states (or any other states that use sequences of automatically-generated moves) are in any of the stacked machine states. If so, don't release axes.
oliof commented 4 months ago

I would suggest reinstating the old behavior on the bare M400 call and having an explicit parameter for the new behavior. Seems like less special casing, and the people requiring the new behavior can enable it if they want.

dc42 commented 4 months ago

The new default behaviour is needed to avoid users getting frequent error due to axes not being available when they start using multiple motion systems. The fact that the default behaviour causes problems when M400 is used in deployprobe.g/retractprobe.g is a bug, it's because RRF subsequently moves axes that the current motion system no longer owns, without re-establishing ownership.

dc42 commented 4 months ago

In RRF 3.5.1 added a workaround to prevent M400 (with or without any parameter) from releasing axes when any entry in the state machine stack is in the middle of any probing sequence. Leaving this issue open because we should still add a check for moving unowned axes in the future.