ports/stm32/can.c: allow mixed id-type bus

Original issue here on forum

Most CAN buses can handle messages with both extended- and standard-id but the current API does not allow mixed id-type bus.

micropython/ports/stm32/can.c

Lines 353 to 369 in 338af99

    
           STATIC mp_obj_t pyb_can_init_helper(pyb_can_obj_t *self, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { 
        
               enum { ARG_mode, ARG_extframe, ARG_prescaler, ARG_sjw, ARG_bs1, ARG_bs2, ARG_auto_restart }; 
        
               static const mp_arg_t allowed_args[] = { 
        
                   { MP_QSTR_mode,         MP_ARG_REQUIRED | MP_ARG_INT,   {.u_int  = CAN_MODE_NORMAL} }, 
        
                   { MP_QSTR_extframe,     MP_ARG_BOOL,                    {.u_bool = false} }, 
        
                   { MP_QSTR_prescaler,    MP_ARG_INT,                     {.u_int  = 100} }, 
        
                   { MP_QSTR_sjw,          MP_ARG_KW_ONLY | MP_ARG_INT,    {.u_int = 1} }, 
        
                   { MP_QSTR_bs1,          MP_ARG_KW_ONLY | MP_ARG_INT,    {.u_int = 6} }, 
        
                   { MP_QSTR_bs2,          MP_ARG_KW_ONLY | MP_ARG_INT,    {.u_int = 8} }, 
        
                   { MP_QSTR_auto_restart, MP_ARG_KW_ONLY | MP_ARG_BOOL,   {.u_bool = false} }, 
        
               }; 
        
               // parse args 
        
               mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; 
        
               mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); 
        
               self->extframe = args[ARG_extframe].u_bool;

There could be hardware modules on the bus with varying degree of tolerance for even the presence of extended IDs:

Concerning the compatibility of standard and extended frames three types of CAN controllers exist:
˚ CAN controllers according to CAN protocol specification vers. 2.0 A, which are able to receive
and transmit standard frames
˚ CAN controllers according to CAN protocol specification vers. 2.0 B, which are able to receive
and transmit standard frames and to tolerate extended frames without detecting an error
(’extended frame passive’)
˚ CAN controllers according to CAN protocol specification vers. 2.0 B, which are able to receive
and transmit standard and extended frames (’extended frame active’).

While it does make sense to say this can bus can only accept standard-id, it doesn't make sense to say this can bus can only accept extended id because if you can accept extended then you can also accept standard.

Therefore, the flag in CAN.init() should be changed to indicate whether or not the bus allows extended ID and CAN.send() should be modified to accept a flag to indicate standard vs extended ID and throw if an extended-message is attempted to be sent on a standard-only bus.

I think this is quite an important fix for CAN comms...
Mixing extended and standard addressing is quite common, especially considering most developer diagnostics tools use extended ID's (to reduce the risk of clashing in the address-space).

Workaround
One (pretty awful) workaround in the meantime is to use both the pyboard's CAN channels, one configured for extended ids, the other for standard.

The user jgui (on the micropython forum) posted an excellent high-level test report on [Nov 21, 2016]
ref: https://forum.micropython.org/viewtopic.php?t=2686 paraphrased below...

I have tested several configurations to understand, using CAN1 to send messages to CAN2, and changing CAN1/CAN2 configuration each test.

Wiring Diagram

Test Table & Results

Thanks for raising this point. I agree it should be possible to send both base and extended frames without reconfiguring the CAN object.

Therefore, the flag in CAN.init() should be changed to indicate whether or not the bus allows extended ID and CAN.send() should be modified to accept a flag to indicate standard vs extended ID and throw if an extended-message is attempted to be sent on a standard-only bus.

When receiving it should already support a mix of extended frames or not, regardless of the extframe setting. extframe only affects sending of frames.

To maintain backwards compatibility we'd need to keep the meaning of extframe in the init() method. This would provide the default type of frame that is sent. Then CAN.send() could also take this same parameter which would override the default. Eg:

can = CAN(1, CAN.NORMAL, extframe=False) # default to base frames
can.send('data', 45) # use base frame
can.send('data, 45, extframe=True) # use extended frame

That seems pretty obvious. I can't think of a simpler or cleaner way to do it. (Autodetecting whether it's base or extended by the size of the address wouldn't work because you could have an address of 1 even in an extended frame.)

@dpgeorge

@dpgeorge

To maintain backwards compatibility we'd need to keep the meaning of extframe in the init() method. This would provide the default type of frame that is sent. Then CAN.send() could also take this same parameter which would override the default. Eg:

Then it's not possible to enforce that a given bus can only send standard-frames (or extended frames).

To maintain backwards compatibility, add another argument to constructor called allowOverride:

allowOverride == 1 -> can.send is allowed to send frames different from extframe default
allowOverride == 0 (default) ->can.send is NOT allowed to send frames different from extframe default

Then it's not possible to enforce that a given bus can only send standard-frames (or extended frames).

I don't think it's really necessary to support this behaviour. The user has to go out of their way to explicitly send an extended or basic frame when calling can.send(), so it can't happen "by accident". And since the CAN bus should always be able to handle extended frames (at least ignore them) I don't see a need to really forbid it in the software.

You’re assuming only ONE user has complete control over all layers of the code.

Scenario: one user creates a layer that initializes the bus and then another user - through another layer like a remote API - attempts to send a message using a message type that’s incompatible given current configuration.

I can’t control the remote code the other author writes so I would have to maintain additional state and checks that could and should be done by the CAN object itself.

You’re assuming only ONE user has complete control over all layers of the code

Well, I'm assuming that if a user has the can instance and can call can.send() then the can also call can.init() and change the configuration. So they could change whether extended frames are allowed or not.

one user creates a layer that initializes the bus and then another user - through another layer like a remote API - attempts to send a message using a message type that’s incompatible given current configuration.

In such a case you'll also need to prevent the other user from calling can.init().

I can’t control the remote code the other author writes so I would have to maintain additional state and checks that could and should be done by the CAN object itself.

In this case it'd be better to wrap the CAN object to expose only the things you want to expose.

@adritium

@adritium

your allowOverride behaviour could be achieved with something like:

import pyb

class MyCAN(pyb.CAN):
    def __init__(self, *args, allowOverride=False, **kwargs):
        self.allowOverride = allowOverride
        super(MyCAN, self).__init__(*args, **kwargs)

    def send(self, *args, **kwargs):
        if not self.allowOverride and kwargs.get('extframe', self.extframe) != self.extframe:
            raise RuntimeError("you can't just change extframe behavior like that!, get off my bus!")
        return super(MyCAN, self).send(*args, **kwargs)

Then use MyCAN instead of the pyb.CAN class
That way you can administer that restriction onto other engineers.

I know I can do that but that logic should be part of CAN module

but that logic should be part of CAN module

Why exactly? What about other (arbitrary) restrictions like on the maximum number of bytes to send, or on the addresses that you can send to, etc? And even if CAN does restrict the extended frame format, how will you prevent users from reconfiguring it via can.init()?

This is not an arbitrary restriction.
It has to do with what the bus itself supports given the capabilities of other modules and therefore is a property of the bus like bitrate.
If modules transmit at different bitrates bus won’t work. If modules transmit extended but some modules start ERRoring because they can’t handle it, bus won’t work.

Reconfiguring the frame type is no different than reconfiguring the bitrate so your question’s answer is the same as “how will you prevent users from reconfiguring the bitrate”.

If modules transmit extended but some modules start ERRoring because they can’t handle it, bus won’t work.

From what I understand about CAN, anything on the bus must at least tolerate (eg ignore) extended frames, even if they don't support them.

Reconfiguring the frame type is no different than reconfiguring the bitrate so your question’s answer is the same as “how will you prevent users from reconfiguring the bitrate”.

So, how will you prevent them reconfiguring the bitrate?

From what I understand about CAN, anything on the bus must at least tolerate (eg ignore) extended frames, even if they don't support them.

That's my understanding as well... and Wikipedia's CAN bus page agrees:

The CAN standard requires the implementation must accept the base frame format and may accept the extended frame format, but must tolerate the extended frame format.

@dpgeorge

@dpgeorge

So, how will you prevent them reconfiguring the bitrate?

If I give them access to modify the bitrate, I should give them access to modify extframe as they're both properties of the CAN bus . . . and if I deny them access to one, I should deny them access to the other.

@dpgeorge

@dpgeorge

From what I understand about CAN, anything on the bus must at least tolerate (eg ignore) extended frames, even if they don't support them.

That's not technically true; see section 5

Maybe the silicon cost to implement tolerance for extended is so trivial that no one will ever encounter this situation but why leave this implementation gap at all if it's part of the spec?

As I see it there are two separate issues here:

Whether an extended frame can lead to an error on the CAN bus with a CAN 2.0A controller.
Whether the stm32's CAN implementation should prevent the user from sending extended frames in the send method if the frame type was configured to basic/standard frame.

As for issue 1, according to the CAN 2.0 specs found here http://esd.cs.ucr.edu/webres/can20.pdf, part A (ie CAN 2.0A controllers), section 3.1.1 on page 13 of the PDF, it states that there are two bits in the control field reserved for future expansion, and that these must be sent 'dominant' (indicating a standard frame). It also states that "receivers accept 'dominant' and 'recessive' bits in all combinations". So I read this as stating that CAN 2.0A controllers must ignore an extended frame. Furthermore in section 6 (error handling) none of the errors there correspond to reporting an error when an extended frame is detected.

That's not technically true; see section 5

It looks like this is an application note, not the CAN specs.

If I give them access to modify the bitrate, I should give them access to modify extframe as they're both properties of the CAN bus . . . and if I deny them access to one, I should deny them access to the other.

So, if you are explicitly denying them access to modify the bitrate and change extframe in the init() method, then you can also deny them access to setting extframe in the send() method, right?

Ok you're right about the case of another user.
Even in the case of single-user, I view this proposed restriction it like the const qualifier: it's there to prevent you from doing something accidentally.

You don't need const keyword but it helps to document and enforce an intention.

With some experience on using the CAN interface I think it was a mistake to put the extframe parameter in the init function. The interface as such should always be able to handle both standard and extended frames. The selection of which types to send and receive should have been delegated to the send,
receive and filtering functions.

The interface as such should always be able to handle both standard and extended frames. The selection of which types to send and receive should have been delegated to the send,
receive and filtering functions.

With the proposal (see above) to add the extframe keyword to the send() method, this would allow to send standard and extended frames easily.

For recv() it already supports standard and extended frames regardless of the extframe setting done at construction/initialisation. The only issue is that it doesn't inform you which frame type was received, it just gives you the id of the frame.

For filtering, we could also add an extframe keyword argument to setfilter() to override the default extframe just like with send().

Passing an additional ide (or extframe) argument to CAN.setfilter (just as rtr argument) would clarify how to configure filter and how filter work.

Also for filter mode CAN.MASK16 and CAN.MASK32, ability to mask IDE or RTR bit is not provided (only ID), and it is not clear how these bits are set by the API. From testing it is difficult to understand the exact logic, which seems to depend on filter mode and extframe. Adding 2 parameters for masking ide and rtr would clarify.

Putting together new ide and mask parameters, API could become:

CAN.setfilter(bank, mode, fifo, params)
with:
- params when mode is LIST16: ((ID_IDValue1, ID_IDE1, ID_RTR1), (ID_IDValue2, ID_IDE2, ID_RTR2), (ID_IDValue3, ID_IDE3, ID_RTR3), (ID_IDValue4, ID_IDE4, ID_RTR4))
- params when mode is LIST32: ((ID_IDValue1, ID_IDE1, ID_RTR1), (ID_IDValue2, ID_IDE2, ID_RTR2))
- params when mode is MASK16: ((ID_IDValue1, ID_IDE1, ID_RTR1), (Mask_IDValue1, Mask_IDE1, Mask_RTR1), (ID_IDValue2, ID_IDE2, ID_RTR2), (Mask_IDValue2, Mask_IDE2, Mask_RTR2))
- params when mode is MASK32: ((ID_IDValue1, ID_IDE1, ID_RTR1), (Mask_IDValue1, Mask_IDE1, Mask_RTR1))

with ID_IDValueX valid ranges:
- (LIST16 or MASK16) and ID_IDEx=0: [0:7FF]
- (LIST16 or MASK16) and ID_IDEx=1: [0:7FF-8000:387FF]
- (LIST32 or MASK32) and ID_IDEx=0: [0:7FF]
- (LIST32 or MASK32) and ID_IDEx=1: [0:1FFFFFFF]
An error should be generated when user set an ID or ID mask out of range.

Here is a mapping with registers I made for my own understanding:

Mixed id types are implemented in e7264e9

Navigation Menu

Repository navigation

ports/stm32/can.c: allow mixed id-type bus #3916

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	STATIC mp_obj_t pyb_can_init_helper(pyb_can_obj_t self, size_t n_args, const mp_obj_t pos_args, mp_map_t *kw_args) {
	enum { ARG_mode, ARG_extframe, ARG_prescaler, ARG_sjw, ARG_bs1, ARG_bs2, ARG_auto_restart };
	static const mp_arg_t allowed_args[] = {
	{ MP_QSTR_mode, MP_ARG_REQUIRED \| MP_ARG_INT, {.u_int = CAN_MODE_NORMAL} },
	{ MP_QSTR_extframe, MP_ARG_BOOL, {.u_bool = false} },
	{ MP_QSTR_prescaler, MP_ARG_INT, {.u_int = 100} },
	{ MP_QSTR_sjw, MP_ARG_KW_ONLY \| MP_ARG_INT, {.u_int = 1} },
	{ MP_QSTR_bs1, MP_ARG_KW_ONLY \| MP_ARG_INT, {.u_int = 6} },
	{ MP_QSTR_bs2, MP_ARG_KW_ONLY \| MP_ARG_INT, {.u_int = 8} },
	{ MP_QSTR_auto_restart, MP_ARG_KW_ONLY \| MP_ARG_BOOL, {.u_bool = false} },
	};

	// parse args
	mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
	mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);

	self->extframe = args[ARG_extframe].u_bool;

ports/stm32/can.c: allow mixed id-type bus #3916

Description

Activity

fragmuffin commented on Jul 9, 2018

fragmuffin commented on Jul 9, 2018

dpgeorge commented on Jul 9, 2018

adritium commented on Jul 9, 2018

dpgeorge commented on Jul 10, 2018

adritium commented on Jul 10, 2018

dpgeorge commented on Jul 10, 2018

fragmuffin commented on Jul 10, 2018

adritium commented on Jul 10, 2018

dpgeorge commented on Jul 10, 2018

adritium commented on Jul 10, 2018

dpgeorge commented on Jul 10, 2018

fragmuffin commented on Jul 10, 2018

adritium commented on Jul 10, 2018

adritium commented on Jul 10, 2018

dpgeorge commented on Jul 11, 2018

dpgeorge commented on Jul 11, 2018

adritium commented on Jul 13, 2018

HenrikSolver commented on Jul 15, 2018

dpgeorge commented on Jul 16, 2018

jgui commented on Jul 16, 2018

dpgeorge commented on Apr 2, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions