Unify bit timing parameters

[christiansandberg]

I've had a quick look at the various interfaces and how you can specify custom bit timing parameters. They vary in abstraction. Some supports setting BTR registers directly, usually using some external tool for help. Some lets you specify SJW, TSEG1, TSEG2, SAM and then BRP is calculated from desired bitrate.

For CAN-FD you must specify different settings for arbitration phase and data phase.

Interfaces supporting BTR register:

• canalyst (named Timing0 and Timing1)
• slcan (named btr as hexadecimal string)
• pcan (possible, see #538)
• ixxat (possible)
• systec (possible)
• ...?

Interfaces supporting SJW, TSEG1, TSEG2, SAM:

• kvaser (named sjw, tseg1, tseg2, no_samp)
• vector

Interfaces supporting CAN-FD:

• kvaser (uses same sjw, tseg1, tseg2 for both arbitration and data)
• vector (sjwAbr, tseg1Abr, tseg2Abr and sjwDbr, tseg1Dbr, tseg2Dbr)
• pcan (f_clock, nom_brp, nom_tseg1, nom_tseg2, nom_sjw and data_brp, data_tseg1, data_tseg2, data_sjw)

As suggested by @bmeisels I propose to create a bit timing class which can bridge the different APIs and reduce clutter in the argument list. See #615 for implementation.

[bmeisels]

Considering what I learned while implementing CAN FD for Pcan I think we should have a timing parameter class.

This class will have multiple constructors from the different parameter options and supply an interface which will calculate the missing parameters based on the existing parameters. This class should be accepted by all CAN bus classes.

I think this change will allow us to simplify the interfaces and allow more easily changes to the parameters.

[christiansandberg]

That would at least make the Bus argument list less cluttered. How would you handle the differences in API’s and controllers?

[bmeisels]

Please correct me if I am wrong, generally it is possible to convert between different specification formats. Each interface should internally use the properties it needs from the timing parameters instance. If these parameters don't exist they should be generated based on the existing parameters.

If the previous statement is wrong and the parameters cannot be converted, an exception with an indicative error message should be thrown.

[christiansandberg]

I started an attempt in #615. I've looked at https://www.mct.net/download/philips/can_timing.pdf for reference, but I don't know if it is applicable for all controllers.

[christiansandberg]

I've got some questions.

• Should we fall back to standard bitrate setting if the interface cannot use the bit timing provided?
• Do we need to support setting some of these parameters using the config file? What about the CLI applications?
• Should we keep backwards compatibility during 4.x or drop the current arguments right away?

[hardbyte]

Nice work Christian. I'll take a stab at your questions:

• I don't know but I'd have thought raising an exception would be a better approach
• yes we need to fully support this in the config file. cli less so
• as annoying as it is I think our users will appreciate a deprecation period. I suggest we add a new optional timing argument which (if present) would take precedence over any of the current arguments.

[lumagi]

Hi,

could you elaborate why setting the bitrate value is mandatory for instantiating the BitTiming class in util.py?

I am using the PCAN interface in an FD configuration with custom bit timings, i.e. f_clock, nom_brp, ... (see example below). The PcanBus actually disregards the bitrate paramter if FD is enabled. Additionally, the parameters are specified twice, once for the nominal part (nom_brp) and once for the data part (data_brp).

After upgrading to v4.1.0, I ran into an issue when constructing a PcanBus via Bus.__new__. When running the following code snippet, I always get the exception shown below:

from can import Bus

from can.interfaces.pcan import PcanBus

def main():
    params = {'interface': 'pcan', 'fd': True, 'f_clock': 80000000, 'nom_brp': 1,
              'nom_tseg1': 129, 'nom_tseg2': 30, 'nom_sjw': 1, 'data_brp': 1,
              'data_tseg1': 9, 'data_tseg2': 6, 'data_sjw': 1, 'channel': 'PCAN_USBBUS1'}

    # Breaks
    bus = Bus(**params)

    # doesn't break
    bus = PcanBus(**params)

    while True:
        r = bus.recv()
        print(r)

if __name__ == "__main__":
    main()

Exception:

Traceback (most recent call last):
  File "<...>", line 23, in <module>
    main()
  File "<...>", line 12, in main
    bus = Bus(**params)
  File "<...>", line 108, in __new__
    kwargs = load_config(config=kwargs, context=context)
  File "<...>", line 192, in load_config
    bus_config = _create_bus_config(config)
  File "<...>", line 252, in _create_bus_config
    timing_conf["bitrate"] = config["bitrate"]
KeyError: 'bitrate'

It looks like _create_bus_config always expects a bitrate parameter although this is not required for FD mode in PcanBus. The code snippet for creating the BitTiming object is triggered because the parameters contain the f_clock parameter. At the moment, I don't see a way for me to adapt the parameters to Bus.__new__ to work around this issue since PcanBus doesn't seem to be using the bittiming parameter as of now.