Fix a cadre of symbol pin alternate issues.

Prevent the file formatter from writing and the file parser from loading
symbol pin alternates that have an empty name or when the alternate name
is the same as the default pin name.  Apparently at some point they were
able to be created and saved.

Do not allow SCH_PIN::SetAlt() to set an alternate name when the name
does not exist in the library symbol pin alternate list.  This required
fixing the associated QA test.

When updating or changing schematic symbols from library, reset the pin
alternate to the default when the alternate no longer exists in the
symbol.

Do not populate pin alternates context menu with duplicate default pin.

Clear orphaned pin alternates to the default when saving symbols from
symbol editor.

(cherry picked from commit d16d2f5a5b1897b0fa3f4e797ff9b9a3a246c09a)
This commit is contained in:
Wayne Stambaugh 2025-05-17 14:19:51 -04:00
parent de0394bf38
commit 8b44bc1344
7 changed files with 64 additions and 3 deletions

View File

@ -2408,6 +2408,18 @@ void SCH_EDIT_FRAME::SaveSymbolToSchematic( const LIB_SYMBOL& aSymbol,
GetCanvas()->GetView()->Update( unit );
}
// Clear any orphaned alternate pins.
for( SCH_PIN* pin : principalSymbol->GetPins() )
{
wxString altName = pin->GetAlt();
if( altName.IsEmpty() )
continue;
if( pin->GetAlternates().count( altName ) == 0 )
pin->SetAlt( wxEmptyString );
}
if( !commit.Empty() )
commit.Push( _( "Save Symbol to Schematic" ) );
}

View File

@ -791,7 +791,10 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
for( const std::unique_ptr<SCH_PIN>& pin : aSymbol->GetRawPins() )
{
if( pin->GetAlt().IsEmpty() )
// There was a bug introduced somewhere in the original alternated pin code that would
// set the alternate pin to the default pin name which caused a number of library symbol
// comparison issues. Clearing the alternate pin resolves this issue.
if( pin->GetAlt().IsEmpty() || ( pin->GetAlt() == pin->GetBaseName() ) )
{
m_out->Print( "(pin %s", m_out->Quotew( pin->GetNumber() ).c_str() );
KICAD_FORMAT::FormatUuid( m_out, pin->m_Uuid );

View File

@ -470,6 +470,12 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::savePin( SCH_PIN* aPin, OUTPUTFORMATTER& aFor
for( const std::pair<const wxString, SCH_PIN::ALT>& alt : aPin->GetAlternates() )
{
// There was a bug somewhere in the alternate pin code that allowed pin alternates with no
// name to be saved in library symbols. This strips any invalid alternates just in case
// that code resurfaces.
if( alt.second.m_Name.IsEmpty() )
continue;
aFormatter.Print( "(alternate %s %s %s)",
aFormatter.Quotew( alt.second.m_Name ).c_str(),
getPinElectricalTypeToken( alt.second.m_Type ),

View File

@ -413,6 +413,25 @@ void SCH_PIN::SetName( const wxString& aName )
}
void SCH_PIN::SetAlt( const wxString& aAlt )
{
wxString alt = aAlt;
// Do not set the alternate pin definition to the default pin name. This breaks the library
// symbol comparison for the ERC and the library diff tool. It also incorrectly causes the
// schematic symbol pin alternate to be set.
if( aAlt.IsEmpty() || ( alt == GetBaseName() ) )
{
m_alt = wxEmptyString;
return;
}
wxCHECK2_MSG( m_libPin && m_libPin->GetAlternates().count( aAlt ), alt = wxEmptyString,
wxString::Format( wxS( "Pin '%s' does not have an alterate '%s'" ), m_number, aAlt ) );
m_alt = aAlt;
}
bool SCH_PIN::IsDangling() const
{
if( GetType() == ELECTRICAL_PINTYPE::PT_NC || GetType() == ELECTRICAL_PINTYPE::PT_NIC )

View File

@ -150,7 +150,17 @@ public:
}
wxString GetAlt() const { return m_alt; }
void SetAlt( const wxString& aAlt ) { m_alt = aAlt; }
/**
* Set the name of the alternate pin.
*
* @note If the alternate pin is the same as the default pin name or does not exist in the
* list of pin alternates, it's set to an empty string which results in the alternate
* being set to the default pin.
*
* @param is the name of the pin alternate in #m_alternates.
*/
void SetAlt( const wxString& aAlt );
void Print( const SCH_RENDER_SETTINGS* aSettings, int aUnit, int aBodyStyle,
const VECTOR2I& aOffset, bool aForceNoFill, bool aDimmed ) override;

View File

@ -201,13 +201,17 @@ private:
wxMenuItem* item = Append( ID_POPUP_SCH_ALT_PIN_FUNCTION, libPin->GetName(), wxEmptyString,
wxITEM_CHECK );
if( pin->GetAlt().IsEmpty() )
if( pin->GetAlt().IsEmpty() || ( pin->GetAlt() == libPin->GetName() ) )
item->Check( true );
int ii = 1;
for( const auto& [ name, definition ] : libPin->GetAlternates() )
{
// The default pin name is set above, avoid setting it again.
if( name == libPin->GetName() )
continue;
item = Append( ID_POPUP_SCH_ALT_PIN_FUNCTION + ii, name, wxEmptyString, wxITEM_CHECK );
if( name == pin->GetAlt() )

View File

@ -26,6 +26,7 @@
// Code under test
#include <lib_symbol.h>
#include <pin_type.h>
#include <sch_pin.h>
#include <sch_symbol.h>
@ -115,6 +116,12 @@ BOOST_AUTO_TEST_CASE( Copy )
BOOST_CHECK_EQUAL( copied.GetNumber(), m_lib_pin->GetNumber() );
BOOST_CHECK_EQUAL( copied.GetAlt(), wxEmptyString );
SCH_PIN::ALT alt;
alt.m_Name = wxS( "alt" );
alt.m_Shape = GRAPHIC_PINSHAPE::INVERTED;
alt.m_Type = ELECTRICAL_PINTYPE::PT_OUTPUT;
copied.GetAlternates()[ wxS( "alt" ) ] = alt;
// Set some non-default values
copied.SetAlt( "alt" );