OpenScenario parameter assignment is broken #832
Comments
|
I just push a new commit that should fix this issue. Please give it a try. |
|
Thanks for the quick reply. I tried the new fix, with that the ouput is |
|
Can you share your scenario file? The traceback is different now... |
|
So I managed to solve the error message for the route maneuver catalog, resolving the error message from my previous comment by adding a string conversion to the line from your fix: -> However, after solving I'm running into similar issues with the rest of the parameters too:
This too was solved by adding ParameterRef in openscenario_parser get_weather_from_env_action(), line 477: -> But the openscenario_parser.py seems to be full of this kind of catalog refs, meaning adding this in a lot places. Should this be addressed here, or on a higher level? As I said, my scenario file was working fine with the earlier commit of scenario runner, so I think there was a change introduced somewhere that causes the parameter parsing to not properly work in the master. Also, the problem doesn't just affect catalogs. This works (check 3rd last line): This doesn't (check 3rd last line): So I think it's a general bug related to parameter parsing. Here is my scenario file for reference anyway. I will be deleting it once we resolve this issue. |
|
Actually, here's the minimum steps to reproduce, based on making the map choice a parameter in the FollowLeadingVehicle.xosc example (changes only on lines 6 and 11, compared to the original): Running this through the ros-bridge rviz2-plugin leads to |
|
Hi, Please give it a try, and let me know if it solved your issues. |
|
Hi, But one more thing. With the changes you suggested, the ParameterAction wasn't working. This was because the config file is parsed at scenario init, so any parameters that would be affected by ParameterAction were actually hard coded at init. Because of this, ParameterAction correctly modified the parameter values stored by CarlaDataProvider, but these new values weren't accessed when evaluating Actions such as UserDefinedAction. I resolved this for UserDefinedAction by moving parameter value resolving into RunScript.update() in atomic_behaviors.py, which is evaluated at runtime. Changes made into parameter values by ParameterAction are now correctly reflected in parameters supplied to UserDefinedAction. I don't know if this issue comes up when using parameters affected by ParameterAction with Actions types other than UserDefinedAction. To avoid circular imports, I had to move ParameterRef definition into it's own file. The solution isn't perfect, but this PR should extend the support for ParameterAction atleast a bit. Please comment on style and naming conventions if necessary, I haven't contributed before. Thanks for your attention on the issue, this really helped me a lot! |
|
Hi, In the past, we went over the XOSC file and basically replaced all parameter calls with the default values. However, to enable dynamic parameters, the ParameterRef class was introduced. The problem that arises is that all xml-tree accesses should be routed through ParameterRef (aka attrib.get(value) --> ParameterRef(attrib.get(value))). This was forgotten for some places, and my PR intended to fix this. |
|
Hi, This is why I moved the ParameterRef for UserDefinedAction into the atomic behaviour update() method. That is evaluated at runtime, not at scenario initialization. I tried the both ways, and this was the one that worked. Calling ParameterRef in openscenario_parser led to the parameter value in the xmltree staying the same despite ParameterAction. I suppose other maneuvers also might need this modification? |
|
Well, the problem is that we construct the entire behavior tree for the scenario at the very beginning. If that construction fails, because a ParameterRef has to be converted to float/str/whatever, we have to do the conversion. But then, even if this is very late in the scenario, the value is already converted and may not be updated. That basically would mean that everything that can be a parameter has to be fowarded to the corresponding atomics and interpreted at runtime. |
|
Yeah, I agree with you that fully supporting ParameterActions would require everything that can be parametrized to be forwarded onto the corresponding atomic behavior.
Is this something that would be desirable?
|
|
I guess, if we touch it now, it is better to do it right ;) Do you want to give it a try? |
|
Hmm. What would it require on code level? From how I understand the code, this would mean doing the same thing I did for UserDefinedAction for
All of this seems a bit much to do by myself, but I could do one piece for starters. For example rework the functions in convert_maneuver_to_atomic(). |
|
Hello @fabianoboril, I began implementing the runtime parameterRef resolution for atomic behaviours. I started by implementing the functionality for AbsoluteTargetSpeed and RelativeTargetSpeed OSC instructions. Could you check the included commit to determine if my approach is okay? The commit can be found here. The relevant changes can be found in openscenario_parser.py lines 1051-1089 Please let me know if this seems good, and I'll implement the changes for the rest of the atomic behaviors. |
|
Hello @fabianoboril ! (Tagging @germanros1987 in case he's not actively working on Carla at the moment.) PR #834 now intoduces ParameterAction support to all OpenSCENARIO Actions - the parameter values are now resolved during runtime inside the AtomicBehaviors. I tested each of the actions with ParameterAction and they now seem to work correctly. Could you review the changes, please? |
Describe the bug
A clear and concise description of what the bug is.
Hello, I updated the scenario runner from
05a811a
to
e5b06cc
to get OpenScenario ParameterAction support. However, now it seems that the parsing of .xosc files is somehow broken. When a parameter is used in the .xosc file, the name of the parameter is passed on instead of the value assigned to the parameter.
To Reproduce
Steps to reproduce the behavior:
Expected behavior
Before updating, this retrieved the catalog maneuver with name "Map01_testroute_maneuver" correctly.
Screenshots
Leads to:
This seems to affect all parameter usage in the .xosc file. Before updating the scenario runner everything was working well. I suspect the breaking changes are introduced by 8ed24c8, however I couldn't find the cause. Can others reproduce the issue, or could it still be something on my end?
Desktop (please complete the following information):
I'm using the scenario runner through the ros-bridge.
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: