From 0dfee209eca393b1e02782f1a05a635b25dbdf5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 12 Aug 2021 17:32:24 +0200 Subject: [PATCH 1/8] p402: Forbid transitions to states which only the drive can enter. The NOT READY TO SWITCH ON and FAULT REACTION ACTIVE states can only be reached if the drive encounters an error or is still busy with its self-testing. Raise an exception in case these states are requested via the property. In extension, only the drive can trigger a transition to the FAULT state, so that is never valid as an end goal requested by the user. --- canopen/profiles/p402.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/canopen/profiles/p402.py b/canopen/profiles/p402.py index f5233ea4..b200189c 100644 --- a/canopen/profiles/p402.py +++ b/canopen/profiles/p402.py @@ -497,6 +497,11 @@ def state(self, target_state): time.sleep(self.INTERVAL_CHECK_STATE) def _next_state(self, target_state): + if target_state in ('NOT READY TO SWITCH ON', + 'FAULT REACTION ACTIVE', + 'FAULT'): + raise ValueError( + 'Target state {} cannot be entered programmatically'.format(target_state)) if target_state == 'OPERATION ENABLED': return State402.next_state_for_enabling(self.state) else: From 6eec07bbdf9c804aa8db1a33fbcb585c01c4231c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 12 Aug 2021 23:22:10 +0200 Subject: [PATCH 2/8] p402: Add a test script to check DS402 State Machine transitions. Go through all possible target states and display where the next_state_for_enabling() function would lead to based on the original state. Mark transitions which can happen directly because they are in the TRANSITIONTABLE. --- canopen/profiles/test_p402_states.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 canopen/profiles/test_p402_states.py diff --git a/canopen/profiles/test_p402_states.py b/canopen/profiles/test_p402_states.py new file mode 100644 index 00000000..a2045932 --- /dev/null +++ b/canopen/profiles/test_p402_states.py @@ -0,0 +1,17 @@ +from canopen.profiles.p402 import State402 + + +if __name__ == '__main__': + for target_state in State402.SW_MASK: + print('\n--- Target =', target_state, '---') + for from_state in State402.SW_MASK: + if target_state == from_state: + continue + if (from_state, target_state) in State402.TRANSITIONTABLE: + print('direct:\t{} -> {}'.format(from_state, target_state)) + else: + next_state = State402.next_state_for_enabling(from_state) + if not next_state: + print('FAIL:\t{} -> {}'.format(from_state, next_state)) + else: + print('\t{} -> {} ...'.format(from_state, next_state)) From f361b6b85ffc5ee196544c2bde26053884574f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Fri, 13 Aug 2021 00:25:02 +0200 Subject: [PATCH 3/8] p402: Extend test script to check the actual implementation. Mock up a BaseNode402 object and compare the _next_state() behavior to the simple lookups from the underlying transition tables. --- canopen/profiles/test_p402_states.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/canopen/profiles/test_p402_states.py b/canopen/profiles/test_p402_states.py index a2045932..e163125e 100644 --- a/canopen/profiles/test_p402_states.py +++ b/canopen/profiles/test_p402_states.py @@ -1,7 +1,10 @@ -from canopen.profiles.p402 import State402 +from canopen.objectdictionary import ObjectDictionary +from canopen.profiles.p402 import State402, BaseNode402 if __name__ == '__main__': + n = BaseNode402(1, ObjectDictionary()) + for target_state in State402.SW_MASK: print('\n--- Target =', target_state, '---') for from_state in State402.SW_MASK: @@ -15,3 +18,12 @@ print('FAIL:\t{} -> {}'.format(from_state, next_state)) else: print('\t{} -> {} ...'.format(from_state, next_state)) + + try: + while from_state != target_state: + n.tpdo_values[0x6041] = State402.SW_MASK[from_state][1] + next_state = n._next_state(target_state) + print('\t\t-> {}'.format(next_state)) + from_state = next_state + except ValueError: + print('\t\t-> disallowed!') From 75fb4079a6138ca3356d814b8ff78b3c3c8e8211 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 12 Aug 2021 23:29:48 +0200 Subject: [PATCH 4/8] p402: Simplify check for intermediate states. Don't special case the OPERATION ENABLED state, as the mechanism formulated for it is actually usable for almost all states. Instead, check whether there is a direct transition and return that immediately before consulting next_state_for_enabling(). --- canopen/profiles/p402.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/canopen/profiles/p402.py b/canopen/profiles/p402.py index b200189c..05e0c21a 100644 --- a/canopen/profiles/p402.py +++ b/canopen/profiles/p402.py @@ -502,10 +502,11 @@ def _next_state(self, target_state): 'FAULT'): raise ValueError( 'Target state {} cannot be entered programmatically'.format(target_state)) - if target_state == 'OPERATION ENABLED': - return State402.next_state_for_enabling(self.state) - else: + from_state = self.state + if (from_state, target_state) in State402.TRANSITIONTABLE: return target_state + else: + return State402.next_state_for_enabling(from_state) def _change_state(self, target_state): try: From c993235189f8d3bb260bc1a01aa199d1db904f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 12 Aug 2021 23:38:38 +0200 Subject: [PATCH 5/8] p402: Rename NEXTSTATE2ENABLE to generalize it. The goal is to provide automatic transition paths for any state, not only OPERATION ENABLED. Reflect that in the naming, without changing the actual logic yet. --- canopen/profiles/p402.py | 14 +++++++------- canopen/profiles/test_p402_states.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/canopen/profiles/p402.py b/canopen/profiles/p402.py index 05e0c21a..10882d16 100644 --- a/canopen/profiles/p402.py +++ b/canopen/profiles/p402.py @@ -46,14 +46,14 @@ class State402(object): 'QUICK STOP ACTIVE': (0x6F, 0x07), } - # Transition path to get to the 'OPERATION ENABLED' state - NEXTSTATE2ENABLE = { + # Transition path to reach and state without a direct transition + NEXTSTATE2ANY = { ('START'): 'NOT READY TO SWITCH ON', ('FAULT', 'NOT READY TO SWITCH ON'): 'SWITCH ON DISABLED', ('SWITCH ON DISABLED'): 'READY TO SWITCH ON', ('READY TO SWITCH ON'): 'SWITCHED ON', ('SWITCHED ON', 'QUICK STOP ACTIVE', 'OPERATION ENABLED'): 'OPERATION ENABLED', - ('FAULT REACTION ACTIVE'): 'FAULT' + ('FAULT REACTION ACTIVE'): 'FAULT', } # Tansition table from the DS402 State Machine @@ -86,14 +86,14 @@ class State402(object): } @staticmethod - def next_state_for_enabling(_from): - """Return the next state needed for reach the state Operation Enabled. + def next_state_indirect(_from): + """Return the next state needed to reach any state indirectly. :param str target: Target state. :return: Next target to change. :rtype: str """ - for cond, next_state in State402.NEXTSTATE2ENABLE.items(): + for cond, next_state in State402.NEXTSTATE2ANY.items(): if _from in cond: return next_state @@ -506,7 +506,7 @@ def _next_state(self, target_state): if (from_state, target_state) in State402.TRANSITIONTABLE: return target_state else: - return State402.next_state_for_enabling(from_state) + return State402.next_state_indirect(from_state) def _change_state(self, target_state): try: diff --git a/canopen/profiles/test_p402_states.py b/canopen/profiles/test_p402_states.py index e163125e..44b6fd72 100644 --- a/canopen/profiles/test_p402_states.py +++ b/canopen/profiles/test_p402_states.py @@ -13,7 +13,7 @@ if (from_state, target_state) in State402.TRANSITIONTABLE: print('direct:\t{} -> {}'.format(from_state, target_state)) else: - next_state = State402.next_state_for_enabling(from_state) + next_state = State402.next_state_indirect(from_state) if not next_state: print('FAIL:\t{} -> {}'.format(from_state, next_state)) else: From 7386d3571730263370641a9521f191813ef8eae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Thu, 12 Aug 2021 23:48:02 +0200 Subject: [PATCH 6/8] p402: Adjust automatic state transition paths. As there is a direct transition from QUICK STOP ACTIVE to OPERATION ENABLED, remove that from the transition paths. Instead go through the SWITCH ON DISABLED state, closing the cycle to make it work for anything between SWITCH ON DISABLED and OPERATION ENABLED. Also remove the self-reference OPERATION ENABLED to itself, which is useless. The whole state changing code will only be called if the target state and the current state do not match. --- canopen/profiles/p402.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/canopen/profiles/p402.py b/canopen/profiles/p402.py index 10882d16..74ed9323 100644 --- a/canopen/profiles/p402.py +++ b/canopen/profiles/p402.py @@ -49,10 +49,10 @@ class State402(object): # Transition path to reach and state without a direct transition NEXTSTATE2ANY = { ('START'): 'NOT READY TO SWITCH ON', - ('FAULT', 'NOT READY TO SWITCH ON'): 'SWITCH ON DISABLED', + ('FAULT', 'NOT READY TO SWITCH ON', 'QUICK STOP ACTIVE'): 'SWITCH ON DISABLED', ('SWITCH ON DISABLED'): 'READY TO SWITCH ON', ('READY TO SWITCH ON'): 'SWITCHED ON', - ('SWITCHED ON', 'QUICK STOP ACTIVE', 'OPERATION ENABLED'): 'OPERATION ENABLED', + ('SWITCHED ON'): 'OPERATION ENABLED', ('FAULT REACTION ACTIVE'): 'FAULT', } @@ -89,6 +89,11 @@ class State402(object): def next_state_indirect(_from): """Return the next state needed to reach any state indirectly. + The chosen path always points toward the OPERATION ENABLED state, except when + coming from QUICK STOP ACTIVE. In that case, it will cycle through SWITCH ON + DISABLED first, as there would have been a direct transition if the opposite was + desired. + :param str target: Target state. :return: Next target to change. :rtype: str From 52cdd66aa551167c5163c80c72a278b4c47d44f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Fri, 13 Aug 2021 00:31:43 +0200 Subject: [PATCH 7/8] p402: Remove two illegal transitions from the state table. Transitions 7 and 10 are duplicated and certainly wrong in the quickstop context. The only transition toward QUICK STOP ACTIVE is from OPERATION ENABLED. --- canopen/profiles/p402.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/canopen/profiles/p402.py b/canopen/profiles/p402.py index 74ed9323..6d09a411 100644 --- a/canopen/profiles/p402.py +++ b/canopen/profiles/p402.py @@ -78,8 +78,6 @@ class State402(object): ('SWITCHED ON', 'OPERATION ENABLED'): CW_OPERATION_ENABLED, # transition 4 ('QUICK STOP ACTIVE', 'OPERATION ENABLED'): CW_OPERATION_ENABLED, # transition 16 # quickstop --------------------------------------------------------------------------- - ('READY TO SWITCH ON', 'QUICK STOP ACTIVE'): CW_QUICK_STOP, # transition 7 - ('SWITCHED ON', 'QUICK STOP ACTIVE'): CW_QUICK_STOP, # transition 10 ('OPERATION ENABLED', 'QUICK STOP ACTIVE'): CW_QUICK_STOP, # transition 11 # fault ------------------------------------------------------------------------------- ('FAULT', 'SWITCH ON DISABLED'): CW_SWITCH_ON_DISABLED, # transition 15 From 831d5c0d5575d2d94614b4fd9cb5c3926aa7eb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Colomb?= Date: Mon, 13 Sep 2021 11:43:12 +0200 Subject: [PATCH 8/8] Move test_p402_states script to a subdirectory and add a docstring. --- canopen/profiles/{ => tools}/test_p402_states.py | 8 ++++++++ 1 file changed, 8 insertions(+) rename canopen/profiles/{ => tools}/test_p402_states.py (80%) diff --git a/canopen/profiles/test_p402_states.py b/canopen/profiles/tools/test_p402_states.py similarity index 80% rename from canopen/profiles/test_p402_states.py rename to canopen/profiles/tools/test_p402_states.py index 44b6fd72..39f085f5 100644 --- a/canopen/profiles/test_p402_states.py +++ b/canopen/profiles/tools/test_p402_states.py @@ -1,3 +1,11 @@ +"""Verification script to diagnose automatic state transitions. + +This is meant to be run for verifying changes to the DS402 power state +machine code. For each target state, it just lists the next +intermediate state which would be set automatically, depending on the +assumed current state. +""" + from canopen.objectdictionary import ObjectDictionary from canopen.profiles.p402 import State402, BaseNode402