← Back to team overview

kicad-developers team mailing list archive

[PATCH] Correct bus junction behavior

 

​The attached patch ​adds junction management to buses in addition to wires.

Of note, this also adds buses to the draggable junctions collection.  Now
that junctions are managed internally, it makes sense to allow any
collection with a junction to allow draggable junctions.

-Seth
From 89386d0e98eb3845685083b7b788fec1d032a338 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Tue, 12 Dec 2017 21:12:06 -0800
Subject: [PATCH] Eeschema: Consider buses for junctions

Allows buses to acquire junctions based on their connections.  Buses
can only have junctions with other buses.  Also allows buses to be
draggable junctions for collections
---
 eeschema/bus-wire-junction.cpp | 42 +++++++++++++-----------
 eeschema/sch_collectors.cpp    | 37 ++-------------------
 eeschema/sch_screen.cpp        | 73 ++++++++++++++++++++++++++----------------
 3 files changed, 72 insertions(+), 80 deletions(-)

diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp
index 1085e699f..9115e3343 100644
--- a/eeschema/bus-wire-junction.cpp
+++ b/eeschema/bus-wire-junction.cpp
@@ -483,7 +483,7 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
     PICKED_ITEMS_LIST   itemList;
     SCH_SCREEN*         screen = GetScreen();
 
-    auto remove_item = [ &itemList, screen ]( SCH_ITEM* aItem ) -> void
+    auto remove_item = [ &itemList ]( SCH_ITEM* aItem ) -> void
     {
         aItem->SetFlags( STRUCT_DELETED );
         itemList.PushItem( ITEM_PICKER( aItem, UR_DELETED ) );
@@ -493,9 +493,9 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
 
     for( item = screen->GetDrawItems(); item; item = item->Next() )
     {
-        if( ( item->Type() != SCH_LINE_T ) &&
-            ( item->Type() != SCH_JUNCTION_T ) &&
-            ( item->Type() != SCH_NO_CONNECT_T ))
+        if( ( item->Type() != SCH_LINE_T )
+            && ( item->Type() != SCH_JUNCTION_T )
+            && ( item->Type() != SCH_NO_CONNECT_T ) )
             continue;
 
         if( item->GetFlags() & STRUCT_DELETED )
@@ -503,14 +503,15 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
 
         // Remove unneeded junctions
         if( ( item->Type() == SCH_JUNCTION_T )
-                && ( !screen->IsJunctionNeeded( item->GetPosition() ) ) )
+            && ( !screen->IsJunctionNeeded( item->GetPosition() ) ) )
         {
             remove_item( item );
             continue;
         }
+
         // Remove zero-length lines
         if( item->Type() == SCH_LINE_T
-                && ( (SCH_LINE*) item )->IsNull() )
+            && ( (SCH_LINE*) item )->IsNull() )
         {
             remove_item( item );
             continue;
@@ -524,24 +525,26 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
             // Merge overlapping lines
             if( item->Type() == SCH_LINE_T )
             {
-                SCH_LINE* firstLine = (SCH_LINE*) item;
-                SCH_LINE* secondLine = (SCH_LINE*) secondItem;
-                SCH_LINE* line = NULL;
+                SCH_LINE*   firstLine   = (SCH_LINE*) item;
+                SCH_LINE*   secondLine  = (SCH_LINE*) secondItem;
+                SCH_LINE*   line = NULL;
                 bool needed = false;
 
                 if( !secondLine->IsParallel( firstLine ) )
                     continue;
 
-                // Check if a junction needs to be kept
-                // This can only happen if:
-                //   1) the endpoints overlap,
-                //   2) the lines are not pointing in the same direction AND
-                //   3) IsJunction Needed is false
-                if( secondLine->IsEndPoint( firstLine->GetStartPoint() )
-                        && !secondLine->IsSameQuadrant( firstLine, firstLine->GetStartPoint() ) )
+                // Remove identical lines
+                if( firstLine->IsEndPoint( secondLine->GetStartPoint() )
+                    && firstLine->IsEndPoint( secondLine->GetEndPoint() ) )
+                {
+                    remove_item( secondItem );
+                    continue;
+                }
+
+                // If the end points overlap, check if we still need the junction
+                if( secondLine->IsEndPoint( firstLine->GetStartPoint() ) )
                     needed = screen->IsJunctionNeeded( firstLine->GetStartPoint() );
-                else if( secondLine->IsEndPoint( firstLine->GetEndPoint() )
-                        && !secondLine->IsSameQuadrant( firstLine, firstLine->GetEndPoint() ) )
+                else if( secondLine->IsEndPoint( firstLine->GetEndPoint() ) )
                     needed = screen->IsJunctionNeeded( firstLine->GetEndPoint() );
 
                 if( !needed && ( line = (SCH_LINE*) secondLine->MergeOverlap( firstLine ) ) )
@@ -558,12 +561,15 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( bool aAppend )
                 remove_item( secondItem );
         }
     }
+
     for( item = screen->GetDrawItems(); item; item = secondItem )
     {
         secondItem = item->Next();
+
         if( item->GetFlags() & STRUCT_DELETED )
             screen->Remove( item );
     }
+
     SaveCopyInUndoList( itemList, UR_CHANGED, aAppend );
 
     return !!( itemList.GetCount() );
diff --git a/eeschema/sch_collectors.cpp b/eeschema/sch_collectors.cpp
index efe859720..970758d82 100644
--- a/eeschema/sch_collectors.cpp
+++ b/eeschema/sch_collectors.cpp
@@ -311,42 +311,11 @@ bool SCH_COLLECTOR::IsNode( bool aIncludePins ) const
 
 bool SCH_COLLECTOR::IsDraggableJunction() const
 {
-    int wireEndCount = 0;
-    int wireMidPoint = 0;
-    int junctionCount = 0;
-
     for( size_t i = 0;  i < m_List.size();  i++ )
-    {
-        SCH_ITEM* item = (SCH_ITEM*) m_List[ i ];
-        KICAD_T type = item->Type();
-
-        if( type == SCH_JUNCTION_T )
-        {
-            junctionCount++;
-            continue;
-        }
+        if( ( (SCH_ITEM*) m_List[ i ] )->Type() == SCH_JUNCTION_T )
+            return true;
 
-        if( type == SCH_LINE_T )
-        {
-            if( item->GetLayer() != LAYER_WIRE )
-                return false;
-
-            SCH_LINE* line = (SCH_LINE*) item;
-
-            if( line->IsEndPoint( m_RefPos ) )
-                wireEndCount++;
-            else
-                wireMidPoint++;
-
-            continue;
-        }
-
-        // Any other item types indicate that this collection is not a draggable junction.
-        return false;
-    }
-
-    return (wireEndCount >= 3) || ((wireEndCount >= 1) && (wireMidPoint == 1))
-        || ((wireMidPoint >= 2) && (junctionCount == 1));
+    return false;
 }
 
 
diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp
index d0c444a88..27090b258 100644
--- a/eeschema/sch_screen.cpp
+++ b/eeschema/sch_screen.cpp
@@ -55,8 +55,6 @@
 #include <lib_pin.h>
 #include <symbol_lib_table.h>
 
-#include <boost/foreach.hpp>
-
 #define EESCHEMA_FILE_STAMP   "EESchema"
 
 /* Default zoom values. Limited to these values to keep a decent size
@@ -349,11 +347,11 @@ void SCH_SCREEN::MarkConnections( SCH_LINE* aSegment )
 
 bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew )
 {
-    bool has_line = false;
-    bool has_nonparallel = false;
-    int end_count = 0;
-    int pin_count = 0;
-    std::vector< SCH_LINE* > lines;
+    bool    has_nonparallel[2] = { false };
+    int     end_count[2] = { 0 };
+    int     pin_count = 0;
+
+    std::vector<SCH_LINE*> lines[2];
 
     for( SCH_ITEM* item = m_drawList.begin(); item; item = item->Next() )
     {
@@ -364,44 +362,63 @@ bool SCH_SCREEN::IsJunctionNeeded( const wxPoint& aPosition, bool aNew )
             return false;
 
         if( ( item->Type() == SCH_LINE_T )
-                && ( item->GetLayer() == LAYER_WIRE )
-                && ( item->HitTest( aPosition, 0 ) ) )
-            lines.push_back( (SCH_LINE*) item );
+            && ( item->HitTest( aPosition, 0 ) ) )
+        {
+            if( item->GetLayer() == LAYER_WIRE )
+                lines[0].push_back( (SCH_LINE*) item );
+            else if( item->GetLayer() == LAYER_BUS )
+                lines[1].push_back( (SCH_LINE*) item );
+        }
 
         if( ( item->Type() == SCH_COMPONENT_T )
                 && ( item->IsConnected( aPosition ) ) )
             pin_count++;
     }
 
-    BOOST_FOREACH( SCH_LINE* line, lines)
+    for( int i = 0; i < 2; i++ )
     {
-        if( !line->IsEndPoint( aPosition ) )
-            has_line = true;
-        else
-            end_count++;
-        BOOST_REVERSE_FOREACH( SCH_LINE* second_line, lines )
+        bool removed_overlapping = false;
+        end_count[i] = lines[i].size();
+
+        for( auto line = lines[i].begin(); line < lines[i].end(); line++ )
         {
-            if( line == second_line )
-                break;
-            if( line->IsEndPoint( second_line->GetStartPoint() )
-                    && line->IsEndPoint( second_line->GetEndPoint() ) )
-                end_count--;
-            if( !line->IsParallel( second_line ) )
-                has_nonparallel = true;
+            // Consider ending on a line to be equivalent to two endpoints because
+            // we will want to split the line if anything else connects
+            if( !(*line)->IsEndPoint( aPosition ) )
+                end_count[i]++;
+
+            for( auto second_line = lines[i].end() - 1; second_line > line; second_line-- )
+            {
+                if( !(*line)->IsParallel( *second_line ) )
+                    has_nonparallel[i] = true;
+                else if( !removed_overlapping
+                         && (*line)->IsSameQuadrant( *second_line, aPosition ) )
+                {
+                    /**
+                     * Overlapping lines that point in the same direction should not be counted
+                     * as extra end_points.  We remove the overlapping lines, being careful to only
+                     * remove them once.
+                     */
+                    removed_overlapping = true;
+                    end_count[i]--;
+                }
+            }
         }
     }
 
-    // If there is line intersecting a pin
-    if( pin_count && has_line )
-        return true;
+    //
 
     // If there are three or more endpoints
-    if( pin_count + end_count > 2 )
+    if( pin_count + end_count[0] > 2 )
         return true;
 
     // If there is at least one segment that ends on a non-parallel line or
     // junction of two other lines
-    if( has_nonparallel && (has_line || end_count > 2 ) )
+    if( has_nonparallel[0] && end_count[0] > 2 )
+        return true;
+
+    // Check for bus - bus junction requirements
+    if( has_nonparallel[1] && end_count[1] > 2 )
         return true;
 
     return false;
-- 
2.11.0


Follow ups