Design discussion for Parameterization of VisualStates

sudo-panda commented 5 years ago

I have already added parameterization to VisualStates tool but as Sir Okan said, and I agree, that it would better to discuss it with everyone to better the design.

Here are some screenshots of the current design:

Pull request #1 adds this feature to VisualStates

pushkalkatara commented 5 years ago

I checked the design for parameterization, It looks good, let's develop parameters taking the easiest example of Kobuki Obstacle Avoidance, further we'll iterate on the design with the help of examples. I believe we have the parameter for LaserThreshold in the example, thus we can develop using it.

sudo-panda commented 5 years ago

kobuki_obstacle_avoidance.txt This the xml file for the modified kobuki obstacle avoidance. I changed the extension to txt so the I can upload it here.

pushkalkatara commented 5 years ago

Hi @sudo-panda. I tried importing the behavior. I have the following reviews -

The list showed after import only describes the parameter details. We have a current PR open on VisualStates repo to select particular states to import. The idea is to give details on a single tab after import of all the properties of the state. Thus we can discuss it more.
The parameters tab in the data option looks good. Your suggestion with tabbed UI would make the GUI more intuitive and easy for the user to alter the parameters, thus you can work further on that.

okanasik commented 5 years ago

First of all, I mean the design of the functionality. We need to discuss a little how the feature will work and how we can implement this feature as part of the current visualstates.

I checked your pull request, you designed params as part of visualstates. However, it is more intuitive to think that parameters are part of states. This would also make more sense when we import states with parameters.
As a small feedback for UI, it would be better, if you add new button at the top of the parameter. Also, it would look more professional if we can create a single look-and-feel of the UI. Can you check the ROS config UI? You can try to match the look similar to those UIs so that users have a same feeling for whole tool.
I think, it is not a good practice to catch index error by try except block, when we can explicitly check whether xml the node and attributes exists.
How do you represent parameters in source codes, maybe we can design a one-to-one mapping of a state variable as parameter. Therefore, when programming, we do not inject some random syntax to refer to the parameters. What do you think?

sudo-panda commented 5 years ago

States already have variables individually so why create a new way to add state wise variables. We can just convert the variables to act the way parameters do now. I had initially thought of parameters as a part of VisualStates and not individual states because I thought that parameters that needed to be tweaked were usually used globally in various states. We could also make the parameters global and mention during import the parameters used by each state. What do we do?

I agree that we should make the UI similar everywhere in the whole tool and I will place the new button at the top and try to make other changes that feel suitable.

okanasik commented 5 years ago

Since, we aim to import particular state from a visualstate, individual parameterization of whole visualstate does not make sense. Parameters can be thought in that sense a public variable of the state/behavior. For example, if parameters are at the root state, they will be used in whole visualstate. However, I am not sure which approach is better. Let's discuss this further by considering the case where the use wants to import a particular state of a complex behavior.

pushkalkatara commented 5 years ago

Currently, in VisualStates we have a Global Namespace which contains the ROS parameters and global variables which are accessible by any state or transition. Each state also contains a Local Namespace in which variables only accessible by that particular state exists. A variable, whether defined in the global namespace or local namespace may act as a parameter. But I think it is not necessary that every variable declared by the user is a parameter. I also think it is a good topic for discussion on whether to keep parameters individually or state specific. I have two examples in mind -

In the current implementation of Kobuki Obstacle Avoidance behavior, the obstacle_threshold is in Local Namespace i.e only accessible by the states under the root states.
PID controller for Prius Examples: Prius car has PID controller in throttle and steering. Both the PID controllers have separate Kp, Ki, kd thus they are separate parameters and apply individually to that state. My vote also goes to parameters being part of states, as it would benefit in complex behaviors and their visualization. Also, it would reduce name conflicts between parameters. We can think of a case in which making a global variable a parameter might be helpful, but I think it would increase the complexity to develop a behavior and understand the tool.

sudo-panda commented 5 years ago

Ok so we make parameters a local namespace variable in the source code. Do we only show the list of parameters during import or allow them to be modified?

sudo-panda commented 5 years ago

Final design:

Parameters are now part of local namespaces and is to be added to the local namespace dialog.
They act like normal variables in the local namespace.
Importing them do not require them to be displayed any longer.

sudo-panda commented 5 years ago

Right now the parameters behave exactly like local variables except for how they are created. So they don't have much advantage over the local namespace variables. This will change when we added the selective import of states and the online importer exporter to the tool.

Since I have to show the use of parameters and their advantage in the demonstration video I have nothing to show. Hence I was wondering if I could add a dialog that comes up during the generation of code that lists out all the parameters along with their states and allows users to only change the values. This will make it easier for the users to tweak their values to perfection

sudo-panda commented 5 years ago

Video for parameterization of VisualStates

sudo-panda commented 5 years ago

@okanasik You asked me to display a list of parameters added on import of a VisualStates file. Do I just show them or let the users edit the value too?

okanasik commented 5 years ago

@sudo-panda I am not sure whether letting the user to change the parameters while importing the state will be useful. The user may need to test the behavior to find the correct parameters. What do you think?

jmplaza commented 5 years ago

The auotmata parameters is more an edition topic than an import topic. Isn't it?

I mean, the user imports an automaton (an existing HFSM from the library) to include it on her new automaton that is being created, and AFTER that import she can fine tune the particular behavior of the imported automaton changing its parameters at will. This remains me like importing a software library: the programmer imports a library and after that calls its functions with the corresponding parameters/arguments....

sudo-panda commented 5 years ago

So what are you suggesting @jmplaza ?

sudo-panda commented 5 years ago

Imported Params

Is this representation fine?

okanasik commented 5 years ago

@sudo-panda this representation looks good. Nice work.

It could be better if you can add some background color to differentiate the parameters from the window background.
Also it could be better to show parameters as table so that you do not repeat Type, Value and Description for every line. Type, Value, Description can be bold at the line of bold Parameter part. Best

okanasik commented 5 years ago

The auotmata parameters is more an edition topic than an import topic. Isn't it?

I mean, the user imports an automaton (an existing HFSM from the library) to include it on her new automaton that is being created, and AFTER that import she can fine tune the particular behavior of the imported automaton changing its parameters at will. This remains me like importing a software library: the programmer imports a library and after that calls its functions with the corresponding parameters/arguments....

Actually, we do not design this way. We assume that parameter update and change is part of the import process. Also, user can change the parameter at any point of the development. The only difference is that user can import states from a repository of behavior. Also, the user can cherry-pick which ever state, she wants to import.

pushkalkatara commented 5 years ago

Is this representation fine?

The representation looks good. I think maybe we can discuss the visuals in the case of large automatons.
Also, when would this representation be shown? Is it right after imports, or would we provide the user an option to show the description?

sudo-panda commented 5 years ago

Week 7 discussion: Tree of states with parameters are going to be shown during import and the the states can be selected right there and states are collapsible

pushkalkatara commented 5 years ago

Hi @sudo-panda , I found a minor bug while checking the PR. Here's the issue - #6

TheRoboticsClub / colab-gsoc2019-Baidyanath_Kundu

Design discussion for Parameterization of VisualStates #2