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. _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
