PCB: Printed Circuit Board CAD package

ipc-d-356 netlist creation

Reported by Jerome Marchand on 2012-05-08
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
pcb
Undecided
Unassigned

Bug Description

This patch provides IPC-D-356 netlist creation ability to PCB.
The units of the netlist is determined by the units set in the main window when the netlister is run (Metric for mm and Imperial for mils).
My testing was quite limited: LED2.pcb (from the examples) and a test design I created.
So let me know how it work for you...

Bert Timmerman (bert-timmerman) wrote :

Hi,

I converted the above patch to a git-patch for others to play with.

First impressions after patching, building and running pcb with this new feature:

1) I'm happy Jerome took the time and effort to code this long outstanding feature request (IIRC in the Sourceforge era).

2) The IPC-D-356 netlist export functionality is not implemented as a HID-exporter, nor as a plug-in for pcb.

3) Furthermore, when tested with the LED2 example, I can't find a netlist file generated by the exporter anywhere, nor is a file-chooser dialog opened to be able to chose a (different) name for the netlist file.

I poke around some more to see what can be done about 1) and 2).

Kind regards,

Bert Timmerman

Bert Timmerman (bert-timmerman) wrote :

Addendum:

3) In the current state the IPC-D-356 netlist code is GTK UI dependant, and can not be used by the lesstiff UI. The build with the lesstif UI fails with:
<quote>
In file included from ipc-d-356.c:39:
hid/gtk/gui.h:36:21: error: gtk/gtk.h: No such file or directory
In file included from hid/gtk/gui.h:37,
                 from ipc-d-356.c:39:
hid/gtk/ghid-coord-entry.h:26: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘*’ token
In file included from hid/gtk/ghid-main-menu.h:8,
                 from hid/gtk/gui.h:38,
</quote>

Ad 2) If/when the IPC-D-356 netlist export functionality is implemented as a plug-in for pcb the code can be maintained outside the official pcb git repository. If/when the IPC-D-356 netlist exporter functionality is implemented as a HID-exporter this code will have to be added to, and has to be assimilated for, the official pcb git repository.

Kind regards,

Bert Timmerman.

Sorry for the long delay in looking at this.

for the example LED2, an error is generated in the message log window. The error is due to the fact that the Crystal / oscillator part in the example is not named. Give it a name and the netlist should work. (it's the part right of C17).

I might have some free time early next year, and will look at implementing it as a HID-exporter...

Jerome

Bert Timmerman (bert-timmerman) wrote :

Hi Jerome,

Have a look at:

https://github.com/bert/pcb/compare/master...LP996319rev1

Kind regards,

Bert Timmerman.

Bert,

Boy, this is great! I'm not sure what I'll do with that free time now :-)

It did require a few minor changes to compile with me ( see the included patch).

It then works fine (In LED2 example the oscillator still needs to be named though).

Bert Timmerman (bert-timmerman) wrote :

Hi Jerome,

Maybe add some tests to the test suite ?

Will have a look at your patch shortly during the coming holiday weeks.

Kind regards,

Bert Timmerman.

Peter Clifton (pcjc2) wrote :
Download full text (3.5 KiB)

Hi guys,

As you might have noticed, I'm currently somewhat in the process of breaking connectivity scanning code, and our existing find.c implementation is a mess.

This might impact upon this patch slightly, so we should adopt one of two strategies..

1. Assume impact will be minimal, and fix-up the patch later.
2. Merge it to core ASAP, and make it my problem not to break it ;)

I was wondering whether you thought it might be appropriate to refactor out some of the netlist extraction code from this branch into the core. (Where it can be cleaned up along withe the existing stuff).

A few style points...

1. Please send unified diffs, as they are easier to read.
2. Please don't add comments with your name or initials into the code, e.g. //JM // END JM etc... use the git history to track these things.
3. Not a requirement, but I usually mark a file's copyright as
"Copyright (C) $YEAR PCB Contributors (see ChangeLog for details)"

If you wrote the whole file, feel free to add yours as well, but I always like to leave the "PCB Contributors" as a second line, as this encourages people to feel contribution is welcome, and not the sole preserve of the original author / copyright holder.

4. I want to see changes to the core (like the addition of LookupConnectionByPin() committed as a separate commit.
5. PLEASE don't re-introduce type aliases with "Ptr" on the end. These are gone now in PCB, good riddance ;)

IPCD356_AliasPtr and IPCD356_AliasListPtr should become IPCD356_Alias * and IPCD356_AliasListPtr *

6. Looking at the above structures, we have a few problems:

+ char NName[11];
+ char NetName[256];

I believe PCB supports arbitrary length in its net names (and if it doesn't, it should!) - so I'm unhappy to see this fixed length field. I appreciate the limit of 14 characters on the IPC standard, so that is OK,

I'm not sure why you limit the aliased net names to 10 characters (+termination), I thought the standard supported up to 14? Is this due to the format of your alias strings?

7. PCB's core now depends on glib, so feel free to use GLists, hash tables, string functions, whatever makes the code easier. I think IPCD356_AliasList is a candidate for being replaced if you wanted.

8. Lots more [256] limited strings, and string handling without bounds checking in the code. I'm really trying to keep people from introducing code like this on both grounds of: 1. Arbitrary length limitations are bad, and 2. Unsafe string handling which could cause a crash is bad.

9. Refactoring is better than copy+paste alteration... e.g. rather than adding ResetVisitPinsViasAndPads() you could have extended ResetFoundPinsViasAndPads().

As it happens, I ran into some buggy code which needed refactoring recently, and did just that... so your calls now become:

- ResetFoundLinesAndPolygons (false);
- ResetFoundPinsViasAndPads (false);
+ ResetConnections (false, FOUNDFLAG);

(ResetConnections combines a call to ResetFoundLinesAndPolygons and ResetFoundPinsViasAndPads)

And I think (not build-tested) your new function ResetVisitPinsViasAndPads can be removed if you change:

- ResetVisitPinsViasAndPads ();
+ ResetFoundPinsViasAndPads (fals...

Read more...

Peter Clifton (pcjc2) wrote :

And yes.. Bert is right, the test-suite is important for merging to PCB.

Any exporter should be tested as part of PCB's test-suite, and it should be simply a matter making sure this exporter gets run in the test-suite, and adding some "golden" output files for the expected results on our existing test input files.

Look at tests/tests.list

You might want to hand-check the output of the golden files to make sure it is vaguely sane, but for the most part, our test-suite is used to verify we didn't accidentally break something which used to work.

Peter Clifton (pcjc2) wrote :

Regarding visualising test-results, perhaps you could look at the gerbv project, and see whether there is any scope (or desire) to add support for importing and overlaying IPC-D-356 data there.

Hi Peter,

<quote>
This might impact upon this patch slightly, so we should adopt one of two strategies..

1. Assume impact will be minimal, and fix-up the patch later.
2. Merge it to core ASAP, and make it my problem not to break it ;)
</quote>

I will look into this stuff later on in the coming week.

I'll go for option #1, for I think merging is best done when tests are in place.

Kind regards,

Bert Timmerman.

BTW: Merging can be speeded up by popular vote: hit the "Does this bug affect you ?" at the top of the page. All votes count !

Peter Clifton (pcjc2) wrote :

"BTW: Merging can be speeded up by popular vote: hit the "Does this bug affect you ?" at the top of the page. All votes count !"

I'm not sure I agree....

IMHO, the best way to improve the speed of merging is to increase the code quality ;)

Peter Clifton (pcjc2) wrote :

Oh, and I renamed the find.c APIs, so you will have to substitute:

ResetFoundLinesAndPolygons -> ClearFlagOnLinesAndPolygons
ResetFoundPinsViasAndPads -> ClearFlagOnPinsViasAndPads
ResetConnections -> ClearFlagOnAllObjects

Hopefully those names are somewhat less confusing than our previous ones.

Hi Peter,

I have implemented most of the changes you mentioned, and I'm now working on the test.

To answer you question on #6: The IPC spec requires Aliases to be composed of: "NNAME" + 5 chars, so with the termination we get Nname[11].

Your question got me to look back at the spec and discover that net names can't have spaces in them. So I added code to replace spaces with underscores in the netlist.

I will post the patch when I'm done with the test.

Jerome

Hi Peter, Bert,

Here's the patch. This includes the test-suite.
Note that I ran this patch from a clean tree so the previous patch (from 2012-19-12) is included in this one.

Jerome

I had some time, so I got the IPC-d-356 netlister to compile / work with the tree in the git repository...

Jerome

Hi Jerome,

Due to a vacation with limited internet access it took some time to test your patch.

I converted your patch to a git-patch and stashed it in my pcb devel repository at https://github.com/bert/pcb/tree/LP996319rev2.

The patch applied, the pcb build (gtk UI) is OK and the IPC-D-356 exporter passed the test script.

What I can't find is the generated netlist (LED2.ipc) when I try to export the LED2.pcb in the example directory.

Kind regards,

Bert Timmerman.

Bert,

I think it's probably the same issue as before (See Comment #4 above). The netlister outputs the error in the message log window.
Maybe it needs to popup a window instead?

Jerome

Changed in pcb:
status: New → In Progress
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers