Repository navigation
Allow user defined PCAN speeds #538
Description
Activity
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
bitrateparameter, I'd suggest having a new configuration parameter specifically for PCAN (for now, perhaps this is more generally applicable to other peripherals) namedbtr0btr1that can be set to the hex value output by this "BaudTool" (e.g. 0x12). This should avoid confusion about whatbitrateactually 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 = 0x12This is merely a suggestion! Would love to discuss pros/cons of this approach. @hardbyte, @felixdivo any thoughts?
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
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:
- User uses PEAK "BaudTool" to calculate
btr0btr1for their particular use-case. - Based on the options output by "BaudTool", the user can type what they want for
btr0btr1in 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
btr0btr1as an additional parameter that is secondary to the currentbitrateparameter. If both are supplied, just usebitrateand ignorebtr0btr1.- Changing the interface will lead to failures if people upgrade from older versions
If we keep the same interface and only add
btr0btr1as 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 = 500000Custom bitrate config files (using
btr0btr1) look like :#~/can.conf [default] interface = pcan channel = PCAN_USBBUS1 btr0btr1 = 0x12If both parameters are supplied, ignore
btr0btr1:#~/can.conf [default] interface = pcan channel = PCAN_USBBUS1 btr0btr1 = 0x12 #Ignored by python-can bitrate = 250000Currently the following interfaces in python-can already support the
BTR0andBTR1bus 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?
Reacted by Brian ThorneReacted by Felix DivoReacted by Bill HassCan’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?
Reacted by bmeisels and Felix DivoCan’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
bitrateparameter and check against a map, if that doesn't contain what we need for baud-rate, check that the interface tool can handlebtr0btr1and 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
btr0btr1parameter that is secondary to the already definedbitrateparameter), but that could be because I'm biased towards how I use and setup python-can.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.
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.
I've created a new issue #614 for unifying the various interfaces.
- added a commit that references this issue
on Jun 16, 2019
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