kicad-developers team mailing list archive

[Seth Hillbrand <seth.hillbrand@xxxxxxxxx>]

The attached patch adjusts how dangling ends are shown for wire-bus entries
to correct the behavior Nick reported.

Previously, wire-bus entries were shown as dangling if there was a wire or
a bus at both ends.  This broke in the case of the Coldfire demo where a
bus was drawn over a wire and there was also an entry point (see attached
image coldfire-broken.png).

The patch implements the logic that if there is _at least_ a wire and a bus
at each end, that the entry is considered not dangling. (see attached image
coldfire-fixed.png)

-Seth

Attachment: coldfire-broken.png
Description: PNG image

Attachment: coldfire-fixed.png
Description: PNG image

From 81b9ecd25b46c665ec372d2ea32a409be5b1b797 Mon Sep 17 00:00:00 2001
From: Seth Hillbrand <hillbrand@xxxxxxxxxxx>
Date: Fri, 15 Dec 2017 10:01:19 -0800
Subject: [PATCH] Eeschema: Mark wire-bus entries correctly

Marks wire-bus entries as not dangling if there is at least a wire
and a bus on each end.  Corrects behavior when wires and buses overlap
at the endpoint.
---
 eeschema/sch_bus_entry.cpp | 73 +++++++++++++++++++++++++++++++++++++---------
 eeschema/sch_bus_entry.h   |  6 ++--
 2 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/eeschema/sch_bus_entry.cpp b/eeschema/sch_bus_entry.cpp
index a9708103c..01bd70b9d 100644
--- a/eeschema/sch_bus_entry.cpp
+++ b/eeschema/sch_bus_entry.cpp
@@ -201,7 +201,7 @@ void SCH_BUS_ENTRY_BASE::Rotate( wxPoint aPosition )
 }
 
 
-bool SCH_BUS_ENTRY_BASE::IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList )
+bool SCH_BUS_WIRE_ENTRY::IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList )
 {
     bool previousStateStart = m_isDanglingStart;
     bool previousStateEnd = m_isDanglingEnd;
@@ -213,11 +213,9 @@ bool SCH_BUS_ENTRY_BASE::IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>&
     // when the end position is found.
     wxPoint seg_start;
 
-    // Special case: if both items are wires, show as dangling. This is because
-    // a bus entry between two wires will look like a connection, but does NOT
-    // actually represent one. We need to clarify this for the user.
-    bool start_is_wire = false;
-    bool end_is_wire = false;
+    // Store the connection type and state for the start (0) and end (1)
+    bool has_wire[2] = { false };
+    bool has_bus[2] = { false };
 
     for( DANGLING_END_ITEM& each_item : aItemList )
     {
@@ -233,11 +231,64 @@ bool SCH_BUS_ENTRY_BASE::IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>&
 
         case WIRE_END_END:
             if( IsPointOnSegment( seg_start, each_item.GetPosition(), m_pos ) )
-                start_is_wire = true;
+                has_wire[0] = true;
+
             if( IsPointOnSegment( seg_start, each_item.GetPosition(), m_End() ) )
-                end_is_wire = true;
-            // Fall through
+                has_wire[1] = true;
+
+            break;
+
+        case BUS_END_END:
+            if( IsPointOnSegment( seg_start, each_item.GetPosition(), m_pos ) )
+                has_bus[0] = true;
+
+            if( IsPointOnSegment( seg_start, each_item.GetPosition(), m_End() ) )
+                has_bus[1] = true;
+
+            break;
+
+        default:
+            break;
+        }
+    }
+
+    /**
+     * A bus-wire entry is connected at both ends if it has a bus and a wire on its
+     * ends.  Otherwise, we connect only one end (in the case of a wire-wire or bus-bus)
+     */
+    if( ( has_wire[0] && has_bus[1] ) || ( has_wire[1] && has_bus[0] ) )
+        m_isDanglingEnd = m_isDanglingStart = false;
+    else if( has_wire[0] || has_bus[0] )
+        m_isDanglingStart = false;
+    else if( has_wire[1] || has_bus[1] )
+        m_isDanglingEnd = false;
+
+    return (previousStateStart != m_isDanglingStart) || (previousStateEnd != m_isDanglingEnd);
+}
+
+
+bool SCH_BUS_BUS_ENTRY::IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList )
+{
+    bool previousStateStart = m_isDanglingStart;
+    bool previousStateEnd = m_isDanglingEnd;
 
+    m_isDanglingStart = m_isDanglingEnd = true;
+
+    // Wires and buses are stored in the list as a pair, start and end. This
+    // variable holds the start position from one iteration so it can be used
+    // when the end position is found.
+    wxPoint seg_start;
+
+    for( DANGLING_END_ITEM& each_item : aItemList )
+    {
+        if( each_item.GetItem() == this )
+            continue;
+
+        switch( each_item.GetType() )
+        {
+        case BUS_START_END:
+            seg_start = each_item.GetPosition();
+            break;
         case BUS_END_END:
             if( IsPointOnSegment( seg_start, each_item.GetPosition(), m_pos ) )
                 m_isDanglingStart = false;
@@ -249,10 +300,6 @@ bool SCH_BUS_ENTRY_BASE::IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>&
         }
     }
 
-    // See above: show as dangling if joining two wires
-    if( start_is_wire && end_is_wire )
-        m_isDanglingStart = m_isDanglingEnd = true;
-
     return (previousStateStart != m_isDanglingStart) || (previousStateEnd != m_isDanglingEnd);
 }
 
diff --git a/eeschema/sch_bus_entry.h b/eeschema/sch_bus_entry.h
index bfe18cecd..fc51873c9 100644
--- a/eeschema/sch_bus_entry.h
+++ b/eeschema/sch_bus_entry.h
@@ -97,8 +97,6 @@ public:
 
     void Rotate( wxPoint aPosition ) override;
 
-    bool IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList ) override;
-
     bool IsDangling() const override;
 
     bool IsUnconnected() const override { return m_isDanglingEnd && m_isDanglingStart; }
@@ -151,6 +149,8 @@ public:
     EDA_ITEM* Clone() const override;
 
     BITMAP_DEF GetMenuImage() const override;
+
+    bool IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList ) override;
 };
 
 /**
@@ -177,6 +177,8 @@ public:
     EDA_ITEM* Clone() const override;
 
     BITMAP_DEF GetMenuImage() const override;
+
+    bool IsDanglingStateChanged( std::vector<DANGLING_END_ITEM>& aItemList ) override;
 };
 
 #endif    // _SCH_BUS_ENTRY_H_
-- 
2.11.0