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
The neovi interface use_system_timestamp
feature has compatibility issues
#1699
Comments
Hi,
@zariiii9003 What do you think? |
The docstring of the |
@zariiii9003 but you would keep the data type as int in milliseconds? |
Tbh, i don't know. |
My thought is that at minimum we should warn about the dangers. I think that it would be even better (a lot better) to not have this two different concepts of "timestamp" that are controlled by the value of a Boolean argument (and isolated to this one Final thought: if we decide that we are OK with a path forward that alters the current behavior, perhaps we can just remove it altogether and provide it differently and universally, i.e. for all of the supported hardware devices. I do like the idea of being able to "timestamp" the messages according to a system clock rather than using the hardware timestamps. I can see that it could be very useful to people, e.g. a bloke who wants to have two devices plugged in at once and have their CAN traffic both timestamped against a common "clock". Maybe there is a standard way that we can build this ability into the python-can library itself, so that the neovi interface would not need to do it in a special way. |
ping @pierreluctg |
@zariiii9003 I have been looking into the ASC writer. Do you know if the localization of the date string is mandatory? It strikes me as quite difficult to test due to the unintended side effects. |
I don't know tbh. But maybe this helps. |
Describe the bug
I am a beginner with python-can, and am up to learning about the asyncio feature. After I adapted the asyncio sample code to use the
neovi
interface, I encountered exceptions being thrown in the code which was logging anasc
file. The code inasc.py
tries to perform asplit
on the decimal point in this line, and this was causing an exception to be thrown due to my timestamps being an integral number of milliseconds. I realized that this was related to theuse_system_timestamp
feature that is specific to theneovi
interface. It turns out that if this Boolean is set toTrue
when creating theBus
instance, timestamps from the Bus are emitted asDWORD
values rather than floats, and the emitted timestamps are in terms of milliseconds rather than seconds.To Reproduce
After getting the asyncio example code running successfully as is, change the constructor arguments passed to
can.Bus
(this line) to instead specify interface of "neovi" and appropriate device serial number, channel, baudrate..., and be sure to includeuse_system_timestamp=True
as well. Run up the example to see exceptions being thrown by the ASC logger.Expected behavior
I expected no exceptions 😎 . (You can get to the expected behavior by changing
use_system_timestamp=False
or removing that constructor argument altogether.)Additional context
OS and version: Windows10
Python version: 3.11.3
python-can version: 4.3.0
python-can interface/s (if applicable): neovi
I think that the goal of the
use_system_timestamp
feature (mentioned here) is worthwhile, but I suggest that there should be mention added to the documentation of it being non-standard (by which I mean peculiar to "neovi"), and warnings given regarding the incompatibilities that it can cause with the rest of the python-can ecosystem. I imagine that the trouble which the milliseconds timestamps caused for the ASC logger could be just one of multiple areas where python-can code might get tripped up by the timestamp values being 1000 times larger than "normal" timestamps. Another example might be this code which uses timestamps to determine whether two messages are equal.Because of the potential usefulness of having it available, maybe there is a different design that could be used to affix OS-compatible timestamps to the incoming/outgoing messages, something universal across all interfaces, and done in a way that doesn't "overload" the traditional timestamping mechanics with new meaning/measurement units? Just a thought...
Traceback and logs
The text was updated successfully, but these errors were encountered: