← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] - File format shim for clearance data

 

Hi Jeff-

A few questions on how you are implementing this:

1) How does the user know what was skipped?  I can imagine team members
with different versions getting into difficulty, especially if the features
being skipped change the board layout.

2) You are removing strict checking for most of the board but you are
adding strict checking for pad options. What's the difference?

3) If we keep these options around but don't allow editing/removing, don't
we run into a risk of generating a really wonk-y board that only looks
wonky in a newer version of KiCad but looks fine in an older version?  For
example, imagine we implement rounded corners as a parameter and then a
user with an older version of KiCad edits and saves the board.  The rounded
corner is only visible in KiCad 6 but the user in KiCad 5 can edit the
board to look right to her only to have her colleague use KiCad 6 and see
the track violating DRC.  I think that might be counter-intuitive for users
but maybe there's a way around it.

In general, if I understand the use-case, this is to allow users to open
newer boards in older KiCad versions?  Is there another use case?

I think I can see clear to this for options like the 3d-model offset where
it could be either inches or mm but there isn't a difference in the actual
layout.  Allowing general unrecognized options would seem to open up a worm
can in terms of support as in "Please post the KiCad version and the file
version in order to figure out the problem."

-S



2018-03-18 9:46 GMT-07:00 Jeff Young <jeff@xxxxxxxxx>:

> OK, for your guys’ (re)viewing pleasure, a patch which losslessly
> round-trips stuff it doesn’t understand.
>
>
>
>
> On 16 Mar 2018, at 19:15, hauptmech <hauptmech@xxxxxxxxx> wrote:
>
> While i would still like to see this (my) shim go in, I agree with wayne.
> Until/unless a less brittle approach is used for the parser, it's better to
> signal a problem painfully and maintain the integrity of the file.
>
> I have to admit that when i first heard the s-expressions idea I assumed
> that the intention was to use a lisp like virtual machine for loading and
> maintaining design data. I've used vm's for data storage before (lua and
> python), and it's great. Parsing is free and you can jam in functions and
> other data when needed.
>
>
>
> On 17 Mar 2018 07:17, "Jeff Young" <jeff@xxxxxxxxx> wrote:
>
>> Hi Wayne,
>>
>> Patch reverted.
>>
>> However, I think your concern is misplaced.  If we want to prevent
>> dataloss (ie: from reading a 6.0 file into 5.0), then we should warn the
>> user based on the version string (and give them a chance to say “I still
>> want to open”).
>>
>> But either way, actually failing to read the file leaves the user in a
>> pickle (especially when it’s easy enough for them to try out a nightly that
>> may very well be ahead of their stable).
>>
>> (And for that reason I think it’s important to put it into 5.0, as
>> otherwise it won’t help until we start making 7.0 file format changes.)
>>
>> Cheers,
>> Jeff.
>>
>> > On 16 Mar 2018, at 18:00, Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>> >
>> > Jeff,
>> >
>> > Please revert this patch.  Any changes to the board file parser and/or
>> > formatter need to be discussed before the changes are committed.  It has
>> > been the intention that the board parser be pendantic by design to
>> > prevent data loss by ignoring unknown formatting.  Allowing anything
>> > outside of the expected formatting in the board file is not something
>> > that I want to introduce without some discussion.  Even should we decide
>> > to accept this patch, I would prefer we put it off until v6.
>> >
>> > That being said, the patch fails to build on windows with following
>> error:
>> >
>> > C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/pcb_parser.cpp: In
>> > member function 'void PCB_PARSER::parseUnknown()':
>> > C:/msys64/home/wstambaugh/src/kicad-trunk/pcbnew/pcb_parser.
>> cpp:1269:12:
>> > error: request for member 'LogText' in '__acrt_iob_func(2)', which is of
>> > pointer type  FILE* {aka _iobuf*}' (maybe you meant to use '->' ?)
>> >     stderr.LogText( msg );
>> >            ^~~~~~~
>> >
>> > Cheers,
>> >
>> > Wayne
>> >
>> > On 3/16/2018 1:08 PM, Jeff Young wrote:
>> >> Perhaps somewhat germane to this discussion I have removed the
>> strict-format nags from the PCB parser.  It now logs warnings to stderr but
>> otherwise ignores structures it doesn’t understand.
>> >>
>> >> I’m not sure that helps hauptmech much as if the file is subsequently
>> written the unknown markup will be lost, but I thought I’d mention it.
>> >>
>> >> Cheers,
>> >> Jeff.
>> >>
>> >>> On 7 Mar 2018, at 20:12, hauptmech <hauptmech@xxxxxxxxx> wrote:
>> >>>
>> >>> Hi Thomasz,
>> >>>
>> >>> I hope I'm able to address you concerns. I'm stuck using kicad stable
>> in many situations. I brought clearances up for discussion last July but no
>> one moved on it, nor are they required to. Clearance management is a major
>> pain point for me so I wrote a fix. This patch will let us (me and the
>> people I collaborate with) work using version 5, but open and close files
>> written with a version patched with clearance handling code.
>> >>>
>> >>> I believe that code exactly like this will go into version 6. Getting
>> it in earlier makes a huge difference to me, so I'm submitting it.
>> >>>
>> >>> On 07/03/18 23:30, Tomasz Wlostowski wrote:
>> >>>> Hi hauptmech,
>> >>>>
>> >>>> I'm sorry but IMHO we can't accept your patch:
>> >>>> - it changes the file format while we are already in feature freeze.
>> >>>> This is a way too big change to accept for the V5.
>> >>>
>> >>> This patch simply adds tokens to the file format. No clearance data
>> is saved for files that use the netclass only. Files without clearance
>> tokens continue to remain without them.
>> >>>
>> >>> Yes it is a backward compatible file format change, but it does no
>> harm to V5 files already in the wild.
>> >>>
>> >>>> - we are planning to overhaul the clearance/design rules system in
>> V6.
>> >>>> Storing the clearance *DIRECTLY* for each track segment/via will
>> >>>> conflict with any more sophisticated design rule management system.
>> >>>>
>> >>> I'm glad you are planning this. I am sure that regardless of the
>> sophistication of the rule system, you will store clearance directly for
>> exactly the same reason that track width is stored directly now. There are
>> always exceptions to the rules.
>> >>>
>> >>> If kicad choose a direction that does not store clearances per item,
>> then it is easy to rip these few lines back out.
>> >>>
>> >>> Did this answer your existing concerns about this patch? Are there
>> any other concerns you have about this patch?
>> >>>
>> >>>
>> >>>
>> >>> _______________________________________________
>> >>> 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
>> >>
>> >>
>> >> _______________________________________________
>> >> 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
>> >>
>> >
>> > _______________________________________________
>> > 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
>>
>>
>> _______________________________________________
>> 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
>>
>
>
> _______________________________________________
> 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
>
>

Follow ups

References