Fix Optional handling for node IDs and PDO variables

[bizfsc]

Fix Optional handling to prevent operations on potentially None values.

Problem

mypy reports [operator] and [union-attr] errors where Optional values are used without None-checks — e.g. 0x600 + self.id when self.id could be None, or divmod(self.offset, 8) when self.offset is None.

Changes

canopen/node/base.py

• Annotate self.id as Optional[int] to match its runtime value (node_id or self.object_dictionary.node_id).

canopen/node/remote.py / canopen/node/local.py

• Add assert self.id is not None in __init__ and re-annotate self.id: int to narrow the type for all methods.

canopen/pdo/base.py

• Add assert guards for self.offset and self.pdo_parent at the start of get_data() and set_data().
• Fix wait_for_reception() return type: Optional[float] (was float, but returns None on timeout).

Impact

Eliminates 21 [operator] and [union-attr] errors in node and PDO modules.", "draft">false

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[acolomb]

I think this is going in the wrong direction. The possibility of None for node IDs causes many problems later on, so instead we should make sure that the ID is definitely set and maybe even in the valid range.

The intention here (according to the docstring) is that a dummy value can be passed for node_id in order to respect the one from the OD. Note that the mentioned passing of None even contradicts the type hint and is bad advice. Passing zero is just as good for an "unspecified" sentinel value. If neither source leads to a usable node ID, I don't think we should even finish constructing the node object.

What do you think? Should we try the opposite way and restrict self.id to purely (valid) integers in general?

[bizfsc]

Agreed — this is the right direction. Making self.id Optional just pushes the problem downstream to every caller. It's better to enforce a valid ID at construction time.

The current pattern node_id or self.object_dictionary.node_id already treats 0 and None identically as "unspecified", which aligns with your suggestion. The fix would be: if the result is still None (or 0), raise a ValueError in BaseNode.init instead of silently leaving the object in a broken state.

Happy to rework the PR in that direction.

[acolomb]

I'm working on a counter suggestion PR right now. See #657.

[friederschueler]

Closed in favor of #657

[acolomb]

I think there are some other improvements in here that #657 doesn't cover. Feel free to reopen with reduced scope, or make a new PR.