diff --git a/canopen/node/local.py b/canopen/node/local.py index 5c7a7b36..d903841e 100644 --- a/canopen/node/local.py +++ b/canopen/node/local.py @@ -81,7 +81,17 @@ def remove_network(self) -> None: def add_read_callback(self, callback: Callable): self._read_callbacks.append(callback) - def add_write_callback(self, callback: Callable): + def add_write_callback(self, callback: Callable) -> None: + """Register a callback invoked before writing to the local data store. + + The callback receives ``index``, ``subindex``, ``od`` and ``data`` as + keyword arguments. Return ``None`` to allow default storage, or any + other value (including ``False`` or ``0``) to suppress it for this write. + Existing stored values are left unchanged when storage is suppressed. + + Callbacks run in registration order unless a callback raises an + exception. A non-``None`` result does not skip later callbacks. + """ self._write_callbacks.append(callback) def get_data( @@ -132,8 +142,14 @@ def set_data( raise SdoAbortedError(0x06070010) # Try callbacks + skip_store = False for callback in self._write_callbacks: - callback(index=index, subindex=subindex, od=obj, data=data) + result = callback(index=index, subindex=subindex, od=obj, data=data) + if result is not None: + skip_store = True + + if skip_store: + return # Store data self.data_store.setdefault(index, {}) diff --git a/test/test_write_callbacks.py b/test/test_write_callbacks.py new file mode 100644 index 00000000..96f446a6 --- /dev/null +++ b/test/test_write_callbacks.py @@ -0,0 +1,165 @@ +import unittest +from unittest.mock import Mock, patch + +import canopen +from canopen.sdo.constants import ABORT_GENERAL_ERROR + +from .util import SAMPLE_EDS + + +class NonBooleanResult: + def __bool__(self): + raise AssertionError("Callback results must not be tested for truthiness") + + +class TestWriteCallbacks(unittest.TestCase): + def setUp(self): + self.node = canopen.LocalNode(2, SAMPLE_EDS) + + def test_non_none_results_suppress_new_storage(self): + for result in (True, False, 0, "", b"", [], NonBooleanResult()): + with self.subTest(result=type(result).__name__): + node = canopen.LocalNode(2, SAMPLE_EDS) + node.add_write_callback(Mock(return_value=result)) + node.set_data(0x2001, 0, b"\x12\x34") + self.assertNotIn(0x2001, node.data_store) + + def test_optout_does_not_skip_or_reset_later_callbacks(self): + for result in (True, False): + with self.subTest(result=result): + node = canopen.LocalNode(2, SAMPLE_EDS) + calls = [] + + def handle(**kwargs): + calls.append(("handle", kwargs["data"])) + return result + + def observe(**kwargs): + calls.append(("observe", kwargs["data"])) + + node.add_write_callback(handle) + node.add_write_callback(observe) + node.set_data(0x2001, 0, b"\x12\x34") + self.assertEqual(calls, [("handle", b"\x12\x34"), + ("observe", b"\x12\x34")]) + self.assertNotIn(0x2001, node.data_store) + + def test_optout_preserves_existing_store(self): + self.node.data_store = {0x2001: {0: b"\x01\x00", 1: b"other"}, + 0x2000: {0: b"unchanged"}} + self.node.add_write_callback(Mock(return_value=True)) + self.node.set_data(0x2001, 0, b"\x02\x00") + self.assertEqual(self.node.data_store, + {0x2001: {0: b"\x01\x00", 1: b"other"}, + 0x2000: {0: b"unchanged"}}) + + def test_default_storage_copies_mutable_data(self): + data = bytearray(b"\x12\x34") + self.node.set_data(0x2001, 0, data) + data[:] = b"\x00\x00" + self.assertEqual(self.node.data_store[0x2001][0], b"\x12\x34") + self.assertIsInstance(self.node.data_store[0x2001][0], bytes) + + def test_none_results_keep_default_storage_and_callback_arguments(self): + first, last = Mock(return_value=None), Mock(return_value=None) + self.node.add_write_callback(first) + self.node.add_write_callback(last) + data = b"\x12\x34" + self.node.set_data(0x2001, 0, data) + expected = dict(index=0x2001, subindex=0, od=self.node.object_dictionary[0x2001], + data=data) + first.assert_called_once_with(**expected) + last.assert_called_once_with(**expected) + self.assertEqual(self.node.data_store[0x2001][0], data) + + def test_callback_abort_still_propagates_before_storage(self): + self.node.data_store[0x2001] = {0: b"\x01\x00"} + self.node.add_write_callback(Mock(return_value=True)) + self.node.add_write_callback( + Mock(side_effect=canopen.SdoAbortedError(ABORT_GENERAL_ERROR))) + later = Mock(return_value=None) + self.node.add_write_callback(later) + with self.assertRaises(canopen.SdoAbortedError) as error: + self.node.set_data(0x2001, 0, b"\x02\x00") + self.assertEqual(error.exception.code, ABORT_GENERAL_ERROR) + later.assert_not_called() + self.assertEqual(self.node.data_store[0x2001][0], b"\x01\x00") + + def test_validation_still_precedes_callbacks(self): + callback = Mock(return_value=True) + self.node.add_write_callback(callback) + for index, data, code in ((0x2001, b"\x01", 0x06070010), + (0x1000, b"\x00" * 4, 0x06010002), + (0xFFFF, b"\x00", 0x06020000)): + with self.subTest(index=index): + with self.assertRaises(canopen.SdoAbortedError) as error: + self.node.set_data(index, 0, data, check_writable=True) + self.assertEqual(error.exception.code, code) + callback.assert_not_called() + self.assertEqual(self.node.data_store, {}) + + def test_read_callbacks_still_use_first_non_none_result(self): + first, later = Mock(return_value=13), Mock(return_value=17) + self.node.add_read_callback(first) + self.node.add_read_callback(later) + self.assertEqual(self.node.get_data(0x2001, 0), b"\x0d\x00") + first.assert_called_once() + later.assert_not_called() + + def test_local_sdo_custom_storage_round_trip(self): + application = {} + + def write(index, data, **kwargs): + application[index] = int.from_bytes(data, "little") + return False + + self.node.add_write_callback(write) + self.node.add_read_callback(lambda index, **kwargs: application[index]) + self.node.sdo.download(0x2001, 0, b"\x12\x34") + self.assertEqual(self.node.sdo.upload(0x2001, 0), b"\x12\x34") + self.assertEqual(application, {0x2001: 0x3412}) + self.assertNotIn(0x2001, self.node.data_store) + + def test_remote_expedited_and_segmented_sdo_round_trip(self): + client_network, server_network = canopen.Network(), canopen.Network() + server_network.add_node(self.node) + remote = client_network.add_node(2, SAMPLE_EDS) + remote.sdo.RESPONSE_TIMEOUT = 0.01 + client_network.send_message = lambda can_id, data, remote=False: server_network.notify( + can_id, data, 0.0) + server_network.send_message = lambda can_id, data, remote=False: client_network.notify( + can_id, data, 0.0) + application, observed = {}, [] + + def write(index, data, **kwargs): + application[index] = bytes(data) + return 0 + + def read(index, **kwargs): + data = application[index] + return int.from_bytes(data, "little") if index == 0x2001 else data.decode("ascii") + + self.node.add_write_callback(write) + self.node.add_write_callback( + lambda index, data, **kwargs: observed.append((index, bytes(data)))) + self.node.add_read_callback(read) + for index, data in ((0x2001, b"\x12\x34"), + (0x2000, b"A payload longer than one CAN frame")): + with self.subTest(index=index): + remote.sdo.download(index, 0, data) + self.assertEqual(remote.sdo.upload(index, 0), data) + self.assertEqual(application[index], data) + self.assertEqual(observed[-1], (index, data)) + self.assertNotIn(index, self.node.data_store) + self.assertIsNone(client_network.bus) + self.assertIsNone(server_network.bus) + + def test_builtin_heartbeat_callback_still_runs_with_optout(self): + self.node.add_write_callback(Mock(return_value=True)) + later = Mock(return_value=None) + self.node.add_write_callback(later) + with patch.object(self.node.nmt, "start_heartbeat") as start: + self.node.set_data(0x1017, 0, b"\x05\x00") + start.assert_called_once_with(5) + later.assert_called_once() + self.assertNotIn(0x1017, self.node.data_store)