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.

Sign up for GitHub

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

Jump to bottom

Add GetTypeDescription.srv (rep2011) #153

Merged
merged 9 commits into from
Mar 23, 2023
Merged

Add GetTypeDescription.srv (rep2011) #153

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2cbbd21
Add GetTypeDescription.srv
emersonknapp Feb 24, 2023
fb7a19c
Add format clarification comments and fix formatting
emersonknapp Mar 10, 2023
6f8d286
Update service from design discussion
emersonknapp Mar 15, 2023
71c97ae
Fix unfinished thought and clarify comments from review
emersonknapp Mar 16, 2023
6756fdb
Type raw sources in a list of submessages
emersonknapp Mar 22, 2023
c5998a7
Use type_hash instead of type_version_hash
emersonknapp Mar 22, 2023
2ec42ec
Update to cpp17 while we are here, like the rest of the pkgs in the repo
emersonknapp Mar 22, 2023
1db9478
Update encoding information and resulting logical updates
emersonknapp Mar 22, 2023
a2b4f4e
Add better documentation for TypeSource
emersonknapp Mar 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions type_description_interfaces/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ cmake_minimum_required(VERSION 3.5)

project(type_description_interfaces)

# Default to C++14
# Default to C++17
if(NOT CMAKE_CXX_STANDARD)
set(CMAKE_CXX_STANDARD 14)
set(CMAKE_CXX_STANDARD 17)
endif()
if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wall -Wextra -Wpedantic)
Expand All @@ -19,7 +19,10 @@ rosidl_generate_interfaces(${PROJECT_NAME}
"msg/Field.msg"
"msg/FieldType.msg"
"msg/IndividualTypeDescription.msg"
"msg/KeyValue.msg"
"msg/TypeDescription.msg"
"msg/TypeSource.msg"
"srv/GetTypeDescription.srv"
ADD_LINTER_TESTS
)

Expand Down
4 changes: 4 additions & 0 deletions type_description_interfaces/msg/KeyValue.msg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Represents an arbitrary key-value pair for application-specific information.

string key
string value
12 changes: 12 additions & 0 deletions type_description_interfaces/msg/TypeSource.msg
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# Represents the original source of a ROS 2 interface definition.

# ROS interface type name, in PACKAGE/NAMESPACE/TYPENAME format.
string type_name

# The type of the original source file, typically matching the file extension.
# Well-known encodings: "idl", "msg", "srv", "action", "dynamic".
string encoding

# Dumped contents of the interface definition source file.
# If this was a type created programmatically (encoding "dynamic"), this field will be empty.
string raw_file_contents
2 changes: 2 additions & 0 deletions type_description_interfaces/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

<buildtool_depend>rosidl_core_generators</buildtool_depend>

<depend>service_msgs</depend>

<exec_depend>rosidl_core_runtime</exec_depend>

<test_depend>ament_lint_auto</test_depend>
Expand Down
26 changes: 26 additions & 0 deletions type_description_interfaces/srv/GetTypeDescription.srv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# ROS interface type name, in PACKAGE/NAMESPACE/TYPENAME format.
string type_name

# REP-2011 RIHS hash string.
string type_hash

# Whether to return the original idl/msg/etc. source file(s) in the response.
bool include_type_sources true
---
# True if the type description information is available and populated in the response.
# If false, all other fields except `failure_reason` are considered undefined.
bool successful
# If `successful` is false, contains a reason for failure.
# If `successful` is true, this is left empty.
string failure_reason
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this to reason instead of failure_reason.

Copy link

@achim-k achim-k Mar 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think something went wrong here, this became failure_reason again (1db9478)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that might have been intentional since it's no longer multi purpose? I'm not sure I haven't re-reviewed this yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, changed back on purpose, see #153 (comment)


# The parsed type description which can be used programmatically.
TypeDescription type_description

# List of sources, including all comments and whitespace.
# Sources can be matched with IndividualTypeDescriptions by their `type_name`.
# The `encoding` field of each entry informs how to interpret its contents.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if someone were to look at this cold, it would be hard for them to understand a) what a "source" is, and b) that this is a list of the type that was requested, and all of the recursive types it depends on. For that reason, I think we should update this comment to something like:

Suggested change
# List of sources, including all comments and whitespace.
# Sources can be matched with IndividualTypeDescriptions by their `type_name`.
# The `encoding` field of each entry informs how to interpret its contents.
# A list containing the interface definition source text of the originally requested type,
# along with all of the types it depends on (recursively).
# Each source text is a copy of the original contents of the
# `.msg`, `.srv`, `.action`, or `.idl` file, including comments and whitespace.
# Sources can be matched with IndividualTypeDescriptions in the `type_description` by
# their `type_name`.
# The `encoding` field of each entry informs how to interpret its contents.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to approximately this language, plus more detail in TypeSource.msg. Added a new "well-known encoding" - "implicit" for subtypes of complex types (services/actions), where the source will be available at the .srv/.action level.

TypeSource[] type_sources

# Key-value pairs of extra information.
KeyValue[] extra_information