Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite gnetlist in scheme #139

Merged
merged 59 commits into from
Jul 11, 2017
Merged

Conversation

vzh
Copy link
Member

@vzh vzh commented Jun 26, 2017

This branch finishes rewriting of gnetlist in Scheme.

This fixes part of #142, first found attached net= for a pin overrides its net name, or first inherited net= if no attached one found, though there is no user visible warning yet.

This also closes #144, and closes #145.

@graahnul-grom
Copy link
Member

@vzh Build, make distcheck - all OK.
Tried backends: spice-sdb, pcbpins, drc2 - work as expected, exit with 0, however there are warnings and one error:

WARNING **: Failed to load config from "/usr/local/etc/xdg/gEDA/geda-system.conf": Error opening file /usr/local/etc/xdg/gEDA/geda-system.conf: No such file or directory

WARNING **: Failed to load config from "/home/dmn/lepton.tests/try_vzh_netlist/geda.conf": Error opening file /home/dmn/lepton.tests/try_vzh_netlist/geda.conf: No such file or directory

CRITICAL **: geda_text_object_calculate_bounds: assertion 'toplevel->rendered_text_bounds_func != NULL' failed

CRITICAL **: geda_text_object_calculate_bounds: assertion 'toplevel->rendered_text_bounds_func != NULL' failed

CRITICAL **: geda_text_object_calculate_bounds: assertion 'toplevel->rendered_text_bounds_func != NULL' failed

CRITICAL **: geda_text_object_calculate_bounds: assertion 'toplevel->rendered_text_bounds_func != NULL' failed

CRITICAL **: geda_text_object_calculate_bounds: assertion 'toplevel->rendered_text_bounds_func != NULL' failed

CRITICAL **: Could not find refdes on component and could not find any special attributes!

CRITICAL **: Could not find refdes on component and could not find any special attributes!

;;; WARNING: compilation of /home/dmn/lepton/bin.vzh/bin/lepton-netlist failed:
;;; ERROR: no code for module (gnetlist option)

@vzh
Copy link
Member Author

vzh commented Jun 28, 2017

@graahnul-grom Thanks for testing and feedback!
While travis-ci checks have failed (due to new Ubuntu containers, I think) you can check out my new fixes and check how it works now.
One note: two first warnings are intentional IIUC, and come from netlist's config.scm; other errors were bugs, large and small. In two words, four last commits will show you what was wrong.

vzh added 28 commits June 28, 2017 16:04
The new test replaces netattrib-geda.out which is the same as
netattrib-config_net_naming_priority_net.out since by default net=
attribute priority is set.
This commit fixes several net= renaming issues visible in test
output files for netattrib.sch:

- Renaming: out3 -> signal3, out8 -> signal 8 due to
  netname-attribute-priority set to #f (net= attribute is
  preferred).

- Renaming: signal12x -> signal12 due to using of the first
  corresponding pin found in the first net= attribute of a symbol.

- Renaming: unnamed_net2 -> unnamed_net4 due to the fact that all
  nets directly connected together without netname= on any segment
  are considered to be unnamed before renaming with net=.

- The same as above for unconnected pins (see 'spice-sdb' and
  'spice' backend output changes).

- Fixed duplication of some nets if they are named with two
  net= attributes on different symbols (see signal6 and signal9).

- Renamining of unconnected pins and unnamed nets is now visible
  in the output of 'geda' backend.
This commit removes parsing of the uref= attribute, which has been
deprecated for a long time.
bottom.sym has no pinlabel= on one pin.
bottom.sch has no refdes= on one port.
This attribute has been deprecated for a long time, so there is no
point to keep it as it is these days.
Structure `CPINLIST' has been changed so it contains now field
`hierarchy_tag' and doesn't contain `nets'.

Structures `NET' and `st_net' have been removed as well as C
functions dealing with them (s_net.c).

Hash tables are not used to track visited objects any more since
their using increases the time of passing tests.
Before, connection refdes and pinnumber were transformed into one
string and then were split again to obtain corresponding values and
check if they do or do not belong to net= attribute. This magic
has been eliminated and now 'net-power symbol is used for the same
task. This avoids special checks for using `%pin-net-prefix' as a
refdes value.
There is no need to make connection pairs package-pinnumber any
more, so it is now possible to create <pin-net> records "on the
fly". The logic (inherited from C code) has been simplified and
some procedures were removed.
Working in the top test directory instead of specialized one leads
to the situation that processing of rc-files supposed to be
processed is missing.
vzh added 15 commits June 28, 2017 16:04
This procedure is needed to move gnetlist logging initialisation
to Scheme code.
This commit also fixes several backends by moving `use-modules'
for required modules into them, thus avoiding re-exporting several
procedures in the (gnetlist) module.
-L option is still tested in `hierarchy-postload' test.
When no backend name was specified on command line, lepton-netlist
did not work even if the interactive mode was set.
This is probably a bug in Guile-2.13. The option `--list-backends'
did not work since instead of stock `sort' function Guile used
same named `(gnetlist sort)' module's #<directory> structure. This
has been fixed by using another function for sorting.
This makes the output more consistent and allows to easily find
the culprit symbol in schematics.
This ensures backward compatibility with `gnetlist' that used
liblepton's function f_open_flags() which parses "gafrc" in every
schematic file's directory.
…able.

Some programs, e.g. `lepton-netlist', don't need to render
placeholders, so this step is just omitted to avoid critical
assertion failures in geda_text_object_calculate_bounds() when
toplevel rendering function is not set.
@vzh vzh force-pushed the rewrite-gnetlist-in-scheme branch from d5a3053 to 19e5fb0 Compare June 28, 2017 13:05
@vzh
Copy link
Member Author

vzh commented Jun 28, 2017

Temporary fixed CI and rebased the branch. It works now.

@graahnul-grom
Copy link
Member

graahnul-grom commented Jun 29, 2017

It works!
By the way, there is no gnetlist symlink in bin/ after installation.
Discovered that on gsch2pcb run. One has to export GNETLIST=/path/to/lepton/bin/lepton-netlist to make it work, because "gnetlist" is hard-coded in gsch2pcb.

@vzh
Copy link
Member Author

vzh commented Jun 29, 2017

Indeed, I've missed that. I was sure I had the compatibility symlink installed. At least, in one of my local branch :-)
The issue with gsch2pcb is also a good point. I think I'll fix it in some next iteration. (Don't you want to do this yourself?) Too many things need TLC at this renaming stage...
Thank you for your support!

@graahnul-grom
Copy link
Member

As far as I can tell, the new gnetlist works fine.
gsch2pcb: will it be sufficient if I simply replace a "gnetlist" string
with "lepton-netlist"?
And about gnetlist backends: is it possible (and worth it) to place them
in separate folder (e.g. share/gEDA/scheme/gnetlist/backends) just to make
scheme folder look more organized? What do you think?

@vzh
Copy link
Member Author

vzh commented Jul 2, 2017

@graahnul-grom Thanks!

gsch2pcb: will it be sufficient if I simply replace a "gnetlist" string
with "lepton-netlist"?

You can try it ;-)

And about gnetlist backends: is it possible (and worth it) to place them
in separate folder (e.g. share/gEDA/scheme/gnetlist/backends) just to make
scheme folder look more organized? What do you think?

I think so. Moreover, I would offer the structure as follows:

  • rename gnetlist to netlist
  • make backend directory
  • rename gnet-allegro.scm to allegro.scm and make a module of it, the same for other backends.

So we would get a structure with modules named, say, (netlist backend allegro).
Of course, we have to support legacy way of backend loading for some time to not break third-party backends.

@vzh vzh mentioned this pull request Jul 4, 2017
@graahnul-grom
Copy link
Member

@vzh Please take a look at this: https://github.com/graahnul-grom/lepton-eda/commits/tb_016_gsch2pcb_hcod
What about renaming gsch2pcb exe to something like lepton-2pcb?
And environment variable GENTLIST to LEPTON_NETLIST? Or do we have to retain its name for backward compatibility?

@vzh
Copy link
Member Author

vzh commented Jul 10, 2017

@graahnul-grom I think lepton-sch2pcb would be better reflect the destination of the program.
I believe there is no sense to support backward compatibility for env vars.
As for your patches, I have a couple of notes:

  • Please make whitespace changes in separate commits.
  • Please make long options more meaningful, i.e. use, say, --backend-cmd instead of --be-cmd

So, IIUC, you want to use some custom backends to work with PCB, right?

@vzh vzh merged commit be8f982 into lepton-eda:master Jul 11, 2017
@vzh
Copy link
Member Author

vzh commented Jul 11, 2017

@graahnul-grom And please do a PR with your changes since I've merged this PR.

@vzh vzh added this to the v1.9.4 milestone Apr 22, 2018
@vzh vzh deleted the rewrite-gnetlist-in-scheme branch November 5, 2018 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants