Open sveinse opened 2 months ago
IMO, the better thing to do would be to improve the documentation to put emphasis on always using Network.add_node
and Network.create_node
[^1] to create nodes.
BTW, are there use-cases for subclassing the *Base classes you mention?
We could make a default Network-like object that will emulate network operations if we need it to work without an actual CAN bus network.
I would instead prefer keeping the status quo.
[^1]: It is slightly confusing that add_node
and create_node
are used for creating remote and local nodes respectively. Something like create_remote_node
and create_local_node
would be clearer.
I would instead prefer keeping the status quo.
I've added all the runtime checks in PR #513 to ensure the None
case of Optional[Network]
is properly handled. We could avoid all that if Network
weren't Optional
. But in many ways, I agree because having as it is today requires less intrusive changes to the codebase.
Picking up my comment on #513... After some prototyping, I think Protocol
will not really help us here. We could still implement a canopen.network._DummyNetwork
class that basically throws targeted exceptions when any methods are actually used. That allows proper static type checking, while punting off error messages to run-time. Which is okay, because it depends on the correct run-time usage of the API (add to a network before using the node object) and a static checker will never be able to judge that. The type annotation on the attribute would be simply Network
then, initialized with the dummy object whose class inherits from Network
but overrides the constructor to not initialize all the attributes.
Would that be a feasible solution? It would require minimal changes only and not touch the "hot" code paths.
See #525 for a small proof of concept. I will extend that one to cover more instances when the approach is deemed acceptable.
With reference to #509 - lots of classes depend on an instance of
class Network
and have it as an attribute in order to communicate with the CAN bus. Examples areSdoBase()
,PdoBase()
,NmtBase()
and others. Their constructor does not require aNetwork
argument so the formal type of the attribute isnetwork: Optional[Network] = None
.These attributes are set when a
RemoteNode()
is created and the reference to the network is injected fromRemoteNode.associate_network()
.The effect of this is that we'll need a lot of checks all over the code to ensure network is properly set. E.g. as here from
class EmcyProducer
:Is is a requirement to be able to create these class instances without a functional Network? We could make a default Network-like object that will emulate network operations if we need it to work without an actual CAN bus network.
What do you think we should do here?