From 53ddff9f338506533085bf3e7e8826fe5935bed0 Mon Sep 17 00:00:00 2001
From: whitequark <whitequark@whitequark.org>
Date: Sat, 1 Jun 2019 16:41:30 +0000
Subject: [PATCH] build.res: always return a Pin record.

In the simple cases, a Pin record consisting of exactly one field
is equivalent in every way to this single field. In the more complex
case however, it can be used as a record, making the code more robust
such that it works with both bidirectional and unidirectional pins.
---
 nmigen/build/res.py           | 31 +++++++++++++++----------------
 nmigen/test/test_build_res.py | 18 +++++++++++-------
 2 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/nmigen/build/res.py b/nmigen/build/res.py
index c14efaf..f6384c8 100644
--- a/nmigen/build/res.py
+++ b/nmigen/build/res.py
@@ -21,8 +21,8 @@ class ConstraintManager:
         self.clocks     = OrderedDict()
 
         self._ports     = []
-        self._tristates = []
-        self._diffpairs = []
+        self._se_pins   = []
+        self._dp_pins   = []
 
         self.add_resources(resources)
         for name_number, frequency in clocks:
@@ -133,19 +133,16 @@ class ConstraintManager:
                 yield (value, subsignal.io[0], subsignal.extras)
 
         for (pin, io, extras) in match_constraints(value, resource):
-            if isinstance(io, DiffPairs):
-                p = Signal(pin.width, name="{}_p".format(pin.name))
-                n = Signal(pin.width, name="{}_n".format(pin.name))
-                self._diffpairs.append((pin, p, n))
-                self._ports.append((p, io.p.names, extras))
-                self._ports.append((n, io.n.names, extras))
-            elif isinstance(io, Pins):
-                if pin.dir == "io":
-                    port = Signal(pin.width, name="{}_io".format(pin.name))
-                    self._tristates.append((pin, port))
-                else:
-                    port = getattr(pin, pin.dir)
+            if isinstance(io, Pins):
+                port = Signal(pin.width, name="{}_io".format(pin.name))
+                self._se_pins.append((pin, port))
                 self._ports.append((port, io.names, extras))
+            elif isinstance(io, DiffPairs):
+                p_port = Signal(pin.width, name="{}_p".format(pin.name))
+                n_port = Signal(pin.width, name="{}_n".format(pin.name))
+                self._dp_pins.append((pin, p_port, n_port))
+                self._ports.append((p_port, io.p.names, extras))
+                self._ports.append((n_port, io.n.names, extras))
             else:
                 assert False # :nocov:
 
@@ -169,8 +166,10 @@ class ConstraintManager:
                 raise ConstraintError("Cannot constrain frequency of resource {}#{} because "
                                       "it has been requested as a tristate buffer"
                                       .format(name, number))
-            if isinstance(resource.io[0], DiffPairs):
+            if isinstance(resource.io[0], Pins):
+                port_name = "{}_io".format(pin.name)
+            elif isinstance(resource.io[0], DiffPairs):
                 port_name = "{}_p".format(pin.name)
             else:
-                port_name = getattr(pin, pin.dir).name
+                assert False
             yield (port_name, period)
diff --git a/nmigen/test/test_build_res.py b/nmigen/test/test_build_res.py
index 43dc7bd..dac5ad5 100644
--- a/nmigen/test/test_build_res.py
+++ b/nmigen/test/test_build_res.py
@@ -64,10 +64,9 @@ class ConstraintManagerTestCase(FHDLTestCase):
 
         ports = list(self.cm.iter_ports())
         self.assertEqual(len(ports), 1)
-        self.assertIs(user_led.o, ports[0])
 
         self.assertEqual(list(self.cm.iter_port_constraints()), [
-            ("user_led_0__o", ["A0"], [])
+            ("user_led_0_io", ["A0"], [])
         ])
 
     def test_request_with_dir(self):
@@ -82,13 +81,16 @@ class ConstraintManagerTestCase(FHDLTestCase):
 
         ports = list(self.cm.iter_ports())
         self.assertEqual(len(ports), 2)
-        self.assertIs(i2c.scl.o, ports[0]),
+        scl, sda = ports
         self.assertEqual(ports[1].name, "i2c_0__sda_io")
         self.assertEqual(ports[1].nbits, 1)
 
-        self.assertEqual(self.cm._tristates, [(i2c.sda, ports[1])])
+        self.assertEqual(self.cm._se_pins, [
+            (i2c.scl, scl),
+            (i2c.sda, sda),
+        ])
         self.assertEqual(list(self.cm.iter_port_constraints()), [
-            ("i2c_0__scl__o", ["N10"], []),
+            ("i2c_0__scl_io", ["N10"], []),
             ("i2c_0__sda_io", ["N11"], [])
         ])
 
@@ -106,7 +108,9 @@ class ConstraintManagerTestCase(FHDLTestCase):
         self.assertEqual(n.name, "clk100_0_n")
         self.assertEqual(n.nbits, clk100.width)
 
-        self.assertEqual(self.cm._diffpairs, [(clk100, p, n)])
+        self.assertEqual(self.cm._dp_pins, [
+            (clk100, p, n),
+        ])
         self.assertEqual(list(self.cm.iter_port_constraints()), [
             ("clk100_0_p", ["H1"], []),
             ("clk100_0_n", ["H2"], [])
@@ -121,7 +125,7 @@ class ConstraintManagerTestCase(FHDLTestCase):
         clk50 = self.cm.request("clk50", 0, dir="i")
         self.assertEqual(list(sorted(self.cm.iter_clock_constraints())), [
             ("clk100_0_p", 10e6),
-            ("clk50_0__i", 5e6)
+            ("clk50_0_io", 5e6)
         ])
 
     def test_wrong_resources(self):
-- 
2.30.2