kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

Hi,

I think with this kind of thing, KiCad should be strict with what it will accept. Otherwise you might find people rely on broken or undefined behaviour and de facto defaults and it all goes wrong when it's fixed or changed on KiCad's end in future. That's how you get "quirks modes". 

However, this should be enforced by the parser, which should reject the file, with a useful UI (not asserts), preferably including a reason and a line number. The file shouldn't make it far enough to cause defects within the program. Defective data in the main program can be checked with asserts, the triggering of which implies that either the parser is missing a check, or the internal logic is defective. Either way there should be no way for an end user to trigger one, no matter what they feed in. 

Ideally, there would be tests on that interface to ensure the parser catches malformed data, particularly when that data is shown to cause defects though bug reports. 

Thus a user can't get bad data into the main program, and also doesn't see asserts (which aren't present in the release code and should never be relied on for anything but development).

Cheers, 

John



On 25 June 2018 19:25:41 BST, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>Hey Seth,
>
>I would agree with you if this was our error.  If I'm reading JP's
>analysis correctly, he could not create this board error using KiCad so
>I'm working on the assumption that this was not our board file output
>formatter causing the problem.  Using defensive measures such as
>assertions and test cases to ensure our board file output formatter is
>correct makes sense.  Adding code to our parser to fix board files that
>are broken due to manual editing or broken scripts is a slippery slope.
>Where do we draw the line?  This isn't the first time this has
>happened.
> In the past, we have chosen not to allow this so I am leaning in that
>direction.  It would be nice if the submitter of the broken board would
>confirm if the board was manually edited with a text editor or
>manipulated with a script.  I'm not completely dismissing the patch but
>it would be easier to make an informed decision if I knew how the
>broken
>board was created.
>
>Cheers,
>
>Wayne
>
>On 6/25/2018 10:25 AM, Seth Hillbrand wrote:
>> ​Hi Wayne-
>> 
>> I would view this patch more as defensive coding than trying to fix
>> others' errors.  The alternatives are to refuse to parse the file or
>(as
>> we currently do) parse as written and encounter errors in display and
>> routing.  JP's approach provides a clear and predictable response to
>a
>> modified file.
>> 
>> -Seth​
>> 
>> 
>> Am Mo., 25. Juni 2018 um 06:16 Uhr schrieb Wayne Stambaugh
>> <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>:
>> 
>>     On 6/25/2018 8:12 AM, jp charras wrote:
>>     > Le 24/06/2018 à 21:52, Wayne Stambaugh a écrit :
>>     >> Are we planning on fixing this for rc3?  If so, what is
>>     required.  I'm fine with waiting a day or
>>     >> two to tag rc3.
>>     >>
>>     >
>>     > Attached a possible fix.
>>     >
>>     > The board test file shows really broken layer set in a few
>pads:
>>     > - no layers for some pads (the layers section is empty).
>>     > This make no sense for me.
>>     >
>>     > After tests, It is not possible to create this empty layer set
>>     from Pcbnew (even with old 2013
>>     > stable version).
>>     >
>>     > My opinion is the board (or the footprints) was modified by
>hand
>>     (or by a script).
>> 
>>     If this is the case, then I'm not thrilled about applying this
>patch
>>     even if it does solve the problem.  We should not be modifying
>KiCad to
>>     fix other people's coding errors.
>> 
>>     >
>>     > This patch is short and fixes silently this kind of issues.
>>     > (I am not sure it is worth to try a better fix, because this
>issue
>>     cannot happens when editing a
>>     > board from pcbnew)
>>     >
>>     >
>>     >
>>     > _______________________________________________
>>     > Mailing list: https://launchpad.net/~kicad-developers
>>     > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>     > Unsubscribe : https://launchpad.net/~kicad-developers
>>     > More help   : https://help.launchpad.net/ListHelp
>>     >
>> 
>>     _______________________________________________
>>     Mailing list: https://launchpad.net/~kicad-developers
>>     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>>     Unsubscribe : https://launchpad.net/~kicad-developers
>>     More help   : https://help.launchpad.net/ListHelp
>> 
>
>_______________________________________________
>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