Actual movements during failed simulation

ÖrjanE

Hi,
I've been experimenting with printing using arcs (G2/G3) and have stumbled upon what seems like two bugs. I'm running RRF3.2 on a Duet2 Maestro with a normal cartesian printer.

First (possible) error occurs when printing an exact semicircle using the G3 Xnnn Ynnn Rnnn format. In the testcode (attached below) it is expected to print a semicircle from X=121.107 to X=113.893 with a radius of 3.607. Note that 113.893+2*3.607=121.107 so this should be fine. However, the firmware complains "Error: G2/G3: bad combination of parameter values". Increasing the radius to 3.608 makes the error disappear, which makes me think it is a problem with floating point precision.

Second worry is that if i run a simulation (!) there is actual, physical, movements happening when the error is detected. This only seems to happen when the axes are homed. The relevant error message in the console is "Error: SetPositions called when DDA ring not empty". Needless to say, physical movements during simulation (even if it is a failed one) is scary.

This is my stripped down test file:

G28 ; Home all axes
G21 ; set units to millimeters
G90 ; use absolute coordinates
M83 ; use relative distances for extrusion

G1 Z20.000 F7800.000
G1 F1800.000
G1 X121.107 Y128.804 E0.15037
G3 X113.893 Y128.804 R3.607 E0.42650
M0 ; Stop print

If relevant, here is my config.g

code_text; Configuration file for Duet Maestro (firmware version 3)
; executed by the firmware on start-up
;
; generated by RepRapFirmware Configuration Tool v3.1.1 on Wed Jun 03 2020 11:09:23 GMT+0200 (centraleuropeisk sommartid)

; General preferences
G90                                            ; send absolute coordinates...
M83                                            ; ...but relative extruder moves
M550 P"Anatidae"                               ; set printer name
M918 P1 E-4 F2000000                           ; configure direct-connect display

; Network
M552 P0.0.0.0 S1                               ; enable network and acquire dynamic address via DHCP
M586 P0 S1                                     ; enable HTTP
M586 P1 S1                                     ; enable FTP
M586 P2 S0                                     ; disable Telnet

; Drives
M569 P0 S0                                     ; physical drive 0 goes backwards
M569 P1 S0                                     ; physical drive 1 goes backwards
M569 P2 S1                                     ; physical drive 2 goes forwards
M569 P3 S0                                     ; physical drive 3 goes backwards (BMG Direct Drive)
M584 X0 Y1 Z2:4 E3                             ; set drive mapping
M350 X16 Y16 Z16 E16 I1                        ; configure microstepping with interpolation
M92 X80.00 Y80.00 Z400.00 E415.00              ; set steps per mm
M566 X{8*60} Y{8*60} Z{1*60} E{5*60}           ; set maximum instantaneous speed changes (mm/min)
M203 X{150*60} Y{150*60} Z{5*60} E{100*60}     ; set maximum speeds (mm/min)
M201 X500.00 Y500.00 Z100.00 E5000.00          ; set accelerations (mm/s^2)
M906 X500 Y500 Z500 E400 I30                   ; set motor currents (mA) and motor idle factor in per cent
M84 S30                                        ; Set idle timeout

; Axis Limits
M208 X-8 Y-10 Z0 S1                            ; set axis minima
M208 X235 Y225 Z260 S0                         ; set axis maxima

; Endstops
M574 X1 S1 P"xstop"                            ; configure active-high endstop for low end on X via pin xstop
M574 Y1 S1 P"ystop"                            ; configure active-high endstop for low end on Y via pin ystop
M574 Z1 S1 P"zstop"                            ; configure active-high endstop for low end on Z via pin zstop

; Z-Probe
M558 P9 C"^zprobe.in" H3 F100 T6000            ; enable Z probe
M950 S0 C"zprobe.mod"                          ; connect to mod pin
G31 X-25 Y-1 Z2.0 P500                         ; offset from nozzle
M557 X25:205 Y25:205 S45                       ; define mesh grid

; Bed leveling
M671 X-28:263 Y0:0 F1.5                        ; Position of the lead screws

; Heaters
M308 S0 P"bedtemp" Y"thermistor" T100000 B4092 A"Bed" ; configure sensor 0 as thermistor on pin bedtemp
M950 H0 C"bedheat" T0                          ; create bed heater output on bedheat and map it to sensor 0
M307 H0 B1 S1.00                               ; enable bang-bang mode for the bed heater and set PWM limit
M140 H0                                        ; map heated bed to heater 0
M143 H0 S120                                   ; set temperature limit for heater 0 to 120C
M308 S1 P"e0temp" Y"thermistor" T100000 B4725 C7.06e-8 A"Nozzle" ; configure sensor 1 as thermistor on pin e0temp
M950 H1 C"e0heat" T1                           ; create nozzle heater output on e0heat and map it to sensor 1
M307 H1 B0 S1.00                               ; disable bang-bang mode for heater  and set PWM limit
M143 H1 S285                                   ; Max nozzle temperature 285C

; Fans
M950 F0 C"fan2" Q500                           ; create fan 0 on pin fan2 and set its frequency
M106 P0 S0 H-1                                 ; set fan 0 value. Thermostatic control is turned off
M950 F1 C"fan1" Q500                           ; create fan 1 on pin fan1 and set its frequency
M106 P1 S1 H1 T45                              ; set fan 1 value. Thermostatic control is turned on

; Tools
M563 P0 D0 H1 F0                               ; define tool 0
G10 P0 X0 Y0 Z0                                ; set tool 0 axis offsets
G10 P0 R0 S0                                   ; set tool 0 active and standby temperatures to 0C

; Custom settings

; Set pressure advance
M572 D0 S0.1

; Set firmware retraction parameters
M207 S1 F1500 Z0   ; 1mm 25mm/s, no lift

; Load the overrides file
M501

jay_s_uk

can you upgrade to 3.2.2
there was a G2/G3 error that was fixed due to rounding

If a G2 or G3 command with R parameter described an arc of exactly 180 degrees, then due to rounding error RRF might report "G2/G3: invalid combination of parameters".

ÖrjanE

@jay_s_uk Thanks; will do!

ÖrjanE

@jay_s_uk Indeed; updating from 3.2 to 3.2.2 fixed the problem. Even when using a clearly too small radius value, the firmware does not throw an error.

Since I can no longer provoke the error in 3.2.2, I do not know if the second problem (movement when an error is triggered during simulation) is also fixed, or simply harder to encounter.

Thanks for the help.

jay_s_uk

@ÖrjanE its probably still something to be looked at.
@Phaedrux or @dc42 to comment as I've not seen any other reports of movement during simulation

dc42

@ÖrjanE, if you make the radius very much too small in the G2 or G3 command then I think you will still get the "Invalid combination" error.

ÖrjanE

@dc42 Surprisingly, I don't. I actually started looking into the source code of your commit (c8ebe81fdaf91fe447ada36a67f5b30b169d80cb), and I see that it is supposed to check if hDivD is negative but very close to zero. But hDivD is the result of a call to sqrtf; will that really give a negative result if the argument is negative? Is it not necessary to check the sign of hSquared instead?

dc42

@ÖrjanE, quite so! The test should be on hSquared instead. Something like this I think:

		const float hSquared = fsquare(rParam) - dSquared/4;				// square of the length of the perpendicular from the mid point to the arc centre

		// When the arc is exactly 180deg, rounding error may make hSquared slightly negative instead of zero
		float hDivD;
		if (hSquared >= 0.0)
		{
			hDivD = sqrtf(hSquared/dSquared);
		}
		else
		{
			if (hSquared < -0.01 * fsquare(rParam))
			{
				err = badArcParametersMessage;
				return true;
			}
			hDivD = 0.0;													// this has the effect of increasing the radius slightly so that the maths works
		}

Do you agree?

ÖrjanE

@dc42 Looks correct. The interpretation of the constant -0.01 is not transparent. I guess it implies that the radius can be 10% (sqrt(0.01)) too short without throwing an error.

The current (3.2.2) code does not throw an error at all, but does not print the arc correctly if the radius is slightly too short. The resulting arc is basically a straight line, probably because the hDivD value actually used is rediculously high.

dc42

I've changed it to:

			if (hSquared < -0.001 * fsquare(rParam))						// allow the radius to be up to sqrt(0.001) ~= 3.2% too short

In the 3.2.2 code, hDivD will be a NaN if the radius is too short, so it's not surprising that it misbehaves.

dc42

Actually that's still wrong. (r + xr)^2 ~= r^2(1 + 2x) if x is small, so I will use this instead:

			if (hSquared < -0.02 * fsquare(rParam))						// allow the radius to be up to 1% too short

ÖrjanE

@dc42 Looks good!

Now, back to the other problem; that an error during simulation may cause physical movement. In 3.2.2 I can't cause the "too small radius"-error any more, but looking at the source code, I realized that I can provoke an error by setting identical start end end points for G2 or G3 with an R parameter.

This code:

G28 ; Home all axes
G21 ; set units to millimeters
G90 ; use absolute coordinates
M83 ; use relative distances for extrusion

G1 Z20.000 F7800.000
G1 F1800.000
G1 X115 Y115
G1 F200.000
G3 X115 Y115 R5
M0 ; Stop print

will cause the message "Error: SetPositions called when DDA ring not empty" as well as "Error: G2/G3: radius is too small to reach endpoint" when run in simulation mode. It also causes a physical movement (some 10mm in the negative X direction).

Hope you can replicate this behaviour.

dc42

Thanks, it sounds to me that when the error occurs, the simulation is exiting before the DDA ring has been emptied, which ties in the message.

ÖrjanE

@dc42 I have also noted another symptom, probably due to the same problem.

When printing (not simulating) the code above, the same unexpected movement as in simulation takes place when the error occurs and printing is aborted. The axes remain in homed-status but the coordinates have not been updated with that last movement.

dc42

@ÖrjanE said in Actual movements during failed simulation:

@dc42 I have also noted another symptom, probably due to the same problem.

When printing (not simulating) the code above, the same unexpected movement as in simulation takes place when the error occurs and printing is aborted. The axes remain in homed-status but the coordinates have not been updated with that last movement.

Do you get the "SetPositions called when DDA ring not empty" message again?

ÖrjanE

@dc42 said in Actual movements during failed simulation:

Do you get the "SetPositions called when DDA ring not empty" message again?

No.
And now I can't replicate that second behaviour (extra movement when not simulation). Perhaps it is state-dependent and du to some leftover state from the failed simulation.

ÖrjanE

@dc42 Forget about the "second symptom" (problem during actual print). I have restarted the duet and fail to repeat the problem. It may be that I misinterpreted what happened when I tested this.

Regarding the move when simulating; it makes sense that it is the last move/moves that remain queued when simulation mode is turned off prematurely. The move does not seem to be random, but consistent with the last move that should have been done (in simulation) before the error occurred.

dc42

Thanks for your example. I have reproduced the issue, and fixed it in the 3.3 branch source code. Moves already prepared and put into the movement queue are now completed when a job is aborted.

ÖrjanE

@dc42 Great!
Thanks for your work in maintaining this software.

dc42

@ÖrjanE said in Actual movements during failed simulation:

@dc42 Great!
Thanks for your work in maintaining this software.

Thanks for your clear description of the issue and GCode file to reproduce it.