From 7a3d23fa3ba114a6cd005c726e7815732c566f6e Mon Sep 17 00:00:00 2001 From: Sebastian Wolf Date: Tue, 8 Oct 2024 16:10:46 +0200 Subject: [PATCH] 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 Co-authored-by: Sebastian Wolf --- can/broadcastmanager.py | 4 +++- can/bus.py | 15 ++++++++++++++- can/interfaces/ixxat/canlib.py | 3 ++- can/interfaces/ixxat/canlib_vcinpl.py | 22 +++++++++++++++++++--- can/interfaces/ixxat/canlib_vcinpl2.py | 22 +++++++++++++++++++--- can/interfaces/socketcan/socketcan.py | 20 ++++++++++++++++---- 6 files changed, 73 insertions(+), 13 deletions(-) diff --git a/can/broadcastmanager.py b/can/broadcastmanager.py index 0fa94c82c..bc65d3a47 100644 --- a/can/broadcastmanager.py +++ b/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 diff --git a/can/bus.py b/can/bus.py index 954c78c5f..a12808ab6 100644 --- a/can/bus.py +++ b/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 diff --git a/can/interfaces/ixxat/canlib.py b/can/interfaces/ixxat/canlib.py index 330ccdcd9..a1693aeed 100644 --- a/can/interfaces/ixxat/canlib.py +++ b/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: diff --git a/can/interfaces/ixxat/canlib_vcinpl.py b/can/interfaces/ixxat/canlib_vcinpl.py index 709780ceb..0579d0942 100644 --- a/can/interfaces/ixxat/canlib_vcinpl.py +++ b/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).""" diff --git a/can/interfaces/ixxat/canlib_vcinpl2.py b/can/interfaces/ixxat/canlib_vcinpl2.py index 18b3a1e57..5872f76b9 100644 --- a/can/interfaces/ixxat/canlib_vcinpl2.py +++ b/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).""" diff --git a/can/interfaces/socketcan/socketcan.py b/can/interfaces/socketcan/socketcan.py index 03bd8c3f8..40da0d094 100644 --- a/can/interfaces/socketcan/socketcan.py +++ b/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, )