Skip to content

Commit

Permalink
Remove .check_parameters() method from signatures.
Browse files Browse the repository at this point in the history
Since RFC 38, the signature of a component can only be assigned within
its constructor. It is no longer useful to decouple the validation of
signature parameters from its constructor.
  • Loading branch information
jfng committed May 17, 2024
1 parent 2dfd84c commit 58d7a69
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 177 deletions.
72 changes: 15 additions & 57 deletions amaranth_soc/csr/bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,17 @@ class Signature(wiring.Signature):
w_stb : Signal()
Write strobe. Registers should update their value or perform the write side effect when
this strobe is asserted.
Raises
------
See :meth:`Element.Signature.check_parameters`.
"""
def __init__(self, width, access):
self.check_parameters(width, access)
if not isinstance(width, int) or width < 0:
raise TypeError(f"Width must be a non-negative integer, not {width!r}")
# TODO(py3.9): Remove this. Python 3.8 and below use cls.__name__ in the error message
# instead of cls.__qualname__.
# Element.Access(access)
try:
Element.Access(access)
except ValueError as e:
raise ValueError(f"{access!r} is not a valid Element.Access") from e

self._width = width
self._access = Element.Access(access)
Expand All @@ -81,27 +85,6 @@ def width(self):
def access(self):
return self._access

@classmethod
def check_parameters(cls, width, access):
"""Validate signature parameters.
Raises
------
:exc:`TypeError`
If ``width`` is not an integer greater than or equal to 0.
:exc:`ValueError`
If ``access`` is not a member of :class:`Element.Access`.
"""
if not isinstance(width, int) or width < 0:
raise TypeError(f"Width must be a non-negative integer, not {width!r}")
# TODO(py3.9): Remove this. Python 3.8 and below use cls.__name__ in the error message
# instead of cls.__qualname__.
# Element.Access(access)
try:
Element.Access(access)
except ValueError as e:
raise ValueError(f"{access!r} is not a valid Element.Access") from e

def create(self, *, path=None, src_loc_at=0):
"""Create a compatible interface.
Expand Down Expand Up @@ -139,10 +122,6 @@ def __repr__(self):
Register access mode.
path : iter(:class:`str`)
Path to this CSR interface. Optional. See :class:`wiring.PureInterface`.
Raises
------
See :meth:`Element.Signature.check_parameters`
"""
def __init__(self, width, access, *, path=None, src_loc_at=0):
super().__init__(Element.Signature(width=width, access=access), path=path, src_loc_at=1 + src_loc_at)
Expand Down Expand Up @@ -188,13 +167,12 @@ class Signature(wiring.Signature):
to the register and causes write side effects to be performed (if any). If ``addr`` points
to any chunk of a register, latches ``w_data`` to the captured value. Otherwise, does
nothing.
Raises
------
See :meth:`Signature.check_parameters`.
"""
def __init__(self, *, addr_width, data_width):
self.check_parameters(addr_width=addr_width, data_width=data_width)
if not isinstance(addr_width, int) or addr_width <= 0:
raise TypeError(f"Address width must be a positive integer, not {addr_width!r}")
if not isinstance(data_width, int) or data_width <= 0:
raise TypeError(f"Data width must be a positive integer, not {data_width!r}")

self._addr_width = addr_width
self._data_width = data_width
Expand All @@ -216,22 +194,6 @@ def addr_width(self):
def data_width(self):
return self._data_width

@classmethod
def check_parameters(cls, *, addr_width, data_width):
"""Validate signature parameters.
Raises
------
:exc:`TypeError`
If ``addr_width`` is not an integer greater than 0.
:exc:`TypeError`
If ``data_width`` is not an integer greater than 0.
"""
if not isinstance(addr_width, int) or addr_width <= 0:
raise TypeError(f"Address width must be a positive integer, not {addr_width!r}")
if not isinstance(data_width, int) or data_width <= 0:
raise TypeError(f"Data width must be a positive integer, not {data_width!r}")

def create(self, *, path=None, src_loc_at=0):
"""Create a compatible interface.
Expand Down Expand Up @@ -288,14 +250,10 @@ class Interface(wiring.PureInterface):
----------
memory_map: :class:`MemoryMap`
Memory map of the bus. Optional.
Raises
------
See :meth:`Signature.check_parameters`.
"""
def __init__(self, *, addr_width, data_width, path=None, src_loc_at=0):
sig = Signature(addr_width=addr_width, data_width=data_width)
super().__init__(sig, path=path, src_loc_at=1 + src_loc_at)
super().__init__(Signature(addr_width=addr_width, data_width=data_width),
path=path, src_loc_at=1 + src_loc_at)
self._memory_map = None

@property
Expand Down
40 changes: 12 additions & 28 deletions amaranth_soc/csr/reg.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,20 @@ class Signature(wiring.Signature):
w_stb : Signal()
Write strobe. Fields should update their value or perform the write side effect when
this strobe is asserted.
Raises
------
See :meth:`FieldPort.Signature.check_parameters`.
"""
def __init__(self, shape, access):
self.check_parameters(shape, access)
try:
Shape.cast(shape)
except TypeError as e:
raise TypeError(f"Field shape must be a shape-castable object, not {shape!r}") from e
# TODO(py3.9): Remove this. Python 3.8 and below use cls.__name__ in the error message
# instead of cls.__qualname__.
# FieldPort.Access(access)
try:
FieldPort.Access(access)
except ValueError as e:
raise ValueError(f"{access!r} is not a valid FieldPort.Access") from e

self._shape = Shape.cast(shape)
self._access = FieldPort.Access(access)

Expand All @@ -76,29 +83,6 @@ def shape(self):
def access(self):
return self._access

@classmethod
def check_parameters(cls, shape, access):
"""Validate signature parameters.
Raises
------
:exc:`TypeError`
If ``shape`` is not a shape-castable object.
:exc:`ValueError`
If ``access`` is not a member of :class:`FieldPort.Access`.
"""
try:
Shape.cast(shape)
except TypeError as e:
raise TypeError(f"Field shape must be a shape-castable object, not {shape!r}") from e
# TODO(py3.9): Remove this. Python 3.8 and below use cls.__name__ in the error message
# instead of cls.__qualname__.
# FieldPort.Access(access)
try:
FieldPort.Access(access)
except ValueError as e:
raise ValueError(f"{access!r} is not a valid FieldPort.Access") from e

def create(self, *, path=None, src_loc_at=0):
"""Create a compatible interface.
Expand Down
33 changes: 8 additions & 25 deletions amaranth_soc/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,16 @@ class Signature(wiring.Signature):
Input line. Sampled in order to detect an event.
trg : Signal()
Event trigger. Asserted when an event occurs, according to the trigger mode.
Raises
------
See :meth:`Source.Signature.check_parameters`.
"""
def __init__(self, *, trigger="level"):
self.check_parameters(trigger=trigger)
# TODO(py3.9): Remove this. Python 3.8 and below use cls.__name__ in the error message
# instead of cls.__qualname__.
# Source.Trigger(trigger)
try:
Source.Trigger(trigger)
except ValueError as e:
raise ValueError(f"{trigger!r} is not a valid Source.Trigger") from e

super().__init__({
"i": Out(1),
"trg": In(1),
Expand All @@ -44,22 +47,6 @@ def __init__(self, *, trigger="level"):
def trigger(self):
return self._trigger

def check_parameters(cls, *, trigger):
"""Validate signature parameters.
Raises
------
:exc:`ValueError`
If ``trigger`` is not a member of :class:`Source.Trigger`.
"""
# TODO(py3.9): Remove this. Python 3.8 and below use cls.__name__ in the error message
# instead of cls.__qualname__.
# Source.Trigger(trigger)
try:
Source.Trigger(trigger)
except ValueError as e:
raise ValueError(f"{trigger!r} is not a valid Source.Trigger") from e

def create(self, *, path=None, src_loc_at=0):
"""Create a compatible interface.
Expand Down Expand Up @@ -94,10 +81,6 @@ def __repr__(self):
----------
event_map : :class:`EventMap`
A collection of event sources.
Raises
------
See :meth:`Source.Signature.check_parameters`.
"""
def __init__(self, *, trigger="level", path=None, src_loc_at=0):
super().__init__(Source.Signature(trigger=trigger), path=path, src_loc_at=1 + src_loc_at)
Expand Down
59 changes: 15 additions & 44 deletions amaranth_soc/wishbone/bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,10 @@ class Signature(wiring.Signature):
----------
addr_width : int
Width of the address signal.
data_width : int
data_width : ``8``, ``16``, ``32`` or ``64``
Width of the data signals ("port size" in Wishbone terminology).
One of 8, 16, 32, 64.
granularity : int
granularity : ``8``, ``16``, ``32``, ``64`` or ``None``
Granularity of select signals ("port granularity" in Wishbone terminology).
One of 8, 16, 32, 64.
Optional. If ``None`` (by default), the granularity is equal to ``data_width``.
features : iter(:class:`Feature`)
Selects additional signals that will be a part of this interface.
Expand Down Expand Up @@ -94,16 +92,22 @@ class Signature(wiring.Signature):
Optional. Corresponds to Wishbone signal ``CTI_O`` (initiator) or ``CTI_I`` (target).
bte : Signal()
Optional. Corresponds to Wishbone signal ``BTE_O`` (initiator) or ``BTE_I`` (target).
Raises
------
See :meth:`Signature.check_parameters`.
"""
def __init__(self, *, addr_width, data_width, granularity=None, features=frozenset()):
if granularity is None:
granularity = data_width
self.check_parameters(addr_width=addr_width, data_width=data_width, granularity=granularity,
features=features)
granularity = data_width

if not isinstance(addr_width, int) or addr_width < 0:
raise TypeError(f"Address width must be a non-negative integer, not {addr_width!r}")
if data_width not in (8, 16, 32, 64):
raise ValueError(f"Data width must be one of 8, 16, 32, 64, not {data_width!r}")
if granularity not in (8, 16, 32, 64):
raise ValueError(f"Granularity must be one of 8, 16, 32, 64, not {granularity!r}")
if granularity > data_width:
raise ValueError(f"Granularity {granularity} may not be greater than data width "
f"{data_width}")
for feature in features:
Feature(feature) # raises ValueError if feature is invalid

self._addr_width = addr_width
self._data_width = data_width
Expand Down Expand Up @@ -150,35 +154,6 @@ def granularity(self):
def features(self):
return self._features

@classmethod
def check_parameters(cls, *, addr_width, data_width, granularity, features):
"""Validate signature parameters.
Raises
------
:exc:`TypeError`
If ``addr_width`` is not an integer greater than or equal to 0.
:exc:`ValueError`
If ``data_width`` is not 8, 16, 32 or 64.
:exc:`ValueError`
If ``granularity`` is not 8, 16, 32 or 64.
:exc:`ValueError`
If ``granularity`` is greater than ``data_width``.
:exc:`ValueError`
If ``features`` contains other values than :class:`Feature` members.
"""
if not isinstance(addr_width, int) or addr_width < 0:
raise TypeError(f"Address width must be a non-negative integer, not {addr_width!r}")
if data_width not in (8, 16, 32, 64):
raise ValueError(f"Data width must be one of 8, 16, 32, 64, not {data_width!r}")
if granularity not in (8, 16, 32, 64):
raise ValueError(f"Granularity must be one of 8, 16, 32, 64, not {granularity!r}")
if granularity > data_width:
raise ValueError(f"Granularity {granularity} may not be greater than data width "
f"{data_width}")
for feature in features:
Feature(feature) # raises ValueError if feature is invalid

def create(self, *, path=None, src_loc_at=0):
"""Create a compatible interface.
Expand Down Expand Up @@ -232,10 +207,6 @@ class Interface(wiring.PureInterface):
----------
memory_map: :class:`MemoryMap`
Memory map of the bus. Optional.
Raises
------
See :meth:`Signature.check_parameters`.
"""
def __init__(self, *, addr_width, data_width, granularity=None, features=frozenset(),
path=None, src_loc_at=0):
Expand Down
5 changes: 1 addition & 4 deletions tests/test_csr_bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,14 +131,11 @@ def test_wrong_addr_width(self):
with self.assertRaisesRegex(TypeError,
r"Address width must be a positive integer, not -1"):
csr.Signature(addr_width=-1, data_width=8)
with self.assertRaisesRegex(TypeError,
r"Address width must be a positive integer, not -1"):
csr.Signature.check_parameters(addr_width=-1, data_width=8)

def test_wrong_data_width(self):
with self.assertRaisesRegex(TypeError,
r"Data width must be a positive integer, not -1"):
csr.Signature.check_parameters(addr_width=16, data_width=-1)
csr.Signature(addr_width=16, data_width=-1)


class InterfaceTestCase(unittest.TestCase):
Expand Down
19 changes: 0 additions & 19 deletions tests/test_wishbone_bus.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,44 +106,25 @@ def test_wrong_addr_width(self):
with self.assertRaisesRegex(TypeError,
r"Address width must be a non-negative integer, not -1"):
wishbone.Signature(addr_width=-1, data_width=8)
with self.assertRaisesRegex(TypeError,
r"Address width must be a non-negative integer, not -1"):
wishbone.Signature.check_parameters(addr_width=-1, data_width=8, granularity=8,
features=())

def test_wrong_data_width(self):
with self.assertRaisesRegex(ValueError,
r"Data width must be one of 8, 16, 32, 64, not 7"):
wishbone.Signature(addr_width=0, data_width=7)
with self.assertRaisesRegex(ValueError,
r"Data width must be one of 8, 16, 32, 64, not 7"):
wishbone.Signature.check_parameters(addr_width=0, data_width=7, granularity=7,
features=())

def test_wrong_granularity(self):
with self.assertRaisesRegex(ValueError,
r"Granularity must be one of 8, 16, 32, 64, not 7"):
wishbone.Signature(addr_width=0, data_width=32, granularity=7)
with self.assertRaisesRegex(ValueError,
r"Granularity must be one of 8, 16, 32, 64, not 7"):
wishbone.Signature.check_parameters(addr_width=0, data_width=32, granularity=7,
features=())

def test_wrong_granularity_wide(self):
with self.assertRaisesRegex(ValueError,
r"Granularity 32 may not be greater than data width 8"):
wishbone.Signature(addr_width=0, data_width=8, granularity=32)
with self.assertRaisesRegex(ValueError,
r"Granularity 32 may not be greater than data width 8"):
wishbone.Signature.check_parameters(addr_width=0, data_width=8, granularity=32,
features=())

def test_wrong_features(self):
with self.assertRaisesRegex(ValueError, r"'foo' is not a valid Feature"):
wishbone.Signature(addr_width=0, data_width=8, features={"foo"})
with self.assertRaisesRegex(ValueError, r"'foo' is not a valid Feature"):
wishbone.Signature.check_parameters(addr_width=0, data_width=8, granularity=8,
features={"foo"})


class InterfaceTestCase(unittest.TestCase):
Expand Down

0 comments on commit 58d7a69

Please sign in to comment.