pcb

G-code exporter outputs all drill sizes into the same file

Bug #846359 reported by dima kogan
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pcb
Fix Released
Undecided
Unassigned

Bug Description

When using the g-code output to manufacture boards, drill bits must be changed to accomodate the different hole sizes. The current gcode exporter doesn't look at the hole size at all, but just dumps all the holes together into a single file. I'm attaching a patch series to fix this by outputting a different file for each drill size present in the layout. It is perhaps better to still have a single file, but to add gcode tool-changing commands instead to separate the hole sizes. It is a very small job to change these patches to have that behavior

Revision history for this message
dima kogan (pjhhh3o25) wrote :
Revision history for this message
dima kogan (pjhhh3o25) wrote :
Revision history for this message
dima kogan (pjhhh3o25) wrote :
Revision history for this message
dima kogan (pjhhh3o25) wrote :
Revision history for this message
dima kogan (pjhhh3o25) wrote :
Peter Clifton (pcjc2)
Changed in pcb:
status: New → Confirmed
importance: Undecided → Wishlist
importance: Wishlist → Undecided
milestone: none → next-feature-release
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi dima kogan,
Whilst trying to apply your patches against git-HEAD, I encountered:
<quote>
An error occurred while executing command:

git am --utf8 --3way --signoff 0002-gcode-exporter-minor-whitespace-corrections.patch

Git says:

fatal: sha1 information is lacking or useless (src/hid/gcode/gcode.c).
</quote>

Hope you can solve this one.
For the time being I will patch my local pcb repo manually for testing purposes.
Kind regards, Bert Timmerman.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi dima kogan,
Both the 0003 and 0004 patches gave the same error regarding the sha1 information, so I patched them by hand editing the gcode.c file.
The last 0005 patch applied OK without any error notice.
As expected, with the patches applied I get one cnc file for every drill size in the pcb.
These files seem to be OK on a short and shallow review.
Kind regards, Bert Timmerman.

Changed in pcb:
status: Confirmed → In Progress
Revision history for this message
dima kogan (pjhhh3o25) wrote :

Hi there.

You're seeing a conflict caused by other patches touching the gcode source.
These were merged in after I made my series. I'm attaching a new series that's
rebased on top of the new code (on top of the master branch in fact).

In looking at this again I discovered that the unit tests were badly broken and
had several bugs in them that prevented them from testing anything at all. I
fixed these and made the g-code tests work. There are some other tests that fail
now, but I haven't looked at them. To be clear, I didn't cause the failures;
rather I fixed the tests that made apparent the failures that were already
there.

The gcode patches and the unit-testing patches live together in the series I'm
attaching. It might make sense to evaluate them separately, but it should all be
good.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi dima kogan,
I see in the generated .cnc that lines like:

(Board size: 3.50x3.30 inches)#100=0.002000 (safe Z)

have changed into:

(Board size: 3.50x3.30 inches)
#100=0.002000 (safe Z)

which looks better to me.
Other than that I see almost identical generated files between the original and revised patch set, only dates change.
One funny thing is that the GTK UI has the generation of advanced g-code ticked, and the lesstif UI has no tick set for advanced g-code generation.
Setting the tick of in the GTK UI I do not get an identical generated files.
Could you check and confirm this with diff ?
Kind regards, Bert Timmerman.

Revision history for this message
dima kogan (pjhhh3o25) wrote :

The new patch is nearly functionally identical to the old patch set, except it has been rebased and thus can cleanly apply to today's HEAD. It also has unit-testing fixes, but these aren't directly tied to the separate g-code files. Thus I wouldn't expect the generated g-code to be significantly different from before (as you're seeing).

I just built the most recent HEAD, and I see the "advanced g-code" button in both gtk and lesstif. The two interfaces also DO produce identical gcode files, as you would expect. Can you confirm that you're NOT seeing an "advanced g-code" button with lesstif and that you're seeing a discrepancy in the generated output?

To be clear, the presense or lack of a button, and the whole "advanced g-code" thing in general has nothing to do with this patch.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi dima,
Sorry, I've misinterpreted the state of the togglebutton in the Lesstif GUI.
I have no in depth knowledge about g-code, and thus can not verify that it actually generates correct code (the ultimate test would be done with a CNC router).
I tried to check on the files with diff and only found some date changes, and notably the "\n" added to split

(Board size: 3.50x3.30 inches)#100=0.002000 (safe Z)

into two lines, which looks good to me.

Kind regards, Bert Timmerman.

BTW: for a nice overlook of all the changes in this patchset have a look at:
https://github.com/bert/pcb/compare/LP846359rev1

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :
Revision history for this message
dima kogan (pjhhh3o25) wrote :

Yeah, that sounds about right. The g-code previews look correct to me, although I admit I haven't made any boards with the new code yet. Do you need anything more from me for these patches?

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Dima,
I think these patches can be reviewed by one of the pcb-devs and committed.
Thanks and kind regards, Bert Timmerman.

Revision history for this message
DJ Delorie (djdelorie) wrote :

I committed these patches. Thanks!

Revision history for this message
DJ Delorie (djdelorie) wrote :

Note: the gcode exporter suffers from the same locale bug the gerber exporter did - you can't just call fprintf("%f") and expect to get '.' as the decimal separator. You have to use g_ascii_formatd("%f") to ascii-format the number into a string buffer first, then use that buffer in your output via "%s"

Try this in the tests directory:

  LC_ALL=cs_CZ.utf8 make check

Revision history for this message
DJ Delorie (djdelorie) wrote :

"make dist" now fails, could you check this?

 (cd hid_gcode1 && make top_distdir=../../../pcb-20111120 distdir=../../../pcb-20111120/tests/golden/hid_gcode1 \
     am__remove_distdir=: am__skip_length_check=: am__skip_mode_fix=: distdir)
make[3]: Entering directory `/home/dj/nightly-geda/pcb-native/tests/golden/hid_gcode1'
make[3]: *** No rule to make target `gcode_oneline.gcode.bottom.cnc', needed by `distdir'. Stop.
make[3]: Leaving directory `/home/dj/nightly-geda/pcb-native/tests/golden/hid_gcode1'

Revision history for this message
dima kogan (pjhhh3o25) wrote :

Here's the fix. The golden test filenames changed (in a different patch series from the one in this bug, actually), so this patch updates the Makefiles accordingly. Also fixing a minor bug where dates were being parsed incorrectly when comparing gcode output during testing.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi,
The two patches seem to work, "make dist" gave no erros over here.
Kind regards, Bert Timmerman.

Revision history for this message
DJ Delorie (djdelorie) wrote :

Thanks, pushed.

Changed in pcb:
status: In Progress → Fix Committed
Changed in pcb:
status: Fix Committed → Fix Released
Changed in pcb:
milestone: next-feature-release → pcb-20140316
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.