Refactor SpiceLibComp dialog #1084
Conversation
|
@ra3xdh: qucs-s crashes (with/without this PR) if
Are these known issues? |
The issue title is a bit misleading. This dialog is not related to netlister. Please rename as something like Refactor SpiceLibComp dialog
I cannot reproduce the crash with my test schematics. Does it crash with every SPICE file or only with specific File/Symbol combination?
Again cannot reproduce the crash. The schematic opens with square placeholders, because it cannot find the LIB file. After I specifiy the Symbol from template, there is no crash. It doesn't allow to assign pins and press OK, because there is no symbol having 8 pins. The LT1057 device also opens dialog on double click and allows to reassign the symbol after fixing the LIB file path. |
ThomasZecha
changed the title
Renamed.
Crashes with every schematic including a spice-lib via <SpLib ..., see callstack:
Just open ./examples/xyce/Xyce_Examples/09-Lorenz/lorenz.sch and than double-click any lib-symbol, see callstack:
|
No crash using this file on my machine. Are you building the application with Qt5? I am using Qt6 for build and test. The Qt5 support will be deprecated since the next release #938 The build of DEB/RPM packages has been switched to Qt6. |
I'm using 5.15.8, wonder that this shall be the cause. |
|
But anyway, I will try it under 6. |
|
Try if the application still be crashing using Qt6. Rebuild using |
|
Yes, it crashes only with Qt5. I can reproduce it. |
|
I can reproduce both crashes under Qt6. |
|
What is your Qt6 version? I am using 6.7.2 |
|
I am using 6.4.2 |
|
Root cause for both cases is line 357 in spicelibcompdialog.cpp: QString file = dir_name + listSymPattern->currentItem()->text() + ".sym";Depending on your local configuration listSymPattern could be empty. In that case currentItem() delivers nullptr. |
|
The key difference was that the application was not installed when I tested Qt5 build. The Qucs-S must be installed while debugging. Otherwise it doesn't find the libraries etc. I would advise you to add install step in your test procedure. The QtCreator IDE allows to perform it. But nevertheless the crash should be fixed. I will prepare a PR containing the fix. |
Reviewed changes from ra3xdh#1049 SpiceLibCompDialog has been reworked to achieve the code quality improvements introduced by ra3xdh#1049. Tested the following dialogs under extsimkernel/: -ExternSimDialog -CustomSimDialog -SimSettingsDialog -SpiceLibCompDialog Rebased.
|
Rebased. |
|
@ra3xdh: Any reason why not merge this improvement? |
I would like to make some corner cases testing of the modified dialogs before merging. I missed this point while accepting #1049 I am short in time this week, because maintaining the project is not my primary occupation. So, expect some delay. All dialogs modified in this PR and in the #1049 are not involved in the netlisting procedure. So, you can proceed with implementing CDL netlister without waiting to merged this. Furthermore the ´externsimdialog.cpp´ is a candidate for remove (See #235). I would not advise you to modify the dialogs in order to implement the CDL netlister. You have to inherit the |
|
@ra3xdh: Is it possible to get a cdl-netlist feature branch where direct push would be possible? |
|
Merged. No issues found with this version. Please avoid modifying the dialogs when implementing the CDL netlister. You need only to add a menu entry or maybe a custom dialog to set the CDL parameters. See how the Verilog-A export is implementing. It is very similar to the planned CDL extension (netlister without simulator). |
Reviewed changes from #1049
SpiceLibCompDialog has been reworked to achieve the code quality improvements introduced by #1049.
Tested the following dialogs under extsimkernel/:
-ExternSimDialog
-CustomSimDialog
-SimSettingsDialog
-SpiceLibCompDialog