canopen: DS402: Homing fails depending on power state, unnecessary state changes

The BaseNode402.homing() method tries to enter state SWITCHED ON upon entry. I think that’s unnecessary, the application should handle these transitions. But more importantly, it actually fails in many cases, namely if the previous state is SWITCH ON DISABLED, the default power-up state of most devices. That’s because there is an automatic way to reach OPERATION ENABLED over multiple intermediate steps, but the library does not know how to reach SWITCHED ON from any other state than OPERATION ENABLED or READY TO SWITCH ON. In addition, setting the op_mode property will already change to SWITCHED ON only if coming from OPERATION ENABLED (which is usually a good idea to avoid unexpected movement).

So I propose to just leave out that initial state switching, but wanted to discuss here first because it will probably break some people’s application on upgrades. Note that switching the operation mode to HOMING is actually safe in any power state, at least on devices I encountered.

Another change I propose is not restoring the previous operation mode, at least as an option. Just needless bus traffic and delay in case the next operation mode must be decided by the application anyway. So I would add an optional argument with the default value restore_op_mode=False. Anyone opposed?

Same goes for restoring the power state after switching the operation mode. Not really necessary and in case of homing() it will even cause additional delays by switching back and forth. Leaving OPERATION ENABLED when switching modes should also be optional, because there are valid applications where e.g. Profile Velocity and Profile Position need to be switched during operation without delay.

I will PR any agreed upon changes, so this issue is just for tracking and finding consensus.

About this issue

Original URL
State: open
Created 3 years ago
Comments: 16 (1 by maintainers)

Commits related to this issue

DS402: Minimize side-effects of operation mode switching (ref #244) (#251) * ds402: Keep target values on operation mode change. As the comment already stated, clearing the target values should p... — committed to christiansandberg/canopen by acolomb 3 years ago

Most upvoted comments

Sorry it took a while longer, but here’s my suggestion for a NEWS entry (wherever we may put that):

Support for the CiA 402 device profile for "drives and motion control" was
overhauled in this release, bringing some backwards-incompatible changes.
The goal is to make the library more flexible and easier to use from an
application requiring DS402 functions, especially when controlling several
synchronized motors.

CAUTION: The following changes will likely require adaptation if you are
         using the respective functions in your application code!

* The BaseNode402.homing() method no longer changs the DS402 power state
  machine.  A suitable state needs to be set by the application before using
  it. (#249)

* Remove the broken parameter set_new_home from the BaseNode402.homing()
  implementation, which did not work as intended. (#250)

* After homing, restore the previous operation mode only when explicitly
  requested using the restore_op_mode parameter. (#262)

* Minimize side-effects of operation mode switching.  Do not clear the
  target values and keep the power state unchanged.  This must now be
  handled in the application, but enables some use-cases where switching
  modes on the fly is desirable. (#251)

* Changes to the Controlword no longer trigger an immediate RPDO
  transmission if already configured as periodic.  That is the case when
  .start() has been called or the .period property was set externally.  For
  an RPDO whose transmission type indicates that it needs a SYNC object to
  apply, the immediate sending is also skipped.  Check the .is_periodic
  property to see whether this will change anything in your usage.

* The .op_mode property is now handled via PDO if possible, otherwise
  setup_402_state_machine() logs a warning and falls back to SDO, just like
  for the Statusword.  Note that if the PDO is expected periodically, the
  getter still blocks (up to 0.2 seconds) until a TPDO update was
  received. (#257)

* Reduce overhead in setup_402_state_machine() procedure by not switching
  the NMT state or the power state automatically.  The application should be
  responsible for that.  Only the NMT STOPPED state will now trigger an
  error before trying to read SDOs.  Reading the PDO configuration can even
  be skipped altogether using the optional read_pdos parameter. (#259)


Enhancements:

* The documentation page concerning device profiles was extended to include
  the relevant API description for the BaseNode402 class.

* All timeout and waiting durations have been moved to constants in the
  BaseNode402 class.  They can thus now be overridden per instance.

* BaseNode402.is_homed() to check whether homing is necessary, switching the
  operation mode automatically as required. (#248)

* Make PDO subscription available without calling read() or save(). (#253)

* The DS402 power state machine is now more flexible by trying to reach any
  target state even if there is no direct transition.  Some bugs were fixed
  as well, such as not trying transitions which only the drive itself can
  trigger. (#264)


Run-time optimizations:

* Supported operation modes are now cached internally to avoid reading
  object 0x6502 repeatedly as it should never change. (#247)

* Several actions which depend on an updated Statusword value now wait for a
  TPDO reception (up to 0.2 seconds), if the PDO is expected periodically.
  This can reduce some delays previously caused by checking loops with fixed
  delays. (#263)

Once we are certain where this should be documented, I will gladly adjust the formatting accordingly. While writing it up, I noticed a bug which will be fixed by #277.

acolomb on Nov 7, 2021