canio: Initial implementation for SAM E5x MCUs

[jepler]

Tested & working:

• Send standard packets
• Receive standard packets (1 FIFO, no filter)

Interoperation between SAM E54 Xplained running this tree and MicroPython running on STM32F405 Feather with an external transceiver was also tested. (SAM-to-SAM is not actually tested yet)

Error detection and recovery are supported.

Transmitter program:

import _canio
import board
import digitalio
import time

d = digitalio.DigitalInOut(board.LED)
d.switch_to_output()

standby = digitalio.DigitalInOut(board.CAN_STANDBY)
standby.switch_to_output(False)

c = _canio.CAN(rx=board.CAN_RX, tx=board.CAN_TX, baudrate=1000000, auto_restart=True)

m = _canio.Message(id=54, data=b'adaf00')
while True:
    c.send(m)
    time.sleep(.1)
    d.value = not d.value

Receiver program:

import _canio
import board
import digitalio
import time

d = digitalio.DigitalInOut(board.LED)
d.switch_to_output()

standby = digitalio.DigitalInOut(board.CAN_STANDBY)
standby.switch_to_output(False)
    
c = _canio.CAN(rx=board.CAN_RX, tx=board.CAN_TX, baudrate=1000000)
        
m = _canio.Message()
l = c.listen(timeout=1)

while True:
    if l.readinto(m):
        d.value = not d.value
        print("received", m.id, m.rtr, m.size, m.data)
    else:
        print("Nothing received before timeout")

[jepler]

[Rebased to clear out some merge conflicts due to camera]

[tannewt]

Here is a first pass. Thanks for your work on this!

[jepler]

I'm about to rebase this, so I've tagged the pre-rebase last revision: https://github.com/jepler/circuitpython/releases/tag/canbus-v1 jepler@5307ed9

[jepler]

I've run into some problems with my final testing, please don't merge this just yet

[jepler]

OK, I think this is ready. It passes my test program, which I made more extensive. Yay for having loopback mode! Test program: https://gist.github.com/3627fbe0659ae8e54dee27d49699ae4d can run without transceiver, self-verifies "all" functionality

[tannewt]

Looks great! Nice job with this!

[tannewt]

(Not for this PR.) We should have a reset_common_hal that wraps all of these. Right now they are don't haphazard in reset_port.

[siddacious]

Previously, in a parallel dimension:

[siddacious]

My though is that RTRs and normal messages are not functionally the same so using the same class for both is a bit awkward. If there were two types, then you'd have to know which type to pass into readinto to be filled by the next message of unknown type.

On the receiving end especially, you're going to want to know if it's an RTR message so at some point I would expect code like

while not timeout_expired():
    message = <MessageGiver>.next_message
    if message is None:
        break
    if <message is an RTR>:
        handle_rtr(message)
    else:
        handle_message(message)

We need to know if a message is an RTR or not on both ends, and using a type works in both cases. On creation the constructor has a different signature, and on the receiving end you can use isinstance(message, RTRMessage) to fulfill <message is an RTR>

readinto makes sense because it allows us to recycle messages so we're not constantly making new messages and discarding them but the fact that having an RTRMessage causes issues with readinto seems like readinto's problem, not RTRMessage's

I feel like I'm barking up a dead tree so I'll leave it at that.

PS:I lied: I think RTRMessage is more likely to be used in user code than readinto so if there's a disagreement I think RTRMessage should win.

[jepler]

@siddacious has been kind enough to point out some other problems in the docs.

I'd like to consider a couple other changes while this still hasn't gone out as beta:

• I used send() and read(), but send() should pair with recv() and write() with read(). So one should be renamed
• Match(address=) should change to Match(id=), ditto the property

[tannewt]

@jepler Yup, those seem like good changes. I would like recv to be receive though. Full words are easier to understand and we don't have an existing API we're trying to interoperate with (/me looks at socket).

@siddacious Are you proposing dropping readinto in favor of splitting Message into two types, Message and RemoteTransmissionRequest? I like that too. readinto isn't that helpful because you've already set filters so you only receive objects you are interested in.

[siddacious]

@siddacious I don't have anything against readinto per se, I think it or something like it could allow for recycling message objects which might be useful. That said, I do like the idea of RemoteTransmisisonRequest.

If others agree that somehting like a "message pool" would be useful, it would be helpful to provide helpers because they're not trivial (I tried)

[tannewt]

This is very similar to how we do BLE advertisements and we do not have a readinto equivalent. Both of these APIs, BLE and CAN, can rely on upfront filtering to minimize memory churn.

The objects are typically small so they do not get harder to allocate as the heap fragments. (Reuse is way more important for large objects.)

[jepler]

If at some point we support CAN FD, the payload of a single message can be up to 64 bytes, rather than just 8.

[siddacious]

@jepler are silent and loopback not mutually exclusive on the SAM-E? Each of those tries to set the mode of the MCP2515 so I'm currently raising an AttributeError if both are set

[jepler]

All 4 combinations of silent= and loopback= work on SAM E5x and (as far as I can tell so far) on the STM32. If some combinations are NOT supported, the constructor should throw.

[jepler]

I agree, the datasheet says your device only supports 3 out of 4 of the modes.

here are the loopback and silent+loopback modes on sam e5x

[siddacious]

I guess I don't know what makes sense here; From a user perspective I think setting a mode that has the behavior you want is more important than what bits get set. I don't know if the MCP is the outlier here or the SAM; do other MCUs support all 4 options of the "transmit y/n" and loopback y/n" matrix?

[tannewt]

If at some point we support CAN FD, the payload of a single message can be up to 64 bytes, rather than just 8.

We can always add readinto if we really need it. I suspect this approach will still be OK because 64 bytes is still only 4 blocks (16b each) of memory.

[jepler]

STM and SAM both support all 4 modes. Feel free to throw an exception in your constructor if a non-supported mode is requested. Constructors will necessarily differ a little bit in their semantics as different HW has different capabilities.

[jepler]

esp32s2 has distinct register bits so it looks like all 4 modes would be supported, but at another level the API only supports 3 of the 4 combinations.

            uint32_t lom: 1;                    /* MOD.1 Listen Only Mode */
            uint32_t stm: 1;                    /* MOD.2 Self Test Mode */

but

static inline void twai_ll_set_mode(twai_dev_t *hw, twai_mode_t mode)
{
    if (mode == TWAI_MODE_NORMAL) {           //Normal Operating mode
        hw->mode_reg.lom = 0;
        hw->mode_reg.stm = 0;
    } else if (mode == TWAI_MODE_NO_ACK) {    //Self Test Mode (No Ack)
        hw->mode_reg.lom = 0;
        hw->mode_reg.stm = 1;
    } else if (mode == TWAI_MODE_LISTEN_ONLY) {       //Listen Only Mode
        hw->mode_reg.lom = 1;
        hw->mode_reg.stm = 0;
    }
}