kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

On 3/28/2019 8:00 AM, Jon Evans wrote:
> Yes :) we should do this in a few other places too
> 
> On Thu, Mar 28, 2019, 07:53 Mark Roszko <mark.roszko@xxxxxxxxx
> <mailto:mark.roszko@xxxxxxxxx>> wrote:
> 
>     Wouldn't it also be smarter to make it an enum? "2" and "1" are
>     already annoying values as you have to hunt them down.

This is only valid if you are working under the assumption that there
are only two possible converts and common to all converts.  I'm not
opposed to this but there was talk at one point to allow more than two
converts so you are back to integers.  Inheritance in the new file
format will make this point moot.

> 
>     On Wed, Mar 27, 2019 at 10:59 PM Jon Evans <jon@xxxxxxxxxxxxx
>     <mailto:jon@xxxxxxxxxxxxx>> wrote:
> 
>         No, reverting that commit causes the problems to reappear, even
>         after 175a2bc0b.
> 
>         In SCH_EDIT_FRAME::ConvertPart(), the logic appears to toggle
>         the parameter
>         between 1 and 2 (value 0 is not used). It appears that the
>         convert value for a
>         component is never supposed to be 0, even though the convert
>         value for component
>         members (shapes and pins) can be 0 to indicate that they are
>         common to all body
>         styles.
> 
>         In SCH_COMPONENT::GetNetListItem(), the following check is made:
> 
>         if( pin->GetConvert() && ( pin->GetConvert() != GetConvert() ) )
>         continue;
> 
>         This has the effect that a component with m_convert set to zero,
>         containing pins
>         with m_convert set to 1, will appear to have no pins when
>         netlisting. I just
>         checked some of the demo projects (that were not converted from
>         Eagle, to rule
>         that out), and it appears that we do default to setting
>         m_convert to 1 when
>         creating new parts.

I'm not opposed to setting m_Convert to 1 but it's highly likely that
this will break something else where the expectation is that m_Convert
was set to 0 in the ctor.  Who ever makes this change will have to pour
through all of the code where library items are instantiated and the
convert is set to make sure nothing gets broken.  It may be less painful
to just fix the Eagle importer.

> 
>         In DIALOG_EDIT_COMPONENT_IN_SCHEMATIC::TransferDataFromWindow():
> 
>         if( m_cbAlternateSymbol->IsEnabled() ) m_cmp->SetConvert(
>             m_cbAlternateSymbol->GetValue() ? 2 : 1 );
> 
>         I am out of time to trace this any deeper, but from brief
>         examination of the
>         code, it looks like the m_convert parameter of SCH_COMPONENT
>         should default to
>         1.  It is initialized this way in all places except for the
>         Eagle conversion,
>         which is probably why this bug was triggered.
> 
> 
>         -Jon
> 
>         On Wed, Mar 27, 2019 at 10:36 AM Wayne Stambaugh
>         <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>> wrote:
> 
>             Jon,
> 
>             Would you please see if reverting commit ab74445df4 and
>             loading the the
>             schematic with the 64 bit time stamps has been fixed by
>             commit 175a2bc0b
>             when you get a chance?  If commit 175a2bc0b resolves the
>             issue, then
>             your commit should be reverted.
> 
>             Thanks,
> 
>             Wayne
> 
>             On 3/27/2019 9:44 AM, Jon Evans wrote:
>             > In my investigation of the issue in that forum thread, I
>             found that
>             > eeschema was choking on the convert value being zero.
>             >
>             > In that case, I have misunderstood the code and we should
>             probably
>             > revert ab74445df4
>             > That commit fixed the loading/netlisting issues in the
>             forum thread but
>             > perhaps it fixed it the wrong way.
>             >
>             > -Jon
>             >
>             > On Wed, Mar 27, 2019 at 8:35 AM Wayne Stambaugh
>             <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>             > <mailto:stambaughw@xxxxxxxxx
>             <mailto:stambaughw@xxxxxxxxx>>> wrote:
>             >
>             >     Oops.
>             >
>             >     On 3/27/2019 8:30 AM, Wayne Stambaugh wrote:
>             >     > Jon,
>             >     >
>             >     > I don't think setting m_Convert to 1 is the correct
>             solution
>             >     unless the
>             >     > part actually has a convert.  According to the
>             Doxygen comment:
>             >
>             >     /**
>             >       * Shape identification for alternate body styles. 
>             Set 0 if the item
>             >       * is common to all body styles.  This is commonly
>             referred to as
>             >       * DeMorgan style and this is typically how it is
>             used in KiCad.
>             >       */
>             >
>             >     Unless someone changed this behavior this should still
>             be valid.
>             >
>             >     >
>             >     >
>             >     > On 3/23/2019 9:08 PM, Jon Evans wrote:
>             >     >> For context, I've been looking at the issues in
>             this forum
>             >     >>
>             >   
>              thread: https://forum.kicad.info/t/before-giving-up-on-kicad-one-last-attempt-erc-misery/15933/63
>             >
>             >     This post is about the timestamps not the convert value.
>             >
>             >     >>
>             >     >> I have already pushed a band-aid fix to "repair" these
>             >     situations, but
>             >     >> it looks like they are being caused by the EAGLE
>             importer (or at
>             >     least
>             >     >> that is one path to this situation coming up)
>             >     >>
>             >     >> Specifically, it looks like m_convert is set to 0 in
>             >     >> SCH_COMPONENT::Init(), but this value is not valid
>             (it should be at
>             >     >> least 1 for components).
>             >     >>
>             >     >> Are there any cases that I am missing, where it
>             makes sense for a
>             >     >> component to have m_convert set to 0?  If not, it's
>             an easy fix
>             >     in that
>             >     >> Init() to set it to 1.  If so, we can just update
>             >     the SCH_EAGLE_PLUGIN
>             >     >> to call SetConvert() wherever it creates new
>             components.
>             >     >>
>             >     >> -Jon
>             >     >>
>             >     >> _______________________________________________
>             >     >> Mailing list: https://launchpad.net/~kicad-developers
>             >     >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>             <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>             >     <mailto: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>
>             >     <mailto: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
> 
> 
> 
>     -- 
>     Mark
>