← Back to team overview

kicad-developers team mailing list archive

Re: Bug 1677282 fix feedback.

 

lol, I was just going to ask you why that line is even there if _no_
components can have a zero unit, and then decided to git-blame it and
realized _I_ wrote that line.

I think the reason I did that is that the new selector returns zero when
the user has selected a root component - it only returns a nonzero unit
if a unit was actually selected. The documentation for the selector
points this out, but clearly it's sufficiently confusing that I got it
wrong in my own code :\

Probably best to fix DIALOG_CHOOSE_COMPONENT::GetSelectedAlias() to
always return a valid unit. I wouldn't go so far as to make all
CMP_TREE_NODE_ALIASes have Unit=1; that would have the desired effect,
but that class is meant to represent just an alias and an alias by
itself doesn't really have a unit number.


On Thu, Mar 30, 2017 at 03:26:11PM -0400, Wayne Stambaugh wrote:
> The problem is line 169 of eeschema/getpart.cpp.  The component unit
> cannot be 0 even for components that do not have multiple units.  There
> probably should be an assertion in the SCH_COMPONENT ctor to warn devs
> about this.
> 
> On 3/30/2017 3:24 PM, Chris Pavlina wrote:
> > Ooooops. Sorry.
> > 
> > I caught one place where I had done that. Thought that was all. :\
> > 
> > 
> > On Thu, Mar 30, 2017 at 03:13:31PM -0400, Wayne Stambaugh wrote:
> >> I believe since the new component chooser dialog has been implement
> >> which hasn't been very long.
> >>
> >> On 3/30/2017 2:53 PM, Maciej Suminski wrote:
> >>> Do you know for how long the bug was present? It could help estimate the
> >>> number of schematic files affected.
> >>>
> >>> As much as I hate windows popping up, I do not think notifying the users
> >>> would be that annoying. It is going to happen only once for each invalid
> >>> file. I doubt there are people working on 10+ designs in parallel.
> >>>
> >>> I vote for #3. IMHO #4 is acceptable too. I would avoid #1 and #2 in
> >>> particular.
> >>>
> >>> Regards,
> >>> Orson
> >>>
> >>> On 03/30/2017 07:33 PM, Wayne Stambaugh wrote:
> >>>> I have found and fixed the bug in this bug report:
> >>>>
> >>>> https://bugs.launchpad.net/kicad/+bug/1677282
> >>>>
> >>>> This bug creates invalid schematic files which sets the component unit
> >>>> flag to 0 and breaks the netlist generator.  The fix itself is simple.
> >>>> What is not simple is what to do about the invalid schematic files that
> >>>> have already been created.  I added code to schematic parser to fix this
> >>>> but this creates a dilemma.  Technically the schematic is modified which
> >>>> begs the question, what to do next.  None of the following choices are
> >>>> particularly appealing:
> >>>>
> >>>> 1) Do nothing and leave the file in an invalid status until the next
> >>>> time the user saves the schematic.  This is probably the most convenient
> >>>> but what about broken schematics being used by versions of kicad prior
> >>>> to this fix?
> >>>>
> >>>> 2) Set the schematic modified status which will trigger a save warning
> >>>> when eeschema is closed even if the user hasn't made any changes.  We
> >>>> already do this with SCH_SCREEN::SchematicCleanUp() that gets called
> >>>> lots of places outside of schematic editing.  The problem with this is
> >>>> that the user has no idea why they are getting a save warning when they
> >>>> didn't actually change anything.
> >>>>
> >>>> 3) Set the schematic modified status and inform the user that there was
> >>>> an error in their schematic file that was repaired on load and requires
> >>>> saving.  This is the most informative for the user but reeks of nagware.
> >>>>
> >>>> 4) Silently save the corrected schematic with no user interaction.  This
> >>>> will undoubtedly make VCS users unhappy.
> >>>>
> >>>> As much as I hate nagware, I like unexpected save warnings when I
> >>>> haven't changed anything even less so I'm leaning towards option 3.  Any
> >>>> feedback would be appreciated.
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Wayne
> >>>>
> >>>> _______________________________________________
> >>>> 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