Add autostart option to BusABC.send_periodic() (#1853)

* Add autostart option (kwarg) to BusABC.send_periodic() to fix issue #1848

* Fix for #1849 (PCAN fails when PCAN_ERROR_ILLDATA is read via ReadFD) (#1850)

* When there is an invalid frame on CAN bus (in our case CAN FD), PCAN first reports result PCAN_ERROR_ILLDATA and then it send the error frame. If the PCAN_ERROR_ILLDATA is not ignored, python-can throws an exception.

This fix add the ignore on the PCAN_ERROR_ILLDATA.

* Fix for ruff error `can/interfaces/pcan/pcan.py:5:1: I001 [*] Import block is un-sorted or un-formatted`
Added comment explaining why to ignore the PCAN_ERROR_ILLDATA.

* Format with black to pass checks

* Do not ignore autostart parameter for Bus.send_periodic() on IXXAT devices

* Do not ignore autostart parameter for Bis.send_periodic() on socketcan devices

* Fix double start socketcan periodic

* Fix link methods in docstring for start() methods of  the tasks can.broadcastmanager.CyclicTask.start

* Change the behaviour of autostart parameter in socketcan implementation of  CyclicSendTask to not call _tx_setup() method instead of adding a parameter to it.

* Fix code style (max 100 chars per line)
Fix wrong docstring reference.

---------

Co-authored-by: Tomas Bures <[email protected]>
Co-authored-by: Sebastian Wolf <[email protected]>

can/broadcastmanager.py

@@ -276,6 +276,7 @@ def __init__(
         period: float,
         duration: Optional[float] = None,
         on_error: Optional[Callable[[Exception], bool]] = None,
+        autostart: bool = True,
         modifier_callback: Optional[Callable[[Message], None]] = None,
     ) -> None:
         """Transmits `messages` with a `period` seconds for `duration` seconds on a `bus`.
@@ -324,7 +325,8 @@ def __init__(
                 stacklevel=1,
             )
 
-        self.start()
+        if autostart:
+            self.start()
 
     def stop(self) -> None:
         self.stopped = True

can/bus.py

@@ -215,6 +215,7 @@ def send_periodic(
         period: float,
         duration: Optional[float] = None,
         store_task: bool = True,
+        autostart: bool = True,
         modifier_callback: Optional[Callable[[Message], None]] = None,
     ) -> can.broadcastmanager.CyclicSendTaskABC:
         """Start sending messages at a given period on this bus.
@@ -237,6 +238,10 @@ def send_periodic(
         :param store_task:
             If True (the default) the task will be attached to this Bus instance.
             Disable to instead manage tasks manually.
+        :param autostart:
+            If True (the default) the sending task will immediately start after creation.
+            Otherwise, the task has to be started by calling the
+            tasks :meth:`~can.RestartableCyclicTaskABC.start` method on it.
         :param modifier_callback:
             Function which should be used to modify each message's data before
             sending. The callback modifies the :attr:`~can.Message.data` of the
@@ -272,7 +277,9 @@ def send_periodic(
         # Create a backend specific task; will be patched to a _SelfRemovingCyclicTask later
         task = cast(
             _SelfRemovingCyclicTask,
-            self._send_periodic_internal(msgs, period, duration, modifier_callback),
+            self._send_periodic_internal(
+                msgs, period, duration, autostart, modifier_callback
+            ),
         )
         # we wrap the task's stop method to also remove it from the Bus's list of tasks
         periodic_tasks = self._periodic_tasks
@@ -299,6 +306,7 @@ def _send_periodic_internal(
         msgs: Union[Sequence[Message], Message],
         period: float,
         duration: Optional[float] = None,
+        autostart: bool = True,
         modifier_callback: Optional[Callable[[Message], None]] = None,
     ) -> can.broadcastmanager.CyclicSendTaskABC:
         """Default implementation of periodic message sending using threading.
@@ -312,6 +320,10 @@ def _send_periodic_internal(
         :param duration:
             The duration between sending each message at the given rate. If
             no duration is provided, the task will continue indefinitely.
+        :param autostart:
+            If True (the default) the sending task will immediately start after creation.
+            Otherwise, the task has to be started by calling the
+            tasks :meth:`~can.RestartableCyclicTaskABC.start` method on it.
         :return:
             A started task instance. Note the task can be stopped (and
             depending on the backend modified) by calling the
@@ -328,6 +340,7 @@ def _send_periodic_internal(
             messages=msgs,
             period=period,
             duration=duration,
+            autostart=autostart,
             modifier_callback=modifier_callback,
         )
         return task

can/interfaces/ixxat/canlib.py

@@ -155,10 +155,11 @@ def _send_periodic_internal(
         msgs: Union[Sequence[Message], Message],
         period: float,
         duration: Optional[float] = None,
+        autostart: bool = True,
         modifier_callback: Optional[Callable[[Message], None]] = None,
     ) -> CyclicSendTaskABC:
         return self.bus._send_periodic_internal(
-            msgs, period, duration, modifier_callback
+            msgs, period, duration, autostart, modifier_callback
         )
 
     def shutdown(self) -> None:

can/interfaces/ixxat/canlib_vcinpl.py

@@ -793,6 +793,7 @@ def _send_periodic_internal(
         msgs: Union[Sequence[Message], Message],
         period: float,
         duration: Optional[float] = None,
+        autostart: bool = True,
         modifier_callback: Optional[Callable[[Message], None]] = None,
     ) -> CyclicSendTaskABC:
         """Send a message using built-in cyclic transmit list functionality."""
@@ -807,7 +808,12 @@ def _send_periodic_internal(
                 self._scheduler_resolution = caps.dwClockFreq / caps.dwCmsDivisor
                 _canlib.canSchedulerActivate(self._scheduler, constants.TRUE)
             return CyclicSendTask(
-                self._scheduler, msgs, period, duration, self._scheduler_resolution
+                self._scheduler,
+                msgs,
+                period,
+                duration,
+                self._scheduler_resolution,
+                autostart=autostart,
             )
 
         # fallback to thread based cyclic task
@@ -821,6 +827,7 @@ def _send_periodic_internal(
             msgs=msgs,
             period=period,
             duration=duration,
+            autostart=autostart,
             modifier_callback=modifier_callback,
         )
 
@@ -863,7 +870,15 @@ def state(self) -> BusState:
 class CyclicSendTask(LimitedDurationCyclicSendTaskABC, RestartableCyclicTaskABC):
     """A message in the cyclic transmit list."""
 
-    def __init__(self, scheduler, msgs, period, duration, resolution):
+    def __init__(
+        self,
+        scheduler,
+        msgs,
+        period,
+        duration,
+        resolution,
+        autostart: bool = True,
+    ):
         super().__init__(msgs, period, duration)
         if len(self.messages) != 1:
             raise ValueError(
@@ -883,7 +898,8 @@ def __init__(self, scheduler, msgs, period, duration, resolution):
         self._msg.uMsgInfo.Bits.dlc = self.messages[0].dlc
         for i, b in enumerate(self.messages[0].data):
             self._msg.abData[i] = b
-        self.start()
+        if autostart:
+            self.start()
 
     def start(self):
         """Start transmitting message (add to list if needed)."""

can/interfaces/ixxat/canlib_vcinpl2.py

@@ -937,6 +937,7 @@ def _send_periodic_internal(
         msgs: Union[Sequence[Message], Message],
         period: float,
         duration: Optional[float] = None,
+        autostart: bool = True,
         modifier_callback: Optional[Callable[[Message], None]] = None,
     ) -> CyclicSendTaskABC:
         """Send a message using built-in cyclic transmit list functionality."""
@@ -953,7 +954,12 @@ def _send_periodic_internal(
                 )  # TODO: confirm
                 _canlib.canSchedulerActivate(self._scheduler, constants.TRUE)
             return CyclicSendTask(
-                self._scheduler, msgs, period, duration, self._scheduler_resolution
+                self._scheduler,
+                msgs,
+                period,
+                duration,
+                self._scheduler_resolution,
+                autostart=autostart,
             )
 
         # fallback to thread based cyclic task
@@ -967,6 +973,7 @@ def _send_periodic_internal(
             msgs=msgs,
             period=period,
             duration=duration,
+            autostart=autostart,
             modifier_callback=modifier_callback,
         )
 
@@ -983,7 +990,15 @@ def shutdown(self):
 class CyclicSendTask(LimitedDurationCyclicSendTaskABC, RestartableCyclicTaskABC):
     """A message in the cyclic transmit list."""
 
-    def __init__(self, scheduler, msgs, period, duration, resolution):
+    def __init__(
+        self,
+        scheduler,
+        msgs,
+        period,
+        duration,
+        resolution,
+        autostart: bool = True,
+    ):
         super().__init__(msgs, period, duration)
         if len(self.messages) != 1:
             raise ValueError(
@@ -1003,7 +1018,8 @@ def __init__(self, scheduler, msgs, period, duration, resolution):
         self._msg.uMsgInfo.Bits.dlc = self.messages[0].dlc
         for i, b in enumerate(self.messages[0].data):
             self._msg.abData[i] = b
-        self.start()
+        if autostart:
+            self.start()
 
     def start(self):
         """Start transmitting message (add to list if needed)."""

can/interfaces/socketcan/socketcan.py

@@ -327,6 +327,7 @@ def __init__(
         messages: Union[Sequence[Message], Message],
         period: float,
         duration: Optional[float] = None,
+        autostart: bool = True,
     ) -> None:
         """Construct and :meth:`~start` a task.
 
@@ -349,10 +350,13 @@ def __init__(
 
         self.bcm_socket = bcm_socket
         self.task_id = task_id
-        self._tx_setup(self.messages)
+        if autostart:
+            self._tx_setup(self.messages)
 
     def _tx_setup(
-        self, messages: Sequence[Message], raise_if_task_exists: bool = True
+        self,
+        messages: Sequence[Message],
+        raise_if_task_exists: bool = True,
     ) -> None:
         # Create a low level packed frame to pass to the kernel
         body = bytearray()
@@ -813,6 +817,7 @@ def _send_periodic_internal(
         msgs: Union[Sequence[Message], Message],
         period: float,
         duration: Optional[float] = None,
+        autostart: bool = True,
         modifier_callback: Optional[Callable[[Message], None]] = None,
     ) -> can.broadcastmanager.CyclicSendTaskABC:
         """Start sending messages at a given period on this bus.
@@ -823,13 +828,17 @@ def _send_periodic_internal(
         :class:`CyclicSendTask` within BCM provides flexibility to schedule
         CAN messages sending with the same CAN ID, but different CAN data.
 
-        :param messages:
+        :param msgs:
             The message(s) to be sent periodically.
         :param period:
             The rate in seconds at which to send the messages.
         :param duration:
             Approximate duration in seconds to continue sending messages. If
             no duration is provided, the task will continue indefinitely.
+        :param autostart:
+            If True (the default) the sending task will immediately start after creation.
+            Otherwise, the task has to be started by calling the
+            tasks :meth:`~can.RestartableCyclicTaskABC.start` method on it.
 
         :raises ValueError:
             If task identifier passed to :class:`CyclicSendTask` can't be used
@@ -854,7 +863,9 @@ def _send_periodic_internal(
             msgs_channel = str(msgs[0].channel) if msgs[0].channel else None
             bcm_socket = self._get_bcm_socket(msgs_channel or self.channel)
             task_id = self._get_next_task_id()
-            task = CyclicSendTask(bcm_socket, task_id, msgs, period, duration)
+            task = CyclicSendTask(
+                bcm_socket, task_id, msgs, period, duration, autostart=autostart
+            )
             return task
 
         # fallback to thread based cyclic task
@@ -868,6 +879,7 @@ def _send_periodic_internal(
             msgs=msgs,
             period=period,
             duration=duration,
+            autostart=autostart,
             modifier_callback=modifier_callback,
         )