Make full use of all hardware acceptance masks and filters

[xyx0826]

Addresses #19.

This PR implements logics to allow for storing multiple message filters under the same mask. Changes validated using a custom RP2040/MCP2515 board on a CAN bus using three different message filters (matches).

[jepler]

thanks, this looks like a good improvement!

There are some changes needed for the automated checks, and I also have some feedback.

Please feel free to @ me when you have updated the branch, or if you have questions about my request or the automated checks.

[xyx0826]

Thanks for looking over this! I will get to fixing the check issues later this week. Also I might've missed something but I can't seem to find any of your comments or requested changes on the change list.

[tekktrik]

Heads up, pre-commit has been updated, so automated checks may yield something different! I think I retriggered the run while patching things.

[HourGlss]

I would love it if this got approved and Masks and filters were updated in the docs as well. I currently have an issue open that relates to Masks as well, hopefully this fixes that too.

[jepler]

This data should be an instance member (self.masks_filters), because otherwise a system with more than one MCP2515 chip in it would not function properly.

[jepler]

I'm sorry, my old comment(s) were apparently never submitted. I looked back at the PR today and besides the automated checks that are failing, I think that the use of a global variable should be replaced by an instance variable.

[xyx0826]

Making acceptance data instanced makes more sense. Thanks for the review, I'll get to implementing those changes.

[HourGlss]

I can help with testing if you want. I really want to make masks as useful as possible. Please remove the print for 1 mask only.

.fyx on discord

[xyx0826]

I can help with testing if you want. I really want to make masks as useful as possible. Please remove the print for 1 mask only.

.fyx on discord

Testing is appreciated as I no longer have a MCP2515 setup by my hand. What do you mean by the print? This PR doesn't actively print anything.

[HourGlss]

check out this issue: #22

This PR is against this issue: #19 I get it. But if we simply include testing with 1 mask we will hit the above issue and can fix it.

If we are going to fix masks, let's fix masks. Adding documentation for Mask would be a good step also.

[anecdata]

There is documentation for the canio module that may be useful
https://docs.circuitpython.org/en/latest/shared-bindings/canio/index.html#canio.Match

[HourGlss]

keeping consistency with canio is a must IMO. diverging from their use of Match is not a good idea.

[xyx0826]

Changes implemented. @jepler Can you help me track down what's being reformatted on line 1? I can't get pylint to lint the same way locally.

[HourGlss]

Did you need me to test? You never reached out via discord.

[jepler]

@tekktrik hey do you know why the change (diff) introduced by black for this PR is not being shown in the results?

@xyx0826 These are the changes made locally when I run pre-commit run --all-files and commit the result:

commit e44aa94985e1df58d2b17650f783e47d32362a80
Author: Jeff Epler <[email protected]>
Date:   Sat Jan 20 15:42:11 2024 -0600

    re-format per black pre-commit

diff --git a/adafruit_mcp2515/__init__.py b/adafruit_mcp2515/__init__.py
index 9d7e6f4..f27e6d1 100644
--- a/adafruit_mcp2515/__init__.py
+++ b/adafruit_mcp2515/__init__.py
@@ -317,16 +317,11 @@ class MCP2515:  # pylint:disable=too-many-instance-attributes
         self._loopback = loopback
         self._silent = silent
         self._masks_filters = {
-            _RXM0SIDH: [None, {
-                _RXF0SIDH: None,
-                _RXF1SIDH: None
-            }],
-            _RXM1SIDH: [None, {
-                _RXF2SIDH: None,
-                _RXF3SIDH: None,
-                _RXF4SIDH: None,
-                _RXF5SIDH: None
-            }]
+            _RXM0SIDH: [None, {_RXF0SIDH: None, _RXF1SIDH: None}],
+            _RXM1SIDH: [
+                None,
+                {_RXF2SIDH: None, _RXF3SIDH: None, _RXF4SIDH: None, _RXF5SIDH: None},
+            ],
         }
 
         self._init_buffers()
@@ -819,8 +814,9 @@ class MCP2515:  # pylint:disable=too-many-instance-attributes
     def _create_mask_and_filter(self, match: Match):
         actual_mask = match.mask
         if match.mask == 0:
-            actual_mask = EXTID_BOTTOM_29_MASK if match.extended \
-                else STDID_BOTTOM_11_MASK
+            actual_mask = (
+                EXTID_BOTTOM_29_MASK if match.extended else STDID_BOTTOM_11_MASK
+            )
 
         result = self._find_mask_and_filter(actual_mask)
         if not result:
@@ -828,7 +824,7 @@ class MCP2515:  # pylint:disable=too-many-instance-attributes
                 "No mask and filter is available for "
                 f"mask 0x{actual_mask:03x}, addr 0x{match.address:03x}"
             )
-        
+
         (mask, filt) = result
         self._set_acceptance_register(mask, actual_mask, match.extended)
         self._set_acceptance_register(filt, match.address, match.extended)
@@ -839,12 +835,11 @@ class MCP2515:  # pylint:disable=too-many-instance-attributes
         """Clears the Receive Mask and Filter Registers"""
         for mask_reg, mask_data in self._masks_filters.items():
             self._set_register(mask_reg, 0)
-            mask_data[0] = None # Mask value
+            mask_data[0] = None  # Mask value
             for filt_reg in mask_data[1]:
                 self._set_register(filt_reg, 0)
                 mask_data[1][filt_reg] = None
 
-
     ######## CANIO API METHODS #############
     @property
     def baudrate(self):
@@ -944,7 +939,6 @@ class MCP2515:  # pylint:disable=too-many-instance-attributes
                 # Mask unused, set it to a match to prevent leakage
                 self._set_acceptance_register(mask_reg, match.mask, match.extended)
 
-
         return Listener(self, timeout)
 
     def deinit(self):

[tekktrik]

@jepler if I understand your question correctly, I don't think black typically announces via diff what it changed, just that it did change a certain number of files, and the result error code causes the CI to fail. That's at least my experience.

[tekktrik]

I realize now that you mean the failure help text, which has an error. That is a good question, I can look into that!

[jepler]

I opened a PR over in adafruit/workflows-circuitpython-libs#34 for showing the changes introduced by black from pre-commit.