kicad-developers team mailing list archive

[Seth Hillbrand <seth@xxxxxxxxxxxxx>]

On 2019-06-21 15:00, 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:
>
> https://github.com/skocjan/kicad_initialconditions.git
>
> 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
> (https://bugs.launchpad.net/kicad/+bug/1821360).
>
> 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::parseCommand( const wxString& aCommand ))
>
> Best regards,
> Sylwek

Hi Sylwek-

I've had a look through the changes and they generally look good.  There 
are a few minor formatting issues that you'll want to address such as 
tabs being used instead of spaces and two blank lines between functions 
in .cpp files.

- It looks like retval in DIALOG_SIM_SETTINGS::TransferDataToWindow() 
isn't getting initialized properly.
- Are the "," replacements in DIALOG_SIM_SETTINGS::updateNetlistOpts() 
there to provide support for alternate locales?

To your questions:
- In KiCad's codebase, \n is standard.  But we don't really worry about 
the generated files or formbuilder files.  I'll write up a 
.gitattributes file to control this so that its transparent in the 
future.
- The color is, I think, a function of your GTK scheme.  On my machine 
the tab panels are white and not grey.  This is true not just for your 
changes.
- The IC field could be added as a key-value parameter in the existing 
format.  Something like SPICE_IC and then pairs of values "[1 0.5]; [2 
0]" where the first element in each pair is the node number and the 
second is the IC.

Thank you very much for taking the time to work on improving this area 
of KiCad!  It will be greatly appreciated by many of our users.

Best-
Seth