Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvaser constructor should allow to set more flags #1587

Open
seobilee opened this issue May 8, 2023 · 4 comments
Open

kvaser constructor should allow to set more flags #1587

seobilee opened this issue May 8, 2023 · 4 comments

Comments

@seobilee
Copy link

seobilee commented May 8, 2023

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
Copy link
Collaborator

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

@seobilee
Copy link
Author

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
Copy link

geynis commented Aug 15, 2024

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
Copy link
Collaborator

Sure, create a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants