network: Avoid notifier re-creating in connect function (#621)

* network: Avoid notifier re-creating in connect function

* test_network: Add regression tests for notifier lifecycle

Add tests to verify that:
- connect() does not recreate an existing notifier
- disconnect() releases the notifier
- disconnect() releases the notifier even when check() raises

Co-authored-by: Frieder Schüler <frieder.schueler@bizerba.com>

Author: xndcn

canopen/network.py

@@ -107,7 +107,8 @@ def connect(self, *args, **kwargs) -> Network:
         if self.bus is None:
             self.bus = can.Bus(*args, **kwargs)
         logger.info("Connected to '%s'", self.bus.channel_info)
-        self.notifier = can.Notifier(self.bus, self.listeners, self.NOTIFIER_CYCLE)
+        if self.notifier is None:
+            self.notifier = can.Notifier(self.bus, self.listeners, self.NOTIFIER_CYCLE)
         return self
 
     def disconnect(self) -> None:
@@ -123,7 +124,11 @@ def disconnect(self) -> None:
         if self.bus is not None:
             self.bus.shutdown()
         self.bus = None
-        self.check()
+        try:
+            self.check()
+        finally:
+            # Release notifier after check
+            self.notifier = None
 
     def __enter__(self):
         return self

test/test_network.py

@@ -69,7 +69,8 @@ def test_network_check(self):
         def cleanup():
             # We must clear the fake exception installed below, since
             # .disconnect() implicitly calls .check() during test tear down.
-            self.network.notifier.exception = None
+            if self.network.notifier is not None:
+                self.network.notifier.exception = None
             self.network.disconnect()
 
         self.addCleanup(cleanup)
@@ -285,6 +286,34 @@ def wait_for_periodicity():
         if msg is not None:
             self.assertIsNone(bus.recv(PERIOD))
 
+    def test_network_connect_does_not_recreate_notifier(self):
+        self.network.connect(interface="virtual")
+        self.addCleanup(self.network.disconnect)
+        notifier1 = self.network.notifier
+        self.assertIsNotNone(notifier1)
+        # Calling connect() again should reuse the existing notifier
+        self.network.connect(interface="virtual")
+        self.assertIs(self.network.notifier, notifier1)
+
+    def test_network_disconnect_releases_notifier(self):
+        self.network.connect(interface="virtual")
+        self.assertIsNotNone(self.network.notifier)
+        self.network.disconnect()
+        self.assertIsNone(self.network.notifier)
+
+    def test_network_disconnect_releases_notifier_on_exception(self):
+        self.network.connect(interface="virtual")
+
+        class Custom(Exception):
+            pass
+
+        self.network.notifier.exception = Custom("fake")
+        with self.assertRaises(Custom):
+            with self.assertLogs(level=logging.ERROR):
+                self.network.disconnect()
+        # Notifier must be released even when check() raises
+        self.assertIsNone(self.network.notifier)
+
 
 class TestScanner(unittest.TestCase):
     TIMEOUT = 0.1