-
Notifications
You must be signed in to change notification settings - Fork 64
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
Enable generation of ROS2 nodes directly from C++ code #984
Changes from all commits
1c8782c
1c914da
fa5fbbb
a7d886d
5a3b1c8
1358dff
ce7911a
f97cd1c
e84256a
321f461
7679227
8431932
2253b3d
e1d8329
67cdfb6
37e20ec
f7a3669
aafc117
c1fdc89
5eb36d7
1a580ee
4e73d4c
accbff4
cc60770
5c2139d
c2c3982
f54f7be
42b0fac
416fb12
456bd27
898e85b
ca8cff8
148fea2
048f85c
02bbc9c
d1370df
6208686
9a41527
798bef5
763c7a8
772d167
3cd3b1c
f583e4d
5727fb0
54b3000
ad261e7
4e42a1c
d959b80
2cd8784
1d27e09
a064d35
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
name: C++ tests | ||
|
||
on: | ||
workflow_call: | ||
inputs: | ||
compiler-ref: | ||
required: false | ||
type: string | ||
runtime-ref: | ||
required: false | ||
type: string | ||
jobs: | ||
run: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Setup Java JDK | ||
uses: actions/setup-java@v1.4.3 | ||
with: | ||
java-version: 11 | ||
- name: Check out lingual-franca repository | ||
uses: actions/checkout@v2 | ||
with: | ||
repository: lf-lang/lingua-franca | ||
submodules: true | ||
ref: ${{ inputs.compiler-ref }} | ||
- name: Check out specific ref of reactor-CPO | ||
uses: actions/checkout@v2 | ||
with: | ||
repository: lf-lang/reactor-cpp | ||
path: org.lflang/src/lib/cpp/reactor-cpp | ||
ref: ${{ inputs.runtime-ref }} | ||
if: ${{ inputs.runtime-ref }} | ||
- name: Setup ROS2 | ||
uses: ros-tooling/setup-ros@0.2.2 | ||
with: | ||
required-ros-distributions: galactic | ||
- name: Run C++ tests; | ||
run: | | ||
source /opt/ros/galactic/setup.bash | ||
./gradlew test --tests org.lflang.tests.runtime.CppRos2Test.* | ||
- name: Report to CodeCov | ||
uses: codecov/codecov-action@v2.1.0 | ||
with: | ||
file: org.lflang.tests/build/reports/xml/jacoco | ||
fail_ci_if_error: false | ||
verbose: true | ||
if: ${{ !inputs.runtime-ref }} # i.e., if this is part of the main repo's CI |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
target Cpp { | ||
ros2: true | ||
} | ||
|
||
public preamble {= | ||
#include "rclcpp/rclcpp.hpp" | ||
#include "std_msgs/msg/string.hpp" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It occurs to me that these two can be automatically included in the generated code since the |
||
=} | ||
|
||
main reactor { | ||
private preamble {= | ||
// FIXME: forward declaration to make the node visible | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this really necessary? Can't you generate this declaration? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just a temporary workaround until I found a better solution. I am trying to keep the code for generating reactor classes agnostic of the target platform, as it keeps the code generator clean. However, there needs to be some way of extending the generated code. I will fix this mechanism as soon as I have found a satisfactory solution. |
||
extern rclcpp::Node* lf_node; | ||
=} | ||
|
||
state publisher: {= rclcpp::Publisher<std_msgs::msg::String>::SharedPtr =} | ||
state count: unsigned(0) | ||
|
||
timer t(0, 500 ms) | ||
|
||
reaction(startup) {= | ||
publisher = lf_node->create_publisher<std_msgs::msg::String>("topic", 10); | ||
=} | ||
|
||
reaction(t) {= | ||
auto message = std_msgs::msg::String(); | ||
message.data = "Hello, world! " + std::to_string(count++); | ||
reactor::log::Info() << "Publishing: " << message.data; | ||
publisher->publish(message); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think the default rclcpp executor is thread-safe, so calling publish for the same topic from multiple LF worker threads will be problematic without acquiring a mutex. This is of course fine for this example, but I think we should be mindful of this limitation, especially if reactors other than the main reactor will be publishing messages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, thread-safety is not discussed in the documentation. According to this post, however, these operations should be thread-safe. |
||
=} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
target Cpp { | ||
ros2: true, | ||
keepalive: true, | ||
} | ||
|
||
public preamble {= | ||
#include "rclcpp/rclcpp.hpp" | ||
#include "std_msgs/msg/string.hpp" | ||
=} | ||
|
||
main reactor { | ||
private preamble {= | ||
// FIXME: forward declaration to make the node visible | ||
extern rclcpp::Node* lf_node; | ||
=} | ||
|
||
state subscription: {= rclcpp::Subscription<std_msgs::msg::String>::SharedPtr =} | ||
state count: unsigned(0) | ||
|
||
physical action message: std::string; | ||
|
||
reaction(startup) -> message {= | ||
subscription = lf_node->create_subscription<std_msgs::msg::String>( | ||
"topic", 10, [&message](const std_msgs::msg::String::SharedPtr msg) { message.schedule(msg->data); } ); | ||
// FIXME: Why can't we use a reference type in the lambda argument? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You mean something like this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess so, but I am not sure. I tried to use |
||
// const std_msgs::msg::String::SharedPtr& msg | ||
=} | ||
|
||
reaction(message) {= | ||
reactor::log::Info() << "I heard: " << *message.get(); | ||
=} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
This is an LF reimplementation of the ROS 2 minimal publisher and sunscriber | ||
[example](https://docs.ros.org/en/galactic/Tutorials/Writing-A-Simple-Cpp-Publisher-And-Subscriber.html). | ||
|
||
It consists of two LF files, MinimalPublisher and MinimalSubscriber, each | ||
implementing the corresponding nodes from the original example. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be very useful to test the C target's ROS 2 support as well.
Is it okay to call this just
ros2-tests
, or should we extract the setup here and share it among two separate workflows?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do something similar as in the benchmark tests, by adding a target parameter and making the jobs conditional. Feel free to add the C tests (but probably in another PR).