pcb

[new feature] TinyCAD Import

Bug #1665862 reported by Milan Prochac
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pcb
Fix Released
Wishlist
Bert Timmerman

Bug Description

Attached patch implements new actions to import netlist and parts from TinyCAD:

ImportTinyCAD()
ImportTinyCAD(netlistfile)

Both actions take the netlist (first one displays dialog), import the netlist and CSV partlist. Name of partlist is derived from netlist name: .csv extension is used instead of .net

First action is bound to menu (Import Schematics -> TinyCAD)

In TinyCAD:
- specify footprint name in Package parameter (exist on most symbols) or in Footprint parameter
- export gEDA copatible netlist
- export CSV partlist

In PCB:
- use the above actions or select menu item to import netlist and partlist in one step.
  * netlist is loaded
  * existing parts are updated or let untouched
  * new parts are added
  * removed parts are selected, so it is easy to delete them

TinyCAD import is wrapper around existing actions: Load(netlist), LoadFrom(netlist, ...) and ElementList(Start|Need|Done,...) and it is as good as implementation of these actions.

Milan

Revision history for this message
Milan Prochac (milan-x) wrote :
Revision history for this message
Milan Prochac (milan-x) wrote :

For those who do not want to patch/build the PCB offline method of TinyCAD import is available.

Attached python script converts TinyCAD CSV partlist to list of PCB commands (sequence of ElementList commands)

In PCB:
- manually import netlist
- run commands from file

Behaviour is identical as with new actions.

Milan

Revision history for this message
Peter Clifton (pcjc2) wrote :

Quick review...

static bool netlist_loaded; should ideally not be added to action.c

Can we ignore if the netlist load failed?
If not, ideally we need a mechanism with API to check this without adding global state to a specific source file.

This is primarily coming from the thought that "at some point we should stop adding all new actions to "action.c", it is getting a little large!". This change as stand ties the new action to living in action.c, due to relying on that file-scope global variable being set here.

(Not a criticism of this patch at all, but longer term, such variables should live in the PCB structure, so it could in theory be possible to load more than one at once.).

Revision history for this message
Peter Clifton (pcjc2) wrote :

I noticed you use strcasecmp...

Unless the tiny cad file format is defined to be case insensitive, I think strcmp would be better (matching whatever they actually output).

Revision history for this message
Milan Prochac (milan-x) wrote : Re: [Bug 1665862] Re: [new feature] TinyCAD Import

On 18.02.2017 11:49, Peter Clifton wrote:
> Quick review...
>
> static bool netlist_loaded; should ideally not be added to action.c
>
> Can we ignore if the netlist load failed?
> If not, ideally we need a mechanism with API to check this without adding global state to a specific source file.
>
> This is primarily coming from the thought that "at some point we should
> stop adding all new actions to "action.c", it is getting a little
> large!". This change as stand ties the new action to living in action.c,
> due to relying on that file-scope global variable being set here.

The variable does not indicate if the netlist is loaded (well, it can
have better name), it's purpose to indicate that user cancelled dialog.

There is no way how to transfer the information - Load(netlist) which is
part of HID common functions calls HID function to display dialog and -
if file is selected and confirmed - calls LoadFrom(netlist,... ) actions
which loads loads anything and always succeeds.

The variable indicates, that whole process (started by Load() function)
with all branching reached the point, where the netlist is loaded.
Therefore the variable is set/reset asymmetrically. It's just kind of
"Kilroy was here!" or hair in drawer.

Loaded netlist is IMO indicated by non-null PCB->Netlistname, we do not
need to duplicate it.

As I wrote - TinyCAD import is as good as underlying actions.

Did you try to load Anna Karenin in PDF as netlist? Try it - for example
this one: http://www.bartleby.com/ebook/adobe/316.pdf

I think that one global variable is very tiny problem.

Milan

> (Not a criticism of this patch at all, but longer term, such variables
> should live in the PCB structure, so it could in theory be possible to
> load more than one at once.).
>

Revision history for this message
Milan Prochac (milan-x) wrote :

On 18.02.2017 11:55, Peter Clifton wrote:
> I noticed you use strcasecmp...
>
> Unless the tiny cad file format is defined to be case insensitive, I
> think strcmp would be better (matching whatever they actually output).

It is case insensitive. If I use footprint and Footprint - they are
handled as one column. In exported file the first occurrence is used.

Milan

Changed in pcb:
status: New → Triaged
importance: Undecided → Wishlist
Changed in pcb:
milestone: none → pcb-4.1.0
tags: added: 3rd-party-import
Changed in pcb:
milestone: pcb-4.1.0 → pcb-4.1.1
Changed in pcb:
milestone: pcb-4.1.1 → pcb-4.2.0
Changed in pcb:
milestone: pcb-4.2.0 → future-feature-release
Changed in pcb:
assignee: nobody → Bert Timmerman (bert-timmerman)
status: Triaged → In Progress
milestone: future-feature-release → pcb-4.2.1
Changed in pcb:
status: In Progress → Fix Committed
Changed in pcb:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Bug attachments

Remote bug watches

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