kicad-developers team mailing list archive

[Sylwester Kocjan <s.kocjan@xxxxx>]

Hi Wayne & KiCad devs,   I made some progress with simulation changes (see the attached patch). I fixed your findings (hopefully this is what you intended)  and made further changes. Currently it is possible to set initial condition for capacitors and inductors in DIALOG_SPICE_MODEL.    At the same time I found a bug, I think fixing would require some architectural changes:  1. User adds initial condition to the element -> additional field "Spice_IC" is saved in component.   2. User opens SPICE options for component and removes initial condition.   3. In DIALOG_SPICE_MODEL::TransferDa IC field from instance of DIALOG_EDIT_COMPONENT_IN_SCHEM will be cleared.   4. DIALOG_EDIT_COMPONENT_IN_SCHEM wants to remove a row from wxGrid based on different number of  elements in DIALOG_EDIT_COMPONENT_IN_SCHEM  5. Before this happen, grid wants to be updated and calls FIELDS_GRID_TABLE<T>::GetValue for a row that is going to be deleted -> wxCHECK( aRow < GetNumberRows(), wxEmptyString ) fails.   This issue is reproducible in 100% cases on my machine with Linux and I think that something similar can occur with DIALOG_EDIT_COMPONENT_IN_LIBRA    Best regards,  Sylwester





        Dnia 27 czerwca 2019 21:04 Wayne Stambaugh <stambaughw@xxxxxxxxx> napisał(a):



         Hi Sylwester,

 I took a look at your repo and I have a few comments on your changes.  I
 did not test it so there may be functional issues as well.

 Please simplify your logic in
 DIALOG_SIM_SETTINGS::TransferD  It's way more complicated
 than it needs to be.

 Get rid of DIALOG_SIM_SETTINGS::disableCt and use the
 appropriate wxUpdateUIEvent[1] handler to enable and/or disable any
 controls.  Manually doing this will almost certainly lead to broken
 control states.  We have done this poorly so many times in the past that
 I am not allowing it in new code.

 If you want to submit your changes for consideration, you should squash
 you changes into a single patch and submit the output of `git
 format-patch` to the mailing list.

 Cheers,

 Wayne

 [1]:
 docs.wxwidgets.org docs.wxwidgets.org

 On 6/21/19 3:00 PM, Sylwester Kocjan wrote:

 Hi,

 I prepared some changes in KiCad simulation dialog. They are about
 additional simulation parameters. According to tutorial, which I've
 found on website, I store them on GitHub repository in branch
 "Sim_InitialConditions_SK" here:

 github.com github.com

 Could you please have a look and do some review if possible? I'd be
 grateful for feedback if these changes are OK. In future I'd like to
 implement OP analysis using some controls I've added
 ( bugs.launchpad.net bugs.launchpad.net

 Additionally I also prepared polish translation for new strings.

 My aim was to make simulation with arbitrary initial conditions
 possible (additionally I added other options). Right now it is possible
 to enable checkbox UIC on TRAN tab, but there is no possibility to set
 IC to any arbitrary value in element properties. I'm afraid it will
 involve change in .sch format. Please take note that it is also
 possible to define initial conditions for entire nodes instead of
 capacitors or inductors (for example: ".ic v(11)=5 v(4)=-5 v(2)=2.2",
 see ngspice manual, chapter 15.2.2). It is also a challenge.

 So in current state we can use for initial conditions results of .OP
 analysis or default zero.

 From my point of view these are the topics, which should be taken into
 account during review:
 - in simulation code sometimes there is used '\n', sometimes "\r\n".
 Maybe it should be unified? Is there a common definition for newline in
 wxWidgets?
 - I added .option savecurrents: someone added TODO that it doesn't
 work. Maybe should we hide this control?
 - something happened to colours on dialog windows, see attached image.
 I don't know what is it and how to fix it. seems to be unrelated to my
 changes, but maybe I'm wrong? (this is visible on Ubuntu 18.04)
 - some feedback regarding how to add IC field to capacitors, inductors
 or nodes will be very appreciated ;)

 Limitations of my changes:
 - parsing SPICE .options is not implemented (in bool
 DIALOG_SIM_SETTINGS::parseComm const wxString& aCommand ))

 Best regards,
 Sylwek


 ______________________________
 Mailing list:  launchpad.net launchpad.net
 Post to     :   kicad-developers@lists.launchp
 Unsubscribe :  launchpad.net launchpad.net
 More help   :  help.launchpad.net help.launchpad.net



 ______________________________
 Mailing list:  launchpad.net launchpad.net
 Post to     :   kicad-developers@lists.launchp
 Unsubscribe :  launchpad.net launchpad.net
 More help   :  help.launchpad.net help.launchpad.net