From e9f64e8f86619202105939b5579c91f16cb0e54f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Colomb?= <src@andre.colomb.de>
Date: Wed, 6 May 2026 10:03:05 +0200
Subject: [PATCH 1/2] Make sure any BaseNode always has a valid Node ID.

The BaseNode.id attribute can be initialized either from the given
node_id parameter, or from the Object Dictionary if zero was passed.
The None value as mentioned in the docstring actually violates the
type hint, so drop that suggestion.  Make sure the inferred type of
the self.id attribute does not allow None, but is always an integer.

Furthermore, check the resulting Node ID against the allowed value
range and raise a ValueError instead of constructing a node object
with an invalid Node ID, which will cause problems down the road.
This is technically an API change, but it just causes things to fail
early on instead of raising exceptions later on.

Copy the docstring for LocalNode as well, to document the expected
node_id "invalid" value.  And because there was none at all before,
include a bit of functional description to set the expectations
straight.
---
 canopen/node/base.py   |  6 ++++--
 canopen/node/local.py  | 15 +++++++++++++++
 canopen/node/remote.py |  2 +-
 test/test_node.py      | 19 +++++++++++++++++++
 4 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/canopen/node/base.py b/canopen/node/base.py
index 45ad35b4..ba2c9764 100644
--- a/canopen/node/base.py
+++ b/canopen/node/base.py
@@ -8,7 +8,7 @@ class BaseNode:
     """A CANopen node.
 
     :param node_id:
-        Node ID (set to None or 0 if specified by object dictionary)
+        Node ID (set to 0 if specified by object dictionary)
     :param object_dictionary:
         Object dictionary as either a path to a file, an ``ObjectDictionary``
         or a file like object.
@@ -25,7 +25,9 @@ def __init__(
             object_dictionary = import_od(object_dictionary, node_id)
         self.object_dictionary = object_dictionary
 
-        self.id = node_id or self.object_dictionary.node_id
+        self.id = node_id or object_dictionary.node_id or 0
+        if not self.id or not 1 <= self.id <= 127:
+            raise ValueError("No valid Node ID provided, %r not in range 1..127")
 
     def has_network(self) -> bool:
         """Check whether the node has been associated to a network."""
diff --git a/canopen/node/local.py b/canopen/node/local.py
index 3620152c..5c7a7b36 100644
--- a/canopen/node/local.py
+++ b/canopen/node/local.py
@@ -18,6 +18,21 @@
 
 
 class LocalNode(BaseNode):
+    """Local CANopen node implementing essential communication services.
+
+    This does not provide a full-fledged communication logic stack, but needs
+    additional application logic to wire up the various services, such as
+    triggering PDO transmissions according to their communication parameters.
+
+    Notable exceptions are a local data store for SDO server access, and using
+    the Heartbeat Producer Time parameter to control Heartbeat transmission.
+
+    :param node_id:
+        Node ID (set to 0 if specified by object dictionary)
+    :param object_dictionary:
+        Object dictionary as either a path to a file, an ``ObjectDictionary``
+        or a file like object.
+    """
 
     def __init__(
         self,
diff --git a/canopen/node/remote.py b/canopen/node/remote.py
index 01bf8f82..b5e4e6b1 100644
--- a/canopen/node/remote.py
+++ b/canopen/node/remote.py
@@ -19,7 +19,7 @@ class RemoteNode(BaseNode):
     """A CANopen remote node.
 
     :param node_id:
-        Node ID (set to None or 0 if specified by object dictionary)
+        Node ID (set to 0 if specified by object dictionary)
     :param object_dictionary:
         Object dictionary as either a path to a file, an ``ObjectDictionary``
         or a file like object.
diff --git a/test/test_node.py b/test/test_node.py
index 43373a2a..973cd664 100644
--- a/test/test_node.py
+++ b/test/test_node.py
@@ -8,6 +8,25 @@ def count_subscribers(network: canopen.Network) -> int:
     return sum(len(n) for n in network.subscribers.values())
 
 
+class TestBaseNode(unittest.TestCase):
+
+    def test_valid_node_id(self):
+        node = canopen.node.base.BaseNode(1, canopen.ObjectDictionary())
+        self.assertEqual(node.id, 1)
+
+    def test_valid_node_id_from_od(self):
+        od = canopen.ObjectDictionary()
+        od.node_id = 2
+        node = canopen.node.base.BaseNode(0, od)
+        self.assertEqual(node.id, 2)
+
+    def test_invalid_node_id(self):
+        with self.assertRaises(ValueError):
+            _ = canopen.node.base.BaseNode(0, canopen.ObjectDictionary())
+        with self.assertRaises(ValueError):
+            _ = canopen.node.base.BaseNode(128, canopen.ObjectDictionary())
+
+
 class TestLocalNode(unittest.TestCase):
 
     @classmethod

From 9edf10b06877144fc8fbddc0b3dc0d287217fe6a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Colomb?= <src@andre.colomb.de>
Date: Thu, 7 May 2026 09:37:48 +0200
Subject: [PATCH 2/2] fix SNAFU caught in review

---
 canopen/node/base.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/canopen/node/base.py b/canopen/node/base.py
index ba2c9764..f3d6888b 100644
--- a/canopen/node/base.py
+++ b/canopen/node/base.py
@@ -26,8 +26,8 @@ def __init__(
         self.object_dictionary = object_dictionary
 
         self.id = node_id or object_dictionary.node_id or 0
-        if not self.id or not 1 <= self.id <= 127:
-            raise ValueError("No valid Node ID provided, %r not in range 1..127")
+        if not 1 <= self.id <= 127:
+            raise ValueError(f"No valid Node ID provided, {self.id} not in range 1..127")
 
     def has_network(self) -> bool:
         """Check whether the node has been associated to a network."""