kicad-developers team mailing list archive

[Lorenzo Marcantonio <l.marcantonio@xxxxxxxxxxxx>]

Still looking at that code after the SEGVIA cleanup.

AFAIK the track/via DL-list on the board is kept sorted (or maybe only
grouped?) by netcode, so all the segments of a net are contiguos on the
list. Presumably the 'rebuild board connectivity' button re-sorts the
list when something changes substantially (usually the netlist).

I'm in the 'merge collinear segments' section in clean_segments();
inside the main 'for each track which is actually a TRACK', just after
the flag and no_inc definitions there is a funny looking for loop
(actually a while in disguise :D). I think the idea here is to find
other tracks attached to the start point of the segment in exam, using
TRACK::GetTrack(), from the next on until the end.

In pseudocode it seems to work in this way:

looping with segStart from the next of the current segment
    find a segment from segStart on attached to the current segment begin point
    if found:
        if it's not a TRACE or if the width is different:
            exit the loop
        if there isn't another segment like the one found before:
            set the flag
        exit the loop
    exit the loop

...then proceed to process the segment found and does the same with the
other endpoint in substantially the same way.

So... have I got it wrong? it's me or it isn't a loop at all? whatever
happen it hits a break. I don't know the exact logic needed here, but
shouldn't retry with another one segment if the width mismatches or it
hits a via? why the only one segment connected? shouldn't it work in
a tee, too?

Typical example:
 -S1b-
--S1--O--S2--
      |
      S3
      |

(S1b is overlapping partially with S1 and ending on the via O).
Shouldn't S1b be 'eaten' by S1. Or it is somehow detected as
unconnected? (how? both edges are connected to S1) *OR* the routine is
expected to only handle the case

--S1--.--S2--

(where the . is only a coincident endpoint)?  If that's what meant for
collinear the the routing makes more sense (but then why a loop which
doesn't ever repeat?)

Another thing:

in TRACK::GetTrack(TRACK*, TRACK*, int), if I got the idea correctly, it
first looks walking in both the directions from the this position
looking for a matching track, for 50 paces (probably to optimize the
usual case) and then do an exhaustive search on the passed range.

In this function:
1) (only nitpicking on this:P) instead of the ugly:

    previousSegment = nextSegment = this;
    
    why not
    
    previousSegment = this->Back();
    nextSegment = this->Next();

    so that explicit check for this are not needed?
    
2) instead of doing the general search why simply don't continue the
bidirectional walk until it its the passed limits?

3) what if I wanted to continue the search (for example in the 'loop' of
the cleaning routine)? with a bidirectional search you can't express it
(unless you return also on which side you found it and keeping track of
two search ranges).

4) in the backward iteration, how could a TRACK* have a PCB_T type?
isn't PCB_T the whole board?

That's a lot of questions but that section of code really confused me...

Thanks in advance,

-- 
Lorenzo Marcantonio
Logos Srl