gcode tests fail

[Bert Timmerman]

Hi,

Running the pcb testsuite, some of the gcode tests fail:

<output>
Test: hid_gcode1
../../../src/pcbtest.sh -x gcode ./inputs/gcode_oneline.pcb
PASSED
----------------------------------------------------------------------
Test: hid_gcode2
../../../src/pcbtest.sh -x gcode --basename out.gcode ./inputs/gcode_oneline.pcb
PASSED
----------------------------------------------------------------------
Test: hid_gcode3
../../../src/pcbtest.sh -x gcode --dpi 1200 ./inputs/gcode_oneline.pcb
--- /tmp/pcb_tests.27427/gcode_oneline-bottom.gcode-ref 2012-11-02 09:13:47.838989817 +0100
+++ /tmp/pcb_tests.27427/gcode_oneline-bottom.gcode-out 2012-11-02 09:13:47.845995282 +0100
@@ -35,6 +35,6 @@
 G1 X28.765500 Y12.488333
 G1 X28.765500 Y12.890500
-G1 X28.638500 Y13.165667
-G1 X28.448000 Y13.377333
+G1 X28.617333 Y13.208000
+G1 X28.405667 Y13.398500
 G1 X28.130500 Y13.525500
 G1 X27.813000 Y13.546667
--- /tmp/pcb_tests.27427/gcode_oneline-top.gcode-ref 2012-11-02 09:13:47.938989652 +0100
+++ /tmp/pcb_tests.27427/gcode_oneline-top.gcode-out 2012-11-02 09:13:47.945994977 +0100
@@ -35,6 +35,6 @@
 G1 X23.706667 Y12.488333
 G1 X23.706667 Y12.890500
-G1 X23.579667 Y13.165667
-G1 X23.389167 Y13.377333
+G1 X23.558500 Y13.208000
+G1 X23.346833 Y13.398500
 G1 X23.071667 Y13.525500
 G1 X22.754167 Y13.546667
FAIL
----------------------------------------------------------------------
Test: hid_gcode4
../../../src/pcbtest.sh -x gcode --iso-mill-depth 0.5 ./inputs/gcode_oneline.pcb
--- /tmp/pcb_tests.27427/gcode_oneline-bottom.gcode-ref 2012-11-02 09:13:48.270990251 +0100
+++ /tmp/pcb_tests.27427/gcode_oneline-bottom.gcode-out 2012-11-02 09:13:48.276995563 +0100
@@ -5,5 +5,5 @@
 (Tool diameter: 0.200000 mm)
 #100=2.000000 (safe Z)
-#101=0.000000 (cutting depth)
+#101=0.500000 (cutting depth)
 #102=25.000000 (plunge feedrate)
 #103=50.000000 (feedrate)
--- /tmp/pcb_tests.27427/gcode_oneline-top.gcode-ref 2012-11-02 09:13:48.370240391 +0100
+++ /tmp/pcb_tests.27427/gcode_oneline-top.gcode-out 2012-11-02 09:13:48.376247030 +0100
@@ -5,5 +5,5 @@
 (Tool diameter: 0.200000 mm)
 #100=2.000000 (safe Z)
-#101=0.000000 (cutting depth)
+#101=0.500000 (cutting depth)
 #102=25.000000 (plunge feedrate)
 #103=50.000000 (feedrate)
FAIL
----------------------------------------------------------------------
Test: hid_gcode5
../../../src/pcbtest.sh -x gcode --safe-Z 10 ./inputs/gcode_oneline.pcb
PASSED
----------------------------------------------------------------------
Test: hid_gcode6
../../../src/pcbtest.sh -x gcode --iso-tool-diameter 0.4 ./inputs/gcode_oneline.pcb
--- /tmp/pcb_tests.27427/gcode_oneline-bottom.gcode-ref 2012-11-02 09:13:49.143248381 +0100
+++ /tmp/pcb_tests.27427/gcode_oneline-bottom.gcode-out 2012-11-02 09:13:49.150240435 +0100
@@ -3,5 +3,5 @@
 (Board size: 50.80 x 25.40 mm)
 (Accuracy 600 dpi)
-(Tool diameter: 0.000000 mm)
+(Tool diameter: 0.400000 mm)
 #100=2.000000 (safe Z)
 #101=-0.050000 (cutting depth)
@@ -13,30 +13,29 @@
 G0 Z#100
 (polygon 1)
-G0 X27.770667 Y13.462000 (start point)
+G0 X27.728333 Y13.631333 (start point)
 G1 Z#101 F#102
 F#103
-G1 X27.516667 Y13.335000
-G1 X27.347333 Y13.208000
-G1 X7.408333 Y13.165667
-G1 X7.112000 Y12.869333
-G1 X7.112000 Y12.488333
-G1 X7.408333 Y12.192000
-G1 X27.347333 Y12.149667
-G1 X27.643667 Y11.938000
-G1 X27.940000 Y11.895667
-G1 X28.278667 Y11.980333
-G1 X28.532667 Y12.192000
-G1 X28.702000 Y12.530667
-G1 X28.702000 Y12.827000
-G1 X28.532667 Y13.165667
-G1 X28.321000 Y13.335000
-G1 X28.024667 Y13.462000
-G1 X27.770667 Y13.462000
+G1 X27.347333 Y13.419667
+G1 X7.493000 Y13.419667
+G1 X7.112000 Y13.250333
+G1 X6.900333 Y12.911667
+G1 X6.900333 Y12.446000
+G1 X7.112000 Y12.107333
+G1 X7.493000 Y11.938000
+G1 X27.305000 Y11.938000
+G1 X27.770667 Y11.726333
+G1 X28.194000 Y11.768667
+G1 X28.575000 Y12.022667
+G1 X28.829000 Y12.403667
+G1 X28.829000 Y12.954000
+G1 X28.575000 Y13.335000
+G1 X28.194000 Y13.589000
+G1 X27.728333 Y13.631333
 G0 Z#100
-(polygon end, distance 44.84)
+(polygon end, distance 45.99)
 (predrilling)
 F#102
 G81 X27.940000 Y12.700000 Z#101 R#100
 (1 predrills)
-(milling distance 44.84mm = 1.77in)
+(milling distance 45.99mm = 1.81in)
 M5 M9 M2
--- /tmp/pcb_tests.27427/gcode_oneline-top.gcode-ref 2012-11-02 09:13:49.250240620 +0100
+++ /tmp/pcb_tests.27427/gcode_oneline-top.gcode-out 2012-11-02 09:13:49.257247480 +0100
@@ -3,5 +3,5 @@
 (Board size: 50.80 x 25.40 mm)
 (Accuracy 600 dpi)
-(Tool diameter: 0.000000 mm)
+(Tool diameter: 0.400000 mm)
 #100=2.000000 (safe Z)
 #101=-0.050000 (cutting depth)
@@ -13,26 +13,25 @@
 G0 Z#100
 (polygon 1)
-G0 X22.733000 Y13.462000 (start point)
+G0 X22.690667 Y13.631333 (start point)
 G1 Z#101 F#102
 F#103
-G1 X22.479000 Y13.335000
-G1 X22.309667 Y13.208000
-G1 X2.370667 Y13.165667
-G1 X2.074333 Y12.869333
-G1 X2.074333 Y12.488333
-G1 X2.370667 Y12.192000
-G1 X22.309667 Y12.149667
-G1 X22.606000 Y11.938000
-G1 X22.902333 Y11.895667
-G1 X23.241000 Y11.980333
-G1 X23.495000 Y12.192000
-G1 X23.664333 Y12.530667
-G1 X23.664333 Y12.827000
-G1 X23.495000 Y13.165667
-G1 X23.283333 Y13.335000
-G1 X22.987000 Y13.462000
-G1 X22.733000 Y13.462000
+G1 X22.309667 Y13.419667
+G1 X2.455333 Y13.419667
+G1 X2.074333 Y13.250333
+G1 X1.862667 Y12.911667
+G1 X1.862667 Y12.446000
+G1 X2.074333 Y12.107333
+G1 X2.455333 Y11.938000
+G1 X22.267333 Y11.938000
+G1 X22.733000 Y11.726333
+G1 X23.156333 Y11.768667
+G1 X23.537333 Y12.022667
+G1 X23.791333 Y12.403667
+G1 X23.791333 Y12.954000
+G1 X23.537333 Y13.335000
+G1 X23.156333 Y13.589000
+G1 X22.690667 Y13.631333
 G0 Z#100
-(polygon end, distance 44.84)
-(milling distance 44.84mm = 1.77in)
+(polygon end, distance 45.99)
+(milling distance 45.99mm = 1.81in)
 M5 M9 M2
FAIL
----------------------------------------------------------------------
Test: hid_gcode7
../../../src/pcbtest.sh -x gcode --drill-depth 7 ./inputs/gcode_oneline.pcb
PASSED
----------------------------------------------------------------------
Test: hid_gcode8
../../../src/pcbtest.sh -x gcode --measurement-unit mm ./inputs/gcode_oneline.pcb
PASSED
----------------------------------------------------------------------
Test: hid_gcode9
../../../src/pcbtest.sh -x gcode --measurement-unit mil ./inputs/gcode_oneline.pcb
PASSED
----------------------------------------------------------------------
Test: hid_gcode10
../../../src/pcbtest.sh -x gcode --measurement-unit um ./inputs/gcode_oneline.pcb
PASSED
----------------------------------------------------------------------
Test: hid_gcode11
../../../src/pcbtest.sh -x gcode --measurement-unit inch ./inputs/gcode_oneline.pcb
PASSED
</output>

I notice that:

- it looks like the date/time stamp is not normalised in a proper way,

- round-offs are occurring,

- tool diameter dimensions differ beyond round-off tolerances.

Running at git-HEAD commit:

5722ae0a55832f525f056d09b002d7f984f90379
gocde HID: refresh all the G-code exporter tests.

Distro: Fedora-13 (patches and updates are up-to-date).

Kind regards,

Bert Timmerman.

[Vladimir Zhbanov (vzhbanov)]

Hello, Bert.

I've found that there are two different bugs here. The hid_gcode4
and hid_gcode6 are affected by a locale issue while the hid_gcode3
issue is a separate bug.

I've attached a diff file which could shed a light on what's
going wrong (I was unable to find any way how to use gdb to
explore the gcode code).

You can apply the test diff using:
  git apply show_cutdepth_values.diff
Then to test what's happening you have to have non-C locale (I use
ru_RU.UTF-8). The command
  make && cd tests && ./run_tests.sh hid_gcode4
outputs
  ...
  ../../../src/pcbtest.sh -x gcode --iso-mill-depth 0.5 ./inputs/gcode_oneline.pcb
  cut_depth after gcode init=-0,050000
  cut_depth aftter getting export options: 0.000000
  cut_depth before export: 0.000000
  PASSED
  ...
Note the comma separator in the first case.

The next command using the C locale
  make && cd tests && LANG=C ./run_tests.sh hid_gcode4
outputs:
  ../../../src/pcbtest.sh -x gcode --iso-mill-depth 0.5 ./inputs/gcode_oneline.pcb
  cut_depth after gcode init=-0.050000
  cut_depth aftter getting export options: 0.500000
  cut_depth before export: 0.500000
  --- /tmp/pcb_tests.7976/gcode_oneline-bottom.gcode-ref 2014-02-04 15:09:37.000000000 +0400
  +++ /tmp/pcb_tests.7976/gcode_oneline-bottom.gcode-out 2014-02-04 15:09:37.000000000 +0400
  @@ -5,5 +5,5 @@
   (Tool diameter: 0.200000 mm)
   #100=2.000000 (safe Z)
  -#101=0.000000 (cutting depth)
  +#101=0.500000 (cutting depth)
   #102=25.000000 (plunge feedrate)
   #103=50.000000 (feedrate)
  --- /tmp/pcb_tests.7976/gcode_oneline-top.gcode-ref 2014-02-04 15:09:37.000000000 +0400
  +++ /tmp/pcb_tests.7976/gcode_oneline-top.gcode-out 2014-02-04 15:09:37.000000000 +0400
  @@ -5,5 +5,5 @@
   (Tool diameter: 0.200000 mm)
   #100=2.000000 (safe Z)
  -#101=0.000000 (cutting depth)
  +#101=0.500000 (cutting depth)
   #102=25.000000 (plunge feedrate)
   #103=50.000000 (feedrate)
  FAIL

You can see that the second test fails while the command output is
indeed correct as the options '--iso-mill-depth 0.5' is used.

The output for hid_gcode4 is all but the same while the output for
hid_gcode6 should be manually verified (though differences are not
very large).

[Vladimir Zhbanov (vzhbanov)]

The attached patch fixes the first bug though test files should be
regenerated and verified anyway.

[Vladimir Zhbanov (vzhbanov)]

The last paragraph in my first message is related
to what should be done after the patch will be
applied (my mistake, sorry). Repeating it here:

The output for hid_gcode4 is all but the same while the output for
hid_gcode6 should be manually verified (though differences are not
very large) in order to consider the first bug fixed.

[Bert Timmerman (bert-timmerman)]

Hi Vladimir,

I have not verified the golden files yet.

What I did find was that the mill tool diameter is handled differently than the tool diameter.

See attached patch.

I have ran this in tests and now even more tests fail, although that was to be expected.

Could you have a look whether this patch should be applied, and if it is related to this bug ? or not ;-)

Kind regards,

Bert Timmerman.

[Andrew Poelstra (asp11)]

Hi guys,

I have found the bug in hid_gcode3. The problem is that when the gcode exporter is deciding how to lay out polygons, it uses an optimization algorithm to find the best way to do so. (Reading the code, it seems it is starting from a bitmap instead of the raw pcb data, for some reason.) In a few cases, the difference in cost between one path or another is in the twelveth decimal place, so on 32- and 64-bit systems different decisions are made.

The cost is calculated in penalty3(), at src/hid/gcode/trace.c:563, and returned as "sqrt(s)".

By removing the sqrt (and thus moving the difference from the twelveth decimal place to something much bigger), a bunch of tests break in similarly subtle ways as the current hid_gcode3 breakage. But after regenerating the goldens, all tests pass for both 32- and 64-bit programs.

I'm happy to push such a change, but since it's a totally voodoo-programming change, and fixes a test failure but no actual bugs, and it breaks a bunch of test cases, I'd need support from a core dev.

[Andrew Poelstra (asp11)]

I should clarify here that penalty3() always returns positive numbers, and is used in exactly one place, where the only relevant property of its return values is their ordering. So removing a sqrt() has no mathematical effect on the surrounding algorithm. It'll just separate values better to help avoid situations where floating-point rounding behaviour determines which of two values is considered larger.

[أحمد المحمودي (Ahmed El-Mahmoudy) (aelmahmoudy)]

Any updates ? Debian is going to freeze soon.

[Traumflug (mah-jump-ing)]

After investigating this problem for a day with lots of googling I came to the conclusion that trying to adjust environment variables is the wrong track. Can't ever work reliably, because there is only one environment for both, exporters and GUI. The standards comittees apparently simply forgot or ignored that not all printf() output is meant to be human readable. Bummer.

The right, reliable solution is to simply not use fprintf() for writing such contents. At least not the fprintf() coming with libc.

One possible replacement would be a complete alternative implementation of fprintf(), quite some libraries are available. But then I found Glib, which is a huge library, but already in use by other parts of pcb. It comes with g_ascii_formatd(), which does exactly what we need: it writes floats with ASCII decimals. There is no g_ascii_fprintf(), but this works on any locale:

    char b[100];
    fputs(g_ascii_formatd(b, 100, "%3.2f\n", 125.646), stdout);

Today I pushed a cleanup of this env variable mess. With NLS disabled, tests pass now all, 'make check' succeeds. Even on a de_DE locale.

Next I'll investigate wether to use pcb_fprintf() (might need fixing) or to rewrite file writing code to use g_ascii_formatd() directly.

[Traumflug (mah-jump-ing)]

Another commit on master implements ascii_fprintf(), which does what the name implies: it's an fprintf() which always writes ascii. For formatting floats/doubles it uses g_ascii_formatd().

With NLS enabled tests don't pass entirely, yet, but it's much better. G-code should be usable now.

[Traumflug (mah-jump-ing)]

ascii_fprintf() was just dropped again, after pcb_fprintf() learned to write generic floats/doubles in file mode, i.e. with a '.' decimal separator independently from the locale settings. All tests pass, with or without NLS enabled. Tested on a system set up with de_DE locale.

It's a whole bunch of commits, along the way I improved pcb-printf, too. Like replacing the risky pcb_sprintf() with the buffer overflow safe pcb_snprintf().

[Traumflug (mah-jump-ing)]

> What I did find was that the mill tool diameter is handled differently than
> the tool diameter.

That's correct and intentional. Tool diameter deals with PCB coordinates, while mill tool diameter is written directly to the G-code file, for outline milling and drill-milling.

[Traumflug (mah-jump-ing)]

Importance medium, because tests didn't pass. Broken code is a showstopper.

[Traumflug (mah-jump-ing)]

Fixes on Git master. Enjoy!

[Traumflug (mah-jump-ing)]

Please keep your fingers off geda-project unless you want to contribute to it.