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

Fix ASC Reader start time

Open Tian-Jionglu opened this issue 1 year ago • 2 comments

to get correct absolute timestamp

Tian-Jionglu avatar Jun 07 '24 08:06 Tian-Jionglu

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 avatar Jun 07 '24 08:06 Tian-Jionglu

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 avatar Jun 11 '24 02:06 Tian-Jionglu