adafruit / Adafruit_CircuitPython_MCP2515

A CircuitPython library for working with the MCP2515 CAN bus controller
MIT License
21 stars 14 forks source link

Using only one mask produces a lot of print spam #22

Open HourGlss opened 1 year ago

HourGlss commented 1 year ago

Please see test case:

import digitalio
import board
import busio
import adafruit_mcp2515

cs = digitalio.DigitalInOut(board.CAN_CS)
cs.switch_to_output()
spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
matches = [
            adafruit_mcp2515.Match(0x060, mask = 0x7F0)
        ]
# for me, the first 3 bits correspond to priority
# the next 4 bits are used for the device number
# the last 4 correspond to the message type.
# each device except the central controller can then only need to listen to a subset of matches that correspond to it's 
# device number

mcp = adafruit_mcp2515.MCP2515(spi, cs, baudrate=1000000)
received_messages = []
while True:
    # If there's a better way to specify mask, let me know.
    with mcp.listen(matches, timeout=.00001) as listener:
        next_message = listener.receive()
        while next_message is not None:
            received_messages.append(next_message)
            print("adding message")
            next_message = listener.receive()
     # just to prove the point       
    while len(received_messages) > 0:
        received_messages.pop()

This produces a LOT of spam on the serial log.

intended functionality would be: the mcp2515 can be used with one mask without something printing.

Adding ANOTHER mask removes the print.

import digitalio
import board
import busio
import adafruit_mcp2515

cs = digitalio.DigitalInOut(board.CAN_CS)
cs.switch_to_output()
spi = busio.SPI(board.SCK, board.MOSI, board.MISO)
matches = [
    adafruit_mcp2515.Match(0x060, mask=0x7F0),  # I think this might be what i want? i cant test.
    adafruit_mcp2515.Match(0x060, mask=0x06F)  # I think this may be what I want... but I cant test?
]
# for me, the first 3 bits correspond to priority
# the next 4 bits are used for the device number
# the last 4 correspond to the message type.
# each device except the central controller can then only need to listen to a subset of matches that correspond to it's
# device number

mcp = adafruit_mcp2515.MCP2515(spi, cs, baudrate=1000000)
received_messages = []
while True:
    # If there's a better way to specify mask, let me know.
    with mcp.listen(matches, timeout=.00001) as listener:
        next_message = listener.receive()
        while next_message is not None:
            received_messages.append(next_message)
            print("adding message")
            next_message = listener.receive()
    # just to prove the point
    while len(received_messages) > 0:
        received_messages.pop()

this seems to be the problem... but I think it may point to a larger issue. https://github.com/adafruit/Adafruit_CircuitPython_MCP2515/blob/a50e24362154677d262e643c8edec9237f810a77/adafruit_mcp2515/__init__.py#L917

Lastly, adding some documentation to Mask would be pretty cool. https://docs.circuitpython.org/projects/mcp2515/en/latest/api.html#adafruit_mcp2515.canio.Match

anecdata commented 1 year ago

Just found this... canio has a more thorough description of Match: https://docs.circuitpython.org/en/latest/shared-bindings/canio/index.html#canio.Match

sneiman commented 11 months ago

I also have printing spam problem. Best would be a flag to silence it. Is any one planning to work on this?

UPDATE: every easy to download the bundle git, follow the instructions, comment out the offending print(), generate a new bundle and use that version of MCP2515

karlfl commented 5 months ago

I actually think the problem lies in the examples and how they recommend using the "listen()" and "receive()" methods.

The 'listen()' method is where the listener is created and also the place where that unused mask message is being printed. If the listen() method is called inside the 'while true:' loop it'll recreate the listener and also display that message on every loop.

Also, according to the documentation it might not be wise to recreate the listener every time due to the cost of creating it...

Creating a listener is an expensive operation and can interfere with reception of messages by other listeners.

I think a better approach (if you're masks and filters don't change over time) is to call the listen() method once outside of the 'while True:' loop. Then just call the receive() method inside the loop to get the incoming messages that haven't been processed. This also has the side benefit of making your while loop much faster (see comment above). So far, this approach has worked very well for me.

# using an array with only one match
matches = [
    adafruit_mcp2515.Match(0x060, mask=0x7F0)
 ]

# Setup the CAN Bus and Listener.
mcp = adafruit_mcp2515.MCP2515(spi, cs, baudrate=1000000)
listener = mcp.listen(matches, timeout=.002)
# You should only see the warning message only once using this approach

while True:
    # use a for loop to process all messages received since the last time this was called.
    message_count = listener.in_waiting()
    print(message_count, "messages available")
    for _i in range(message_count):
        msg = listener.receive()
        # Handle the incoming messages here.
        print("Message from ", hex(msg.id))
        if isinstance(msg, Message):
            print("message data:", msg.data)
        if isinstance(msg, RemoteTransmissionRequest):
            print("RTR length:", msg.length)

Hope that helps!