Fix: Runaway in jog velocity mode

[hdiethelm]

While correcting issues in the xhc-whb04b-6 module (#3810), I found an issue in axis.c.

This can result in a crash, especially in Z axis if you have a tool in the spindle.

Steps to reproduce

1. Start linuxcnc and load axis_mm sample
2. Enable and home
3. Go to MDI and execute G53 G0 X-254 (Negative limit)
4. Go to Manual Control
5. Open Halshow and add axis.x.jog-vel-mode / axis.x.jog-scale / axis.x.jog-enable / axis.x.jog-counts
6. Set axis.x.jog-vel-mode = true / axis.x.jog-scale = 0.12 / axis.x.jog-enable = true
7. Set axis.x.jog-counts = 1 -> Machine moves to x=-0.914 with full speed

I would expect the machine to move to x=-253.88

This happens on all axis and all limits. For simplicity, I just described X negative.

Analysis

The issue lies in https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/motion/axis.c

After homing, axis->pos_cmd is 0.0

G53 G0 X-254 moves the machine exactly to one of the limits.

Due to this, update_teleop_with_check returns true and axis->pos_cmd is not updated with the actual position:
https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/motion/axis.c#L677

When jogging in velocity mode, axis->pos_cmd is used to limit the next set-point:
https://github.com/LinuxCNC/linuxcnc/blob/master/src/emc/motion/axis.c#L421

Now with axis->pos_cmd == 0.0 while the machine is at -254, it moves with full speed to x=-0.914.

This issue exists on any axis when moving with gcode to one of the limits or your home position is at one of the limits and then manually jog in velocity mode to any direction.

I analyzed this with some printf debugging. See:
https://github.com/hdiethelm/linuxcnc-fork/tree/printf-debug

This was changed quite some time ago in:
784db03 control.c line 1312

Suggested fix

• update_teleop_with_check already sets teleop_tp.curr_pos to the last position before the limit and teleop_tp.curr_vel = 0 if at or over the limit
• axis->pos_cmd: This is only used for jogging in velocity mode. So this change should be save.
• axis->teleop_vel_cmd: I think this should also be changed. However, I'm not so deep in the code that I'm sure they will be no side effects.

This fixes the above behavior. I will do some future testing next week in simulation and on my machine, but until now, I did not see any side effects.

[hdiethelm]

Yesterday, I did a whole day of milling with a lot of jogging between and no issues so far.

[andypugh]

Sorry to take so long to look at this.
This is clearly a bug, and your fix makes sense.
As a bug it should probably be against the 2.9 branch.

One thought that I have is whether potentially the velocity command should be set to zero in the case where the axis is on the limit.

[andypugh]

Rebased on 2.9 and pushed as 7242b3f

[hdiethelm]

Thanks for merging!
How do you handle 2.9 vs. master? Bugfixes go to 2.9 and 2.9 is regularly merged to master? So I can re-base my #3810 also to 2.9 if it's more apropriate.

One thought that I have is whether potentially the velocity command should be set to zero in the case where the axis is on the limit.

Reading the code, axis->teleop_tp.curr_vel is set to 0 in:
https://github.com/hdiethelm/linuxcnc-fork/blob/7bcbdd6762ab1cca4fe7d65361067e23e24926ad/src/emc/motion/axis.c#L652 and
https://github.com/hdiethelm/linuxcnc-fork/blob/7bcbdd6762ab1cca4fe7d65361067e23e24926ad/src/emc/motion/axis.c#L659
so axis->teleop_vel_cmd is already set to 0 with:
https://github.com/hdiethelm/linuxcnc-fork/blob/7bcbdd6762ab1cca4fe7d65361067e23e24926ad/src/emc/motion/axis.c#L680

I did not test that, but I don't see a reason how this could work any other way.

[hdiethelm]

Rebased on 2.9 and pushed as 7242b3f

Ah I just reviewed you commit 7242b3f. The two lines should be after update_teleop_with_check() as they are in my PR. Otherwise, the values are not limited properly any you introduce one sample delay.

[andypugh]

The two lines should be after update_teleop_with_check() as they are in my PR. Otherwise, the values are not limited properly and you introduce one sample delay.

I had forgotten to consider the side effects of the function call when I decided to move the lines to somewhere that felt more logical.

As for your other question, yes, bug fixes go into 2.9 and then 2.9 is periodically merged upstream.