python-can icon indicating copy to clipboard operation
python-can copied to clipboard

socketcan: support use of SO_TIMESTAMPING for hardware timestamps

Open pevsonic opened this issue 1 year ago • 4 comments

The current implemenation of socketcan utilises SO_TIMESTAMPNS which only offers system timestamps.

I've looked at how can-utils candump.c configures hardware timestamping and implemented this in socketcan as an option which is disabled by default to avoid any potential adverse impact on existing usage. This is using the same param 'use_system_timestamp' as established by neovi_bus.py.

I've also modified logger.py to provide an additional '-H' flag in the same way that candump does in order to use this functionality.

pevsonic avatar Oct 30 '24 13:10 pevsonic

Any update on this?

jayceslesar avatar Apr 07 '25 15:04 jayceslesar

Any update on this?

I’d love to get it in as I’m using it in derived code we have here and I’m sure it’s helpful, but I’m not sure what changes I need to make to get accepted?

pevsonic avatar Apr 07 '25 15:04 pevsonic

I’d love to get it in as I’m using it in derived code we have here and I’m sure it’s helpful, but I’m not sure what changes I need to make to get accepted?

I think you can just remove the argparse code you added and things should just work as the library knows to just pass in kwargs to the bustype by using the following:

    parser.add_argument(
        "extra_args",
        nargs=argparse.REMAINDER,
        help="The remaining arguments will be used for the interface and "
        "logger/player initialisation. "
        "For example, `-i vector -c 1 --app-name=MyCanApp` is the equivalent "
        "to opening the bus with `Bus('vector', channel=1, app_name='MyCanApp')",
    )

So you would just pass --can-hardware-timestamps=True like @zariiii9003 commented in the initial review.

This is beneficial because this library acts as an abstraction over many bustypes abstracted over and made to look like CAN buses even though they are not and allows users to pass in arbitrary "kwargs" to the underlying interface that actually gets initialized without having to worry about what arguments actually live on the argument parser object

The rest just look like minor docs changes

jayceslesar avatar Apr 07 '25 18:04 jayceslesar

I’d love to get it in as I’m using it in derived code we have here and I’m sure it’s helpful, but I’m not sure what changes I need to make to get accepted?

I think you can just remove the argparse code you added and things should just work as the library knows to just pass in kwargs to the bustype by using the following:

    parser.add_argument(
        "extra_args",
        nargs=argparse.REMAINDER,
        help="The remaining arguments will be used for the interface and "
        "logger/player initialisation. "
        "For example, `-i vector -c 1 --app-name=MyCanApp` is the equivalent "
        "to opening the bus with `Bus('vector', channel=1, app_name='MyCanApp')",
    )

So you would just pass --can-hardware-timestamps=True like @zariiii9003 commented in the initial review.

This is beneficial because this library acts as an abstraction over many bustypes abstracted over and made to look like CAN buses even though they are not and allows users to pass in arbitrary "kwargs" to the underlying interface that actually gets initialized without having to worry about what arguments actually live on the argument parser object

The rest just look like minor docs changes

That's really helpful thanks! I didn't know that's how kwargs works - while I can get by with Python, I'm really a 'C' guy..! Hopefully that should be enough for a merge now...

pevsonic avatar Apr 08 '25 10:04 pevsonic