examples/ex17_loop_labels.yml

@@ -1,23 +0,0 @@
-# Connector loops using pin labels
-# RS-232 loopback adapter: RTS-CTS and DSR-DTR-DCD jumpered,
-# with TX/RX/GND passed through to a cable.
-
-connectors:
-  X1:
-    type: D-Sub
-    subtype: female
-    pinlabels: [DCD, RX, TX, DTR, GND, DSR, RTS, CTS, RI]
-    loops:
-      - [RTS, CTS]      # pin labels instead of numbers
-      - [DSR, DTR]
-      - [4, DCD]         # mixed: pin number + label
-
-cables:
-  W1:
-    wirecount: 3
-    colors: [BK, RD, GN]
-
-connections:
-  -
-    - X1: [RX, TX, GND]  # labels in connections too
-    - W1: [1-3]

src/wireviz/DataClasses.py

@@ -203,81 +203,24 @@ class Connector:
             # hide pincount for simple (1 pin) connectors by default
             self.show_pincount = self.style != "simple"
 
-        for i, loop in enumerate(self.loops):
+        for loop in self.loops:
+            # TODO: allow using pin labels in addition to pin numbers, just like when defining regular connections
             # TODO: include properties of wire used to create the loop
             if len(loop) != 2:
                 raise Exception("Loops must be between exactly two pins!")
-            resolved = []
             for pin in loop:
-                pin = self.resolve_pin(pin)
+                if pin not in self.pins:
-                resolved.append(pin)
+                    raise Exception(
+                        f'Unknown loop pin "{pin}" for connector "{self.name}"!'
+                    )
                 # Make sure loop connected pins are not hidden.
                 self.activate_pin(pin, None)
-            if resolved[0] == resolved[1]:
-                raise Exception(
-                    f'Loop in connector "{self.name}" connects pin '
-                    f'"{resolved[0]}" to itself.'
-                )
-            self.loops[i] = resolved
 
         for i, item in enumerate(self.additional_components):
             if isinstance(item, dict):
                 self.additional_components[i] = AdditionalComponent(**item)
 
-    def resolve_pin(self, pin: Pin) -> Pin:
+    def activate_pin(self, pin: Pin, side: Side) -> None:
-        """Resolve a pin identifier to its canonical pin number.
-
-        Given a value that may be either a pin number (from self.pins)
-        or a pin label (from self.pinlabels), returns the corresponding
-        pin number from self.pins.
-
-        Callers needing a positional index should use
-        self.pins.index(return_value).
-
-        Resolution order:
-            1. Value in both pins and pinlabels at the same position
-               -> return directly (no ambiguity).
-            2. Value in both at different positions -> raise.
-            3. Value only in pinlabels -> return corresponding pin number.
-            4. Value only in pins -> return directly.
-            5. Not found -> raise.
-
-        Note: Lookups are type-sensitive (int 1 != str "1").
-        """
-        in_pins = pin in self.pins
-        in_labels = pin in self.pinlabels if self.pinlabels else False
-
-        if in_pins and in_labels:
-            # present in both lists — check for duplicate labels first
-            if self.pinlabels.count(pin) > 1:
-                raise Exception(
-                    f'Pin label "{pin}" in connector "{self.name}" '
-                    f"is defined more than once in pinlabels."
-                )
-            # then check for positional ambiguity
-            if self.pins.index(pin) != self.pinlabels.index(pin):
-                raise Exception(
-                    f'"{pin}" in connector "{self.name}" exists in both '
-                    f"pins and pinlabels at different positions."
-                )
-            return pin  # same position, no ambiguity
-
-        if in_labels:
-            if self.pinlabels.count(pin) > 1:
-                raise Exception(
-                    f'Pin label "{pin}" in connector "{self.name}" '
-                    f"is defined more than once."
-                )
-            return self.pins[self.pinlabels.index(pin)]
-
-        if in_pins:
-            return pin
-
-        raise Exception(
-            f'Unknown pin "{pin}" for connector "{self.name}"!'
-        )
-
-    def activate_pin(self, pin: Pin, side: Optional[Side]) -> None:
         self.visible_pins[pin] = True
         if side == Side.LEFT:
             self.ports_left = True

src/wireviz/Harness.py

@@ -105,17 +105,28 @@ class Harness:
         to_name: str,
         to_pin: (int, str),
     ) -> None:
-        # resolve pin labels to pin numbers via Connector.resolve_pin()
+        # check from and to connectors
-        for name, pin, is_from in [
+        for name, pin in zip([from_name, to_name], [from_pin, to_pin]):
-            (from_name, from_pin, True),
-            (to_name, to_pin, False),
-        ]:
             if name is not None and name in self.connectors:
-                resolved = self.connectors[name].resolve_pin(pin)
+                connector = self.connectors[name]
-                if is_from:
+                # check if provided name is ambiguous
-                    from_pin = resolved
+                if pin in connector.pins and pin in connector.pinlabels:
-                else:
+                    if connector.pins.index(pin) != connector.pinlabels.index(pin):
-                    to_pin = resolved
+                        raise Exception(
+                            f"{name}:{pin} is defined both in pinlabels and pins, for different pins."
+                        )
+                    # TODO: Maybe issue a warning if present in both lists but referencing the same pin?
+                if pin in connector.pinlabels:
+                    if connector.pinlabels.count(pin) > 1:
+                        raise Exception(f"{name}:{pin} is defined more than once.")
+                    index = connector.pinlabels.index(pin)
+                    pin = connector.pins[index]  # map pin name to pin number
+                    if name == from_name:
+                        from_pin = pin
+                    if name == to_name:
+                        to_pin = pin
+                if not pin in connector.pins:
+                    raise Exception(f"{name}:{pin} not found.")
 
         # check via cable
         if via_name in self.cables:
@@ -269,14 +280,9 @@ class Harness:
                 else:
                     raise Exception("No side for loops")
                 for loop in connector.loops:
-                    # Convert pin numbers to 1-based port indices.
-                    # Ports are named p{index+1} in the connector table,
-                    # not p{pin_number} — these differ for non-sequential pins.
-                    idx0 = connector.pins.index(loop[0]) + 1
-                    idx1 = connector.pins.index(loop[1]) + 1
                     dot.edge(
-                        f"{connector.name}:p{idx0}{loop_side}:{loop_dir}",
+                        f"{connector.name}:p{loop[0]}{loop_side}:{loop_dir}",
-                        f"{connector.name}:p{idx1}{loop_side}:{loop_dir}",
+                        f"{connector.name}:p{loop[1]}{loop_side}:{loop_dir}",
                         label=" ",  # Work-around to avoid over-sized loops.
                     )
 

tests/test_resolve_pin.py

@@ -1,297 +0,0 @@
-# -*- coding: utf-8 -*-
-
-"""Tests for Connector.resolve_pin() and loop pin resolution.
-
-Covers the fix for https://github.com/wireviz/WireViz/issues/432
-and related safety-critical pin resolution correctness.
-"""
-
-import pytest
-
-from wireviz.DataClasses import Connector
-
-
-def make_connector(pins=None, pinlabels=None, loops=None, **kwargs):
-    """Helper to build a Connector with minimal required fields."""
-    args = {"name": "X1"}
-    if pins is not None:
-        args["pins"] = pins
-    if pinlabels is not None:
-        args["pinlabels"] = pinlabels
-    if loops is not None:
-        args["loops"] = loops
-    args.update(kwargs)
-    return Connector(**args)
-
-
-# --- resolve_pin() happy paths ---
-
-
-class TestResolvePinHappyPaths:
-    def test_pin_number_passthrough(self):
-        """Pin number that exists in self.pins returns unchanged."""
-        c = make_connector(pins=[1, 2, 3])
-        assert c.resolve_pin(2) == 2
-
-    def test_label_resolves_to_pin_number(self):
-        """Pin label resolves to the corresponding pin number."""
-        c = make_connector(pins=[1, 2, 3], pinlabels=["VCC", "GND", "SIG"])
-        assert c.resolve_pin("GND") == 2
-
-    def test_label_with_non_sequential_pins(self):
-        """Labels work correctly even when pin numbers are non-sequential."""
-        c = make_connector(pins=[10, 20, 30], pinlabels=["A", "B", "C"])
-        assert c.resolve_pin("B") == 20
-
-    def test_value_in_both_lists_same_position(self):
-        """Value that exists in both pins and pinlabels at the same index."""
-        c = make_connector(pins=["A", "B", "C"], pinlabels=["A", "X", "Y"])
-        assert c.resolve_pin("A") == "A"
-
-    def test_empty_pinlabels_falls_through(self):
-        """When pinlabels is empty, pin numbers still resolve."""
-        c = make_connector(pins=[1, 2, 3], pinlabels=[])
-        assert c.resolve_pin(3) == 3
-
-
-# --- resolve_pin() error paths ---
-
-
-class TestResolvePinErrors:
-    def test_unknown_pin_raises(self):
-        """Resolving a pin that doesn't exist in either list raises."""
-        c = make_connector(pins=[1, 2, 3], pinlabels=["A", "B", "C"])
-        with pytest.raises(Exception, match="Unknown pin"):
-            c.resolve_pin("NONEXISTENT")
-
-    def test_unknown_number_raises(self):
-        """Resolving a pin number not in self.pins raises."""
-        c = make_connector(pins=[1, 2, 3])
-        with pytest.raises(Exception, match="Unknown pin"):
-            c.resolve_pin(99)
-
-    def test_ambiguous_pin_different_positions(self):
-        """Value in both pins and pinlabels at different positions raises."""
-        c = make_connector(pins=["A", "B", "C"], pinlabels=["X", "A", "Y"])
-        with pytest.raises(Exception, match="exists in both"):
-            c.resolve_pin("A")
-
-    def test_duplicate_label_only_in_labels(self):
-        """Duplicate label in pinlabels (label-only path) raises."""
-        c = make_connector(pins=[1, 2, 3], pinlabels=["A", "B", "A"])
-        with pytest.raises(Exception, match="defined more than once"):
-            c.resolve_pin("A")
-
-    def test_duplicate_label_in_both_lists(self):
-        """Duplicate label detected even when value also exists in pins (C-2 fix)."""
-        c = make_connector(pins=["A", "B", "C"], pinlabels=["A", "X", "A"])
-        with pytest.raises(Exception, match="defined more than once"):
-            c.resolve_pin("A")
-
-
-# --- Loop resolution ---
-
-
-class TestLoopResolution:
-    def test_loop_with_labels(self):
-        """Loops specified with pin labels resolve to pin numbers."""
-        c = make_connector(
-            pins=[1, 2, 3, 4],
-            pinlabels=["VCC", "GND", "TX", "RX"],
-            loops=[["VCC", "GND"]],
-        )
-        assert c.loops == [[1, 2]]
-
-    def test_loop_with_mixed_number_and_label(self):
-        """Loop with one pin number and one label resolves correctly."""
-        c = make_connector(
-            pins=[1, 2, 3, 4],
-            pinlabels=["VCC", "GND", "TX", "RX"],
-            loops=[[1, "GND"]],
-        )
-        assert c.loops == [[1, 2]]
-
-    def test_loop_with_non_sequential_pins(self):
-        """Loops resolve correctly with non-sequential pin numbering."""
-        c = make_connector(
-            pins=[10, 20, 30, 40],
-            pinlabels=["VCC", "GND", "TX", "RX"],
-            loops=[["VCC", "GND"]],
-        )
-        assert c.loops == [[10, 20]]
-
-    def test_loop_self_reference_raises(self):
-        """A loop from a pin to itself raises (I-2 fix)."""
-        with pytest.raises(Exception, match="to itself"):
-            make_connector(
-                pins=[1, 2, 3],
-                pinlabels=["A", "B", "C"],
-                loops=[["A", "A"]],
-            )
-
-    def test_loop_self_reference_via_label_and_number(self):
-        """Self-loop detected even when specified as label + number."""
-        with pytest.raises(Exception, match="to itself"):
-            make_connector(
-                pins=[1, 2, 3],
-                pinlabels=["A", "B", "C"],
-                loops=[[2, "B"]],
-            )
-
-    def test_loop_activates_pins(self):
-        """Loop-connected pins are marked visible (for hide_disconnected_pins)."""
-        c = make_connector(
-            pins=[1, 2, 3, 4],
-            pinlabels=["VCC", "GND", "TX", "RX"],
-            loops=[["TX", "RX"]],
-        )
-        assert c.visible_pins.get(3) is True
-        assert c.visible_pins.get(4) is True
-        # Unconnected pins should not be in visible_pins
-        assert 1 not in c.visible_pins
-
-    def test_multiple_loops(self):
-        """Multiple loops on the same connector all resolve correctly."""
-        c = make_connector(
-            pins=[1, 2, 3, 4, 5, 6],
-            pinlabels=["A", "B", "C", "D", "E", "F"],
-            loops=[["A", "B"], ["E", "F"]],
-        )
-        assert c.loops == [[1, 2], [5, 6]]
-
-
-# --- Loop rendering: non-sequential pins (C-1 fix) ---
-
-
-class TestLoopRendering:
-    """Verify that the GraphViz output uses port indices, not pin numbers.
-
-    This is the critical C-1 fix: Harness.py must convert resolved pin
-    numbers to 1-based position indices for GraphViz port references.
-    """
-
-    def _render_gv(self, yaml_str):
-        """Parse a YAML string through WireViz and return the GV source."""
-        import yaml
-
-        from wireviz.DataClasses import Metadata, Options, Tweak
-        from wireviz.Harness import Harness
-
-        data = yaml.safe_load(yaml_str)
-        harness = Harness(
-            metadata=Metadata(data.get("metadata", {})),
-            options=Options(**data.get("options", {})),
-            tweak=Tweak(**data.get("tweak", {})),
-        )
-        for name, conn_data in data.get("connectors", {}).items():
-            harness.add_connector(name, **conn_data)
-        for name, cable_data in data.get("cables", {}).items():
-            harness.add_cable(name, **cable_data)
-
-        graph = harness.create_graph()
-        return graph.source
-
-    def test_non_sequential_pins_use_indices(self):
-        """C-1 regression test: loops must use port indices, not pin numbers.
-
-        With pins: [10, 20, 30, 40], a loop between pins 10 and 20
-        must produce port references p1 and p2 (1-based indices),
-        NOT p10 and p20 (pin numbers).
-        """
-        yaml_str = """
-connectors:
-  X1:
-    pins: [10, 20, 30, 40]
-    pinlabels: [VCC, GND, TX, RX]
-    loops:
-      - [VCC, GND]
-cables:
-  W1:
-    wirecount: 2
-    colors: [BK, RD]
-connections:
-  -
-    - X1: [TX, RX]
-    - W1: [1-2]
-"""
-        gv = self._render_gv(yaml_str)
-        # The loop should reference p1 and p2 (positions), not p10 and p20.
-        # Look specifically for the loop edge pattern with port names.
-        import re
-
-        loop_edges = re.findall(r"X1:p(\d+)\w:\w -- X1:p(\d+)\w:\w", gv)
-        assert len(loop_edges) == 1, f"Expected 1 loop edge, found {len(loop_edges)}"
-        idx_a, idx_b = loop_edges[0]
-        # Pin 10 is at index 1, pin 20 is at index 2
-        assert idx_a == "1" and idx_b == "2", (
-            f"Loop ports should be p1/p2 (indices), got p{idx_a}/p{idx_b}"
-        )
-
-    def test_sequential_pins_still_work(self):
-        """Sequential pins (the common case) continue to work correctly."""
-        yaml_str = """
-connectors:
-  X1:
-    pinlabels: [VCC, GND, TX, RX]
-    loops:
-      - [VCC, GND]
-cables:
-  W1:
-    wirecount: 2
-    colors: [BK, RD]
-connections:
-  -
-    - X1: [TX, RX]
-    - W1: [1-2]
-"""
-        gv = self._render_gv(yaml_str)
-        # With sequential pins [1,2,3,4], p1/p2 are both the index and number
-        assert ":p1" in gv
-        assert ":p2" in gv
-
-
-# --- Harness.connect() delegation (I-1 fix) ---
-
-
-class TestHarnessConnectDelegation:
-    """Verify Harness.connect() now delegates to Connector.resolve_pin()."""
-
-    def test_connect_resolves_labels(self):
-        """Wire connections using pin labels resolve correctly."""
-        from wireviz.DataClasses import Cable, Metadata, Options, Tweak
-        from wireviz.Harness import Harness
-
-        harness = Harness(
-            metadata=Metadata({}),
-            options=Options(),
-            tweak=Tweak(),
-        )
-        harness.add_connector(
-            "X1", pins=[1, 2, 3], pinlabels=["VCC", "GND", "SIG"]
-        )
-        harness.add_connector("X2", pins=[1, 2, 3])
-        harness.add_cable("W1", wirecount=1, colors=["BK"])
-
-        # Connect using a label on the from side
-        harness.connect("X1", "SIG", "W1", 1, "X2", 1)
-
-        # The connection should store the resolved pin number (3), not the label
-        conn = harness.cables["W1"].connections[0]
-        assert conn.from_pin == 3
-
-    def test_connect_rejects_unknown_pin(self):
-        """Wire connections with unknown pins raise via resolve_pin()."""
-        from wireviz.DataClasses import Metadata, Options, Tweak
-        from wireviz.Harness import Harness
-
-        harness = Harness(
-            metadata=Metadata({}),
-            options=Options(),
-            tweak=Tweak(),
-        )
-        harness.add_connector("X1", pins=[1, 2, 3])
-        harness.add_connector("X2", pins=[1, 2, 3])
-        harness.add_cable("W1", wirecount=1, colors=["BK"])
-
-        with pytest.raises(Exception, match="Unknown pin"):
-            harness.connect("X1", 99, "W1", 1, "X2", 1)