addressing feedback from @jepler

Author: siddacious

adafruit_mcp2515/__init__.py

@@ -5,7 +5,8 @@
 `adafruit_mcp2515`
 ================================================================================
 
-A CircuitPython library for working with the MCP2515 CAN bus controller
+A CircuitPython library for working with the MCP2515 CAN bus controller using the
+CircuitPython `canio` API
 
 
 * Author(s): Bryan Siepert
@@ -20,8 +21,7 @@
 * Adafruit CircuitPython firmware for the supported boards:
   https://github.com/adafruit/circuitpython/releases
 
-# * Adafruit's Bus Device library: https://github.com/adafruit/Adafruit_CircuitPython_BusDevice
-# * Adafruit's Register library: https://github.com/adafruit/Adafruit_CircuitPython_Register
+* Adafruit's Bus Device library: https://github.com/adafruit/Adafruit_CircuitPython_BusDevice
 """
 
 from collections import namedtuple
@@ -30,6 +30,7 @@
 from micropython import const
 import adafruit_bus_device.spi_device as spi_device
 from .canio import *
+from .timer import Timer
 
 __version__ = "0.0.0-auto.0"
 __repo__ = "https://github.com/adafruit/Adafruit_CircuitPython_MCP2515.git"
@@ -162,12 +163,11 @@
     "TransmitBuffer",
     ["CTRL_REG", "STD_ID_REG", "INT_FLAG_MASK", "LOAD_CMD", "SEND_CMD"],
 )
-# perhaps this will be stateful later? #TODO : dedup with above
-FilterMask = namedtuple(
-    "FilterMask", ["CTRL_REG", "STD_ID_REG", "INT_FLAG_MASK", "LOAD_CMD", "SEND_CMD"],
-)
 
+# This is magic, don't disturb the dragon
+# expects a 16Mhz crystal
 _BAUD_RATES = {
+    # CNF1, CNF2, CNF3
     1000000: (0x00, 0xD0, 0x82),
     500000: (0x00, 0xF0, 0x86),
     250000: (0x41, 0xF1, 0x85),
@@ -209,7 +209,24 @@ def _tx_buffer_status_decode(status_byte):
 
 
 class MCP2515:  # pylint:disable=too-many-instance-attributes
-    """Library for the MCP2515 CANbus controller"""
+    """
+    A common shared-bus protocol.
+
+    :param ~busio.SPI spi: The SPI bus used to communicate with the MCP2515
+    :param ~digitalio.DigitalInOut cs_pin:  SPI bus enable pin
+    :param int baudrate: The bit rate of the bus in Hz, using a 16Mhz crystal. All devices on\
+        the bus must agree on this value. Defaults to 250000.
+    :param bool loopback: Receive only packets sent from this device, and send only to\
+        this device. Requires that `silent` is also set to `False`, but only prevents\
+        transimssion to other devices. Otherwise the send/receive behavior is normal.
+    :param bool silent:When `True` the controller does not transmit and all messages\
+        are received, ignoring errors and filters. This mode can be used to “sniff” a CAN\
+        bus without interfering. Defaults to `False`.
+    :param bool auto_restart: **Not supported by hardware. An `AttributeError`
+        will be raised if `auto_restart` is set to `True`** If `True`, will restart\
+        communications after entering bus-off state. Defaults to `False`.
+    :param bool debug: If `True`, will enable printing debug information. Defaults to `False`.
+    """
 
     def __init__(
         self,
@@ -222,24 +239,12 @@ def __init__(
         auto_restart: bool = False,
         debug: bool = False
     ):
-        """[summary]
 
-        Args:
-            spi_bus (busio.SPI): The SPI bus used to communicate with the MCP2515
-            cs_pin (digitalio.DigitalInOut): SPI bus enable pin
-            baudrate (int, optional):  The bit rate of the bus in Hz. All devices on the bus must \
-                agree on this value. Defaults to 250000.
-            loopback (bool, optional): When True the rx pin’s value is ignored, and the device \
-                receives the packets it sends. Defaults to False.
-            silent (bool, optional): When True the tx pin is always driven to the high logic level.\
-                 This mode can be used to “sniff” a CAN bus without interfering.. Defaults to False.
-            auto_restart (bool, optional): If True, will restart communications after entering \
-                bus-off state. Defaults to False.
-            debug (bool, optional): If True, will enable printing debug information. Defaults to \
-                False.
-        """
-        if loopback and silent:
-            raise AttributeError("only loopback or silent mode can bet set, not both")
+        if loopback and not silent:
+            raise AttributeError("Loopback mode requires silent to be set")
+        if auto_restart:
+            raise AttributeError("`auto-restart` is not supported by hardware")
+
         self._auto_restart = auto_restart
         self._debug = debug
         self._bus_device_obj = spi_device.SPIDevice(spi_bus, cs_pin)
@@ -331,6 +336,7 @@ def send(self, message_obj, wait_sent=True):
         Args:
             message (canio.Message): The message to send. Must be a valid `canio.Message`
         """
+
         # TODO: Timeout
         tx_buff = self._get_tx_buffer()  # info = addr.
 
@@ -350,7 +356,7 @@ def send(self, message_obj, wait_sent=True):
             if send_confirmed:
                 return True
 
-        raise RuntimeError("Timeout occoured waiting for transmit confirmation")
+        raise RuntimeError("Timeout occurred waiting for transmit confirmation")
 
     @property
     def unread_message_count(self):
@@ -830,7 +836,7 @@ def listen(self, matches=None, *, timeout: float = 10):
 
     An empty filter list causes all messages to be accepted.
 
-    Timeout dictates how long ``receive()`` and ``next()`` will block.
+    Timeout dictates how long ``receive()`` will block.
 
         Args:
             match (Optional[Sequence[Match]], optional): [description]. Defaults to None.
@@ -841,6 +847,11 @@ def listen(self, matches=None, *, timeout: float = 10):
         """
         if matches is None:
             matches = []
+        elif self.silent and not self.loopback:
+            raise AttributeError(
+                "Hardware does not support setting `matches` in when\
+                `silent`==`True` and `loopback` == `False`"
+            )
 
         for match in matches:
             self._dbg("match:", match)

adafruit_mcp2515/canio/__init__.py

@@ -4,11 +4,11 @@
 """Python implementation of the CircuitPython core `canio` API"""
 # pylint:disable=too-few-public-methods, invalid-name, redefined-builtin
 import time
+from ..timer import Timer
 
 
 class Message:
-    """A class representing a message to send on a `canio` bus
-    """
+    """A class representing a CANbus data frame"""
 
     # pylint:disable=too-many-arguments,invalid-name,redefined-builtin
     def __init__(self, id, data, extended=False):
@@ -50,10 +50,10 @@ def data(self, new_data):
 
 
 class RemoteTransmissionRequest:
-    """RRTTTTRRRR """
+    """A class representing a CANbus remote frame"""
 
     def __init__(self, id: int, length: int, *, extended: bool = False):
-        """Construct a Message to send on a CAN bus
+        """Construct a RemoteTransmissionRequest to send on a CAN bus
 
         Args:
             id (int): The numeric ID of the requested message
@@ -205,29 +205,3 @@ def __init__(self, address: int, *, mask: int = 0, extended: bool = False):
         self.address = address
         self.mask = mask
         self.extended = extended
-
-
-############# non-api classes and methods
-class Timer:
-    """A class to track timeouts, like an egg timer
-    """
-
-    def __init__(self, timeout=0.0):
-        self._timeout = None
-        self._start_time = None
-        if timeout:
-            self.rewind_to(timeout)
-
-    @property
-    def expired(self):
-        """Returns the expiration status of the timer
-
-        Returns:
-            bool: True if more than `timeout` seconds has past since it was set
-        """
-        return (time.monotonic() - self._start_time) > self._timeout
-
-    def rewind_to(self, new_timeout):
-        """Re-wind the timer to a new timeout and start ticking"""
-        self._timeout = float(new_timeout)
-        self._start_time = time.monotonic()

adafruit_mcp2515/timer.py

@@ -0,0 +1,29 @@
+# SPDX-FileCopyrightText: Copyright (c) 2020 Bryan Siepert for Adafruit Industries
+#
+# SPDX-License-Identifier: MIT
+"""Provides a simple timer class; see `Timer`"""
+from time import monotonic
+
+
+class Timer:
+    """A reusable class to track timeouts, like an egg timer"""
+
+    def __init__(self, timeout=0.0):
+        self._timeout = None
+        self._start_time = None
+        if timeout:
+            self.rewind_to(timeout)
+
+    @property
+    def expired(self):
+        """Returns the expiration status of the timer
+
+        Returns:
+            bool: True if more than `timeout` seconds has past since it was set
+        """
+        return (monotonic() - self._start_time) > self._timeout
+
+    def rewind_to(self, new_timeout):
+        """Re-wind the timer to a new timeout and start ticking"""
+        self._timeout = float(new_timeout)
+        self._start_time = monotonic()

examples/canio_test.py

@@ -1,6 +1,14 @@
 # SPDX-FileCopyrightText: Copyright (c) 2020 Jeff Epler for Adafruit Industries
 #
 # SPDX-License-Identifier: MIT
+
+###################### Board/ Config selection #################
+# This is is only on way to make this test work for other boards.
+# As an alternative, CAN_TYPE could be set manually and
+# board-specific expectations can be if/else'd, and support for a
+# new CAN controller or peripheral would start with defining the
+# controller-specifc bits
+
 CAN_TYPE = None
 try:
     from canio import (
@@ -37,8 +45,10 @@ def builtin_bus_factory():
     def builtin_bus_factory():
         cs = DigitalInOut(board.D5)
         cs.switch_to_output()
-        return CAN(board.SPI(), cs, baudrate=1000000, loopback=True)
+        return CAN(board.SPI(), cs, baudrate=1000000, loopback=True, silent=True)
+
 
+################################################################
 
 max_standard_id = 0x7FF
 max_extended_id = 0x1FFFFFFF

examples/mcp2515_simpletest.py

@@ -1,25 +1,31 @@
 # SPDX-FileCopyrightText: Copyright (c) 2020 Bryan Siepert for Adafruit Industries
 #
 # SPDX-License-Identifier: MIT
-
 from time import sleep
-from board import SPI, D5 as CS_PIN
-import digitalio
-import adafruit_mcp2515
+import board
+import busio
+from digitalio import DigitalInOut
 from adafruit_mcp2515.canio import Message
+from adafruit_mcp2515 import MCP2515 as CAN
+
 
-cs = digitalio.DigitalInOut(CS_PIN)
+cs = DigitalInOut(board.D5)
 cs.switch_to_output()
-mcp = adafruit_mcp2515.MCP2515(SPI(), cs)
+spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
 
-max_ext_id = 0x3FFFF
-mb1 = [0xDE, 0xAD, 0xBE, 0xEF]
-mb2 = [0xCA, 0xFE, 0xFA, 0xDE]
-can_id = 0b100000000000000001
+can_bus = CAN(
+    spi, cs, loopback=True, silent=True
+)  # use loopback to test without another device
 while True:
-    mb1.insert(0, mb2.pop())
-    mb2.insert(0, mb1.pop())
-    message = Message(id=0xFFAA, data=bytes(mb1 + mb2), extended=True)
-    mcp.send(message)
-    sleep(0.3)
-    print("*" * 40)
+    with can_bus.listen(timeout=1.0) as listener:
+
+        message = Message(id=0x1234ABCD, data=b"adafruit", extended=True)
+        can_bus.send(message)
+        message_count = listener.in_waiting()
+        print(message_count, "messages available")
+        for _i in range(message_count):
+            msg = listener.receive()
+            print("Message from ", hex(msg.id))
+            print("message data:", msg.data)
+            print("")
+    sleep(1)