hardbyte / python-can

The can package provides controller area network support for Python developers
https://python-can.readthedocs.io
GNU Lesser General Public License v3.0
1.27k stars 597 forks source link

kvaser constructor should allow to set more flags #1587

Open seobilee opened 1 year ago

seobilee commented 1 year ago

Is your feature request related to a problem? Please describe.

I'd like to point out that the constructor of Kvaser Canlib has limited argument options available.

  1. Unlike Vector's constructor, there doesn't appear to be a way to set tseg1, tseg2, and sjw of data bitrate.
  2. Additionally, upon reviewing the code below, it seems that the flags parameter is restricted to only two options.

    flags = 0
    if accept_virtual:
        flags |= canstat.canOPEN_ACCEPT_VIRTUAL
    if fd:
        flags |= canstat.canOPEN_CAN_FD
    
    log.debug("Creating read handle to bus channel: %s", channel)
    self._read_handle = canOpenChannel(channel, flags)

so, several flags of constants are unavailable.

    canOPEN_ACCEPT_VIRTUAL = 0x0020
    canOPEN_OVERRIDE_EXCLUSIVE = 0x0040
    canOPEN_REQUIRE_INIT_ACCESS = 0x0080
    canOPEN_NO_INIT_ACCESS = 0x0100
    canOPEN_ACCEPT_LARGE_DLC = 0x0200
    canOPEN_CAN_FD = 0x0400
    canOPEN_CAN_FD_NONISO = 0x0800

Describe the solution you'd like

I would suggest considering the following enhancements for the Kvaser constructor:

  1. It may be beneficial to support additional FD parameters such as tseg1_dbr, tseg2_dbr, and sjw_dbr.
  2. Adding support for more diverse flags in canOpenChannel's flags would be beneficial.

Describe alternatives you've considered

In my opinion, the list provided above should suffice as an explanation. Currently, I am using a modified version of canlib.py from python-can 4.2.0, but I believe it would be beneficial to have native support for it.

Although I haven't put much thought into it, I would like to share a code snippet that could be helpful. Please kindly consider it as an example.

    flags = kwargs.get("flags", 0)
    if accept_virtual:
        flags |= canstat.canOPEN_ACCEPT_VIRTUAL
    if fd:
        flags |= canstat.canOPEN_CAN_FD

    tseg1_dbr = kwargs.get("tseg1_dbr", 0)
    tseg2_dbr = kwargs.get("tseg2_dbr", 0)
    sjw_dbr = kwargs.get("sjw_dbr", 0)

    canSetBusParamsFd(self._read_handle, data_bitrate, tseg1_dbr, tseg2_dbr, sjw_dbr)

Additional context

zariiii9003 commented 1 year ago

You could help getting #1510 merged by testing and giving feedback

seobilee commented 1 year ago

You could help getting #1510 merged by testing and giving feedback

currently I don't have access to a testable CAN-FD environment, so it might take some time for me.

geynis commented 1 month ago

Hi, #1510 is still restricting the flags parameter to only two options, I want to use the "canOPEN_ACCEPT_LARGE_DLC " for example. Are there any plans to add that or should I suggest this small fix in another PR?

zariiii9003 commented 1 month ago

Sure, create a PR