pick and place (xy) file export in dmil unit

[Sergey Alyoshin]

If dmil units are used in XY file export, the result x- and
y-coord is not correct and in kind of mixed units.

cmil is correct.

Checked on example file LED2.pcb with exporting and
comparing XY file in mil and dmil.

[Sergey Alyoshin (alyoshin-s)]

Same bug was fixed in r21297 of svn://repo.hu/pcb-rnd

[Bert Timmerman (bert-timmerman)]

Hi,

I tried to manually apply the pcb-rnd diff r21297.

The pcb-rnd code differs massively from the pcb code.

A git-patch from a first satb at this bug is in a following attachment, and I did push to topic branch LP1806044 in the pcb git repository.

make distcheck gives a diff from the golden files (expected ?) in which the x value of coordinates of the first component is not altered, log spew applied in another attachment.

IMO this is probably due to another bug in pcb-printf.[ch] code.

Kind regards,

Bert Timmerman.

[Chad Parker (parker-charles)]

Was the golden file wrong?

[Bert Timmerman (bert-timmerman)]

Hi Chad,

I did a quick scan, the golden file looks correct for default units (cmil).

There are just no regression tests for other units, so this one went unnoticed.

Follow up in this bug report or start a new bug report ?

Kind regards,

Bert Timmerman.

[Chad Parker (parker-charles)]

pcb-printf actually has quite a few tests. They're run using the glib testing framework though, not the run_tests.sh script. They're in pcb-printf.c at the end of the file: pcb_printf_test_printf().

I haven't looked at this much, but, I'm not sure that %mN is a valid format specifier. Perhaps it's new to pcb-rnd?

If the output is supposed to be dmil, I think the format specifier should be %mt.

[Bert Timmerman (bert-timmerman)]

Hi Charles,

At a first glance the %mt gives dmil output. Now we need additional tests and golden files.

Kind regards,

Bert Timmerman.

[Chad Parker (parker-charles)]

So, the output of the file is a mix of mils and inches. I think you were on the right track yesterday. The line:

ALLOW_ALL = ~ALLOW_DMIL

means that DMIL isn't generally allowed. I suspect that the reason for this is that when the feature was added (3a8891a6) they didn't want the "most natural unit" to suddenly start selecting and outputting dmil in places where it wasn't before. However, clearly this means that "most natural unit" doesn't work with dmil. "Most natural unit" uses the following line to generate its output (bom.c:617):

                case 'S': unit_str = CoordsToString(value, 1, spec->str, mask & ALLOW_ALL, suffix); break;

The value of "mask" is set by the earlier format to ALLOW_DMIL, so mask & ALLOW_ALL -> 0. So, CoordsToString gets told that no units are allowed, which by default it assumes to mean that ALLOW_ALL units are allowed, which excludes dmil.

I see a couple of possible solutions:

1. Change ALLOW_ALL to ~0 instead of ~ALLOW_DMIL.
This seems like a reasonable thing to do, except that it may cause other output to switch to dmil, which we don't generally want.

2. Change ALLOW_ALL to ~0 and add an ALLOW_COMMON = ~ALLOW_DMIL, update the 'S' specifier to use ALLOW_COMMON, and add a new format specifier that actually allows any unit to be used with a natural scaling.

3. Dynamically generate the format string in bom.c using the format specifier that the user requested.
I think this is the easiest. The line:
char fmt[256];

sprintf(fmt, "%%s,\"%%s\",\"%%s\",%%.2`m%c,%%.2`m%c,%%g,%%s\n", xy_unit->printf_code, xy_unit->printf_code);

at the top of PrintBOM does the trick, then use fmt as the format string in the pcb_fprintf call.

I've pushed a branch LP1806044 that takes care of this. It also adds some additional tests for various units.