ASCWriter: use correct channel for error frame (#1583)

* use correct channel for error frame

* add test

can/io/asc.py

@@ -420,8 +420,15 @@ def log_event(self, message: str, timestamp: Optional[float] = None) -> None:
         self.file.write(line)
 
     def on_message_received(self, msg: Message) -> None:
+        channel = channel2int(msg.channel)
+        if channel is None:
+            channel = self.channel
+        else:
+            # Many interfaces start channel numbering at 0 which is invalid
+            channel += 1
+
         if msg.is_error_frame:
-            self.log_event(f"{self.channel}  ErrorFrame", msg.timestamp)
+            self.log_event(f"{channel}  ErrorFrame", msg.timestamp)
             return
         if msg.is_remote_frame:
             dtype = f"r {msg.dlc:x}"  # New after v8.5
@@ -432,12 +439,6 @@ def on_message_received(self, msg: Message) -> None:
         arb_id = f"{msg.arbitration_id:X}"
         if msg.is_extended_id:
             arb_id += "x"
-        channel = channel2int(msg.channel)
-        if channel is None:
-            channel = self.channel
-        else:
-            # Many interfaces start channel numbering at 0 which is invalid
-            channel += 1
         if msg.is_fd:
             flags = 0
             flags |= 1 << 12

can/message.py

@@ -291,6 +291,7 @@ def equals(
         self,
         other: "Message",
         timestamp_delta: Optional[float] = 1.0e-6,
+        check_channel: bool = True,
         check_direction: bool = True,
     ) -> bool:
         """
@@ -299,6 +300,7 @@ def equals(
         :param other: the message to compare with
         :param timestamp_delta: the maximum difference in seconds at which two timestamps are
                                 still considered equal or `None` to not compare timestamps
+        :param check_channel: whether to compare the message channel
         :param check_direction: whether to compare the messages' directions (Tx/Rx)
 
         :return: True if and only if the given message equals this one
@@ -322,7 +324,7 @@ def equals(
                 and self.data == other.data
                 and self.is_remote_frame == other.is_remote_frame
                 and self.is_error_frame == other.is_error_frame
-                and self.channel == other.channel
+                and (self.channel == other.channel or not check_channel)
                 and self.is_fd == other.is_fd
                 and self.bitrate_switch == other.bitrate_switch
                 and self.error_state_indicator == other.error_state_indicator

test/logformats_test.py

@@ -591,6 +591,26 @@ def test_no_triggerblock(self):
     def test_can_dlc_greater_than_8(self):
         _msg_list = self._read_log_file("issue_1299.asc")
 
+    def test_error_frame_channel(self):
+        # gh-issue 1578
+        err_frame = can.Message(is_error_frame=True, channel=4)
+
+        temp_file = tempfile.NamedTemporaryFile("w", delete=False)
+        temp_file.close()
+
+        try:
+            with can.ASCWriter(temp_file.name) as writer:
+                writer.on_message_received(err_frame)
+
+            with can.ASCReader(temp_file.name) as reader:
+                msg_list = list(reader)
+                assert len(msg_list) == 1
+                assert err_frame.equals(
+                    msg_list[0], check_channel=True
+                ), f"{err_frame!r}!={msg_list[0]!r}"
+        finally:
+            os.unlink(temp_file.name)
+
 
 class TestBlfFileFormat(ReaderWriterTest):
     """Tests can.BLFWriter and can.BLFReader.
@@ -814,7 +834,7 @@ def test_not_crashes_with_stdout(self):
                 printer(message)
 
     def test_not_crashes_with_file(self):
-        with tempfile.NamedTemporaryFile("w", delete=False) as temp_file:
+        with tempfile.NamedTemporaryFile("w") as temp_file:
             with can.Printer(temp_file) as printer:
                 for message in self.messages:
                     printer(message)

test/message_helper.py

@@ -4,8 +4,6 @@
 This module contains a helper for writing test cases that need to compare messages.
 """
 
-from copy import copy
-
 
 class ComparingMessagesTestCase:
     """
@@ -28,31 +26,14 @@ def assertMessageEqual(self, message_1, message_2):
         Checks that two messages are equal, according to the given rules.
         """
 
-        if message_1.equals(message_2, timestamp_delta=self.allowed_timestamp_delta):
-            return
-        elif self.preserves_channel:
+        if not message_1.equals(
+            message_2,
+            check_channel=self.preserves_channel,
+            timestamp_delta=self.allowed_timestamp_delta,
+        ):
             print(f"Comparing: message 1: {message_1!r}")
             print(f"           message 2: {message_2!r}")
-            self.fail(
-                "messages are unequal with allowed timestamp delta {}".format(
-                    self.allowed_timestamp_delta
-                )
-            )
-        else:
-            message_2 = copy(message_2)  # make sure this method is pure
-            message_2.channel = message_1.channel
-            if message_1.equals(
-                message_2, timestamp_delta=self.allowed_timestamp_delta
-            ):
-                return
-            else:
-                print(f"Comparing: message 1: {message_1!r}")
-                print(f"           message 2: {message_2!r}")
-                self.fail(
-                    "messages are unequal with allowed timestamp delta {} even when ignoring channels".format(
-                        self.allowed_timestamp_delta
-                    )
-                )
+            self.fail(f"messages are unequal: \n{message_1}\n{message_2}")
 
     def assertMessagesEqual(self, messages_1, messages_2):
         """