kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Sylwester,

Either your patch was removed by the launchpad moderation system or you
forgot to attach it.

Cheers,

Wayne

On 8/18/19 4:11 PM, Sylwester Kocjan wrote:
> 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
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>