diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index b7453b7b..9fa10643 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -265,6 +265,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/contexts/ContextTest.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" @@ -283,6 +284,7 @@ if(BUILD_TESTING) "org.ros2.rcljava.SpinTest" "org.ros2.rcljava.TimeTest" # "org.ros2.rcljava.client.ClientTest" + "org.ros2.rcljava.contexts.ContextTest" "org.ros2.rcljava.node.NodeOptionsTest" "org.ros2.rcljava.node.NodeParametersTest" "org.ros2.rcljava.node.NodeUndeclaredParametersTest" diff --git a/rcljava/include/org_ros2_rcljava_contexts_ContextImpl.h b/rcljava/include/org_ros2_rcljava_contexts_ContextImpl.h index 049891d6..b6cef1fb 100644 --- a/rcljava/include/org_ros2_rcljava_contexts_ContextImpl.h +++ b/rcljava/include/org_ros2_rcljava_contexts_ContextImpl.h @@ -27,7 +27,8 @@ extern "C" { * Signature: (J)V */ JNIEXPORT void -JNICALL Java_org_ros2_rcljava_contexts_ContextImpl_nativeInit(JNIEnv *, jclass, jlong); +JNICALL Java_org_ros2_rcljava_contexts_ContextImpl_nativeInit( + JNIEnv *, jclass, jlong, jobjectArray); /* * Class: org_ros2_rcljava_contexts_ContextImpl diff --git a/rcljava/src/main/cpp/org_ros2_rcljava_contexts_ContextImpl.cpp b/rcljava/src/main/cpp/org_ros2_rcljava_contexts_ContextImpl.cpp index 8f70afad..4ae9d471 100644 --- a/rcljava/src/main/cpp/org_ros2_rcljava_contexts_ContextImpl.cpp +++ b/rcljava/src/main/cpp/org_ros2_rcljava_contexts_ContextImpl.cpp @@ -14,7 +14,9 @@ #include +#include #include +#include #include "rcl/context.h" #include "rcl/error_handling.h" @@ -25,6 +27,7 @@ #include "org_ros2_rcljava_contexts_ContextImpl.h" +using rcljava_common::exceptions::rcljava_throw_exception; using rcljava_common::exceptions::rcljava_throw_rclexception; JNIEXPORT jboolean JNICALL @@ -36,7 +39,8 @@ Java_org_ros2_rcljava_contexts_ContextImpl_nativeIsValid(JNIEnv *, jclass, jlong } JNIEXPORT void JNICALL -Java_org_ros2_rcljava_contexts_ContextImpl_nativeInit(JNIEnv * env, jclass, jlong context_handle) +Java_org_ros2_rcljava_contexts_ContextImpl_nativeInit( + JNIEnv * env, jclass, jlong context_handle, jobjectArray jargs) { // TODO(jacobperron): Encapsulate init options into a Java class rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); @@ -49,8 +53,42 @@ Java_org_ros2_rcljava_contexts_ContextImpl_nativeInit(JNIEnv * env, jclass, jlon } rcl_context_t * context = reinterpret_cast(context_handle); - // TODO(esteve): parse args - ret = rcl_init(0, nullptr, &init_options, context); + + // jsize should always fit in a size_t, so the following cast is safe. + const auto argc = static_cast(env->GetArrayLength(jargs)); + // rcl_init takes an int, check for overflow just in case + if (argc > static_cast(std::numeric_limits::max())) { + std::ostringstream oss( + "args length is longer than expected, maximum length is ", std::ios_base::ate); + oss << std::numeric_limits::max(); + rcljava_throw_exception( + env, "java/lang/IllegalArgumentException", oss.str().c_str()); + return; + } + const char ** argv = nullptr; + if (argc) { + argv = static_cast(malloc(argc * sizeof(char *))); + for (size_t i = 0; i < argc; ++i) { + auto item = static_cast(env->GetObjectArrayElement(jargs, i)); + // an exception here should never happen, + // as the only possible exception is array out of bounds + RCLJAVA_COMMON_CHECK_FOR_EXCEPTION(env); + argv[i] = env->GetStringUTFChars(item, NULL); + } + } + + ret = rcl_init(static_cast(argc), argv, &init_options, context); + if (argc) { + for (size_t i = 0; i < argc; ++i) { + auto item = static_cast(env->GetObjectArrayElement(jargs, i)); + // an exception here should never happen, + // as the only possible exception is array out of bounds + RCLJAVA_COMMON_CHECK_FOR_EXCEPTION(env); + env->ReleaseStringUTFChars(item, argv[i]); + argv[i] = env->GetStringUTFChars(item, NULL); + } + free(argv); + } if (RCL_RET_OK != ret) { std::string msg = "Failed to init context: " + std::string(rcl_get_error_string().str); rcl_ret_t ignored_ret = rcl_init_options_fini(&init_options); @@ -98,8 +136,15 @@ Java_org_ros2_rcljava_contexts_ContextImpl_nativeDispose(JNIEnv * env, jclass, j rcl_context_t * context = reinterpret_cast(context_handle); - rcl_ret_t ret = rcl_context_fini(context); + // TODO(ivanpauno): Currently, calling `rcl_context_fini` in a zero initialized context fails: + // That's incongruent with most other rcl objects. + // rcl issue: https://github.com/ros2/rcl/issues/814 + if (!context->impl) { + return; + } + + rcl_ret_t ret = rcl_context_fini(context); if (RCL_RET_OK != ret) { std::string msg = "Failed to destroy context: " + std::string(rcl_get_error_string().str); rcl_reset_error(); diff --git a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java index 635fdd2b..d23f5122 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java +++ b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java @@ -154,12 +154,20 @@ public static boolean isInitialized() { * This also initializes the default context. */ public static synchronized void rclJavaInit() { + String args[] = {}; + rclJavaInit(args); + } + + /** + * Initialize the RCLJava API, passing command line arguments. + */ + public static synchronized void rclJavaInit(String args[]) { if (RCLJava.ok()) { return; } // Initialize default context - getDefaultContext().init(); + getDefaultContext().init(args); logger.info("Using RMW implementation: {}", RCLJava.getRMWIdentifier()); } diff --git a/rcljava/src/main/java/org/ros2/rcljava/contexts/Context.java b/rcljava/src/main/java/org/ros2/rcljava/contexts/Context.java index 717eea77..3a458cb1 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/contexts/Context.java +++ b/rcljava/src/main/java/org/ros2/rcljava/contexts/Context.java @@ -32,11 +32,15 @@ public interface Context extends Disposable { /** * Initialize the context. - * // TODO(jacobperron): Pass arguments for parsing * // TODO(jacobperron): Pass in InitOptions object */ void init(); + /** + * Initialize the context passing command line arguments. + */ + void init(String args[]); + /** * Shutdown the context. */ diff --git a/rcljava/src/main/java/org/ros2/rcljava/contexts/ContextImpl.java b/rcljava/src/main/java/org/ros2/rcljava/contexts/ContextImpl.java index 3d6bf9f0..d39e09d0 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/contexts/ContextImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/contexts/ContextImpl.java @@ -61,10 +61,14 @@ public ContextImpl(final long handle) { * {@inheritDoc} */ public final void dispose() { - // Ensure the context is shutdown first - shutdown(); - nativeDispose(this.handle); - this.handle = 0; + if (0 != this.handle) { + // Ensure the context is shutdown first. + if (this.isValid()) { + shutdown(); + } + nativeDispose(this.handle); + this.handle = 0; + } } /** @@ -79,13 +83,21 @@ public final long getHandle() { * * @param contextHandle The pointer to the context structure. */ - private static native void nativeInit(long contextHandle); + private static native void nativeInit(long contextHandle, String args[]); /** * {@inheritDoc} */ public final void init() { - nativeInit(this.handle); + String args[] = {}; + nativeInit(this.handle, args); + } + + /** + * {@inheritDoc} + */ + public final void init(String args[]) { + nativeInit(this.handle, args); } /** diff --git a/rcljava/src/test/java/org/ros2/rcljava/contexts/ContextTest.java b/rcljava/src/test/java/org/ros2/rcljava/contexts/ContextTest.java new file mode 100644 index 00000000..52216587 --- /dev/null +++ b/rcljava/src/test/java/org/ros2/rcljava/contexts/ContextTest.java @@ -0,0 +1,82 @@ +/* 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.contexts; + +import static org.junit.Assert.assertEquals; + +import org.junit.After; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Before; +import org.junit.Test; + +import org.ros2.rcljava.RCLJava; +import org.ros2.rcljava.contexts.Context; + +public class ContextTest { + public Context context = null; + + @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(); + } + + @Before + public void setup() throws Exception { + this.context = RCLJava.createContext(); + } + + @After + public void tearDown() { + this.context.dispose(); + } + + @Test + public final void testCreateAndDispose() { + assertEquals(false, this.context.isValid()); + } + + @Test + public final void testInitShutdown() { + assertEquals(false, this.context.isValid()); + this.context.init(); + assertEquals(true, this.context.isValid()); + this.context.shutdown(); + assertEquals(false, this.context.isValid()); + } + + @Test + public final void testInitWithArgsAndShutdown() { + String args[] = { + "--user-arg-1", "user-arg-2", "-p,", + "--ros-args", "-r", "asd:=bsd" + }; + assertEquals(false, this.context.isValid()); + this.context.init(args); + assertEquals(true, this.context.isValid()); + this.context.shutdown(); + assertEquals(false, this.context.isValid()); + } +} +