Lua preprocessor for yaml #45
Conversation
… in 3d vis rendering. Fixed boolean params.
|
cool! So this embeds Lua code into the yaml files? |
|
Yeah. The use case for Avidbots is being able to create one flatland configuration file for each robot, and parametrically load in the cleaning head dimensions, enable/disable sensors based on configuration, etc. I was originally going to create a domain specific language for conditionally enabling parts of a model, but it was a complexity explosion. Lua is designed for fast, easy integration, and supports way more functionality that we really need. I was mostly interested in arbitrary math equations, nested conditionals, and the ability to integrate with rosparam/environment variables, which this PR achieves in <700 lines of code including documentation and unit test coverage. The intent is to provide near feature parity with xacro+urdf in terms of being able to parametrically define everything, and conditionally load things. |
|
|
||
| /** | ||
| */ | ||
| class YamlPreprocessor { |
There was a problem hiding this comment.
Since all members of this class are public static, why don't just make it a namespace? At this stage it has nothing to do with class.
| ROS_WARN("======== 3d visualize? ==========="); | ||
| // Don't try to visualized uninitalized layers | ||
| if (viz_name_.length() == 0) { | ||
| return; |
There was a problem hiding this comment.
Not sure when will this happen, but do we need a warning here?
There was a problem hiding this comment.
That was an old debugging statement. I removed it.
flatland_server/src/model.cpp
Outdated
| << " disabled"); | ||
| continue; | ||
| } else { | ||
| ROS_WARN_STREAM("Body " |
There was a problem hiding this comment.
Do we really need a warning for enabled bodies? If we don't we can get rid of the entire else clause. Same is true for the other object loadings.
flatland_server/src/model.cpp
Outdated
| << body_reader.Get<std::string>("name", "unnamed") | ||
| << " disabled"); | ||
| continue; | ||
| } else { |
There was a problem hiding this comment.
Just a style preference, we don't really need else here if we have continue above.
| ProcessScalarNode(node); | ||
| } | ||
| break; | ||
| } |
There was a problem hiding this comment.
Add a case default to catch any unexpected types, if any. It also is a clear statement that all cases have been considered.
| ROS_INFO_STREAM("Attempting to parse lua " << value); | ||
|
|
||
| try { | ||
| if (value.find("return ") == |
There was a problem hiding this comment.
The scope of this try{} block can be reduced by moving these string manipulations out.
| bindings for env and param | ||
| ------------------------------- | ||
|
|
||
| Additional lua function bindings (beyond the normal standard libraries such as string, math, etc.): |
There was a problem hiding this comment.
Are these bindings used for reading the environment variable and rosparam? If so can you explicitly say it?
There was a problem hiding this comment.
That's a good point! I'll add that in another pr.
Lua preprocessor for yaml
Added a lua preprocessor for YAML with simple bindings for the environment variables and rosparam.
The intent is to be able to build parametric models that are defined by dimensions and flags found in either environment variables or rosparam, in a similar way to xacro+urdf. Because this is parsed at model load time, any roslaunch rosparam loading will have completed and those parameters will be available.
Documentation: https://flatland-simulator.readthedocs.io/en/lua-preprocessor-for-yaml/core_functions/yaml_preprocessor.html