RAFCON icon indicating copy to clipboard operation
RAFCON copied to clipboard

Introduction of explicit handling of invalid elements

Open franzlst opened this issue 4 years ago • 8 comments

Generally, close to every change in RAFCON is checked for invalid constellation by there properties. All state elements are checked and maybe only the states and those constellations are not fully checked. But invalidities can be introduced by developers who create complex state machines with multiple library hierarchies. The most often invalidity that occurs is that a type of a data port of an used library state has changed and is connected by a data flow to another state's data port that still has the old data type.

If this problem occurs the first time an advanced user still don't know what to do. At the moment a user has to decode the error/traceback message, find the state machine (the right path) or respective state by searching the state machine file system folder and retry till the problem is maybe solved. In case of a invalid data flow there are three options, change the origin port (can be a library state), remove or change the data flow, or change the target data port (can be a Library State, too).

The issue propose a solution to solve those problems and similar possible things by the following concept:

  • every state holds a list of invalid state elements
  • invalidities found by the parent setter-check (calls validity check of state-elements) are insert into this list
  • a recursive notification/flag that marks states with invalid elements (or we add the state with invalid state elements, but this is kind of inconsistent)
  • a flag tolerate_invalidity in the runtime config that always is False when RAFCON is started but can be set while runtime to True (which is checked in the parent setter)
    • by tolerating the invalid elements the state machine can be loaded, visualized and by listing those they can be found very easily
  • as long as the invalidity flag is true and there are invalid elements an additional widget is generated that lists all invalid elements and maybe newly generated invalid elements while fixing those
    • the controller of this widget supports selection of all invalid elements (e.g. by state_machine.selection)
      • here also a already discussed feature (but not implemented yet) could be helpful the forced focus by "forced selection" to find the selected element not only in the state(s)-editor but also in the graphical editor
    • the respective widget controller observes all invalid elements and visualize solved situations
    • after all issues are solved and the widget is closed the tolerate_invalidity flag is set to False again and the cycle can start again
    • general this widget-controller has to observe all state machines because the fixing of an issue maybe needs to open another state machine and which also could have invalidities

The issue is considered to start as a discussion. Please at you opinion, concerns or possible improvements. This is the first raw shot on the topic. :)

Originally created by @Rbelder at 2017-09-18 12:34:34+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

For a more differentiated and transparent implementation I also would extend the flag to be a dictionary tolerate_invalid with respective flags for any type of state element that is tolerated to be invalid. This is something to be discussed.

This also become a topic because @doem-an would like to generate state machines more liberal as a expert user by for example tolerate invalid data flows by default to generate more generic library states. In this case the new widget would still collect invalid element and provide an interface to search those but the procedure to solve those would not be forced as described above.

Originally created by @Rbelder at 2017-09-19 13:54:48+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

Can we discuss this in the next RAFCON meeting? Actually I do not like the concept that much. I will talk to Andreas and check his motivation again.

Originally created by @sebastian-brunner ([email protected]) at 2017-09-25 06:10:16+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

I looked into the topic a bit deeper and as mentioned in the description the wrong data type is only one of the most common things.

Here some more options to create inconsistencies while developing libraries that cause smaller or more complex situations which are not very easily handled if you have close to no GUI-Support for those:

  • DataFlows:
    • target or origin port is missing
    • target/origin data port id has changed and thereby can not be found
    • and as mentioned/common data type conflicts of origin and target port
    • missing target/origin state (possible but not by using the GUI)
  • Transition:
    • missing origin or target outcome
    • target/origin outcome id has changed and thereby can not be found
    • missing target/origin state (possible but not by using the GUI)

The argument of Andreas is only the sub-sub-optional thing somebody could do with it. Major aspect is to give the user an option/support to solve issues of existing state machines (libraries) and there interface conflicts in a transparent way. It can not be the solution to tell the user to solve his issue with cryptic error messages with which even I need 5-10 minutes to find the origin and to access it to finally solve it in the way I wanna have it after maybe 30min (which is very hard if you don't have a mind map like the GUI is supporting it).

Only as some starting point before having the meeting.

Originally created by @Rbelder at 2017-09-25 09:19:49+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

Yeah let's discuss this in the meeting. I just saw that the LIBRARY_RECOVERY_MODE is not documented. Actually this is what I use to tackle these kind of problems.

Originally created by @sebastian-brunner ([email protected]) at 2017-09-25 09:33:50+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

So checked the LIBRARY_RECOVERY_MODE flag and I have seen it while checking for this issue already and I could have investigate it more. 😄 The flag in its current implementation would allow invalid data type constellations but remove all data flows that miss any target or origin element (state or port).

The user would not have any direct hint (library name is a bit less then a direct hint) or an option to directly open the state machine with the defect linkage inside. So the user has to be expert user of RAFCON and of the project he is working with (library-tree structure and so on).

Transitions would not be handled.

@sebastian-brunner If you have time come by today. I think it could be good to document this flag and integrate it into this issue (by e.g. generating only the GUI and enabling this kind of options only with this major expert flag).

Originally created by @Rbelder at 2017-09-25 12:37:41+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

I have given it a shoot, see branch allow_and_handle_invalid_elements_explicit.

  • The handling for data flow and also transitions should be possible.
  • Corrections of io-data-ports should also result in data flows and transitions to become valid.
  • The Data flow widget should be able to show in parent or origin/target state editor the invalid data flows even if gaphas is not showing them. Have fun while trying to freeze the GUI (feedback is welcome).

Future adaptation in mind:

  • [ ] the invalid elements widget will offer to remove invalid elements explicit
  • [ ] the selection should be in-cooperated better
  • [ ] the selection and opening should be handled more explicit by right click menu for invalid elements in libraries (at the moment the respective state machine is directly opened)
  • [ ] tab only shown if the flag LIBRARY_RECOVERY_MODE is True, also it would help to change the color of the icon if invalidity is resolved or a state machine becomes selected which has invalid elements

Originally created by @Rbelder at 2017-09-27 13:26:11+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

For data flows it works fine. Also the feature is nice, that the user can select the invalid data flow in the invalid elements list and the data flow becomes selected in gaphas.

The grammer is wrong in the following logging output:

In state HierarchyState with name 'new root state' and id 'MOUNIZ' [4 child states] MOUNIZ is invalid element Data flow - from_state: VRRMZI, from_key: 0, to_state: SNMBIO, to_key: 0, id: 3

Furthermore invalid transitions are not supported: When I delete an outcome of a library which is already connected in a state machine containing this library then a stack trace is printed and no entry appears in the invalid elements list.

Originally created by @sebastian-brunner ([email protected]) at 2017-09-28 07:25:52+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst

You are right have not tested it fully:

  • [x] have to adapt the parent assignment like for the data flow
  • [x] I have to improve the robustness of the transitions widget for this case, too.
  • [x] I also have to improve the dialog generation for this to support the user to choose the right config setting

@sebastian-brunner But generally I think your TOLERATE_INVALID was not with a 'Transition': True, right? And you don't have seen it because the dialog was not popping up and so on.

Originally created by @Rbelder at 2017-09-28 08:04:12+00:00 (moved from RMC internal repository)

franzlst avatar Apr 17 '20 09:04 franzlst