kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

Tom,

On 4/5/19 12:41 PM, Wayne Stambaugh wrote:
> On 4/5/19 12:24 PM, Tomasz Wlostowski wrote:
>> On 05/04/2019 18:04, Wayne Stambaugh wrote:
>>> Tom,
>>>
>>> On 4/4/19 8:16 PM, Tomasz Wlostowski wrote:
>>>> Hi,
>>>>
>>>> We needed to do some signal/power integrity simulations on one of our
>>>> Kicad designs and in order to do that, we needed to convert a Kicad PCB
>>>> to Hyperlynx format. Luckily, the format is simple, all text and well
>>>> documented in [1], so here comes a patch that adds a Hyperlynx exporter.
>>>>
>>>> Notes:
>>>> - since Kicad doesn't have a concept of board stackup (permittivities,
>>>> loss tangent, dielectric types, etc.), the exporter writes a dummy
>>>> stackup. Edit it to match the PCB spec in Hyperlynx.
>>>> - no support for offset pad holes, slotted pad holes,
>>>> trapezoid/polygonal pads (it seems HL format doesn't support such
>>>> features or I need to figure out how to emulate them).
>>>> - no support for thermal pads (yet)
>>>> - no error reporting.
>>>>
>>>> Looking forward to your feedback & wish you happy testing,
>>>> Tom
>>>>
>>>> [1] http://www.ibis.org/birds/bird33.txt
>>>
>>> Your patch built and tested without issue.  I just have a few minor
>>> comments:
>>>
>>> Please remove all commented out debugging output code and one instance
>>> of wxLogDebug unless you are planning to some additional debugging in
>>> the future.  In which case, use wxLogTrace.
>>>
>>> Per section 4.2 in the coding policy[1], in source files there should be
>>> 2 blank lines between functions except when they are in the class
>>> definition in which case there should be 1 blank line.  I also saw a
>>> couple of if{} statements with missing blank lines above.
>>>
>>> It is no longer necessary to wrap strings with the wxT() macro when
>>> using the wxString assignment operator.
>>>
>>> The OUPUTFORMATTER::Print function can throw an IO_ERROR exception.  If
>>> you don't handle this, KiCad will most likely crash when it occurs.  It
>>> would be a good idea to add a try/catch block in
>>> HYPERLYNX_EXPORTER::Run() and return false when a exception is caught.
>>>
>>> The copyright dates in the qa files are 2018.
>>
>> Thanks Wayne,
>>
>> It looks my settings for VSCode formatter don't catch all Kicad Coding
>> Style rules. Need to fix that ;-). I'll push a version with fixes
>> (including error handling via exceptions).
>>
>> Tom
>>
>> PS. We need to think about factoring out the exporters (including their
>> private settings dialogs) to some sort of plugins...
>>
> 
> A plugin architecture for board exporters is a great idea.  This would
> be an ideal candidate for a true external plugin architecture similar to
> the 3D model plugin.  Maybe there is code that can be factored out into
> a base external plugin object that can be reused for other plugins.
> 
> Wayne
> 

One other thing I noticed is it looks like the VSCode formatter is
wrapping at 100.  The coding policy states 99 so you might want to
change your formatter settings.  There were a few places in your patch
where I noticed that the code wrapped by exactly one character.