Skip to content

Commit 31b982e

Browse files
emcy: catch callback exceptions and expand test coverage
- Fix bug where an exception in one EMCY callback would abort all subsequent callbacks; now logs the exception and continues - Add test for callback exception handling (verifies fix) - Add tests for EmcyConsumer initialization, multiple callback order, error reset variants, wait timeout edge cases, concurrent errors - Add tests for EmcyError initialization types and str representation - Add tests for EmcyProducer initialization, send/reset edge cases - Add TestEmcyIntegration class using network.subscribe() for producer-consumer roundtrip tests - Remove superfluous inline comments per reviewer feedback (acolomb) - Convert leading comment to docstring in test_emcy_consumer_on_emcy Addresses review feedback from PR #604 by yuvraj-kolkar17.
1 parent c05b49d commit 31b982e

2 files changed

Lines changed: 169 additions & 5 deletions

File tree

canopen/emcy.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,10 @@ def on_emcy(self, can_id, data, timestamp):
3939
self.emcy_received.notify_all()
4040

4141
for callback in self.callbacks:
42-
callback(entry)
42+
try:
43+
callback(entry)
44+
except Exception:
45+
logger.exception("Exception in EMCY callback")
4346

4447
def add_callback(self, callback: Callable[[EmcyError], None]):
4548
"""Get notified on EMCY messages from this node.

test/test_emcy.py

Lines changed: 165 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,12 @@ def check_error(self, err, code, reg, data, ts):
2525
self.assertAlmostEqual(err.timestamp, ts)
2626

2727
def test_emcy_consumer_on_emcy(self):
28-
# Make sure multiple callbacks receive the same information.
28+
"""Make sure multiple callbacks receive the same information."""
2929
acc1 = []
3030
acc2 = []
3131
self.emcy.add_callback(lambda err: acc1.append(err))
3232
self.emcy.add_callback(lambda err: acc2.append(err))
3333

34-
# Dispatch an EMCY datagram.
3534
self.emcy.on_emcy(0x81, b'\x01\x20\x02\x00\x01\x02\x03\x04', 1000)
3635

3736
self.assertEqual(len(self.emcy.log), 1)
@@ -45,7 +44,6 @@ def test_emcy_consumer_on_emcy(self):
4544
data=bytes([0, 1, 2, 3, 4]), ts=1000,
4645
)
4746

48-
# Dispatch a new EMCY datagram.
4947
self.emcy.on_emcy(0x81, b'\x10\x90\x01\x04\x03\x02\x01\x00', 2000)
5048
self.assertEqual(len(self.emcy.log), 2)
5149
self.assertEqual(len(self.emcy.active), 2)
@@ -58,7 +56,6 @@ def test_emcy_consumer_on_emcy(self):
5856
data=bytes([4, 3, 2, 1, 0]), ts=2000,
5957
)
6058

61-
# Dispatch an EMCY reset.
6259
self.emcy.on_emcy(0x81, b'\x00\x00\x00\x00\x00\x00\x00\x00', 2000)
6360
self.assertEqual(len(self.emcy.log), 3)
6461
self.assertEqual(len(self.emcy.active), 0)
@@ -123,6 +120,65 @@ def push_reset():
123120
t.start()
124121
self.assertIsNone(self.emcy.wait(0x9000, TIMEOUT))
125122

123+
def test_emcy_consumer_initialization(self):
124+
consumer = canopen.emcy.EmcyConsumer()
125+
self.assertEqual(consumer.log, [])
126+
self.assertEqual(consumer.active, [])
127+
self.assertEqual(consumer.callbacks, [])
128+
129+
def test_emcy_consumer_multiple_callbacks(self):
130+
"""Test adding multiple callbacks and their execution order."""
131+
call_order = []
132+
self.emcy.add_callback(lambda err: call_order.append('callback1'))
133+
self.emcy.add_callback(lambda err: call_order.append('callback2'))
134+
self.emcy.add_callback(lambda err: call_order.append('callback3'))
135+
self.emcy.on_emcy(0x81, b'\x01\x20\x02\x00\x01\x02\x03\x04', 1000)
136+
self.assertEqual(call_order, ['callback1', 'callback2', 'callback3'])
137+
138+
def test_emcy_consumer_callback_exception_handling(self):
139+
"""Test that callback exceptions don't break other callbacks or the system."""
140+
successful_callbacks = []
141+
self.emcy.add_callback(lambda err: successful_callbacks.append('success1'))
142+
self.emcy.add_callback(
143+
lambda err: exec('raise ValueError("Test exception in callback")')
144+
)
145+
self.emcy.add_callback(lambda err: successful_callbacks.append('success2'))
146+
self.emcy.on_emcy(0x81, b'\x01\x20\x02\x00\x01\x02\x03\x04', 1000)
147+
self.assertEqual(successful_callbacks, ['success1', 'success2'])
148+
149+
def test_emcy_consumer_error_reset_variants(self):
150+
"""Test different error reset code patterns."""
151+
self.emcy.on_emcy(0x81, b'\x01\x20\x02\x00\x01\x02\x03\x04', 1000)
152+
self.emcy.on_emcy(0x81, b'\x10\x90\x01\x04\x03\x02\x01\x00', 2000)
153+
self.assertEqual(len(self.emcy.active), 2)
154+
self.emcy.on_emcy(0x81, b'\x00\x00\x00\x00\x00\x00\x00\x00', 3000)
155+
self.assertEqual(len(self.emcy.active), 0)
156+
self.emcy.on_emcy(0x81, b'\x01\x30\x02\x00\x01\x02\x03\x04', 4000)
157+
self.assertEqual(len(self.emcy.active), 1)
158+
self.emcy.on_emcy(0x81, b'\x99\x00\x01\x00\x00\x00\x00\x00', 5000)
159+
self.assertEqual(len(self.emcy.active), 0)
160+
161+
def test_emcy_consumer_wait_timeout_edge_cases(self):
162+
"""Test wait method with various timeout scenarios."""
163+
result = self.emcy.wait(timeout=0)
164+
self.assertIsNone(result)
165+
result = self.emcy.wait(timeout=0.001)
166+
self.assertIsNone(result)
167+
168+
def test_emcy_consumer_wait_concurrent_errors(self):
169+
"""Test wait method when multiple errors arrive concurrently."""
170+
def push_multiple_errors():
171+
self.emcy.on_emcy(0x81, b'\x01\x20\x01\x01\x02\x03\x04\x05', 100)
172+
self.emcy.on_emcy(0x81, b'\x02\x20\x01\x01\x02\x03\x04\x05', 101)
173+
self.emcy.on_emcy(0x81, b'\x03\x20\x01\x01\x02\x03\x04\x05', 102)
174+
t = threading.Timer(TIMEOUT / 2, push_multiple_errors)
175+
with self.assertLogs(level=logging.INFO):
176+
t.start()
177+
err = self.emcy.wait(0x2003, timeout=TIMEOUT)
178+
t.join(TIMEOUT)
179+
self.assertIsNotNone(err)
180+
self.assertEqual(err.code, 0x2003)
181+
126182

127183
class TestEmcyError(unittest.TestCase):
128184
def test_emcy_error(self):
@@ -180,6 +236,26 @@ def check(code, expected):
180236
check(0xff00, "Device Specific")
181237
check(0xffff, "Device Specific")
182238

239+
def test_emcy_error_initialization_types(self):
240+
"""Test EmcyError initialization with various data types."""
241+
error = EmcyError(0x1000, 0, b'', 123.456)
242+
self.assertEqual(error.code, 0x1000)
243+
self.assertEqual(error.register, 0)
244+
self.assertEqual(error.data, b'')
245+
self.assertEqual(error.timestamp, 123.456)
246+
error = EmcyError(0xFFFF, 0xFF, b'\xFF' * 5, float('inf'))
247+
self.assertEqual(error.code, 0xFFFF)
248+
self.assertEqual(error.register, 0xFF)
249+
self.assertEqual(error.data, b'\xFF' * 5)
250+
self.assertEqual(error.timestamp, float('inf'))
251+
252+
def test_emcy_error_str_edge_cases(self):
253+
for code in (0x0000, 0x0001, 0x0100, 0xFFFF):
254+
error = EmcyError(code, 0, b'', 1000)
255+
s = str(error)
256+
self.assertIsInstance(s, str)
257+
self.assertIn(f"0x{code:04X}", s)
258+
183259

184260
class TestEmcyProducer(unittest.TestCase):
185261
def setUp(self):
@@ -220,6 +296,91 @@ def check(*args, res):
220296
check(3, res=b'\x00\x00\x03\x00\x00\x00\x00\x00')
221297
check(3, b"\xaa\xbb", res=b'\x00\x00\x03\xaa\xbb\x00\x00\x00')
222298

299+
def test_emcy_producer_initialization(self):
300+
producer = canopen.emcy.EmcyProducer(0x123)
301+
self.assertEqual(producer.cob_id, 0x123)
302+
self.assertIsNotNone(producer.network)
303+
304+
def test_emcy_producer_send_edge_cases(self):
305+
self.emcy.send(0xFFFF, 0xFF, b'\xFF\xFF\xFF\xFF\xFF')
306+
self.check_response(b'\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF')
307+
self.emcy.send(0x0000, 0x00)
308+
self.check_response(b'\x00\x00\x00\x00\x00\x00\x00\x00')
309+
self.emcy.send(0x1234, 0x56, b'\xAB\xCD')
310+
self.check_response(b'\x34\x12\x56\xAB\xCD\x00\x00\x00')
311+
self.emcy.send(0x1234, 0x56, b'\xAB\xCD\xEF\x12\x34')
312+
self.check_response(b'\x34\x12\x56\xAB\xCD\xEF\x12\x34')
313+
314+
def test_emcy_producer_reset_edge_cases(self):
315+
self.emcy.reset(0xFF)
316+
self.check_response(b'\x00\x00\xFF\x00\x00\x00\x00\x00')
317+
self.emcy.reset(0xFF, b'\xFF\xFF\xFF\xFF\xFF')
318+
self.check_response(b'\x00\x00\xFF\xFF\xFF\xFF\xFF\xFF')
319+
self.emcy.reset(0x12, b'\xAB\xCD')
320+
self.check_response(b'\x00\x00\x12\xAB\xCD\x00\x00\x00')
321+
322+
323+
class TestEmcyIntegration(unittest.TestCase):
324+
"""Integration tests for EMCY producer and consumer."""
325+
326+
def setUp(self):
327+
self.txbus = can.Bus(interface="virtual")
328+
self.rxbus = can.Bus(interface="virtual")
329+
self.net = canopen.Network(self.txbus)
330+
self.net.NOTIFIER_SHUTDOWN_TIMEOUT = 0.0
331+
self.net.connect()
332+
self.rx_net = canopen.Network(self.rxbus)
333+
self.rx_net.NOTIFIER_SHUTDOWN_TIMEOUT = 0.0
334+
self.rx_net.connect()
335+
self.producer = canopen.emcy.EmcyProducer(0x081)
336+
self.producer.network = self.net
337+
self.consumer = canopen.emcy.EmcyConsumer()
338+
self.rx_net.subscribe(0x081, self.consumer.on_emcy)
339+
340+
def tearDown(self):
341+
self.net.disconnect()
342+
self.rx_net.disconnect()
343+
self.txbus.shutdown()
344+
self.rxbus.shutdown()
345+
346+
def test_producer_consumer_integration(self):
347+
"""Test that producer and consumer work together."""
348+
received_errors = []
349+
self.consumer.add_callback(lambda err: received_errors.append(err))
350+
t = threading.Timer(
351+
TIMEOUT / 2,
352+
lambda: self.producer.send(0x2001, 0x02, b'\x01\x02\x03\x04\x05'),
353+
)
354+
with self.assertLogs(level=logging.INFO):
355+
t.start()
356+
err = self.consumer.wait(0x2001, timeout=TIMEOUT)
357+
t.join(TIMEOUT)
358+
self.assertIsNotNone(err)
359+
self.assertEqual(err.code, 0x2001)
360+
self.assertEqual(err.register, 0x02)
361+
self.assertEqual(err.data, b'\x01\x02\x03\x04\x05')
362+
self.assertEqual(received_errors, [err])
363+
364+
def test_producer_reset_consumer_integration(self):
365+
"""Test producer reset clears consumer active errors."""
366+
t = threading.Timer(
367+
TIMEOUT / 2,
368+
lambda: self.producer.send(0x2001, 0x02, b'\x01\x02\x03\x04\x05'),
369+
)
370+
with self.assertLogs(level=logging.INFO):
371+
t.start()
372+
self.consumer.wait(0x2001, timeout=TIMEOUT)
373+
t.join(TIMEOUT)
374+
self.assertEqual(len(self.consumer.active), 1)
375+
t = threading.Timer(TIMEOUT / 2, self.producer.reset)
376+
with self.assertLogs(level=logging.INFO):
377+
t.start()
378+
err = self.consumer.wait(timeout=TIMEOUT)
379+
t.join(TIMEOUT)
380+
self.assertIsNotNone(err)
381+
self.assertEqual(len(self.consumer.active), 0)
382+
self.assertEqual(len(self.consumer.log), 2)
383+
223384

224385
if __name__ == "__main__":
225386
unittest.main()

0 commit comments

Comments
 (0)