G2 ZAxis Moves Not Completing

[madgrizzle]

Firmware/cnc_ctrl_v1/Motion.cpp

Line 348 in 400db40

zPosition += zStepSizeMM;

In the G0/G1 moves (coordinatedMove) this function is higher up in the loop than in G2/G3 moves (arc). I think this needs to be moved up to be after line 332?

[blurfl]

Good spotting @madgrizzle. I'm not in a position to make that change or test it - can you do that?

[BarbourSmith]

I agree that after 332 looks like a more logical place for that line to go. I'm not in a position to test it either but I can put the call out on the forums and hopefully recruit some testers

[madgrizzle]

Yeah, but after thinking about it, I'm not sure its the cause of the problem. The issue was a G1 Z-0.8 F30 move not completing... I'm thinking maybe if it only happens at that depth, then it might be a binding issue of some sort (maybe its bottoming out). Need more info,

[blurfl]

If the file runs in FAKE_SERVO mode but fails with real equipment, that would further suggest a mechanical cause.

[gb0101010101]

Originally reported in forum Ground Control not using correct Z height

I think I have found the problem but looking through change history I can't understand what was going on. Axis::read() is reporting the current encoder position whilst it is still moving and not the last target dimension set by Axis::write() or Axis::set().

I thought that gcode is executed sequentially and waits until motors have stopped moving from previous command? It looks like the next Gcode command can start processing before this (even using fakeServo).

There is Axis::setpoint() which seems to return the last target dimension but I have no idea why its is named setpoint and it does not appear to be used anywhere.

If you change GCode.cpp line 778 from
float Z1 = zAxis.read();
to
float Z1 = zAxis.setpoint();

You will see that Z does reach target dimension. Before this change, testing with fakeServo, the Z stopped at -7.96 for me. Afterwards it stops at -8 as expected. Using physical equipment will probably make the discrepancy worse.

Test Gcode

Planar G2

This exhibits the problem because Z is not provided in the G2 line.

G0 G40 G90 G17  
G21  
G0 X0 Y0 Z0  
G1 Z-8  
G2 X37.93360 Y5.515 I43.2022 J129.306  
G1 Z0  

Helical G2

This always works because Z is provided.

G0 G40 G90 G17  
G21  
G0 X0 Y0 Z0  
G1 Z-8  
G2 X37.93360 Y5.515 I43.2022 J129.306 Z-16
G1 Z0  

Perhaps zAxis.setpoint() should be renamed zAxis.readTarget()?

[madgrizzle]

I'm using my phone to look at all this, but if you look at the end of the arc function, you will see rightaxis.endmove and leftaxis.endmove called, but not zaxis.endmove. Compare the arc function with coordinatedmove and you'll see the difference.

[gb0101010101]

if you look at the end of the arc function, you will see rightaxis.endmove and leftaxis.endmove called, but not zaxis.endmove

@madgrizzle That might be another problem. The problem I am talking about happens before arc() starts. Z1 passed to arc() is already wrong. If the G2 code line does not have Z in it then Z2 is also wrong because it equals Z1. Removing my fix and adding zAxis.endMove(Z2); at end of arc() does not fix the problem I am reporting but it might fix another problem.

[madgrizzle]

Yeah, I agree.. but if we are inside the code to fix one thing, might as well fix something else that's there.

I'm wondering if loop is being exited one step too early.

[gb0101010101]

I have created pull request with fix. #539
It fixes this issue by passing Z1 and Z2 as NAN if Z was not specified in the G2 gcode line. This stops arc() from using the wrong Z height that was read whilst Z axis was still in motion.
It also removes all Z axis calculation and movement when Z1 or Z2 are NAN.