Skip to content

Fix timestamp offset bug in BLFWriter #1921

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

Merged
merged 3 commits into from
Feb 25, 2025
Merged

Conversation

pierreluctg
Copy link
Collaborator

@pierreluctg pierreluctg commented Feb 20, 2025

In the BLF files, the frame timestamp are stored as a delta from the "start time". The "start time" stored in the file is the first frame timestamp rounded to 3 decimals. When we calculate the timestamp delta for each frame, we were previously calculating it using the non-rounded "start time".

This new code, use the "start time" as the first frame timestamp truncated to 3 decimals.

This code can be used to test if the written timestamps are the same that we read back from the BLF file.

import time

import can

messages = [
    can.Message(arbitration_id=_, is_fd=True, is_extended_id=False, timestamp=time.time() + _) for _ in range(10)
]


def main():
    can_log_files = [
        "logfile1.blf",
    ]

    for file_name in [None] + can_log_files:
        with can.Logger(file_name) as logger:
            for message in messages:
                logger(message)

    for file_name in can_log_files:
        print("Reading", file_name)
        with can.LogReader(file_name) as reader:
            for message in reader:
                print(message)

    for timestamp_delta in [1.0e-3, 1.0e-4, 1.0e-6]:
        print(f"timestamp_delta={timestamp_delta}")
        for file_name in can_log_files:
            with can.LogReader(file_name) as reader:
                for i, message in enumerate(reader):
                    assert message.equals(messages[i], timestamp_delta=timestamp_delta, check_channel=False), (
                    messages[i], message)
        print("OK")


if __name__ == "__main__":
    main()

@pierreluctg pierreluctg added bug file-io about reading & writing to files labels Feb 20, 2025
Copy link
Collaborator

@zariiii9003 zariiii9003 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. Maybe add a code comment, so this does not get "optimized" away in the future.

In the BLF files, the frame timestamp are stored as a delta from the "start time". The "start time" stored in the file is the first frame timestamp rounded to 3 decimals. When we calculate the timestamp delta for each frame, we were previously calculating it using the non-rounded "start time".

This new code, use the "start time" as the first frame timestamp truncated to 3 decimals.
Making sure all tests messages contains a timestamp to avoid situation where the first written message have timestamps equal to zero.
Adding comment
@pierreluctg pierreluctg force-pushed the blf-timestamp-offset-fix branch from d54c2ca to 001e5eb Compare February 25, 2025 15:04
@pierreluctg
Copy link
Collaborator Author

That makes sense. Maybe add a code comment, so this does not get "optimized" away in the future.

I have also updated/fix the unit tests test data to avoid hitting this issue in the future.

@pierreluctg pierreluctg merged commit 319a9f2 into main Feb 25, 2025
62 checks passed
@pierreluctg pierreluctg deleted the blf-timestamp-offset-fix branch February 25, 2025 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug file-io about reading & writing to files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants