kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #39931
Re: Should SCH_COMPONENT::m_convert default to 1?
Yes :) we should do this in a few other places too
On Thu, Mar 28, 2019, 07:53 Mark Roszko <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.
>
> On Wed, Mar 27, 2019 at 10:59 PM Jon Evans <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.
>>
>> 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>
>> 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>> 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>
>>> > >> 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
>>
>
>
> --
> Mark
>
Follow ups
References