Allow user defined PCAN speeds

[ttcontrol-brunnthaler]

Hello,

In pcan.py currently the PcanBus(BusABC) class allows to initialize the speeds which are defined in pcan_bitrate_objs.

I would like to extend the functionality to allow all possible speeds by setting SJW, BRP, TSEG2, TSEG1. So basically user defined speeds.

My idea is to create a new argument flag user_bit which either takes the old approach or creates out of the bitrate a TPCANHandle() with the above mentioned bits.

Best regards

[bhass1]

Hi @ttcontrol-FBU, from your comment in the pull-request ( #539 ):

If you download the https://www.peak-system.com/Software-APIs.305.0.html PCANView you can enter the speed you want and get the Bus Timing Register as parameter back

I found a different tool named "BaudTool" that does what you describe:

It can calculate the BTR0/BTR1 registers based on a desired Baud Rate....

Instead of overloading the bitrate parameter, I'd suggest having a new configuration parameter specifically for PCAN (for now, perhaps this is more generally applicable to other peripherals) named btr0btr1 that can be set to the hex value output by this "BaudTool" (e.g. 0x12). This should avoid confusion about what bitrate actually represents (is it an integer of the baud rate or is it the btr0btr1 register value?)

The user interface would then be either pass it in as a dict key-value pair: can.interface.Bus(btr0btr1=0x12) or pass it in from the config file:

#~/can.conf
[default]
interface = pcan
channel = PCAN_USBBUS1
btr0btr1 = 0x12

This is merely a suggestion! Would love to discuss pros/cons of this approach. @hardbyte, @felixdivo any thoughts?

[ttcontrol-brunnthaler]

Hey @bhass1,
Thanks for investigating further.
My concerns:

• How will we manage all the different combinations that are possible (in your example 2). If we always use the first output of the tool we will limit the user.
• User written btr0btr1 registers are quite advanced CAN properties which most of the users will never use, so does it make sense to replace the current user friendly interface (the default bitrates from the dictionary already cover almost every usecase)
• Changing the interface will lead to failures if people upgrade from older versions
  Happy to discuss further :)
  Best regards

[bhass1]

Hi @ttcontrol-FBU some thoughts:

• How will we manage all the different combinations that are possible (in your example 2). If we always use the first output of the tool we will limit the user.

You are right. I think it is better to encourage the workflow that is:

1. User uses PEAK "BaudTool" to calculate btr0btr1 for their particular use-case.
2. Based on the options output by "BaudTool", the user can type what they want for btr0btr1 in the configuration file or API call.

• User written btr0btr1 registers are quite advanced CAN properties which most of the users will never use, so does it make sense to replace the current user friendly interface (the default bitrates from the dictionary already cover almost every usecase)

I am suggesting to add btr0btr1 as an additional parameter that is secondary to the current bitrate parameter. If both are supplied, just use bitrate and ignore btr0btr1.

• Changing the interface will lead to failures if people upgrade from older versions

If we keep the same interface and only add btr0btr1 as a new, additional parameter it will not break current setups.

So... standard bitrate config files (the most common and currently used) look like :

#~/can.conf
[default]
interface = pcan
channel = PCAN_USBBUS1
bitrate = 500000

Custom bitrate config files (using btr0btr1) look like :

#~/can.conf
[default]
interface = pcan
channel = PCAN_USBBUS1
btr0btr1 = 0x12

If both parameters are supplied, ignore btr0btr1:

#~/can.conf
[default]
interface = pcan
channel = PCAN_USBBUS1
btr0btr1 = 0x12 #Ignored by python-can
bitrate = 250000

[karlding]

Currently the following interfaces in python-can already support the BTR0 and BTR1 bus bit-timing registers:

• slcan
• systec

A quick look through each seems to indicate that they both use a different API with respect to how they handle Bus instantiation (and how the bit rate is chosen). Perhaps it makes sense to pick one (either one of the existing ones, or a new proposal) and unify?

[christiansandberg]

Can’t we calculate the register values ourselves? It doesn’t seem to be that complicated after all.

Maybe we should also take the opportunity for the next major release to unify the various bit timing parameters?

[bhass1]

Can’t we calculate the register values ourselves? It doesn’t seem to be that complicated after all.

This seems ideal from the user-perspective, but more complex and hence more work. Do you have a source to the functions? Is the idea here to take the user-supplied bitrate parameter and check against a map, if that doesn't contain what we need for baud-rate, check that the interface tool can handle btr0btr1 and calculate it ourselves?

Maybe we should also take the opportunity for the next major release to unify the various bit timing parameters?

This makes sense. I haven't had a chance to review the slcan and systec implementations. I prefer the interface I outlined (add new btr0btr1 parameter that is secondary to the already defined bitrate parameter), but that could be because I'm biased towards how I use and setup python-can.

[christiansandberg]

My idea was to take BRP (or a calculate this as well), number of samples, tseg1, tseg2 and SJW to calculate the register values. Some interfaces use this abstraction level already. See https://www.kvaser.com/lesson/can-bit-timing/ for more info.

[christiansandberg]

Interfaces like kvaser and vector seem to calculate the BRP based on bitrate. PCAN (CAN FD) forces you to provide BRP yourself. Other just use BTR directly.

Maybe it is best to keep whatever abstraction the driver provides and not try too hard. But we could anyway use the same arguments between interfaces.