Behavior and failure for uninitialized network

[sveinse]

https://github.com/christiansandberg/canopen/blob/5382f245d8ede61b113c6a648cb99b199667f0cd/canopen/node/local.py#L51-L59

Calling LocalNode.remove_network() or RemoteNode.remove_network() on an already closed network will fail, due to self.network no longer have the .unsubscribe() method (which is expected). Depending on what behavior is wanted, there lacks some functionality with regards to the logic of uninitialized LocalNode. Related to #525

Option 1 - multiple calls to .remove_network() should be ok

If it should be acceptable to call .remove_network() multiple times without failing -> then these methods should support that the network is uninitialized

    def remove_network(self) -> None:
        if self.network is canopen.network._UNINITIALIZED_NETWORK:
            return
        # Rest of the function....

Option 2 - multiple calls to .remove_network() should fail

If a repeated call to .remove_network() should fail or fail on uninitialized network, I think we're missing a function to test if the LocalNode object is uninitialized.

LocalNode and friends it sets the network uninitialized on its __init__() and then later initialize it by .associate_network(). This means that there are code paths through the usage of LocalNode where a cleanup code might need to call .remove_network() conditionally depending on the state of its association with a network.

There is no current way to test if a LocalNode object is associated without using the private canopen.network._UNINITIALIZED_NETWORK object. Moreso, _UNINITIALIZED_NETWORK is inherited from Network so a negative isinstance(node.network, Network) does not help to determine if its initialized.

To prevent access to the private object, I suggest that the best approach would be to add a new .has_network() method to the LocalNode and RemoteNode implementation.

    def has_network(self):
       """Return True if the node has an associated network."""
       return self.network is not canopen.network._UNINITIALIZED_NETWORK

[acolomb]

Thanks for pointing this out. I prefer the first option, it does not make much sense to fail on that call.

[sveinse]

While working on this, it was uncovered that calling associate_network() multiple times is not safe either. The Network.subscribe() method happily appends multiple callbacks to its lists. This is addressed in the PR.

[sveinse]

FYI I just discovered that we already have the a method for testing that a node is associated with a network or not:

canopen/canopen/node/base.py

Lines 30 to 32 in 48cca7c

	def has_network(self) -> bool:
	"""Check whether the node has been associated to a network."""
	return not isinstance(self.network, canopen.network._UninitializedNetwork)