examples/simple_ds402_node.py uses contradictory PDO mapping patterns

[acolomb]

The example uses the RemoteNode.load_configuration() API to send ParameterValue settings given in the sample EDS file to the device. This used to work in conjunction with PdoBase.read() to synchronize the locally cached PDO configuration and mapping info. Calling that method (on any PDO) was therefore not really necessary, as it was implied as a final step in load_configuration().

But #427 changed this and made load_configuration() apply the PDO configuration through calls to PdoBase.save() instead. Thus the need to read it back was eliminated.

In any case, the example goes on to call BaseNode402.setup_402_state_machine(), which then reads back the PDO configuration again. Here it would help to pass read_pdos=False since we know that the configuration was just saved, skipping the unnecessary upload.

That convenience method also checks whether needed values like the status and control words are configured in the respective PDOs. Which should succeed because of the initialization values in the sample EDS (haven't tested).

But then the example code continues with re-mapping the PDOs, which (probably?) invalidates the rpdo_pointers and tpdo_pointers entries pointing at the previous PDO mapping's PdoVariable objects. This should be done before setup_402_state_machine() to avoid this error. In addition, the example first uses node.tpdo.read() and node.rpdo.read() before adjusting the mappings, which, again, is unnecessary after load_configuration().

Furthermore, before the PDO re-mapping, it tries to change the node.state attribute, but there is no prior change of node.nmt.state = 'OPERATIONAL', thus the exchanges of control and status words cannot work. There is a fallback to SDO implemented if no PDO maps the required objects, but that is not triggered because they are in fact mapped. The problem is that in PRE-OPERATIONAL state, the node's PDOs simply do not function. Re-mapping them after the previous PdoVariable objects have been cached (see above) doesn't make this any better.

These are some observations from trying to understand the sample code, showing that it is mostly broken for actual usage and suggests wrong or inefficient patterns of the library API usage. There may be more problems hidden, which could be revealed after testing with actual hardware. Maybe that was done at some point, but probably not after the library API has evolved and some functionality (especially regarding profile CiA402) moved around.

We should discuss how to make this example actually useful again, or at the very minimum, document that it serves only as a demonstration of the API and will not work with real hardware unless the user takes care of the issues with their hopefully firm CANopen background themselves.

[erlend-aasland]

We should discuss how to make this example actually useful again, or at the very minimum, document that it serves only as a demonstration of the API and will not work with real hardware unless the user takes care of the issues with their hopefully firm CANopen background themselves.

FWIW, I think it would make sense to convert it to a full-fledged tutorial, alternatively a more concise how-to.

[sveinse]

The way a user needs to setup any node is highly dependent on the node capabilities.

• Some node might not have any PDO configuration on startup and needs .load_configuration().
• Some nodes have static PDO configuration and the local PDO configuration must be loaded from the OD (pdo.read(from_od=True)).
• Or perhaps the node has a variable PDO (compared to the OD) and it needs to be fetched from the node before use (.pdo.read(from_od=False)). I think the examples should highlight these three use cases

[erlend-aasland]

This sounds like a series of How-to guides to me.