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

Fix ASC Reader start time #1788

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Tian-Jionglu
Copy link

to get correct absolute timestamp

@Tian-Jionglu
Copy link
Author

Tian-Jionglu commented Jun 7, 2024

An example.asc:

date Thu May 16 06:16:50.884 pm 2024
base hex  timestamps absolute
internal events logged
// version 8.2.0
Begin Triggerblock Thu May 16 06:16:50.884 pm 2024
   0.000000 Start der Messung
   594680.670773 CANFD  1 Rx 168   1 0 8 08 95 0b 80 00 00 00 00 00 0 0 3000 0 0 0 0 0
   594680.670873 CANFD  2 Rx 121   1 0 8 08 35 0b 00 00 00 00 00 00 0 0 3000 0 0 0 0 0
   594680.670895  5  1af    Rx    d 8 d3 0b 1c 6a 00 00 00 00
End TriggerBlock

In release-4.4.0 branch io/asc.py snipped:

            if trigger_match := ASC_TRIGGER_REGEX.match(line):
                datetime_str = trigger_match.group("datetime_string")
                self.start_time = (
                    0.0
                    if self.relative_timestamp
                    else self._datetime_to_timestamp(datetime_str)
                )
                continue

here, self.relative_timestamp is always True, therefor self.start_time is always 0.0. (What is relative_timestamp stand for? no records found in ASC files)
As a result, msg.timestamp can't get correct value with code snipped

timestamp = float(_timestamp) + self.start_time

Besides, 2 more changes to unify ASCReader with other Reader(BLFReader):

  1. move _extract_header to __init__, to make it possible to get start_timestamp via reader.start_timestamp;
  2. add attribution start_timestamp as BLFReader;

To reproduce the issue, use the code below to read the example.asc

from datetime import datetime, timedelta
import can

if __name__ == '__main__':
    log_file = "example.asc"
    with can.LogReader(log_file) as reader:
        print(datetime.fromtimestamp(reader.start_time))
        for msg in reader:
            print("{}\t{}\t{}".format(
                datetime.fromtimestamp(msg.timestamp), 
                msg.channel + 1,
                msg.data)
                )

@Tian-Jionglu
Copy link
Author

At present, tests below failed:

FAILED test/logformats_test.py::TestAscFileFormat::test_read_can_and_canfd_error_frames - AssertionError: messages are unequal:
FAILED test/logformats_test.py::TestAscFileFormat::test_read_can_fd_message - AssertionError: messages are unequal:
FAILED test/logformats_test.py::TestAscFileFormat::test_read_can_fd_message_64 - AssertionError: messages are unequal:
FAILED test/logformats_test.py::TestAscFileFormat::test_read_can_fd_remote_message - AssertionError: messages are unequal:
FAILED test/logformats_test.py::TestAscFileFormat::test_read_can_message - AssertionError: messages are unequal:
FAILED test/logformats_test.py::TestAscFileFormat::test_read_can_remote_message - AssertionError: messages are unequal:

This is because it is asserted between relative and absolute timestamps.

For example, snipped codes in test/logformats_test.py:

    def test_read_can_and_canfd_error_frames(self):
        expected_messages = [
            can.Message(timestamp=2.501000, channel=0, is_error_frame=True),
            can.Message(timestamp=3.501000, channel=0, is_error_frame=True),
            can.Message(timestamp=4.501000, channel=1, is_error_frame=True),
            can.Message(
                timestamp=30.806898,
                channel=4,
                is_rx=False,
                is_error_frame=True,
                is_fd=True,
            ),
        ]
        actual = self._read_log_file("test_CanErrorFrames.asc")
        self.assertMessagesEqual(actual, expected_messages)

here actual are absolute timestamps, but expected_messages relative.

To solve this, we can delete start_timestamp when reading messages in can/io/asc.py:

    def __iter__(self) -> Generator[Message, None, None]:

        for _line in self.file:
            line = _line.strip()

            if not ASC_MESSAGE_REGEX.match(line):
                # line might be a comment, chip status,
                # J1939 message or some other unsupported event
                continue

            msg_kwargs: Dict[str, Union[float, bool, int]] = {}
            try:
                _timestamp, channel, rest_of_message = line.split(None, 2)
                timestamp = float(_timestamp) + self.start_timestamp

just do + self.start_timestamp

From my side, this change is correct. Users can get absolute timestamp in their own scripts.
But..., it would be not unified with BLFReader.
Please take a review about this point, thanks.

@Tian-Jionglu Tian-Jionglu changed the base branch from release-4.4.0 to main July 14, 2024 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants