Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow custom type names in JSONExporter #877

Open
b-adkins opened this issue Oct 7, 2024 · 0 comments
Open

Allow custom type names in JSONExporter #877

b-adkins opened this issue Oct 7, 2024 · 0 comments
Assignees
Labels
enhancement New feature or request

Comments

@b-adkins
Copy link
Contributor

b-adkins commented Oct 7, 2024

Problem Statement

Parsing complex objects from JSON uses a __type field inside the JSON to determine the C++ type. Conveniently, when registering the type with the JSON exporter, the template automatically infers a string to represent the type. (See tutorial page for how serialization is done and registered with BehaviorTree.CPP.)

C++ templates are infamous for their verbosity. Even something relatively svelte

std::vector<std::double>

hides default value complexity, and so gets converted to something verbose in its full name:

"std::vector<double, std::allocator<double> >"

This is what happens to some common ROS2 types (at least in humble):

  • "geometry_msgs::msg::Pose_<std::allocator<void> >"
  • "geometry_msgs::msg::Quaternion_<std::allocator<void> >"
  • "geometry_msgs::msg::Transform_<std::allocator<void> >"
  • "geometry_msgs::msg::Vector3_<std::allocator<void> >"
  • "geometry_msgs::msg::Point_<std::allocator<void> >"

Issues caused

  • It's verbose and ugly. These full template names aren't commonly used in C++ for a reason, and even the short ones are commonly abbreviated with aliases.
  • It requires an exact string match, instead of merely valid C++. E.g. this does not get parsed
    {
      "__type": "std::vector<double, std::allocator<double>>"
    }
    and fails silently in certain parts of the BehaviorTree.CPP stack, e.g. ImportBlackBoardFromJSON()
  • It is specific to C++. Currently, the __type field must match what BehaviorTree.CPP expects -- which could cause a name collision for other languages and libraries deserializing the same JSON. It is common in large systems to define types using some common schema language, e.g. ROS2 IDL, JSON schema, or protobuf, then generate the types in C++ and other languages. These serialization/schema languages have their own programming-language-independent notation for representing types, e.g. geometry_msgs/Transform in ROS2 IDL, e.g. type hints (.NET docs) "Circle:http://schemas.datacontract.org/2004/07/MyApp.Shapes" or "Circle:#MyApp.Shapes".

Requested solution

When registering JSON function, support a custom type name string.

Interface Change

Add an optional type_name parameter to the BT::JsonExporter::addConverter() functions and BT::RegisterJsonDefinition(). E.g.

template <typename T>  
inline void JsonExporter::addConverter()

becomes

template <typename T>  
inline void JsonExporter::addConverter(const std::string& type_name="")

Behavior

If the type name is left blank, it behaves as it does currently, with BT::demangle()'s best effort to create a readable type name.

If the type name is provided, then it is used instead in the JSONExporter::type_names_ map.

Ideally, if called multiple times, it would register multiple names for the same type.

Discussion

This added freedom addresses all three issues. Application developers can:

  • Can use short names in __type
  • Register several variations of the type name for less brittle JSON
  • Use existing, language-non-specific type name conventions
@facontidavide facontidavide self-assigned this Nov 1, 2024
@facontidavide facontidavide added the enhancement New feature or request label Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants