From 201676de4d83da3362e6dd967fe6d17311d40409 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 4 Aug 2020 16:27:21 +0000 Subject: [PATCH 1/8] Get the node name and namespace from the native interface. Signed-off-by: Chris Lalancette --- .../include/org_ros2_rcljava_node_NodeImpl.h | 19 ++++++++++++++ .../cpp/org_ros2_rcljava_node_NodeImpl.cpp | 14 +++++++++++ .../main/java/org/ros2/rcljava/RCLJava.java | 2 +- .../main/java/org/ros2/rcljava/node/Node.java | 13 ++++++++++ .../java/org/ros2/rcljava/node/NodeImpl.java | 25 +++++++++++++++---- 5 files changed, 67 insertions(+), 6 deletions(-) diff --git a/rcljava/include/org_ros2_rcljava_node_NodeImpl.h b/rcljava/include/org_ros2_rcljava_node_NodeImpl.h index 181c0c30..40d40867 100644 --- a/rcljava/include/org_ros2_rcljava_node_NodeImpl.h +++ b/rcljava/include/org_ros2_rcljava_node_NodeImpl.h @@ -20,6 +20,25 @@ #ifdef __cplusplus extern "C" { #endif + +/* + * Class: org_ros2_rcljava_node_NodeImpl + * Method: nativeGetName + * Signature: (J)Ljava/lang/String + */ +JNIEXPORT jstring +JNICALL Java_org_ros2_rcljava_node_NodeImpl_nativeGetName( + JNIEnv * env, jclass, jlong node_handle); + +/* + * Class: org_ros2_rcljava_node_NodeImpl + * Method: nativeGetNamespace + * Signature: (J)Ljava/lang/String + */ +JNIEXPORT jstring +JNICALL Java_org_ros2_rcljava_node_NodeImpl_nativeGetNamespace( + JNIEnv * env, jclass, jlong node_handle); + /* * Class: org_ros2_rcljava_node_NodeImpl * Method: nativeCreatePublisherHandle diff --git a/rcljava/src/main/cpp/org_ros2_rcljava_node_NodeImpl.cpp b/rcljava/src/main/cpp/org_ros2_rcljava_node_NodeImpl.cpp index 16b6ed4c..e2c2e829 100644 --- a/rcljava/src/main/cpp/org_ros2_rcljava_node_NodeImpl.cpp +++ b/rcljava/src/main/cpp/org_ros2_rcljava_node_NodeImpl.cpp @@ -31,6 +31,20 @@ using rcljava_common::exceptions::rcljava_throw_rclexception; +JNIEXPORT jstring JNICALL +Java_org_ros2_rcljava_node_NodeImpl_nativeGetName( + JNIEnv * env, jclass, jlong node_handle) +{ + return env->NewStringUTF(rcl_node_get_name(reinterpret_cast(node_handle))); +} + +JNIEXPORT jstring JNICALL +Java_org_ros2_rcljava_node_NodeImpl_nativeGetNamespace( + JNIEnv * env, jclass, jlong node_handle) +{ + return env->NewStringUTF(rcl_node_get_namespace(reinterpret_cast(node_handle))); +} + JNIEXPORT jlong JNICALL Java_org_ros2_rcljava_node_NodeImpl_nativeCreatePublisherHandle( JNIEnv * env, jclass, jlong node_handle, jclass jmessage_class, jstring jtopic, diff --git a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java index 0e18d6aa..64467fe4 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java +++ b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java @@ -234,7 +234,7 @@ public static Node createNode(final String nodeName, final String namespace, fin public static Node createNode(final String nodeName, final String namespace, final Context context, final boolean allowUndeclaredParameters) { long nodeHandle = nativeCreateNodeHandle(nodeName, namespace, context.getHandle()); - Node node = new NodeImpl(nodeHandle, nodeName, context, allowUndeclaredParameters); + Node node = new NodeImpl(nodeHandle, context, allowUndeclaredParameters); nodes.add(node); return node; } diff --git a/rcljava/src/main/java/org/ros2/rcljava/node/Node.java b/rcljava/src/main/java/org/ros2/rcljava/node/Node.java index e5d875cb..f413ab04 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/Node.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/Node.java @@ -175,8 +175,21 @@ Client createClient(final Class serviceType, WallTimer createWallTimer(final long period, final TimeUnit unit, final Callback callback); + /** Get the name of the node. + * + * @return The name of the node. + */ String getName(); + /** Get the namespace of the node. + * + * This namespace is the "node's" namespace, and therefore is not affected + * by any sub-namespace's that may affect entities created with this instance. + * + * @return The namespace of the node. + */ + String getNamespace(); + /** * Declare and initialize a parameter, return the effective value. * diff --git a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java index 36affa63..1f638133 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -120,8 +120,6 @@ public class NodeImpl implements Node { */ private final Collection timers; - private final String name; - private Object parametersMutex; class ParameterAndDescriptor { @@ -141,10 +139,9 @@ class ParameterAndDescriptor { * @param handle A pointer to the underlying ROS2 node structure. Must not * be zero. */ - public NodeImpl(final long handle, final String name, final Context context, final boolean allowUndeclaredParameters) { + public NodeImpl(final long handle, final Context context, final boolean allowUndeclaredParameters) { this.clock = new Clock(ClockType.SYSTEM_TIME); this.handle = handle; - this.name = name; this.context = context; this.publishers = new LinkedBlockingQueue(); this.subscriptions = new LinkedBlockingQueue(); @@ -158,6 +155,20 @@ public NodeImpl(final long handle, final String name, final Context context, fin this.parameterCallbacks = new ArrayList(); } + /** + * Get the ROS 2 node name. + * @param handle A pointer to the underlying ROS2 node structure. + * @return The name of the node. + */ + private static native String nativeGetName(long handle); + + /** + * Get the ROS 2 node namespace. + * @param handle A pointer to the underlying ROS2 node structure. + * @return The namespace of the node. + */ + private static native String nativeGetNamespace(long handle); + /** * Create a ROS2 publisher (rcl_publisher_t) and return a pointer to * it as an integer. @@ -416,7 +427,11 @@ public final Collection getTimers() { * {@inheritDoc} */ public final String getName() { - return this.name; + return nativeGetName(this.handle); + } + + public final String getNamespace() { + return nativeGetNamespace(this.handle); } public ParameterVariant declareParameter(ParameterVariant parameter) { From ac2852315d66753c5b74ce303f9241b87fe1437e Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 20:45:50 +0000 Subject: [PATCH 2/8] Add in node options to nativeCreateNodeHandle. Signed-off-by: Chris Lalancette --- rcljava/include/org_ros2_rcljava_RCLJava.h | 2 +- .../src/main/cpp/org_ros2_rcljava_RCLJava.cpp | 67 +++++++++++++++++-- .../main/java/org/ros2/rcljava/RCLJava.java | 11 +-- .../node/NodeUndeclaredParametersTest.java | 2 +- 4 files changed, 68 insertions(+), 14 deletions(-) diff --git a/rcljava/include/org_ros2_rcljava_RCLJava.h b/rcljava/include/org_ros2_rcljava_RCLJava.h index 5ef6aff1..4d8d1b50 100644 --- a/rcljava/include/org_ros2_rcljava_RCLJava.h +++ b/rcljava/include/org_ros2_rcljava_RCLJava.h @@ -35,7 +35,7 @@ JNICALL Java_org_ros2_rcljava_RCLJava_nativeCreateContextHandle(JNIEnv *, jclass */ JNIEXPORT jlong JNICALL Java_org_ros2_rcljava_RCLJava_nativeCreateNodeHandle( - JNIEnv *, jclass, jstring, jstring, jlong); + JNIEnv *, jclass, jstring, jstring, jlong, jobject, jboolean, jboolean); /* * Class: org_ros2_rcljava_RCLJava diff --git a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp index a4f5717f..ff97f0d4 100644 --- a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp +++ b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp @@ -46,28 +46,81 @@ Java_org_ros2_rcljava_RCLJava_nativeCreateContextHandle(JNIEnv *, jclass) JNIEXPORT jlong JNICALL Java_org_ros2_rcljava_RCLJava_nativeCreateNodeHandle( - JNIEnv * env, jclass, jstring jnode_name, jstring jnamespace, jlong context_handle) + JNIEnv * env, jclass, jstring jnode_name, jstring jnamespace, jlong context_handle, + jobject cli_args, jboolean use_global_arguments, jboolean enable_rosout) { const char * node_name = env->GetStringUTFChars(jnode_name, 0); const char * node_namespace = env->GetStringUTFChars(jnamespace, 0); rcl_context_t * context = reinterpret_cast(context_handle); + // TODO(clalancette): we could probably make this faster by just doing these + // method lookups during some initialization time. But we'd have to add a lot + // more infrastructure for that, and we don't expect to call + // 'nativeCreateNodeHandle' that often, so I think this is OK for now. + jclass java_util_ArrayList = + static_cast(env->NewGlobalRef(env->FindClass("java/util/ArrayList"))); + jmethodID java_util_ArrayList_size = env->GetMethodID(java_util_ArrayList, "size", "()I"); + jmethodID java_util_ArrayList_get = env->GetMethodID( + java_util_ArrayList, "get", "(I)Ljava/lang/Object;"); + + rcl_arguments_t arguments = rcl_get_zero_initialized_arguments(); + char ** argv = NULL; + jint argc = env->CallIntMethod(cli_args, java_util_ArrayList_size); + argv = static_cast(malloc(argc * sizeof(char *))); + for (jint i = 0; i < argc; ++i) { + jstring element = + static_cast(env->CallObjectMethod(cli_args, java_util_ArrayList_get, i)); + argv[i] = const_cast(env->GetStringUTFChars(element, nullptr)); + } + rcl_ret_t ret = rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &arguments); + for (jint i = 0; i < argc; ++i) { + jstring element = + static_cast(env->CallObjectMethod(cli_args, java_util_ArrayList_get, i)); + env->ReleaseStringUTFChars(element, argv[i]); + } + free(argv); + if (ret != RCL_RET_OK) { + std::string msg = "Failed to parse node arguments: " + std::string(rcl_get_error_string().str); + rcl_reset_error(); + rcljava_throw_rclexception(env, ret, msg); + return 0; + } + rcl_node_t * node = static_cast(malloc(sizeof(rcl_node_t))); *node = rcl_get_zero_initialized_node(); - rcl_node_options_t default_options = rcl_node_get_default_options(); - rcl_ret_t ret = rcl_node_init(node, node_name, node_namespace, context, &default_options); - env->ReleaseStringUTFChars(jnode_name, node_name); - env->ReleaseStringUTFChars(jnamespace, node_namespace); + rcl_node_options_t options = rcl_node_get_default_options(); + options.use_global_arguments = use_global_arguments; + options.arguments = arguments; + options.enable_rosout = enable_rosout; + ret = rcl_node_init(node, node_name, node_namespace, context, &options); if (ret != RCL_RET_OK) { std::string msg = "Failed to create node: " + std::string(rcl_get_error_string().str); rcl_reset_error(); + free(node); + if (rcl_arguments_fini(&arguments) != RCL_RET_OK) { + // We are already throwing an exception, just ignore the return here + } + rcljava_throw_rclexception(env, ret, msg); + return 0; + } + + ret = rcl_arguments_fini(&arguments); + if (ret != RCL_RET_OK) { + // We failed to cleanup + std::string msg = "Failed to cleanup after creating node: " + std::string( + rcl_get_error_string().str); + rcl_reset_error(); + if (rcl_node_fini(node) != RCL_RET_OK) { + // We are already throwing an exception, just ignore the return here + } + free(node); rcljava_throw_rclexception(env, ret, msg); return 0; } - jlong node_handle = reinterpret_cast(node); - return node_handle; + + return reinterpret_cast(node); } JNIEXPORT jstring JNICALL diff --git a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java index 64467fe4..00cfccd1 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java +++ b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java @@ -18,6 +18,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.ArrayList; import java.util.Collection; import java.util.Map; import java.util.concurrent.ConcurrentSkipListMap; @@ -160,7 +161,7 @@ public static synchronized void rclJavaInit() { * @param contextHandle Pointer to a context (rcl_context_t) with which to associated the node. * @return A pointer to the underlying ROS2 node structure. */ - private static native long nativeCreateNodeHandle(String nodeName, String namespace, long contextHandle); + private static native long nativeCreateNodeHandle(String nodeName, String namespace, long contextHandle, ArrayList arguments, boolean useGlobalArguments, boolean enableRosout); /** * @return The identifier of the currently active RMW implementation via the @@ -217,7 +218,7 @@ public static Context createContext() { * structure. */ public static Node createNode(final String nodeName) { - return createNode(nodeName, "", RCLJava.getDefaultContext(), false); + return createNode(nodeName, "", RCLJava.getDefaultContext(), true, true, new ArrayList(), false); } /** @@ -229,11 +230,11 @@ public static Node createNode(final String nodeName) { * structure. */ public static Node createNode(final String nodeName, final String namespace, final Context context) { - return createNode(nodeName, namespace, context, false); + return createNode(nodeName, namespace, context, true, true, new ArrayList(), false); } - public static Node createNode(final String nodeName, final String namespace, final Context context, final boolean allowUndeclaredParameters) { - long nodeHandle = nativeCreateNodeHandle(nodeName, namespace, context.getHandle()); + public static Node createNode(final String nodeName, final String namespace, final Context context, final boolean useGlobalArguments, final boolean enableRosout, final ArrayList cliArgs, final boolean allowUndeclaredParameters) { + long nodeHandle = nativeCreateNodeHandle(nodeName, namespace, context.getHandle(), cliArgs, useGlobalArguments, enableRosout); Node node = new NodeImpl(nodeHandle, context, allowUndeclaredParameters); nodes.add(node); return node; diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java index 9d9f3991..607c67bb 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java @@ -65,7 +65,7 @@ public static void tearDownOnce() { @Before public void setUp() { - node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), true); + node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), true, true, new ArrayList(), true); } @After From 87eba17b29820ac0849b667832dd0a6b6a8ff41b Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 4 Aug 2020 16:27:55 +0000 Subject: [PATCH 3/8] Add in NodeOptions test. Signed-off-by: Chris Lalancette --- rcljava/CMakeLists.txt | 2 + .../ros2/rcljava/node/NodeOptionsTest.java | 54 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index 7f667541..8eacd184 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -229,6 +229,7 @@ if(BUILD_TESTING) "src/test/java/org/ros2/rcljava/SpinTest.java" "src/test/java/org/ros2/rcljava/TimeTest.java" "src/test/java/org/ros2/rcljava/client/ClientTest.java" + "src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java" "src/test/java/org/ros2/rcljava/node/NodeParametersTest.java" "src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java" "src/test/java/org/ros2/rcljava/node/NodeTest.java" @@ -244,6 +245,7 @@ if(BUILD_TESTING) "org.ros2.rcljava.SpinTest" "org.ros2.rcljava.TimeTest" "org.ros2.rcljava.client.ClientTest" + "org.ros2.rcljava.node.NodeOptionsTest" "org.ros2.rcljava.node.NodeParametersTest" "org.ros2.rcljava.node.NodeUndeclaredParametersTest" "org.ros2.rcljava.node.NodeTest" diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java new file mode 100644 index 00000000..121adad2 --- /dev/null +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java @@ -0,0 +1,54 @@ +/* Copyright 2020 Open Source Robotics Foundation, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.ros2.rcljava.node; + +import static org.junit.Assert.assertEquals; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import java.util.ArrayList; +import java.util.Arrays; + +import org.ros2.rcljava.RCLJava; +import org.ros2.rcljava.node.Node; + +public class NodeOptionsTest { + @BeforeClass + public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + + RCLJava.rclJavaInit(); + } + + @AfterClass + public static void tearDownOnce() { + RCLJava.shutdown(); + } + + @Test + public final void testCreateNodeWithArgs() { + ArrayList args = new ArrayList(Arrays.asList("--ros-args", "-r", "__ns:=/foo")); + //ArrayList args = new ArrayList(); + Node node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), true, true, args, false); + assertEquals("test_node", node.getName()); + assertEquals("/foo", node.getNamespace()); + + node.dispose(); + } +} From bfb2b71a99555b715f20411fd1e7adf51ad12cfc Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 10 Aug 2020 18:04:33 +0000 Subject: [PATCH 4/8] Move argument parsing to its own helper function. This should make the code much easier to read. Signed-off-by: Chris Lalancette --- .../src/main/cpp/org_ros2_rcljava_RCLJava.cpp | 50 ++++++++++++------- 1 file changed, 32 insertions(+), 18 deletions(-) diff --git a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp index ff97f0d4..492bebc5 100644 --- a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp +++ b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include "rcl/error_handling.h" #include "rcl/node.h" @@ -44,46 +45,59 @@ Java_org_ros2_rcljava_RCLJava_nativeCreateContextHandle(JNIEnv *, jclass) return context_handle; } -JNIEXPORT jlong JNICALL -Java_org_ros2_rcljava_RCLJava_nativeCreateNodeHandle( - JNIEnv * env, jclass, jstring jnode_name, jstring jnamespace, jlong context_handle, - jobject cli_args, jboolean use_global_arguments, jboolean enable_rosout) +bool parse_arguments(JNIEnv * env, jobject cli_args, rcl_arguments_t * arguments) { - const char * node_name = env->GetStringUTFChars(jnode_name, 0); - const char * node_namespace = env->GetStringUTFChars(jnamespace, 0); - - rcl_context_t * context = reinterpret_cast(context_handle); - // TODO(clalancette): we could probably make this faster by just doing these // method lookups during some initialization time. But we'd have to add a lot // more infrastructure for that, and we don't expect to call // 'nativeCreateNodeHandle' that often, so I think this is OK for now. - jclass java_util_ArrayList = - static_cast(env->NewGlobalRef(env->FindClass("java/util/ArrayList"))); + jclass java_util_ArrayList = static_cast(env->FindClass("java/util/ArrayList")); jmethodID java_util_ArrayList_size = env->GetMethodID(java_util_ArrayList, "size", "()I"); jmethodID java_util_ArrayList_get = env->GetMethodID( java_util_ArrayList, "get", "(I)Ljava/lang/Object;"); - rcl_arguments_t arguments = rcl_get_zero_initialized_arguments(); - char ** argv = NULL; + *arguments = rcl_get_zero_initialized_arguments(); jint argc = env->CallIntMethod(cli_args, java_util_ArrayList_size); - argv = static_cast(malloc(argc * sizeof(char *))); + std::vector argv(argc); for (jint i = 0; i < argc; ++i) { jstring element = static_cast(env->CallObjectMethod(cli_args, java_util_ArrayList_get, i)); - argv[i] = const_cast(env->GetStringUTFChars(element, nullptr)); + argv[i] = env->GetStringUTFChars(element, nullptr); } - rcl_ret_t ret = rcl_parse_arguments(argc, argv, rcl_get_default_allocator(), &arguments); + rcl_ret_t ret = rcl_parse_arguments(argc, &argv[0], rcl_get_default_allocator(), arguments); for (jint i = 0; i < argc; ++i) { jstring element = static_cast(env->CallObjectMethod(cli_args, java_util_ArrayList_get, i)); env->ReleaseStringUTFChars(element, argv[i]); } - free(argv); if (ret != RCL_RET_OK) { std::string msg = "Failed to parse node arguments: " + std::string(rcl_get_error_string().str); rcl_reset_error(); rcljava_throw_rclexception(env, ret, msg); + return false; + } + + return true; +} + +JNIEXPORT jlong JNICALL +Java_org_ros2_rcljava_RCLJava_nativeCreateNodeHandle( + JNIEnv * env, jclass, jstring jnode_name, jstring jnamespace, jlong context_handle, + jobject cli_args, jboolean use_global_arguments, jboolean enable_rosout) +{ + const char * node_name_tmp = env->GetStringUTFChars(jnode_name, 0); + std::string node_name(node_name_tmp); + env->ReleaseStringUTFChars(jnode_name, node_name_tmp); + + const char * namespace_tmp = env->GetStringUTFChars(jnamespace, 0); + std::string namespace_(namespace_tmp); + env->ReleaseStringUTFChars(jnamespace, namespace_tmp); + + rcl_context_t * context = reinterpret_cast(context_handle); + + rcl_arguments_t arguments; + if (!parse_arguments(env, cli_args, &arguments)) { + // All of the exception setup was done by parse_arguments, just return here. return 0; } @@ -94,7 +108,7 @@ Java_org_ros2_rcljava_RCLJava_nativeCreateNodeHandle( options.use_global_arguments = use_global_arguments; options.arguments = arguments; options.enable_rosout = enable_rosout; - ret = rcl_node_init(node, node_name, node_namespace, context, &options); + rcl_ret_t ret = rcl_node_init(node, node_name.c_str(), namespace_.c_str(), context, &options); if (ret != RCL_RET_OK) { std::string msg = "Failed to create node: " + std::string(rcl_get_error_string().str); rcl_reset_error(); From 18a8279bf05162312e213dd6016e2a84facbc3af Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 10 Aug 2020 18:52:44 +0000 Subject: [PATCH 5/8] Add a NodeOptions class. This makes it a lot nicer to use. Signed-off-by: Chris Lalancette --- rcljava/CMakeLists.txt | 1 + .../main/java/org/ros2/rcljava/RCLJava.java | 11 +-- .../org/ros2/rcljava/node/NodeOptions.java | 68 +++++++++++++++++++ .../ros2/rcljava/node/NodeOptionsTest.java | 7 +- .../node/NodeUndeclaredParametersTest.java | 4 +- 5 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 rcljava/src/main/java/org/ros2/rcljava/node/NodeOptions.java diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index 8eacd184..3cbe0318 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -134,6 +134,7 @@ set(${PROJECT_NAME}_sources "src/main/java/org/ros2/rcljava/node/ComposableNode.java" "src/main/java/org/ros2/rcljava/node/Node.java" "src/main/java/org/ros2/rcljava/node/NodeImpl.java" + "src/main/java/org/ros2/rcljava/node/NodeOptions.java" "src/main/java/org/ros2/rcljava/parameters/client/AsyncParametersClient.java" "src/main/java/org/ros2/rcljava/parameters/client/AsyncParametersClientImpl.java" "src/main/java/org/ros2/rcljava/parameters/client/SyncParametersClient.java" diff --git a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java index 00cfccd1..c4be74ef 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java +++ b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java @@ -34,6 +34,7 @@ import org.ros2.rcljava.node.ComposableNode; import org.ros2.rcljava.node.Node; import org.ros2.rcljava.node.NodeImpl; +import org.ros2.rcljava.node.NodeOptions; import org.ros2.rcljava.publisher.Publisher; import org.ros2.rcljava.qos.QoSProfile; import org.ros2.rcljava.service.RMWRequestId; @@ -218,7 +219,7 @@ public static Context createContext() { * structure. */ public static Node createNode(final String nodeName) { - return createNode(nodeName, "", RCLJava.getDefaultContext(), true, true, new ArrayList(), false); + return createNode(nodeName, "", RCLJava.getDefaultContext(), new NodeOptions()); } /** @@ -230,12 +231,12 @@ public static Node createNode(final String nodeName) { * structure. */ public static Node createNode(final String nodeName, final String namespace, final Context context) { - return createNode(nodeName, namespace, context, true, true, new ArrayList(), false); + return createNode(nodeName, namespace, context, new NodeOptions()); } - public static Node createNode(final String nodeName, final String namespace, final Context context, final boolean useGlobalArguments, final boolean enableRosout, final ArrayList cliArgs, final boolean allowUndeclaredParameters) { - long nodeHandle = nativeCreateNodeHandle(nodeName, namespace, context.getHandle(), cliArgs, useGlobalArguments, enableRosout); - Node node = new NodeImpl(nodeHandle, context, allowUndeclaredParameters); + public static Node createNode(final String nodeName, final String namespace, final Context context, final NodeOptions options) { + long nodeHandle = nativeCreateNodeHandle(nodeName, namespace, context.getHandle(), options.getCliArgs(), options.getUseGlobalArguments(), options.getEnableRosout()); + Node node = new NodeImpl(nodeHandle, context, options.getAllowUndeclaredParameters()); nodes.add(node); return node; } diff --git a/rcljava/src/main/java/org/ros2/rcljava/node/NodeOptions.java b/rcljava/src/main/java/org/ros2/rcljava/node/NodeOptions.java new file mode 100644 index 00000000..c64d65e1 --- /dev/null +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeOptions.java @@ -0,0 +1,68 @@ +/* Copyright 2020 Open Source Robotics Foundation, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.ros2.rcljava.node; + +import java.util.ArrayList; + +public class NodeOptions { + private boolean useGlobalArguments; + private boolean enableRosout; + private boolean allowUndeclaredParameters; + private ArrayList cliArgs; + + public NodeOptions() { + this.useGlobalArguments = true; + this.enableRosout = true; + this.allowUndeclaredParameters = false; + this.cliArgs = new ArrayList(); + } + + public final boolean getUseGlobalArguments() { + return this.useGlobalArguments; + } + + public NodeOptions setUseGlobalArguments(boolean useGlobal) { + this.useGlobalArguments = useGlobal; + return this; + } + + public final boolean getEnableRosout() { + return this.enableRosout; + } + + public NodeOptions setEnableRosout(boolean enable) { + this.enableRosout = enable; + return this; + } + + public final boolean getAllowUndeclaredParameters() { + return this.allowUndeclaredParameters; + } + + public NodeOptions setAllowUndeclaredParameters(boolean allow) { + this.allowUndeclaredParameters = allow; + return this; + } + + public final ArrayList getCliArgs() { + return this.cliArgs; + } + + public NodeOptions setCliArgs(ArrayList newArgs) { + this.cliArgs = newArgs; + return this; + } +} diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java index 121adad2..ce3bca84 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java @@ -26,6 +26,7 @@ import org.ros2.rcljava.RCLJava; import org.ros2.rcljava.node.Node; +import org.ros2.rcljava.node.NodeOptions; public class NodeOptionsTest { @BeforeClass @@ -43,9 +44,9 @@ public static void tearDownOnce() { @Test public final void testCreateNodeWithArgs() { - ArrayList args = new ArrayList(Arrays.asList("--ros-args", "-r", "__ns:=/foo")); - //ArrayList args = new ArrayList(); - Node node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), true, true, args, false); + NodeOptions options = new NodeOptions(); + options.setCliArgs(new ArrayList(Arrays.asList("--ros-args", "-r", "__ns:=/foo"))); + Node node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), options); assertEquals("test_node", node.getName()); assertEquals("/foo", node.getNamespace()); diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java index 607c67bb..454e62bd 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java @@ -65,7 +65,9 @@ public static void tearDownOnce() { @Before public void setUp() { - node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), true, true, new ArrayList(), true); + NodeOptions options = new NodeOptions(); + options.setAllowUndeclaredParameters(true); + node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), options); } @After From 6407fc3014d42832e9acdd401dfeca871bd5d104 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 10 Aug 2020 19:00:44 +0000 Subject: [PATCH 6/8] Make sure to DeleteLocalRef after use. Signed-off-by: Chris Lalancette --- rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp index 492bebc5..3ea1d473 100644 --- a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp +++ b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp @@ -63,12 +63,14 @@ bool parse_arguments(JNIEnv * env, jobject cli_args, rcl_arguments_t * arguments jstring element = static_cast(env->CallObjectMethod(cli_args, java_util_ArrayList_get, i)); argv[i] = env->GetStringUTFChars(element, nullptr); + env->DeleteLocalRef(element); } rcl_ret_t ret = rcl_parse_arguments(argc, &argv[0], rcl_get_default_allocator(), arguments); for (jint i = 0; i < argc; ++i) { jstring element = static_cast(env->CallObjectMethod(cli_args, java_util_ArrayList_get, i)); env->ReleaseStringUTFChars(element, argv[i]); + env->DeleteLocalRef(element); } if (ret != RCL_RET_OK) { std::string msg = "Failed to parse node arguments: " + std::string(rcl_get_error_string().str); From 9fbb2603512708efa690fd9c1d6f0e7fc9c7ab8c Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 11 Aug 2020 13:10:45 +0000 Subject: [PATCH 7/8] Fixes from review. 1. Check return value of malloc. 2. Get rid of unnecessary cast. Signed-off-by: Chris Lalancette --- rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp index 3ea1d473..1a3b640d 100644 --- a/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp +++ b/rcljava/src/main/cpp/org_ros2_rcljava_RCLJava.cpp @@ -51,7 +51,7 @@ bool parse_arguments(JNIEnv * env, jobject cli_args, rcl_arguments_t * arguments // method lookups during some initialization time. But we'd have to add a lot // more infrastructure for that, and we don't expect to call // 'nativeCreateNodeHandle' that often, so I think this is OK for now. - jclass java_util_ArrayList = static_cast(env->FindClass("java/util/ArrayList")); + jclass java_util_ArrayList = env->FindClass("java/util/ArrayList"); jmethodID java_util_ArrayList_size = env->GetMethodID(java_util_ArrayList, "size", "()I"); jmethodID java_util_ArrayList_get = env->GetMethodID( java_util_ArrayList, "get", "(I)Ljava/lang/Object;"); @@ -97,15 +97,20 @@ Java_org_ros2_rcljava_RCLJava_nativeCreateNodeHandle( rcl_context_t * context = reinterpret_cast(context_handle); + rcl_node_t * node = static_cast(malloc(sizeof(rcl_node_t))); + if (node == nullptr) { + env->ThrowNew(env->FindClass("java/lang/OutOfMemoryError"), "Failed to allocate node"); + return 0; + } + *node = rcl_get_zero_initialized_node(); + rcl_arguments_t arguments; if (!parse_arguments(env, cli_args, &arguments)) { // All of the exception setup was done by parse_arguments, just return here. + free(node); return 0; } - rcl_node_t * node = static_cast(malloc(sizeof(rcl_node_t))); - *node = rcl_get_zero_initialized_node(); - rcl_node_options_t options = rcl_node_get_default_options(); options.use_global_arguments = use_global_arguments; options.arguments = arguments; From cd14a19056dda0760f32f7eef216724abac31337 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Tue, 16 Nov 2021 15:38:42 -0300 Subject: [PATCH 8/8] android compatibility Signed-off-by: Ivan Santiago Paunovic --- .../org/ros2/rcljava/node/NodeOptionsTest.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java index ce3bca84..bde90763 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeOptionsTest.java @@ -21,6 +21,7 @@ import org.junit.BeforeClass; import org.junit.Test; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; @@ -32,7 +33,18 @@ public class NodeOptionsTest { @BeforeClass public static void setupOnce() throws Exception { // Just to quiet down warnings - org.apache.log4j.BasicConfigurator.configure(); + try + { + // Configure log4j. Doing this dynamically so that Android does not complain about missing + // the log4j JARs, SLF4J uses Android's native logging mechanism instead. + Class c = Class.forName("org.apache.log4j.BasicConfigurator"); + Method m = c.getDeclaredMethod("configure", (Class[]) null); + Object o = m.invoke(null, (Object[]) null); + } + catch (Exception e) + { + e.printStackTrace(); + } RCLJava.rclJavaInit(); }