Fixed issue #591

[ra3xdh]

I have added quick fix for #591. Library name is checked is it relative (system libraries) or absolute (user libraries). It will allow to process user libraries in SymbolWidget::setSymbol() method. Otherwise we always have an error when trying to process user library which is defined by absolute path.

[in3otd]

it may be the same in the end, since I don't think it's safe to have user libraries with the same name as system libraries, but you could do as in libcomp.cpp, so something like

QDir Directory(QucsSettings.LibDir);
QString libpath = Directory.absoluteFilePath(Lib_ + ".lib");

It will then be even clearer that relative library paths are for system libraries while the other libraries have absolute paths.

Besides, the search is not yet working correctly: searching for "741" and selecting the IRF7410 I get an error similar to #591.
The line here should likely be if (i < UserLibCount) but I didn't test much.
Still I have the impression that the current code is quite fragile.

[ra3xdh]

@in3otd , I accepted your suggestion with QDir usage. I have modified my fix. It seems error massages have gone. Please check all again. I also added fix for empty symbols in some libraries (Diodes.lib, Transistors.lib, etc.). See #591 (comment) for details.

[guitorri]

There is something else going on. I did the following:
Copy MOSFETs.lib library into $HOME/.qucs/user_lib

If I try to select the MOSFETSs from the system library section, the one from the user_lib section ends up selected. It searches for the first match of the library name. It is confusing, the combo-box is assigning with the file name (minus suffix) while there is also a library name defined inside the .lib file.

Second issue. If one tries to use the search field. I may error out again, as it first tries to resolve symbols into the user lib path. I searched for BSS123, when selected one of them (from system NMOSFETs.lib) it went looking for it in user_lib.

Somehow it should keep track of the full path of the libraries. The Qucs library dock does that and works just fine, even with system and user libs with the same name.

[in3otd]

yep, this I what I saw also and was hinting at above...

Are you sure that the Qucs library dock works fine for user and system libs with the same name? Here if I select a component from these two libraries it points always to the user lib.

[in3otd]

BTW, a solution could be that every item in CompList contains also the information about the library where the component comes from, similarly to what done here, adding QListWidgetItem() to the CompList and storing the library path/info using QListWidgetItem::setData().
This will be reused also if/when adding the "instant search" to the Qucs library dock...

[ra3xdh]

I used the same name for user libraries and system libraries only to demonstrate error messages introduced by recent addition in qucslib. You can also use every library that has DefaultSymbol XML section for this purpose. Current code gives an error for every user library that uses DefaultSymbol. There is no matter how it is named. But copying of system NMOSFETs.lib is the easiest way to demonstrate these error messages.

If it's hard to add the proper search implementation for system and user libraries with the same name, we can simply forbid to have the same system and user libraries. As alternative, we can introduce library priority. User library will override system library, and system library become invisible in the library manager. If search issues concerns only system and user libraries with the same name, we can also leave all as is. The same name of system and user libraries is rare artificial corner case.

[in3otd]

another library issue is that "base" components (i.e. not defined by a subcircuit) do not get a valid component model once placed in the schematic using the GUI; to see this

• in the Qucs GUI, select the Libraries dock
• open the NMOSFETs system library
• drag the first component (2N3796) in a schematic
• try to run a simulation with this and see that it fails due to a wrong component model
• alternatively, copy the component just placed and paste it to a text document: you'll see that its definition is not correct for a MOSFET.
• or place the same component using the Library Tool and compare the model

...I'm working on this (and other semi-related stuff)...

[in3otd]

I have just pushed a couple of commits directly to the ra3xdh branch, seems to work 😁
These commits solve the wrong model issue for base components mentioned above and moves/simplifies things a little to keep the symbol-handling stuff in SymbolWidget() instead of having part of it in QucsLib()

I had also started to fix the local/system libraries path confusion but I feel it would take a good rewrite to do all properly. If you think that it would be better to have it fixed for the 0.0.19 release I can try to hack something together (but qucslib seems already hacked together enough).

[guitorri]

I also think it qucslib could benefit from redesign.
Can we just patch the libraries path issue to make it usable for the release?
For a fix, I was thinking of adding a list (containing full paths to libs) initialized in parallel with the combobox. Then use the combobox to as index to fetch the full path form the list. I don't know how that would fit with the search function.

For the future, searching on startup for *.lib is convenient but I think there should be a configuration file to store the included libs, the library manager could be used to add/remove libs (or by hand on the config file).

[in3otd]

I also think it qucslib could benefit from redesign

yep, all the component library stuff actually... e.g. note that some functions are reused in both qucslib and qucs, like parseComponentLibrary(), while for others we have separate functions in the two applications doing almost the same thing, like SymbolWidget::theModel() and makeModelString(), defined in qucslib_common.h (...) .

For a fix, I was thinking of adding a list (containing full paths to libs) initialized in parallel with the combobox

I started doing a similar thing but adding the path to the userData passed to addItem(), as mentioned above. Seemed slightly nicer but I'm no longer so sure 😁

[ra3xdh]

@in3otd , Your code works as expected. I have tested different combinations. I am surprised that Github allows mainline project members push to forks. I didn't know about this feature.

I think we should move QucsLib redesign to 0.0.20 ore even the next releases. It we start it right now , it will further delay release, probably again for a months. But error messages in QucsLib should be fixed in upcoming release, because it can make QucsLib usage impossible. As alternative, we can revert old code before #556 and #580 merging, make release, and redesign QucsLib later.

[in3otd]

thanks for testing the code. Guitorri mentioned that pushing to team members' forks by other team members can be enabled when opening a PR, so I just tried to see if it worked on your PR. I understood this needs to be enabled explicitly, though, and is not active by default ?

I agree that the QucsLib redesign is not a priority now and maybe not even for the next release; I'm working to just include the local vs system directory naming check without touching anything else, hope to have it ready "soon"

[ra3xdh]

pushing to team members' forks by other team members can be enabled when opening a PR

Yes, I now understood how it works.

[guitorri]

The #580 (merged) is a cleanup from #556 (not merged).

Reverting #580 fix this issue and reopen another one.

@ra3xdh, yes the idea is to do a release ASAP. qucslib redesign/removal can wait for later.

I also recently discovered these two things about PRs which are quite handy:

• change of base branch
• allow edits from maintainers (checked by default for maintainers?)
  Both can be changed on the edit button on the title of the PR.

[guitorri]

@in3otd any news? No pressure, just checking otherwise I will give it a try.

[in3otd]

yep, about to push some commits but saw other issues, will explain later 😁

[in3otd]

the latest commits fix the user vs system directory issues, now the components models are taken from the right libraries even when there are system and user libraries with the same name or file name.
In the libraries selection combobox of the Components Library tool the library name as defined in the library header is shown, while the library path is used to retrieve the component model from the right library.
So far, seems to work here but please double check.

While testing I have noticed another issue:

• copy a system library which contains components defined as subcircuits to user_lib, as NMOSFETs.lib
• place one component defined as subcircuit (e.g. 2N7000)
• try to run a simulation
• an error checker error, found 2 definitions of 'Def:NMOSFETs_2N7000' occurs

this is because the name of the subcircuits are hardcoded in the libraries and simply copied in the netlist, so if the components use the same name for their subcircuits there are two conflicting definitions.
This error of course does not occur with components not defined as subcircuits, whose definition is simply copied from the library (e.g. the 2N3796 in NMOSFETs.lib)

I think that a solution could be to prepend user_lib/ to the subcircuits names coming from user libraries.
I won't have time to try to fix this also in the near future, though.

[guitorri]

Hum. If I try what you said, copy and paste the component:
User lib

<Qucs Schematic 0.0.19>
<Components>
  <Lib T 1 0 0 8 -26 0 0 "/Users/guitorri/qucs_area/_demo/user_lib/NMOSFETs" 0 "2N7000" 0>
</Components>

System lib

<Qucs Schematic 0.0.19>
<Components>
  <Lib T 1 0 0 8 -26 0 0 "NMOSFETs" 0 "2N7000" 0>
</Components>

Which seems right, I haven't tried running the simulation yet. I think the netlist cannot handle the same component name from 2 different libs...

[guitorri]

Yep. The name collision is another problem. The fix belongs either to the .lib files of needs to be solved by the netlister (or both).

I am testing you code and it looks good so far.

[in3otd]

I think that problem should be handled by the netlister, as said above the subcircuits coming from the user lib could be defined as
.Def:user_lib/NMOSFETs_2N7000 _net4 _net3 _net5
instead of simply
.Def:NMOSFETs_2N7000 _net4 _net3 _net5, which would be used only for system libs. qucsator might need to be modified, not sure it handles slashes in the subcircuit name properly.

[felix-salfelder]

what do other programs do?

what does python do about import foo, if foo is available locally and on the system? what does cpp do, if you do #include "file.h" and there are multiple file.h on your harddisk? do you need to change the input file? (no).

qucs (all parts of it) should behave in a similar non-surprising and efficient way. mangling paths with symbol names in a netlist is not one of them. replacing one implementation for another must not require a netlist edit.

qucsator might need to be modified.

if qucsator does the include (i.e. the fopen), then yes. the search for the file must be implemented in qucsator.

possible solution: pass -I to qucsator, to tell it where to look for files. use different names for different components.

[guitorri]

Shall we open another issue and keep things into context?
The issues are becoming more entangled than needed.

[in3otd]

yep, I saw you just did it 😁. Seems better to have a dedicated issue for that.

[guitorri]

The issue with symbols and wrong paths to libs seems fixed. Thank you. Merging.