kicad-developers team mailing list archive

[Janis Skujenieks <janis.skujenieks@xxxxxxxxx>]

OK, I understand what your saying.

Jean-Pierre, maybe you can check the patch and give your thoughts on using
this kind of solution.

P.S. Thanks for the tips.

On Wed, Feb 22, 2012 at 4:33 PM, Dick Hollenbeck <dick@xxxxxxxxxxx> wrote:

> On 02/21/2012 06:00 PM, Janis Skujenieks wrote:
>
> Offering a critique of this patch is perhaps a waste of time if there has
> been no
> agreement on its need.
>
> But for future reference:
>
> a) accessors are to hide data layouts, and protect against silly changes.
>  ORing in a an
> upper bit into the netcode would be fine, but only if this is hidden
> behind a pair of
> accessors.  It is self defeating to do this outside the accessor.
>  Likewise, your GetNet()
> accessor would have to and off the upper bit before returning the netcode.
>  Hide the shit
> inside the accessors please.  Setting a NetCode now has to OR in all the
> bits except the
> upper one.  (In the end, it is probably easier to simply add another bool
> data field.)
>
> #define OLD_TRACK_NET       0x40000000      // Flag for old netcodes that
> must be updated
>
> That means this define above should be a secret, behind the 4 accessors
> which have to know
> about it.  Two fields, two accessors each = 4 accessors.
>
>
>
> b) typedef on structs are not needed in C++.
>
> c) use spaces, NEVER tabs, indentation of 4.
>
> +               curr_track->SetNet( 0 );
>
>
>
>
>
> > === modified file 'pcbnew/connect.cpp'
> > --- pcbnew/connect.cpp        2012-02-19 04:02:19 +0000
> > +++ pcbnew/connect.cpp        2012-02-21 23:49:28 +0000
> > @@ -39,12 +39,23 @@
> >  #include <pcbnew.h>
> >
> >
> > +
> > +#define OLD_TRACK_NET       0x40000000      // Flag for old netcodes
> that must be updated
> > +
> > +/// Structure for holding needed substitution data
> > +typedef struct NetSubstitutionStruct
> > +{
> > +     int OldNetcode;
> > +     int NewNetcode;
> > +}NET_SUBS;
>
>
>
> > +
> > +
> >  extern void Merge_SubNets_Connected_By_CopperAreas( BOARD* aPcb );
> >  extern void Merge_SubNets_Connected_By_CopperAreas( BOARD* aPcb, int
> aNetcode );
> >
> >  // Local functions
> >  static void RebuildTrackChain( BOARD* pcb );
> > -
> > +static bool NetSubsCompare( NET_SUBS A, NET_SUBS B );
> >
> >  // A helper class to handle connection points (i.e. candidates) for
> tracks
> >  class CONNECTED_POINT
> > @@ -969,6 +980,19 @@
> >      return;
> >  }
> >
> > +/**
> > + * Function SubsCompare
> > + * compares two parameters of type NET_SUBS.
> > + *
> > + * @param A is first member for comparision.
> > + * @param B is second member for comparision.
> > + * @return bool, true if A.OldNetcode < B.OldNetcode.
> > + */
> > +static bool NetSubsCompare( NET_SUBS A, NET_SUBS B )
> > +{
> > +     if( A.OldNetcode < B.OldNetcode ) return true;
> > +     return false;
> > +}
> >
> >  /* search connections between tracks and pads and propagate pad net
> codes to the track
> >   * segments.
> > @@ -977,6 +1001,12 @@
> >  void PCB_BASE_FRAME::RecalculateAllTracksNetcode()
> >  {
> >      TRACK*              curr_track;
> > +    NET_SUBS            subst_pair;
> > +
> > +    /// Stores substitution set for netcodes
> > +    std::set<NET_SUBS, bool (*)(NET_SUBS, NET_SUBS)> substitut_set(
> NetSubsCompare );
> > +    std::set<NET_SUBS>::iterator subs_el;
> > +
> >
> >      // Build the net info list
> >      GetBoard()->BuildListOfNets();
> > @@ -991,28 +1021,32 @@
> >          curr_track->end = NULL;
> >          curr_track->SetState( BUSY | IN_EDIT | BEGIN_ONPAD | END_ONPAD,
> OFF );
> >          curr_track->SetZoneSubNet( 0 );
> > -        curr_track->SetNet( 0 );    // net code = 0 means not connected
> > +        curr_track->SetNet( curr_track->GetNet() | OLD_TRACK_NET );
> >      }
> > -
> > -    // If no pad, reset pointers and netcode, and do nothing else
> > -    if( m_Pcb->GetPadCount() == 0 )
> > -        return;
> > -
> > +
> >      CONNECTIONS connections( m_Pcb );
> >      connections.BuildPadsList();
> > -    connections.BuildTracksCandidatesList(m_Pcb->m_Track);
> > +    connections.BuildTracksCandidatesList( m_Pcb->m_Track );
> >
> >      // First pass: build connections between track segments and pads.
> >      connections.SearchTracksConnectedToPads();
> >
> > -    /* For tracks connected to at least one pad,
> > -     * set the track net code to the pad netcode
> > +    /* If pads > 0, store netcode substitution pairs for tracks
> > +     * connected to pads. So later we can change all tracks with this
> code to
> > +     * netcode of pad.
> >       */
> > -    curr_track = m_Pcb->m_Track;
> > -    for( ; curr_track != NULL; curr_track = curr_track->Next() )
> > +    if( m_Pcb->GetPadCount() > 0 )
> >      {
> > -        if( curr_track->m_PadsConnected.size() )
> > -            curr_track->SetNet(
> curr_track->m_PadsConnected[0]->GetNet() );
> > +        curr_track = m_Pcb->m_Track;
> > +        for( ; curr_track != NULL; curr_track = curr_track->Next() )
> > +        {
> > +            if( curr_track->m_PadsConnected.size() )
> > +            {
> > +                subst_pair.OldNetcode = curr_track->GetNet();
> > +                subst_pair.NewNetcode =
> curr_track->m_PadsConnected[0]->GetNet();
> > +                substitut_set.insert(subst_pair);
> > +            }
> > +        }
> >      }
> >
> >      // Pass 2: build connections between track ends
> > @@ -1022,46 +1056,22 @@
> >          connections.GetConnectedTracks( curr_track );
> >      }
> >
> > -    // Propagate net codes from a segment to other connected segments
> > -    bool new_pass_request = true;   // set to true if a track has its
> netcode changed from 0
> > -                                    // to a known netcode to
> re-evaluate netcodes
> > -                                    // of connected items
> > -    while( new_pass_request )
> > +    // Go trough tracks and apply substitution list or set to 0 if not
> found
> > +    for( curr_track = m_Pcb->m_Track; curr_track != NULL; curr_track =
> curr_track->Next() )
> >      {
> > -        new_pass_request = false;
> > -
> > -        for( curr_track = m_Pcb->m_Track; curr_track; curr_track =
> curr_track->Next() )
> > -        {
> > -            int netcode = curr_track->GetNet();
> > -            if( netcode == 0 )
> > -            {   // try to find a connected item having a netcode
> > -                for( unsigned kk = 0; kk <
> curr_track->m_TracksConnected.size(); kk++ )
> > -                {
> > -                    int altnetcode =
> curr_track->m_TracksConnected[kk]->GetNet();
> > -                    if( altnetcode )
> > -                    {
> > -                        new_pass_request = true;
> > -                        netcode = altnetcode;
> > -                        curr_track->SetNet(netcode);
> > -                        break;
> > -                    }
> > -                }
> > -            }
> > -            if( netcode )    // this track has a netcode
> > -            {   // propagate this netcode to connected tracks having no
> netcode
> > -                for( unsigned kk = 0; kk <
> curr_track->m_TracksConnected.size(); kk++ )
> > -                {
> > -                    int altnetcode =
> curr_track->m_TracksConnected[kk]->GetNet();
> > -                    if( altnetcode == 0 )
> > -                    {
> > -
>  curr_track->m_TracksConnected[kk]->SetNet(netcode);
> > -                        new_pass_request = true;
> > -                    }
> > -                }
> > -            }
> > +        subst_pair.OldNetcode = curr_track->GetNet();
> > +        subs_el = substitut_set.find(subst_pair);
> > +
> > +        if( subs_el != substitut_set.end() )
> > +        {
> > +            curr_track->SetNet( (*subs_el).NewNetcode );
> > +        }
> > +        else
> > +        {
> > +             curr_track->SetNet( 0 );
> >          }
> >      }
> > -
> > +
> >      // Sort the track list by net codes:
> >      RebuildTrackChain( m_Pcb );
> >  }
> >
>
>
> _______________________________________________
> 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
>