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

RST-6618 fixes for action bridge #2

Merged
merged 4 commits into from
Feb 7, 2024
Merged

RST-6618 fixes for action bridge #2

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55d7193
RST-6618 fixes for action bridge
abencz Jul 7, 2023
f7b36e4
Check for diffs when generating factories
abencz Jul 13, 2023
8a90a8e
Create generated folder explicitly
abencz Jan 19, 2024
9571dab
Added comment on why a temporary folder is used
abencz Feb 7, 2024
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
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,14 +532,14 @@ Note that the `qos` section can be omitted entirely and options not set are left

This bridge extends the `ros1_bridge` to support actions. The bridge works in both directions, meaning an action goal can be sent from ROS 1 client to ROS 2 server, or from ROS 2 client to ROS 1 server.

The arguments for the `action_bridge` node are:
The arguments for the `action_bridge` node are:
`direction`: from client (`ros1` or `ros2`)
e.g.:
- `ROS1` client to `ROS2` server --> `direction` = `ros1`
- `ROS2` client to `ROS1` server --> `direction` = `ros2`
- `ROS1` client to `ROS2` server --> `direction` = `ros2`
- `ROS2` client to `ROS1` server --> `direction` = `ros1`
abencz marked this conversation as resolved.
Show resolved Hide resolved

`package`: package of the `ROS1` server node
`type`: action interface type of `ROS1`
`package`: package of the `ROS1` server node
`type`: action interface type of `ROS1`
`name`: action name

For sending goals from a ROS 2 action client to a ROS 1 action server
Expand Down
2 changes: 1 addition & 1 deletion include/ros1_bridge/action_factory.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ class ActionFactory_1_2 : public ActionFactoryInterface
std::shared_future<ROS2ClientGoalHandle> gh2_future;
auto send_goal_ops = ROS2SendGoalOptions();
send_goal_ops.goal_response_callback =
[this, &gh2_future](std::shared_future<ROS2GoalHandle> gh2) mutable {
[this, &gh2_future](ROS2GoalHandle gh2) mutable {
Copy link
Author

Choose a reason for hiding this comment

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

API change from Foxy to Rolling.

Copy link

Choose a reason for hiding this comment

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

I assume this also works for iron?

Copy link

Choose a reason for hiding this comment

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

Why make these changes on our fork of ros1_bridge instead of either

  1. Making a change upstream and back porting to iron?
  2. or switching over to rolling if it's fixed there
  3. or just cherry-picking commits from rolling onto our fork?

Copy link
Author

@abencz abencz Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The action bridge stuff is not merged into upstream ros1_bridge. The PR for it is probably abandoned. Notice that this merge is into an action_bridge branch and not locus-devel. I'll see what Paul wants to do with the action_bridge branch once he's back.

auto goal_handle = gh2_future.get();
if (!goal_handle) {
gh1_.setRejected(); // goal was not accepted by remote server
Expand Down
22 changes: 11 additions & 11 deletions resource/interface_factories.cpp.em
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,27 +405,27 @@ void ActionFactory_@(frm_)_@(to_)<
@(action["ros2_package"])::action::@(action["ros2_name"])
>::translate_@(type.lower())_@(frm)_to_@(to)(
@[ if type == "Goal"]@
@(const_1)ROS@(frm)@(type) &@(type.lower())@(frm),
@(const_2)ROS@(to)@(type) &@(type.lower())@(to))
@(const_1)ROS@(frm)@(type) &@(type.lower())_msg@(frm),
@(const_2)ROS@(to)@(type) &@(type.lower())_msg@(to))
AadityaRavindran marked this conversation as resolved.
Show resolved Hide resolved
@[ else]@
@(const_1)ROS@(to)@(type) &@(type.lower())@(to),
@(const_2)ROS@(frm)@(type) &@(type.lower())@(frm))
@(const_1)ROS@(to)@(type) &@(type.lower())_msg@(to),
@(const_2)ROS@(frm)@(type) &@(type.lower())_msg@(frm))
@[ end if]@
{
@[ for field in action["fields"][type.lower()]]@
@[ if field["array"]]@
@(type.lower())@(to).@(field["ros" + to]["name"]).resize(@(type.lower())@(frm).@(field["ros" + frm]["name"]).size());
auto @(field["ros" + frm]["name"])@(frm)_it = @(type.lower())@(frm).@(field["ros" + frm]["name"]).begin();
auto @(field["ros" + to]["name"])@(to)_it = @(type.lower())@(to).@(field["ros" + to]["name"]).begin();
@(type.lower())_msg@(to).@(field["ros" + to]["name"]).resize(@(type.lower())_msg@(frm).@(field["ros" + frm]["name"]).size());
auto @(field["ros" + frm]["name"])@(frm)_it = @(type.lower())_msg@(frm).@(field["ros" + frm]["name"]).begin();
auto @(field["ros" + to]["name"])@(to)_it = @(type.lower())_msg@(to).@(field["ros" + to]["name"]).begin();
while (
@(field["ros" + frm]["name"])@(frm)_it != @(type.lower())@(frm).@(field["ros" + frm]["name"]).end() &&
@(field["ros" + to]["name"])@(to)_it != @(type.lower())@(to).@(field["ros" + to]["name"]).end()
@(field["ros" + frm]["name"])@(frm)_it != @(type.lower())_msg@(frm).@(field["ros" + frm]["name"]).end() &&
@(field["ros" + to]["name"])@(to)_it != @(type.lower())_msg@(to).@(field["ros" + to]["name"]).end()
) {
auto & @(field["ros" + frm]["name"])@(frm) = *(@(field["ros" + frm]["name"])@(frm)_it++);
auto & @(field["ros" + to]["name"])@(to) = *(@(field["ros" + to]["name"])@(to)_it++);
@[ else]@
auto & @(field["ros" + frm]["name"])@(frm) = @(type.lower())@(frm).@(field["ros" + frm]["name"]);
auto & @(field["ros" + to]["name"])@(to) = @(type.lower())@(to).@(field["ros" + to]["name"]);
auto & @(field["ros" + frm]["name"])@(frm) = @(type.lower())_msg@(frm).@(field["ros" + frm]["name"]);
auto & @(field["ros" + to]["name"])@(to) = @(type.lower())_msg@(to).@(field["ros" + to]["name"]);
abencz marked this conversation as resolved.
Show resolved Hide resolved
@[ end if]@
@[ if field["basic"]]@
@[ if field["ros2"]["type"].startswith("builtin_interfaces") ]@
Expand Down
51 changes: 46 additions & 5 deletions ros1_bridge/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@
from collections import OrderedDict
import os
import re
import shutil
import sys
import tempfile
import filecmp

import ament_index_python
from catkin_pkg.package import parse_package
Expand Down Expand Up @@ -72,6 +75,15 @@


def generate_cpp(output_path, template_dir):
# Temporary directory is used to hold generated files, which are only
# copied into place if they differ from previously generated files. This
# ensures that timestamps on files that have not changed are not updated
# and saves a ton of compile time.
with tempfile.TemporaryDirectory() as tempdir:
_generate_cpp(output_path, template_dir, tempdir)
AadityaRavindran marked this conversation as resolved.
Show resolved Hide resolved


def _generate_cpp(output_path, template_dir, temporary_dir):
rospack = rospkg.RosPack()
data = generate_messages(rospack)
message_string_pairs = {
Expand All @@ -84,11 +96,17 @@ def generate_cpp(output_path, template_dir):
data.update(generate_actions(
rospack, message_string_pairs=message_string_pairs))

# create output folder if it doesn't exist
if not os.path.exists(output_path):
os.makedirs(output_path)

template_file = os.path.join(template_dir, 'get_mappings.cpp.em')
temporary_file = os.path.join(temporary_dir, 'get_mappings.cpp')
output_file = os.path.join(output_path, 'get_mappings.cpp')
data_for_template = {
'mappings': data['mappings'], 'services': data['services'], 'actions': data['actions']}
expand_template(template_file, data_for_template, output_file)
expand_template(template_file, data_for_template, temporary_file)
update_output_file(temporary_file, output_file)

unique_package_names = set(
data['ros2_package_names_msg'] + data['ros2_package_names_srv'])
Expand All @@ -97,8 +115,10 @@ def generate_cpp(output_path, template_dir):
data['ros2_package_names'] = list(unique_package_names)

template_file = os.path.join(template_dir, 'get_factory.cpp.em')
temporary_file = os.path.join(temporary_dir, 'get_factory.cpp')
output_file = os.path.join(output_path, 'get_factory.cpp')
expand_template(template_file, data, output_file)
expand_template(template_file, data, temporary_file)
update_output_file(temporary_file, output_file)

for ros2_package_name in data['ros2_package_names']:
data_pkg_hpp = {
Expand Down Expand Up @@ -126,9 +146,12 @@ def generate_cpp(output_path, template_dir):
if m.ros2_msg.package_name == ros2_package_name],
}
template_file = os.path.join(template_dir, 'pkg_factories.hpp.em')
temporary_file = os.path.join(
temporary_dir, '%s_factories.hpp' % ros2_package_name)
output_file = os.path.join(
output_path, '%s_factories.hpp' % ros2_package_name)
expand_template(template_file, data_pkg_hpp, output_file)
expand_template(template_file, data_pkg_hpp, temporary_file)
update_output_file(temporary_file, output_file)

data_pkg_cpp = {
'ros2_package_name': ros2_package_name,
Expand All @@ -138,9 +161,12 @@ def generate_cpp(output_path, template_dir):
'ros2_action_types': data_pkg_hpp['ros2_action_types'],
}
template_file = os.path.join(template_dir, 'pkg_factories.cpp.em')
temporary_file = os.path.join(
temporary_dir, '%s_factories.cpp' % ros2_package_name)
output_file = os.path.join(
output_path, '%s_factories.cpp' % ros2_package_name)
expand_template(template_file, data_pkg_cpp, output_file)
expand_template(template_file, data_pkg_cpp, temporary_file)
update_output_file(temporary_file, output_file)

for interface_type, interfaces in zip(
['msg', 'srv', 'action'], [data['all_ros2_msgs'],
Expand Down Expand Up @@ -175,10 +201,25 @@ def generate_cpp(output_path, template_dir):

template_file = os.path.join(
template_dir, 'interface_factories.cpp.em')
temporary_file = os.path.join(
temporary_dir, '%s__%s__%s__factories.cpp' %
(ros2_package_name, interface_type, interface.message_name))
output_file = os.path.join(
output_path, '%s__%s__%s__factories.cpp' %
(ros2_package_name, interface_type, interface.message_name))
expand_template(template_file, data_idl_cpp, output_file)
expand_template(template_file, data_idl_cpp, temporary_file)
update_output_file(temporary_file, output_file)


def update_output_file(temporary_file, output_file):
abencz marked this conversation as resolved.
Show resolved Hide resolved
"""Copy temporary file to output file if the two differ"""
try:
files_same = filecmp.cmp(temporary_file, output_file)
except FileNotFoundError:
files_same = False

if not files_same:
shutil.copyfile(temporary_file, output_file)


def generate_messages(rospack=None):
Expand Down
Loading