From eb8dffcd2409bf86c54f58b2eefd6325393e4531 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Thu, 16 Jul 2020 15:45:11 +0000 Subject: [PATCH 01/14] Implement all parameter types. This implementation was missing BoolArray, IntegerArray, DoubleArray, and StringArray. Doing this required a change to the constructor for ParameterVariant, since List is a Generic and we can't have multiple constructors with the same signature. Signed-off-by: Chris Lalancette --- .../rcljava/parameters/ParameterNames.java | 2 +- .../rcljava/parameters/ParameterType.java | 8 +- .../rcljava/parameters/ParameterVariant.java | 84 +++++++++++++++++-- 3 files changed, 85 insertions(+), 9 deletions(-) diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterNames.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterNames.java index db899443..a2b30e77 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterNames.java +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterNames.java @@ -24,4 +24,4 @@ public class ParameterNames { public static final String SET_PARAMETERS_ATOMICALLY = "set_parameters_atomically"; private ParameterNames() {} -} \ No newline at end of file +} diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterType.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterType.java index 9a56e8c7..73704382 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterType.java +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterType.java @@ -21,7 +21,11 @@ public enum ParameterType { PARAMETER_INTEGER(rcl_interfaces.msg.ParameterType.PARAMETER_INTEGER), PARAMETER_DOUBLE(rcl_interfaces.msg.ParameterType.PARAMETER_DOUBLE), PARAMETER_STRING(rcl_interfaces.msg.ParameterType.PARAMETER_STRING), - PARAMETER_BYTE_ARRAY(rcl_interfaces.msg.ParameterType.PARAMETER_BYTE_ARRAY); + PARAMETER_BYTE_ARRAY(rcl_interfaces.msg.ParameterType.PARAMETER_BYTE_ARRAY), + PARAMETER_BOOL_ARRAY(rcl_interfaces.msg.ParameterType.PARAMETER_BOOL_ARRAY), + PARAMETER_INTEGER_ARRAY(rcl_interfaces.msg.ParameterType.PARAMETER_INTEGER_ARRAY), + PARAMETER_DOUBLE_ARRAY(rcl_interfaces.msg.ParameterType.PARAMETER_DOUBLE_ARRAY), + PARAMETER_STRING_ARRAY(rcl_interfaces.msg.ParameterType.PARAMETER_STRING_ARRAY); private final byte value; @@ -43,4 +47,4 @@ public static ParameterType fromByte(final byte code) { } return null; } -} \ No newline at end of file +} diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java index aa274fb9..263b5b5a 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java @@ -94,13 +94,41 @@ public ParameterVariant(final String name, final String stringValue) { this.value.setType(ParameterType.PARAMETER_STRING.getValue()); } - public ParameterVariant(final String name, final List byteArrayValue) { + public ParameterVariant(final String name, final Byte[] byteArrayValue) { this.name = name; this.value = new rcl_interfaces.msg.ParameterValue(); - this.value.setByteArrayValue(byteArrayValue); + this.value.setByteArrayValue(Arrays.asList(byteArrayValue)); this.value.setType(ParameterType.PARAMETER_BYTE_ARRAY.getValue()); } + public ParameterVariant(final String name, final Boolean[] boolArrayValue) { + this.name = name; + this.value = new rcl_interfaces.msg.ParameterValue(); + this.value.setBoolArrayValue(Arrays.asList(boolArrayValue)); + this.value.setType(ParameterType.PARAMETER_BOOL_ARRAY.getValue()); + } + + public ParameterVariant(final String name, final Long[] integerArrayValue) { + this.name = name; + this.value = new rcl_interfaces.msg.ParameterValue(); + this.value.setIntegerArrayValue(Arrays.asList(integerArrayValue)); + this.value.setType(ParameterType.PARAMETER_INTEGER_ARRAY.getValue()); + } + + public ParameterVariant(final String name, final Double[] doubleArrayValue) { + this.name = name; + this.value = new rcl_interfaces.msg.ParameterValue(); + this.value.setDoubleArrayValue(Arrays.asList(doubleArrayValue)); + this.value.setType(ParameterType.PARAMETER_DOUBLE_ARRAY.getValue()); + } + + public ParameterVariant(final String name, final String[] stringArrayValue) { + this.name = name; + this.value = new rcl_interfaces.msg.ParameterValue(); + this.value.setStringArrayValue(Arrays.asList(stringArrayValue)); + this.value.setType(ParameterType.PARAMETER_STRING_ARRAY.getValue()); + } + public ParameterType getType() { return ParameterType.fromByte(this.value.getType()); } @@ -117,6 +145,14 @@ public String getTypeName() { return "string"; case PARAMETER_BYTE_ARRAY: return "byte_array"; + case PARAMETER_BOOL_ARRAY: + return "bool_array"; + case PARAMETER_INTEGER_ARRAY: + return "integer_array"; + case PARAMETER_DOUBLE_ARRAY: + return "double_array"; + case PARAMETER_STRING_ARRAY: + return "string_array"; case PARAMETER_NOT_SET: return "not set"; default: @@ -161,11 +197,39 @@ public final boolean asBool() { return this.value.getBoolValue(); } - public final List asByteArray() { + public final Byte[] asByteArray() { if (getType() != ParameterType.PARAMETER_BYTE_ARRAY) { throw new IllegalArgumentException("Invalid type"); } - return this.value.getByteArrayValue(); + return this.value.getByteArrayValue().toArray(new Byte[0]); + } + + public final Boolean[] asBooleanArray() { + if (getType() != ParameterType.PARAMETER_BOOL_ARRAY) { + throw new IllegalArgumentException("Invalid type"); + } + return this.value.getBoolArrayValue().toArray(new Boolean[0]); + } + + public final Long[] asIntegerArray() { + if (getType() != ParameterType.PARAMETER_INTEGER_ARRAY) { + throw new IllegalArgumentException("Invalid type"); + } + return this.value.getIntegerArrayValue().toArray(new Long[0]); + } + + public final Double[] asDoubleArray() { + if (getType() != ParameterType.PARAMETER_DOUBLE_ARRAY) { + throw new IllegalArgumentException("Invalid type"); + } + return this.value.getDoubleArrayValue().toArray(new Double[0]); + } + + public final String[] asStringArray() { + if (getType() != ParameterType.PARAMETER_STRING_ARRAY) { + throw new IllegalArgumentException("Invalid type"); + } + return this.value.getStringArrayValue().toArray(new String[0]); } public final rcl_interfaces.msg.Parameter toParameter() { @@ -186,7 +250,15 @@ public static ParameterVariant fromParameter(final rcl_interfaces.msg.Parameter case rcl_interfaces.msg.ParameterType.PARAMETER_STRING: return new ParameterVariant(parameter.getName(), parameter.getValue().getStringValue()); case rcl_interfaces.msg.ParameterType.PARAMETER_BYTE_ARRAY: - return new ParameterVariant(parameter.getName(), parameter.getValue().getByteArrayValue()); + return new ParameterVariant(parameter.getName(), parameter.getValue().getByteArrayValue().toArray(new Byte[0])); + case rcl_interfaces.msg.ParameterType.PARAMETER_BOOL_ARRAY: + return new ParameterVariant(parameter.getName(), parameter.getValue().getBoolArrayValue().toArray(new Boolean[0])); + case rcl_interfaces.msg.ParameterType.PARAMETER_INTEGER_ARRAY: + return new ParameterVariant(parameter.getName(), parameter.getValue().getIntegerArrayValue().toArray(new Long[0])); + case rcl_interfaces.msg.ParameterType.PARAMETER_DOUBLE_ARRAY: + return new ParameterVariant(parameter.getName(), parameter.getValue().getDoubleArrayValue().toArray(new Double[0])); + case rcl_interfaces.msg.ParameterType.PARAMETER_STRING_ARRAY: + return new ParameterVariant(parameter.getName(), parameter.getValue().getStringArrayValue().toArray(new String[0])); case rcl_interfaces.msg.ParameterType.PARAMETER_NOT_SET: throw new IllegalArgumentException("Type from ParameterValue is not set"); default: @@ -194,4 +266,4 @@ public static ParameterVariant fromParameter(final rcl_interfaces.msg.Parameter "Unexpected type from ParameterVariant: " + parameter.getValue().getType()); } } -} \ No newline at end of file +} From e262013500f2cba2afc5ffa2f379075498d25b42 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 13:03:21 +0000 Subject: [PATCH 02/14] Add log4j initialization to all tests. It gets rid of a warning when starting the tests that looks like: log4j:WARN No appenders could be found for logger (org.ros2.rcljava.common.JNIUtils). log4j:WARN Please initialize the log4j system properly. log4j:WARN See http://logging.apache.org/log4j/1.2/faq.html#noconfig for more info. Signed-off-by: Chris Lalancette --- .../test/java/org/ros2/rcljava/RCLJavaTest.java | 7 +++++++ .../src/test/java/org/ros2/rcljava/SpinTest.java | 3 +++ .../src/test/java/org/ros2/rcljava/TimeTest.java | 8 ++++++++ .../test/java/org/ros2/rcljava/node/NodeTest.java | 15 +++------------ .../org/ros2/rcljava/publisher/PublisherTest.java | 7 +++++++ .../rcljava/subscription/SubscriptionTest.java | 7 +++++++ .../java/org/ros2/rcljava/timer/TimerTest.java | 7 +++++++ 7 files changed, 42 insertions(+), 12 deletions(-) diff --git a/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java b/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java index dba322cf..16a00dfe 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java @@ -18,9 +18,16 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; +import org.junit.BeforeClass; import org.junit.Test; public class RCLJavaTest { + @BeforeClass + public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + } + @Test public final void testInitShutdown() { assertFalse(RCLJava.ok()); diff --git a/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java b/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java index dd0aea31..f015d260 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java @@ -58,6 +58,9 @@ public int getCounter() { @BeforeClass public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + RCLJava.rclJavaInit(); } diff --git a/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java b/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java index 30b5b96d..5e8abf32 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java @@ -20,11 +20,19 @@ import java.util.concurrent.TimeUnit; +import org.junit.BeforeClass; import org.junit.Test; + import org.ros2.rcljava.RCLJava; import org.ros2.rcljava.time.ClockType; public class TimeTest { + @BeforeClass + public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + } + // @Test public final void testSystemTime() { builtin_interfaces.msg.Time now = Time.now(ClockType.SYSTEM_TIME); diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java index 2287b9de..6c5e45be 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java @@ -86,19 +86,10 @@ boolean checkPrimitives(rcljava.msg.Primitives primitives, boolean booleanValue, @BeforeClass public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + RCLJava.rclJavaInit(); - 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(); - } } public class TestConsumer implements Consumer { diff --git a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java index 2ce379fd..649c4aa8 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java @@ -18,12 +18,19 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import org.junit.BeforeClass; import org.junit.Test; import org.ros2.rcljava.RCLJava; import org.ros2.rcljava.node.Node; public class PublisherTest { + @BeforeClass + public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + } + @Test public final void testCreateAndDispose() { RCLJava.rclJavaInit(); diff --git a/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java b/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java index 8c990480..6b01a634 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import org.junit.BeforeClass; import org.junit.Test; import org.ros2.rcljava.RCLJava; @@ -25,6 +26,12 @@ import org.ros2.rcljava.node.Node; public class SubscriptionTest { + @BeforeClass + public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + } + @Test public final void testCreateAndDispose() { RCLJava.rclJavaInit(); diff --git a/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java b/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java index 28378805..2500f5cf 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java @@ -24,6 +24,7 @@ import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import org.junit.BeforeClass; import org.junit.Test; import org.ros2.rcljava.RCLJava; @@ -55,6 +56,12 @@ public int getCounter() { } } + @BeforeClass + public static void setupOnce() throws Exception { + // Just to quiet down warnings + org.apache.log4j.BasicConfigurator.configure(); + } + @Test public final void testCreate() { int max_iterations = 4; From bbed9ad9206e1c4168eaa600d742c09341433297 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 17:33:02 +0000 Subject: [PATCH 03/14] Make the parameter separator a class variable. It doesn't need to change, so it can just be a class variable. Signed-off-by: Chris Lalancette --- rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 8bf5e69d..34a5316c 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -61,6 +61,7 @@ */ public class NodeImpl implements Node { private static final Logger logger = LoggerFactory.getLogger(NodeImpl.class); + private static final String separator = "."; static { try { @@ -479,7 +480,6 @@ public rcl_interfaces.msg.ListParametersResult listParameters( rcl_interfaces.msg.ListParametersResult result = new rcl_interfaces.msg.ListParametersResult(); - String separator = "."; for (Map.Entry entry : this.parameters.entrySet()) { boolean getAll = (prefixes.size() == 0) From 19ec023e8b82dc0fdd8b85a6a0435a6579909db6 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 17:40:03 +0000 Subject: [PATCH 04/14] Store the parameter and the descriptor. That way we have the information available when other methods want to retrieve it. Signed-off-by: Chris Lalancette --- .../java/org/ros2/rcljava/node/NodeImpl.java | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) 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 34a5316c..e62a9e4e 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -117,7 +117,12 @@ public class NodeImpl implements Node { private Object mutex; - private Map parameters; + class ParameterAndDescriptor { + public ParameterVariant parameter; + public rcl_interfaces.msg.ParameterDescriptor descriptor; + } + + private Map parameters; /** * Constructor. @@ -136,7 +141,7 @@ public NodeImpl(final long handle, final String name, final Context context) { this.clients = new LinkedBlockingQueue(); this.timers = new LinkedBlockingQueue(); this.mutex = new Object(); - this.parameters = new ConcurrentHashMap(); + this.parameters = new ConcurrentHashMap(); } /** @@ -404,9 +409,9 @@ public List getParameters(List names) { synchronized (mutex) { List results = new ArrayList(); for (String name : names) { - for (Map.Entry entry : this.parameters.entrySet()) { + for (Map.Entry entry : this.parameters.entrySet()) { if (name.equals(entry.getKey())) { - results.add(entry.getValue()); + results.add(entry.getValue().parameter); } } } @@ -418,9 +423,9 @@ public List getParameterTypes(List names) { synchronized (mutex) { List results = new ArrayList(); for (String name : names) { - for (Map.Entry entry : this.parameters.entrySet()) { + for (Map.Entry entry : this.parameters.entrySet()) { if (name.equals(entry.getKey())) { - results.add(entry.getValue().getType()); + results.add(entry.getValue().parameter.getType()); } else { results.add(ParameterType.fromByte(rcl_interfaces.msg.ParameterType.PARAMETER_NOT_SET)); } @@ -447,7 +452,15 @@ public rcl_interfaces.msg.SetParametersResult setParametersAtomically( synchronized (mutex) { rcl_interfaces.msg.SetParametersResult result = new rcl_interfaces.msg.SetParametersResult(); for (ParameterVariant p : parameters) { - this.parameters.put(p.getName(), p); + ParameterAndDescriptor pandd = new ParameterAndDescriptor(); + pandd.parameter = p; + + rcl_interfaces.msg.ParameterDescriptor parameterDescriptor = + new rcl_interfaces.msg.ParameterDescriptor(); + parameterDescriptor.setName(p.getName()); + parameterDescriptor.setType(p.getType().getValue()); + + this.parameters.put(p.getName(), pandd); } result.setSuccessful(true); return result; @@ -460,13 +473,9 @@ public List describeParameters( List results = new ArrayList(); for (String name : names) { - for (Map.Entry entry : this.parameters.entrySet()) { + for (Map.Entry entry : this.parameters.entrySet()) { if (name.equals(entry.getKey())) { - rcl_interfaces.msg.ParameterDescriptor parameterDescriptor = - new rcl_interfaces.msg.ParameterDescriptor(); - parameterDescriptor.setName(entry.getKey()); - parameterDescriptor.setType(entry.getValue().getType().getValue()); - results.add(parameterDescriptor); + results.add(entry.getValue().descriptor); } } } @@ -480,7 +489,7 @@ public rcl_interfaces.msg.ListParametersResult listParameters( rcl_interfaces.msg.ListParametersResult result = new rcl_interfaces.msg.ListParametersResult(); - for (Map.Entry entry : this.parameters.entrySet()) { + for (Map.Entry entry : this.parameters.entrySet()) { boolean getAll = (prefixes.size() == 0) && ((depth == rcl_interfaces.srv.ListParameters_Request.DEPTH_RECURSIVE) From bfafaf5449a5b25e60e9773f2e4fc7edfb3ff681 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 17:51:49 +0000 Subject: [PATCH 05/14] Switch parameters to using map lookups. The parameters are stored in a HashMap of name -> parameter, but we were unnecessarily iterating over the HashMap to find things. Instead, look up the items directly in the map which should be much faster. Signed-off-by: Chris Lalancette --- .../java/org/ros2/rcljava/node/NodeImpl.java | 22 +++++++------------ 1 file changed, 8 insertions(+), 14 deletions(-) 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 e62a9e4e..5200a52e 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -409,10 +409,8 @@ public List getParameters(List names) { synchronized (mutex) { List results = new ArrayList(); for (String name : names) { - for (Map.Entry entry : this.parameters.entrySet()) { - if (name.equals(entry.getKey())) { - results.add(entry.getValue().parameter); - } + if (this.parameters.containsKey(name)) { + results.add(this.parameters.get(name).parameter); } } return results; @@ -423,12 +421,10 @@ public List getParameterTypes(List names) { synchronized (mutex) { List results = new ArrayList(); for (String name : names) { - for (Map.Entry entry : this.parameters.entrySet()) { - if (name.equals(entry.getKey())) { - results.add(entry.getValue().parameter.getType()); - } else { - results.add(ParameterType.fromByte(rcl_interfaces.msg.ParameterType.PARAMETER_NOT_SET)); - } + if (this.parameters.containsKey(name)) { + results.add(this.parameters.get(name).parameter.getType()); + } else { + results.add(ParameterType.fromByte(rcl_interfaces.msg.ParameterType.PARAMETER_NOT_SET)); } } return results; @@ -473,10 +469,8 @@ public List describeParameters( List results = new ArrayList(); for (String name : names) { - for (Map.Entry entry : this.parameters.entrySet()) { - if (name.equals(entry.getKey())) { - results.add(entry.getValue().descriptor); - } + if (this.parameters.containsKey(name)) { + results.add(this.parameters.get(name).descriptor); } } return results; From 42901a6f55632d5ba9235f43930c7631dba7430d Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 18:28:49 +0000 Subject: [PATCH 06/14] Plumb undeclaredParameters through the Node constructor. Whether to allow undeclared parameters is a decision that is made during Node creation. Plumb through the necessary option so that the user can choose to allow undeclared parameters when they create the node. Signed-off-by: Chris Lalancette --- rcljava/src/main/java/org/ros2/rcljava/RCLJava.java | 8 ++++++-- rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java | 4 +++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java index 75e2148d..0e18d6aa 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java +++ b/rcljava/src/main/java/org/ros2/rcljava/RCLJava.java @@ -217,7 +217,7 @@ public static Context createContext() { * structure. */ public static Node createNode(final String nodeName) { - return createNode(nodeName, "", RCLJava.getDefaultContext()); + return createNode(nodeName, "", RCLJava.getDefaultContext(), false); } /** @@ -229,8 +229,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, false); + } + + 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); + Node node = new NodeImpl(nodeHandle, nodeName, context, allowUndeclaredParameters); nodes.add(node); return node; } 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 5200a52e..5b741434 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -123,6 +123,7 @@ class ParameterAndDescriptor { } private Map parameters; + private boolean allowUndeclaredParameters; /** * Constructor. @@ -130,7 +131,7 @@ 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) { + public NodeImpl(final long handle, final String name, final Context context, final boolean allowUndeclaredParameters) { this.clock = new Clock(ClockType.SYSTEM_TIME); this.handle = handle; this.name = name; @@ -142,6 +143,7 @@ public NodeImpl(final long handle, final String name, final Context context) { this.timers = new LinkedBlockingQueue(); this.mutex = new Object(); this.parameters = new ConcurrentHashMap(); + this.allowUndeclaredParameters = allowUndeclaredParameters; } /** From 3f741f90c6b744e77ffb998c959c829800d85d06 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 17:26:58 +0000 Subject: [PATCH 07/14] Add in another ParameterVariant. This one just takes a name and sets the type to NOT_SET. Signed-off-by: Chris Lalancette --- .../java/org/ros2/rcljava/parameters/ParameterVariant.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java index 263b5b5a..8f7484c7 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java @@ -52,6 +52,12 @@ public ParameterVariant() { this.value.setType(ParameterType.PARAMETER_NOT_SET.getValue()); } + public ParameterVariant(final String name) { + this.name = name; + this.value = new rcl_interfaces.msg.ParameterValue(); + this.value.setType(ParameterType.PARAMETER_NOT_SET.getValue()); + } + public ParameterVariant(final String name, final boolean boolValue) { this.name = name; this.value = new rcl_interfaces.msg.ParameterValue(); From 5e252b8ec406f868987843cd43f00f622d7a4313 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 18:44:45 +0000 Subject: [PATCH 08/14] Add in a ParameterCallback interface. This interface is what users will have to implement in order to have their callback called when parameters are set. Signed-off-by: Chris Lalancette --- rcljava/CMakeLists.txt | 1 + .../rcljava/parameters/ParameterCallback.java | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterCallback.java diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index 48a8e7bc..95553286 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -138,6 +138,7 @@ set(${PROJECT_NAME}_sources "src/main/java/org/ros2/rcljava/parameters/client/AsyncParametersClientImpl.java" "src/main/java/org/ros2/rcljava/parameters/client/SyncParametersClient.java" "src/main/java/org/ros2/rcljava/parameters/client/SyncParametersClientImpl.java" + "src/main/java/org/ros2/rcljava/parameters/ParameterCallback.java" "src/main/java/org/ros2/rcljava/parameters/ParameterNames.java" "src/main/java/org/ros2/rcljava/parameters/ParameterType.java" "src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java" diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterCallback.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterCallback.java new file mode 100644 index 00000000..51ffcea2 --- /dev/null +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterCallback.java @@ -0,0 +1,24 @@ +/* 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.parameters; + +import java.util.List; + +import org.ros2.rcljava.parameters.ParameterVariant; + +public interface ParameterCallback { + rcl_interfaces.msg.SetParametersResult callback(List parameters); +} From b074d610e963c9fda237a26dfb5a2239fe8e4626 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 20 Jul 2020 13:59:25 +0000 Subject: [PATCH 09/14] Update the Node parameter methods to Dashing/Foxy equivalents. The parameter API was significantly updated during the Dashing release cycle. Update the API in rcljava to provide similar functionality and make the API look a lot more like rclcpp and rclpy. Signed-off-by: Chris Lalancette --- .../main/java/org/ros2/rcljava/node/Node.java | 355 +++++++++++++++++- .../java/org/ros2/rcljava/node/NodeImpl.java | 300 ++++++++++++--- 2 files changed, 606 insertions(+), 49 deletions(-) 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 0a3ba7ad..4f276c82 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/Node.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/Node.java @@ -17,6 +17,7 @@ import java.util.Collection; import java.util.List; +import java.util.Map; import java.util.concurrent.TimeUnit; import org.ros2.rcljava.client.Client; @@ -27,6 +28,7 @@ import org.ros2.rcljava.interfaces.Disposable; import org.ros2.rcljava.interfaces.MessageDefinition; import org.ros2.rcljava.interfaces.ServiceDefinition; +import org.ros2.rcljava.parameters.ParameterCallback; import org.ros2.rcljava.parameters.ParameterType; import org.ros2.rcljava.parameters.ParameterVariant; import org.ros2.rcljava.publisher.Publisher; @@ -175,15 +177,366 @@ Client createClient(final Class serviceType, String getName(); + /* + * TODO(clalancette): The parameter APIs below all tend to throw + * IllegalArgumentException on failure. We should instead add in custom + * exception classes to make it easier for the caller to determine the + * cause of the exception. + */ + + /** + * Declare and initialize a parameter, return the effective value. + * + * See the documentation for the full signature of declareParameter for + * more details. + * This method will use a default descriptor that sets only the name. + */ + ParameterVariant declareParameter(ParameterVariant parameter); + + /** + * Declare and initialize a parameter, return the effective value. + * + * This method is used to declare that a parameter exists on this node. + * Whatever name and value is in the given ParameterVariant will be used + * as the name and initial value of the parameter. + * If successful, the passed in ParameterVariant is returned. + * + * The passed in parameter descriptor will be stored alongside the parameter. + * This descriptor can be used for additional meta-information about the + * parameter, like ranges, read-only, etc. + * + * This method, if successful, will result in any callback registered with + * addOnSetParametersCallback to be called. + * If that callback prevents the initial value for the parameter from being + * set then an IllegalArgumentException is thrown. + * + * The returned reference will remain valid until the parameter is + * undeclared. + * + * @param parameter The parameter to declare. + * @param descriptor The descriptor to declare. + * @return The parameter. + * @throws IllegalArgumentException if parameter has already been declared. + * @throws rclcpp::exceptions::InvalidParametersException if a parameter + * name is invalid. + * @throws IllegalArgumentException if initial value fails to be set. + */ + ParameterVariant declareParameter(ParameterVariant parameter, rcl_interfaces.msg.ParameterDescriptor descriptor); + + /** + * Declare and initialize several parameters. + * + * Each parameter in the parameters List is declared on the node along with + * the corresponding descriptor. + * Note that the length of the parameters list and descriptors list must be + * the same, or an IllegalArgumentException is thrown. + * A list of the ParameterVariants will be returned. + * + * This method, if successful, will result in any callback registered with + * addOnSetParametersCallback to be called, once for each parameter. + * If that callback prevents the initial value for any parameter from being + * set then an IllegalArgumentException is thrown. + * + * @param parameters The parameters to set. + * @param descriptors The descriptors to set. + * @return The list of parameters that were declared. + * @throws IllegalArgumentException if the parameters list and descriptors + * list are not the same length. + * @throws IllegalArgumentException if any parameter in the list has already + * been declared. + */ + List declareParameters(List parameters, List descriptors); + + /** + * Undeclare a previously declared parameter. + * + * This method will not cause a callback registered with + * addOnSetParametersCallback to be called. + * + * @param name The name of the parameter to be undeclared. + * @throws IllegalArgumentException if the parameter + * has not been declared. + */ + void undeclareParameter(String name); + + /** + * Return true if a given parameter is declared. + * + * @param name The name of the parameter to check for being declared. + * @return true if the parameter name has been declared, otherwise false. + */ + boolean hasParameter(String name); + + /** + * Return a list of parameters by the given list of parameter names. + * + * This method will throw an IllegalArgumentException if any of the requested + * parameters have not been declared and undeclared parameters are not + * allowed. + * + * If undeclared parameters are allowed and the parameter has not been + * declared, then the returned rclcpp::Parameter will be default initialized + * and therefore have the type ParameterType::PARAMETER_NOT_SET. + * + * @param names The names of the parameters to be retrieved. + * @return The parameters that were retrieved. + * @throws IllegalArgumentException if any of the parameters have not been + * declared and undeclared parameters are not allowed. + */ List getParameters(List names); - List getParameterTypes(List names); + /** + * Return a list of parameters by the given list of parameter names. + * + * See the documentation for getParameters for more details. + * + * @param name The names of the parameters to be retrieved. + * @return The parameter that was retrieved. + * @throws IllegalArgumentException if the parameter has not been declared + * and undeclared parameters are not allowed. + */ + ParameterVariant getParameter(String name); + + /** + * Return the named parameter value, or the "alternativeValue" if not set. + * + * This method will not throw IllegalArgumentException if a parameter is + * not declared. + * Instead, it will return the alternativeValue. + * + * In all cases, the parameter is never set or declared within the node. + * + * @param name The name of the parameter to get. + * @param alternativeValue Value to be returned if the parameter was not set. + * @return The parameter that was retrieved. + */ + ParameterVariant getParameterOr(String name, ParameterVariant alternateValue); + + /** + * Get parameters that have a given prefix in their names. + * + * The names which are used as keys in the returned Map have the prefix + * removed. + * For example, if you use the prefix "foo" and the parameters "foo.ping", + * "foo.pong" and "bar.baz" exist, then the returned dictionary will have the + * keys "ping" and "pong". + * Note that the parameter separator is also removed from the parameter name + * to create the keys. + * + * An empty string for the prefix will match all parameters. + * + * If no parameters with the prefix are found, an empty Map will be returned. + * + * @param prefix The prefix of the parameters to get. + * @return A set of parameters with the given prefix. + */ + Map getParametersByPrefix(String prefix); + + /** + * Set the given parameter and then return result of the set action. + * + * If the parameter has not been declared and undeclared parameters are not + * allowed, this function will throw an IllegalArgumentException. + * + * If undeclared parameters are allowed, then the parameter is implicitly + * declared with the default parameter meta data before being set. + * Parameter overrides are ignored by set_parameter. + * + * This method will result in any callback registered with + * addOnSetParametersCallback to be called. + * If the callback prevents the parameter from being set, then it will be + * reflected in the SetParametersResult that is returned, but no exception + * will be thrown. + * + * If the value type of the passed in parameter is + * ParameterType::PARAMETER_NOT_SET, and the existing parameter type is + * something else, then the parameter will be implicitly undeclared. + * + * @param parameter The parameter to be set. + * @return The result of the set action. + * @throws IllegalArgumentException if the parameter + * has not been declared and undeclared parameters are not allowed. + */ + rcl_interfaces.msg.SetParametersResult setParameter(ParameterVariant parameter); + /** + * Set the given parameters, one at a time, and then return the result of each + * set action. + * + * Parameters are set in the order they are given within the input List. + * + * Like setParameter, if any of the parameters to be set have not first been + * declared, and undeclared parameters are not allowed (the default), then + * this method will throw an IllegalArgumentException. + * + * If setting a parameter fails due to not being declared, then the + * parameters which have already been set will stay set, and no attempt will + * be made to set the parameters which come after. + * + * If a parameter fails to be set due to any other reason, like being + * rejected by the user's callback (basically any reason other than not + * having been declared beforehand), then that is reflected in the + * corresponding SetParametersResult in the vector returned by this function. + * + * This method will result in any callback registered with + * addOnSetParametersCallback to be called, once for each parameter. + * If the callback prevents the parameter from being set, then, as mentioned + * before, it will be reflected in the corresponding SetParametersResult + * that is returned, but no exception will be thrown. + * + * Like setParameter this method will implicitly undeclare parameters + * with the type ParameterType::PARAMETER_NOT_SET. + * + * @param parameters The list of parameters to be set. + * @return The results for each set action as a list. + * @throws IllegalArgumentException if any parameter has not been declared + * and undeclared parameters are not allowed. + */ List setParameters(List parameters); + /** + * Set the given parameters, all at one time, and then aggregate result. + * + * In contrast to setParameters, either all of the parameters are set or none + * of them are set. + * + * Like setParameter and setParameters, this method may throw an + * IllegalArgumentException if any of the parameters to be set have not first + * been declared. + * If the exception is thrown then none of the parameters will have been set. + * + * This method will result in any callback registered with + * addOnSetParametersCallback to be called, just one time. + * If the callback prevents the parameters from being set, then it will be + * reflected in the SetParametersResult which is returned, but no exception + * will be thrown. + * + * If multiple ParameterVariant instances in the list have the same name, then + * only the last one in the list (forward iteration) will be set. + * + * Like setParameter this method will implicitly undeclare parameters + * with the type ParameterType::PARAMETER_NOT_SET. + * + * @param parameters The list of parameters to be set. + * @return The aggregate result of setting all the parameters atomically. + * @throws IllegalArgumentException if any parameter has not been declared + * and undeclared parameters are not allowed. + */ rcl_interfaces.msg.SetParametersResult setParametersAtomically(List parameters); + /** + * Add a callback to be called when parameters are being set or declared. + * + * The callback signature is designed to allow handling of any of the above + * `setParameter*` or `declareParameter*` methods, and so it takes a list of + * parameters to be set, and returns an instance of + * rcl_interfaces::msg::SetParametersResult to indicate whether or not the + * parameters should be set or not, and if not why. + * + * Users who wish to register a callback must implement the ParameterCallback + * class, then call this method with an instance of the implemented class. + * + * Note that the callback is called when declareParameter and its variants + * are called, and so it cannot be assumed any of the parameter has been set + * before this callback. If checking a new value against the existing one, + * the case where the parameter is not yet set must be accounted for. + * + * The callback may introspect other already set parameters (by calling any + * of the {get,list,describe}Parameter methods), but may *not* modify + * other parameters (by calling any of the {set,declare}Parameter methods) + * or modify the registered callback itself (by calling the + * addOnSetParametersCallback method). + * + * The registered callbacks are called when a parameter is set. + * When a callback returns a not successful result, the remaining callbacks aren't called. + * The order of the callback is the reverse from the registration order. + * + * @param cb The callback to register. + */ + void addOnSetParametersCallback(ParameterCallback cb); + + /** + * Remove a callback registered with `addOnSetParametersCallback`. + * + * Calling `remove_on_set_parameters_callback` more than once is an error. + * + * @param cb The callback handler to remove. + */ + void removeOnSetParametersCallback(ParameterCallback cb); + + /** + * Return the parameter descriptor for the given parameter name. + * + * This method will throw an IllegalArgumentException if the requested + * parameter has not been declared and undeclared parameters are not allowed. + * + * If undeclared parameters are allowed, then a default initialized + * descriptor will be returned. + * + * @param name The name of the parameter to describe. + * @return The descriptor for the given parameter name. + * @throws IllegalArgumentException if the parameter has not been declared + * and undeclared parameters are not allowed. + */ + rcl_interfaces.msg.ParameterDescriptor describeParameter(String name); + + /** + * Return a List of parameter descriptors, one for each of the given names. + * + * This method will throw an IllegalArgumentException if any of the requested + * parameters have not been declared and undeclared parameters are not allowed. + * + * If undeclared parameters are allowed, then a default initialized + * descriptor will be returned for each of the undeclared parameter's descriptor. + * + * If the names list is empty, then an empty list will be returned. + * + * @param names The list of parameter names to describe. + * @return A list of parameter descriptors, one for each parameter given. + * @throws IllegalArgumentException if any of the parameters have not been + * declared and undeclared parameters are not allowed. + */ List describeParameters(List names); + /** + * Return a list of parameter types, one for each of the given names. + * + * This method will throw an IllegalArgumentException if any of the requested + * parameters have not been declared and undeclared parameters are not allowed. + * + * If undeclared parameters are allowed, then the default type + * rclcpp::ParameterType::PARAMETER_NOT_SET will be returned. + * + * @param names The list of parameter names to get the types. + * @return A list of parameter types, one for each parameter given. + * @throws IllegalArgumentException if any of the parameters have not been + * declared and undeclared parameters are not allowed. + */ + List getParameterTypes(List names); + + /** + * List the parameters with the given prefixes at the given depth. + * + * This method finds all parameters with the given prefixes at no deeper than + * the given depth and returns them in an aggregated result. + * If the prefixes list is empty, then all parameters up to the given depth + * are returned. + * If the depth is rcl_interfaces.srv.ListParameters_Request.DEPTH_RECURSIVE, + * then all depths are considered. + * + * Thus, it is possible to get a list of all of the parameters by calling this + * method with an empty prefix and a depth of DEPTH_RECURSIVE. + * To get all parameters up to a depth of 2, call this method with an empty + * prefix and a depth equal to 2. + * To get all parameters with a prefix of "foo", call this method with a prefix + * containing "foo" and a depth of DEPTH_RECURSIVE. + * + * The return value is a ListParametersResult message, which contains the + * prefixes and the names of all matching parameters. + * + * @param prefixes The list of prefixes to find (may be the empty list). + * @param depth How deep to look for parameters (may be DEPTH_RECURSIVE for all depths). + * @return rcl_interfaces.msg.ListParametersResult + */ rcl_interfaces.msg.ListParametersResult listParameters(List prefixes, long depth); } 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 5b741434..dffbb707 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -27,6 +27,7 @@ import org.ros2.rcljava.interfaces.Disposable; import org.ros2.rcljava.interfaces.MessageDefinition; import org.ros2.rcljava.interfaces.ServiceDefinition; +import org.ros2.rcljava.parameters.ParameterCallback; import org.ros2.rcljava.parameters.ParameterType; import org.ros2.rcljava.parameters.ParameterVariant; import org.ros2.rcljava.publisher.Publisher; @@ -49,6 +50,8 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; +import java.util.Iterator; import java.util.List; import java.util.Map; @@ -115,7 +118,7 @@ public class NodeImpl implements Node { private final String name; - private Object mutex; + private Object parametersMutex; class ParameterAndDescriptor { public ParameterVariant parameter; @@ -125,6 +128,9 @@ class ParameterAndDescriptor { private Map parameters; private boolean allowUndeclaredParameters; + private Object parameterCallbacksMutex; + private List parameterCallbacks; + /** * Constructor. * @@ -141,9 +147,11 @@ public NodeImpl(final long handle, final String name, final Context context, fin this.services = new LinkedBlockingQueue(); this.clients = new LinkedBlockingQueue(); this.timers = new LinkedBlockingQueue(); - this.mutex = new Object(); + this.parametersMutex = new Object(); this.parameters = new ConcurrentHashMap(); this.allowUndeclaredParameters = allowUndeclaredParameters; + this.parameterCallbacksMutex = new Object(); + this.parameterCallbacks = new ArrayList(); } /** @@ -407,84 +415,277 @@ public final String getName() { return this.name; } + public ParameterVariant declareParameter(ParameterVariant parameter) { + return declareParameter(parameter, new rcl_interfaces.msg.ParameterDescriptor().setName(parameter.getName())); + } + + public ParameterVariant declareParameter(ParameterVariant parameter, rcl_interfaces.msg.ParameterDescriptor descriptor) { + return declareParameters(java.util.Arrays.asList(new ParameterVariant[] {parameter}), + java.util.Arrays.asList(new rcl_interfaces.msg.ParameterDescriptor[] {descriptor})).get(0); + } + + public List declareParameters(List parameters, List descriptors) { + if (parameters.size() != descriptors.size()) { + throw new IllegalArgumentException("Parameters length must be equal to descriptors length"); + } + + synchronized (parameterCallbacksMutex) { + for (ParameterCallback cb : this.parameterCallbacks) { + rcl_interfaces.msg.SetParametersResult result = cb.callback(parameters); + if (!result.getSuccessful()) { + throw new IllegalArgumentException("Failed to declare parameters; rejected by callback"); + } + } + } + + List results = new ArrayList(); + Iterator itp = parameters.iterator(); + Iterator itpd = descriptors.iterator(); + synchronized (parametersMutex) { + while (itp.hasNext() && itpd.hasNext()) { + ParameterVariant parameter = itp.next(); + rcl_interfaces.msg.ParameterDescriptor descriptor = itpd.next(); + + if (this.parameters.containsKey(parameter.getName())) { + throw new IllegalArgumentException(String.format("Parameter '%s' is already declared", parameter.getName())); + } + + ParameterAndDescriptor pandd = new ParameterAndDescriptor(); + pandd.parameter = parameter; + pandd.descriptor = descriptor; + + this.parameters.put(parameter.getName(), pandd); + results.add(parameter); + } + } + + return results; + } + + public void undeclareParameter(String name) { + synchronized (parametersMutex) { + if (!this.parameters.containsKey(name)) { + throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", name)); + } + + this.parameters.remove(name); + } + } + + public boolean hasParameter(String name) { + synchronized (parametersMutex) { + return this.parameters.containsKey(name); + } + } + public List getParameters(List names) { - synchronized (mutex) { - List results = new ArrayList(); + List results = new ArrayList(); + + synchronized (parametersMutex) { for (String name : names) { if (this.parameters.containsKey(name)) { - results.add(this.parameters.get(name).parameter); + results.add(this.parameters.get(name).parameter); + } else if (this.allowUndeclaredParameters) { + results.add(new ParameterVariant(name)); + } else { + throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", name)); } } - return results; } + + return results; } - public List getParameterTypes(List names) { - synchronized (mutex) { - List results = new ArrayList(); - for (String name : names) { - if (this.parameters.containsKey(name)) { - results.add(this.parameters.get(name).parameter.getType()); + public ParameterVariant getParameter(String name) { + return getParameters(java.util.Arrays.asList(new String[] {name})).get(0); + } + + public ParameterVariant getParameterOr(String name, ParameterVariant alternateValue) { + synchronized (parametersMutex) { + if (this.parameters.containsKey(name)) { + return this.parameters.get(name).parameter; + } + } + + return alternateValue; + } + + public Map getParametersByPrefix(String prefix) { + String new_prefix; + if (prefix.length() > 0) { + new_prefix = prefix.concat(separator); + } else { + new_prefix = prefix; + } + + Map results = new HashMap(); + synchronized (parametersMutex) { + for (Map.Entry entry : this.parameters.entrySet()) { + if (entry.getKey().startsWith(new_prefix)) { + results.put(entry.getKey().substring(new_prefix.length()), entry.getValue().parameter); + } + } + } + + return results; + } + + private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically( + List parameters) { + rcl_interfaces.msg.SetParametersResult result = new rcl_interfaces.msg.SetParametersResult(); + + synchronized (parameterCallbacksMutex) { + for (ParameterCallback cb : this.parameterCallbacks) { + result = cb.callback(parameters); + if (!result.getSuccessful()) { + return result; + } + } + } + + // Walk through the list of new parameters, checking if they have been + // declared. If they haven't, and we are not allowing undeclared + // parameters, throw an exception. If they haven't and we are allowing + // undeclared parameters, collect the names so we can default initialize + // them in the parameter list before setting them to the new value. We + // do this multi-stage thing so that all of the parameters are set, or + // none of them. + List parametersToDeclare = new ArrayList(); + List parametersToUndeclare = new ArrayList(); + for (ParameterVariant parameter : parameters) { + if (this.parameters.containsKey(parameter.getName())) { + if (parameter.getType() == ParameterType.PARAMETER_NOT_SET) { + parametersToUndeclare.add(parameter.getName()); + } + } else { + if (this.allowUndeclaredParameters) { + parametersToDeclare.add(parameter.getName()); } else { - results.add(ParameterType.fromByte(rcl_interfaces.msg.ParameterType.PARAMETER_NOT_SET)); + throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", parameter.getName())); } } - return results; } + + // Check to make sure that a parameter isn't both trying to be declared + // and undeclared simultaneously. + for (String name : parametersToDeclare) { + if (parametersToUndeclare.contains(name)) { + throw new IllegalArgumentException(String.format("Cannot both declare and undeclare the same parameter name '%s'", name)); + } + } + + // Undeclare any parameters that have their type set to NOT_SET + for (String name : parametersToUndeclare) { + if (!hasParameter(name)) { + throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", name)); + } + + this.parameters.remove(name); + } + + for (String name : parametersToDeclare) { + this.parameters.put(name, new ParameterAndDescriptor()); + } + + for (ParameterVariant parameter : parameters) { + // We already dealt with unsetting parameters above, so skip them + if (parameter.getType() == ParameterType.PARAMETER_NOT_SET) { + continue; + } + this.parameters.get(parameter.getName()).parameter = parameter; + } + + result.setSuccessful(true); + return result; + } + + public rcl_interfaces.msg.SetParametersResult setParameter(ParameterVariant parameter) { + return this.setParametersAtomically(java.util.Arrays.asList(new ParameterVariant[] {parameter})); } public List setParameters( List parameters) { List results = new ArrayList(); - for (ParameterVariant p : parameters) { - rcl_interfaces.msg.SetParametersResult result = - this.setParametersAtomically(java.util.Arrays.asList(new ParameterVariant[] {p})); - results.add(result); + + synchronized (parametersMutex) { + for (ParameterVariant parameter : parameters) { + rcl_interfaces.msg.SetParametersResult result = + this.internalSetParametersAtomically(java.util.Arrays.asList(new ParameterVariant[] {parameter})); + results.add(result); + } } + return results; } public rcl_interfaces.msg.SetParametersResult setParametersAtomically( List parameters) { - synchronized (mutex) { - rcl_interfaces.msg.SetParametersResult result = new rcl_interfaces.msg.SetParametersResult(); - for (ParameterVariant p : parameters) { - ParameterAndDescriptor pandd = new ParameterAndDescriptor(); - pandd.parameter = p; + synchronized (parametersMutex) { + return internalSetParametersAtomically(parameters); + } + } - rcl_interfaces.msg.ParameterDescriptor parameterDescriptor = - new rcl_interfaces.msg.ParameterDescriptor(); - parameterDescriptor.setName(p.getName()); - parameterDescriptor.setType(p.getType().getValue()); + public void addOnSetParametersCallback(ParameterCallback cb) { + synchronized (parameterCallbacksMutex) { + this.parameterCallbacks.add(0, cb); + } + } - this.parameters.put(p.getName(), pandd); - } - result.setSuccessful(true); - return result; + public void removeOnSetParametersCallback(ParameterCallback cb) { + synchronized (parameterCallbacksMutex) { + this.parameterCallbacks.remove(cb); } } + public rcl_interfaces.msg.ParameterDescriptor describeParameter(String name) { + return this.describeParameters(java.util.Arrays.asList(new String[] {name})).get(0); + } + public List describeParameters( List names) { - synchronized (mutex) { - List results = - new ArrayList(); + List results = + new ArrayList(); + + synchronized (parametersMutex) { for (String name : names) { if (this.parameters.containsKey(name)) { results.add(this.parameters.get(name).descriptor); + } else if (this.allowUndeclaredParameters) { + results.add(new rcl_interfaces.msg.ParameterDescriptor().setName(name)); + } else { + throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", name)); } } - return results; } + + return results; + } + + public List getParameterTypes(List names) { + List results = new ArrayList(); + + synchronized (parametersMutex) { + for (String name : names) { + if (this.parameters.containsKey(name)) { + results.add(this.parameters.get(name).parameter.getType()); + } else if (this.allowUndeclaredParameters) { + results.add(ParameterType.fromByte(rcl_interfaces.msg.ParameterType.PARAMETER_NOT_SET)); + } else { + throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", name)); + } + } + } + + return results; } public rcl_interfaces.msg.ListParametersResult listParameters( List prefixes, long depth) { - synchronized (mutex) { - rcl_interfaces.msg.ListParametersResult result = - new rcl_interfaces.msg.ListParametersResult(); + rcl_interfaces.msg.ListParametersResult result = + new rcl_interfaces.msg.ListParametersResult(); + synchronized (parametersMutex) { for (Map.Entry entry : this.parameters.entrySet()) { boolean getAll = (prefixes.size() == 0) @@ -492,18 +693,21 @@ public rcl_interfaces.msg.ListParametersResult listParameters( || ((entry.getKey().length() - entry.getKey().replace(separator, "").length()) < depth)); boolean prefixMatches = false; - for (String prefix : prefixes) { - if (entry.getKey() == prefix) { - prefixMatches = true; - break; - } else if (entry.getKey().startsWith(prefix + separator)) { - int length = prefix.length(); - String substr = entry.getKey().substring(length); - prefixMatches = (depth == rcl_interfaces.srv.ListParameters_Request.DEPTH_RECURSIVE) - || ((entry.getKey().length() - entry.getKey().replace(separator, "").length()) - < depth); + if (!getAll) { + for (String prefix : prefixes) { + if (entry.getKey() == prefix) { + prefixMatches = true; + break; + } else if (entry.getKey().startsWith(prefix + separator)) { + int length = prefix.length(); + String substr = entry.getKey().substring(length); + prefixMatches = (depth == rcl_interfaces.srv.ListParameters_Request.DEPTH_RECURSIVE) + || ((entry.getKey().length() - entry.getKey().replace(separator, "").length()) + < depth); + } } } + if (getAll || prefixMatches) { result.getNames().add(entry.getKey()); int lastSeparator = entry.getKey().lastIndexOf(separator); From 6157fb1bf36fe902c1cd8a318a0d77a6d7f65ebf Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 17:24:13 +0000 Subject: [PATCH 10/14] Add tests for Node parameters. Signed-off-by: Chris Lalancette --- rcljava/CMakeLists.txt | 2 + .../ros2/rcljava/node/NodeParametersTest.java | 405 ++++++++++++++++++ 2 files changed, 407 insertions(+) create mode 100644 rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index 95553286..d0ec05c9 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -225,6 +225,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/NodeParametersTest.java" "src/test/java/org/ros2/rcljava/node/NodeTest.java" "src/test/java/org/ros2/rcljava/parameters/AsyncParametersClientTest.java" "src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java" @@ -238,6 +239,7 @@ if(BUILD_TESTING) "org.ros2.rcljava.SpinTest" "org.ros2.rcljava.TimeTest" "org.ros2.rcljava.client.ClientTest" + "org.ros2.rcljava.node.NodeParametersTest" "org.ros2.rcljava.node.NodeTest" "org.ros2.rcljava.parameters.SyncParametersClientTest" "org.ros2.rcljava.publisher.PublisherTest" diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java new file mode 100644 index 00000000..b5caf4a7 --- /dev/null +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java @@ -0,0 +1,405 @@ +/* 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 static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.AfterClass; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Before; +import org.junit.Test; + +import org.ros2.rcljava.RCLJava; +import org.ros2.rcljava.parameters.ParameterCallback; +import org.ros2.rcljava.parameters.ParameterType; +import org.ros2.rcljava.parameters.ParameterVariant; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; + +public class NodeParametersTest { + private Node node; + private boolean callbackCalled = false; + + @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() { + node = RCLJava.createNode("test_node"); + } + + @After + public void tearDown() { + node.dispose(); + } + + @Test + public final void testDeclareParameter() { + ParameterVariant p = new ParameterVariant("foo", true); + ParameterVariant returnp = node.declareParameter(p); + assertTrue(node.hasParameter("foo")); + assertEquals("foo", returnp.getName()); + assertTrue(returnp.asBool()); + } + + @Test(expected = IllegalArgumentException.class) + public final void testDeclareParameterTwice() { + ParameterVariant p = new ParameterVariant("foo", true); + node.declareParameter(p); + node.declareParameter(p); + } + + @Test(expected = IllegalArgumentException.class) + public final void testDeclareCallbackRejected() { + class MyCB implements ParameterCallback { + public rcl_interfaces.msg.SetParametersResult callback(List parameters) { + return new rcl_interfaces.msg.SetParametersResult().setSuccessful(false); + } + } + + node.addOnSetParametersCallback(new MyCB()); + node.declareParameter(new ParameterVariant("foo.bar", true)); + } + + @Test + public final void testDeclareParameters() { + List plist = new ArrayList(); + List pdlist = new ArrayList(); + + plist.add(new ParameterVariant("foo", true)); + pdlist.add(new rcl_interfaces.msg.ParameterDescriptor()); + + node.declareParameters(plist, pdlist); + assertTrue(node.hasParameter("foo")); + } + + @Test(expected = IllegalArgumentException.class) + public final void testDeclareParametersBadListLength() { + List plist = new ArrayList(); + List pdlist = new ArrayList(); + + plist.add(new ParameterVariant("foo", true)); + + node.declareParameters(plist, pdlist); + } + + @Test + public final void testUndeclareParameter() { + ParameterVariant p = new ParameterVariant("foo", true); + node.declareParameter(p); + assertTrue(node.hasParameter("foo")); + node.undeclareParameter("foo"); + assertFalse(node.hasParameter("foo")); + } + + @Test(expected = IllegalArgumentException.class) + public final void testUndeclareParameterDoesNotExist() { + node.undeclareParameter("foo"); + } + + @Test + public final void testGetParameters() { + ParameterVariant p = new ParameterVariant("foo", true); + node.declareParameter(p); + assertTrue(node.hasParameter("foo")); + List results = node.getParameters(new ArrayList(Arrays.asList("foo"))); + assertEquals(1, results.size()); + assertEquals("foo", results.get(0).getName()); + assertTrue("foo", results.get(0).asBool()); + } + + @Test + public final void testGetParameter() { + ParameterVariant p = new ParameterVariant("foo", true); + node.declareParameter(p); + assertTrue(node.hasParameter("foo")); + ParameterVariant param = node.getParameter("foo"); + assertEquals("foo", param.getName()); + } + + @Test(expected = IllegalArgumentException.class) + public final void testGetParameterNotDeclared() { + ParameterVariant param = node.getParameter("foo"); + } + + @Test + public final void testGetParameterOr() { + ParameterVariant p = new ParameterVariant("foo", true); + node.declareParameter(p); + ParameterVariant param = node.getParameterOr("foo", new ParameterVariant("none", false)); + assertEquals("foo", param.getName()); + } + + @Test + public final void testGetParameterOrNotDeclared() { + ParameterVariant param = node.getParameterOr("foo", new ParameterVariant("none", false)); + assertEquals("none", param.getName()); + } + + @Test + public final void testGetParametersByPrefix() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + node.declareParameter(new ParameterVariant("foo.baz", true)); + node.declareParameter(new ParameterVariant("doh.baz", true)); + + Map params = node.getParametersByPrefix("foo"); + assertEquals(2, params.size()); + assertEquals("foo.bar", params.get("bar").getName()); + assertEquals("foo.baz", params.get("baz").getName()); + } + + @Test + public final void testSetParameter() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + assertTrue(node.getParameter("foo.bar").asBool()); + + rcl_interfaces.msg.SetParametersResult result = node.setParameter(new ParameterVariant("foo.bar", false)); + assertTrue(result.getSuccessful()); + assertFalse(node.getParameter("foo.bar").asBool()); + } + + @Test(expected = IllegalArgumentException.class) + public final void testSetParameterNotDeclared() { + node.setParameter(new ParameterVariant("foo.bar", false)); + } + + @Test + public final void testSetParameterNotSet() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + assertTrue(node.getParameter("foo.bar").asBool()); + + rcl_interfaces.msg.SetParametersResult result = node.setParameter(new ParameterVariant("foo.bar")); + assertTrue(result.getSuccessful()); + assertFalse(node.hasParameter("foo.bar")); + } + + @Test(expected = IllegalArgumentException.class) + public final void testSetParameterNotSetInvalidName() { + node.setParameter(new ParameterVariant("unset")); + } + + @Test + public final void testSetParameters() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + assertTrue(node.getParameter("foo.bar").asBool()); + + List params = new ArrayList(Arrays.asList( + new ParameterVariant("foo.bar", false) + )); + + List result = node.setParameters(params); + assertEquals(1, result.size()); + assertTrue(result.get(0).getSuccessful()); + assertFalse(node.getParameter("foo.bar").asBool()); + } + + @Test + public final void testSetParametersPartialUpdate() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + assertTrue(node.getParameter("foo.bar").asBool()); + + node.declareParameter(new ParameterVariant("foo.baz", true)); + assertTrue(node.getParameter("foo.baz").asBool()); + + List params = new ArrayList(Arrays.asList( + new ParameterVariant("foo.bar", false), + new ParameterVariant("unset", true), + new ParameterVariant("foo.baz", false) + )); + + try { + List result = node.setParameters(params); + } catch (IllegalArgumentException e) { + } + assertTrue(node.hasParameter("foo.bar")); + assertFalse(node.getParameter("foo.bar").asBool()); // Got updated + assertFalse(node.hasParameter("unset")); + assertTrue(node.hasParameter("foo.baz")); + assertTrue(node.getParameter("foo.baz").asBool()); // Didn't get updated because of previous failure + } + + @Test + public final void testSetParametersAtomicallyNoUpdate() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + assertTrue(node.getParameter("foo.bar").asBool()); + + node.declareParameter(new ParameterVariant("foo.baz", true)); + assertTrue(node.getParameter("foo.baz").asBool()); + + List params = new ArrayList(Arrays.asList( + new ParameterVariant("foo.bar", false), + new ParameterVariant("unset", true), + new ParameterVariant("foo.baz", false) + )); + + try { + node.setParametersAtomically(params); + } catch (IllegalArgumentException e) { + } + assertTrue(node.hasParameter("foo.bar")); + assertTrue(node.getParameter("foo.bar").asBool()); // Didn't get updated because of failure + assertFalse(node.hasParameter("unset")); + assertTrue(node.hasParameter("foo.baz")); + assertTrue(node.getParameter("foo.baz").asBool()); // Also didn't get updated because of failure + } + + @Test + public final void testDescribeParameter() { + node.declareParameter(new ParameterVariant("foo.bar", true), + new rcl_interfaces.msg.ParameterDescriptor().setName("foo.bar").setDescription("description")); + + rcl_interfaces.msg.ParameterDescriptor newdesc = node.describeParameter("foo.bar"); + assertEquals("foo.bar", newdesc.getName()); + assertEquals("description", newdesc.getDescription()); + } + + @Test(expected = IllegalArgumentException.class) + public final void testDescribeParameterNotDeclared() { + node.describeParameter("foo.bar"); + } + + @Test + public final void testDescribeParameters() { + node.declareParameter(new ParameterVariant("foo.bar", true), + new rcl_interfaces.msg.ParameterDescriptor().setName("foo.bar").setDescription("description")); + + node.declareParameter(new ParameterVariant("foo.baz", true), + new rcl_interfaces.msg.ParameterDescriptor().setName("foo.baz").setDescription("description2")); + + List descs = node.describeParameters(new ArrayList(Arrays.asList("foo.bar", "foo.baz"))); + assertEquals(2, descs.size()); + assertEquals("foo.bar", descs.get(0).getName()); + assertEquals("description", descs.get(0).getDescription()); + assertEquals("foo.baz", descs.get(1).getName()); + assertEquals("description2", descs.get(1).getDescription()); + } + + @Test + public final void testGetParameterTypes() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + List types = node.getParameterTypes(new ArrayList(Arrays.asList("foo.bar"))); + assertEquals(1, types.size()); + assertEquals(ParameterType.PARAMETER_BOOL, types.get(0)); + } + + @Test(expected = IllegalArgumentException.class) + public final void testGetParameterTypesUndeclared() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + node.getParameterTypes(new ArrayList(Arrays.asList("foo.bar", "unset"))); + } + + @Test + public final void testListParameters() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + node.declareParameter(new ParameterVariant("thing.do", true)); + + rcl_interfaces.msg.ListParametersResult result = node.listParameters(new ArrayList(), rcl_interfaces.srv.ListParameters_Request.DEPTH_RECURSIVE); + assertEquals(2, result.getNames().size()); + } + + @Test + public final void testParameterCallback() { + this.callbackCalled = false; + + class MyCB implements ParameterCallback { + private NodeParametersTest npt; + public MyCB(NodeParametersTest npt) { + this.npt = npt; + } + public rcl_interfaces.msg.SetParametersResult callback(List parameters) { + rcl_interfaces.msg.SetParametersResult result = new rcl_interfaces.msg.SetParametersResult(); + result.setSuccessful(true); + this.npt.callbackCalled = true; + return result; + } + } + node.addOnSetParametersCallback(new MyCB(this)); + + node.declareParameter(new ParameterVariant("foo.bar", true)); + rcl_interfaces.msg.SetParametersResult result = node.setParameter(new ParameterVariant("foo.bar", false)); + assertTrue(result.getSuccessful()); + assertTrue(this.callbackCalled); + assertFalse(node.getParameter("foo.bar").asBool()); + } + + @Test + public final void testParameterCallbackRejected() { + this.callbackCalled = false; + + class MyCB implements ParameterCallback { + private NodeParametersTest npt; + public MyCB(NodeParametersTest npt) { + this.npt = npt; + } + public rcl_interfaces.msg.SetParametersResult callback(List parameters) { + rcl_interfaces.msg.SetParametersResult result = new rcl_interfaces.msg.SetParametersResult(); + result.setSuccessful(false); + this.npt.callbackCalled = true; + return result; + } + } + + node.declareParameter(new ParameterVariant("foo.bar", true)); + node.addOnSetParametersCallback(new MyCB(this)); + rcl_interfaces.msg.SetParametersResult result = node.setParameter(new ParameterVariant("foo.bar", false)); + assertFalse(result.getSuccessful()); + assertTrue(this.callbackCalled); + assertTrue(node.getParameter("foo.bar").asBool()); + } + + @Test + public final void testParameterRemoveCallback() { + this.callbackCalled = false; + + class MyCB implements ParameterCallback { + private NodeParametersTest npt; + public MyCB(NodeParametersTest npt) { + this.npt = npt; + } + public rcl_interfaces.msg.SetParametersResult callback(List parameters) { + rcl_interfaces.msg.SetParametersResult result = new rcl_interfaces.msg.SetParametersResult(); + result.setSuccessful(true); + this.npt.callbackCalled = true; + return result; + } + } + + node.declareParameter(new ParameterVariant("foo.bar", true)); + MyCB cb = new MyCB(this); + node.addOnSetParametersCallback(cb); + node.removeOnSetParametersCallback(cb); + rcl_interfaces.msg.SetParametersResult result = node.setParameter(new ParameterVariant("foo.bar", false)); + assertTrue(result.getSuccessful()); + assertFalse(this.callbackCalled); + assertFalse(node.getParameter("foo.bar").asBool()); + } +} From 7b0bd43a61d997179a2be6b5ec6686ae294a644e Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Wed, 22 Jul 2020 17:25:19 +0000 Subject: [PATCH 11/14] Add node parameters test with undeclared parameters. Signed-off-by: Chris Lalancette --- rcljava/CMakeLists.txt | 2 + .../node/NodeUndeclaredParametersTest.java | 98 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index d0ec05c9..f0903726 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -226,6 +226,7 @@ if(BUILD_TESTING) "src/test/java/org/ros2/rcljava/TimeTest.java" "src/test/java/org/ros2/rcljava/client/ClientTest.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" "src/test/java/org/ros2/rcljava/parameters/AsyncParametersClientTest.java" "src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java" @@ -240,6 +241,7 @@ if(BUILD_TESTING) "org.ros2.rcljava.TimeTest" "org.ros2.rcljava.client.ClientTest" "org.ros2.rcljava.node.NodeParametersTest" + "org.ros2.rcljava.node.NodeUndeclaredParametersTest" "org.ros2.rcljava.node.NodeTest" "org.ros2.rcljava.parameters.SyncParametersClientTest" "org.ros2.rcljava.publisher.PublisherTest" diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java new file mode 100644 index 00000000..d4004873 --- /dev/null +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java @@ -0,0 +1,98 @@ +/* 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 static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.AfterClass; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Before; +import org.junit.Test; + +import org.ros2.rcljava.RCLJava; +import org.ros2.rcljava.parameters.ParameterCallback; +import org.ros2.rcljava.parameters.ParameterType; +import org.ros2.rcljava.parameters.ParameterVariant; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Set; + +public class NodeUndeclaredParametersTest { + private Node node; + + @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() { + node = RCLJava.createNode("test_node", "", RCLJava.getDefaultContext(), true); + } + + @After + public void tearDown() { + node.dispose(); + } + + @Test + public final void testGetParameterNotDeclared() { + ParameterVariant param = node.getParameter("foo"); + assertEquals("foo", param.getName()); + } + + @Test + public final void testSetParametersAtomicallyNotDeclared() { + List params = new ArrayList(Arrays.asList( + new ParameterVariant("foo.bar", false), + new ParameterVariant("foo.baz", true) + )); + + node.setParametersAtomically(params); + assertTrue(node.hasParameter("foo.bar")); + assertFalse(node.getParameter("foo.bar").asBool()); + assertTrue(node.hasParameter("foo.baz")); + assertTrue(node.getParameter("foo.baz").asBool()); + } + + @Test + public final void testDescribeParameterNotDeclared() { + rcl_interfaces.msg.ParameterDescriptor newdesc = node.describeParameter("foo.bar"); + assertEquals("foo.bar", newdesc.getName()); + } + + @Test + public final void testGetParameterTypesNotDeclared() { + node.declareParameter(new ParameterVariant("foo.bar", true)); + List types = node.getParameterTypes(new ArrayList(Arrays.asList("foo.bar", "unset"))); + assertEquals(2, types.size()); + assertEquals(ParameterType.PARAMETER_BOOL, types.get(0)); + assertEquals(ParameterType.PARAMETER_NOT_SET, types.get(1)); + } +} From 086e700298866ebfeae574ffc59da9a5febb176a Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 12 Nov 2021 13:53:02 -0300 Subject: [PATCH 12/14] Fix rebasing error Signed-off-by: Ivan Santiago Paunovic --- rcljava/CMakeLists.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index f0903726..7dd71e37 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -228,8 +228,8 @@ if(BUILD_TESTING) "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" - "src/test/java/org/ros2/rcljava/parameters/AsyncParametersClientTest.java" - "src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java" + # "src/test/java/org/ros2/rcljava/parameters/AsyncParametersClientTest.java" + # "src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java" "src/test/java/org/ros2/rcljava/publisher/PublisherTest.java" "src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java" "src/test/java/org/ros2/rcljava/timer/TimerTest.java" @@ -243,7 +243,7 @@ if(BUILD_TESTING) "org.ros2.rcljava.node.NodeParametersTest" "org.ros2.rcljava.node.NodeUndeclaredParametersTest" "org.ros2.rcljava.node.NodeTest" - "org.ros2.rcljava.parameters.SyncParametersClientTest" + # "org.ros2.rcljava.parameters.SyncParametersClientTest" "org.ros2.rcljava.publisher.PublisherTest" "org.ros2.rcljava.subscription.SubscriptionTest" "org.ros2.rcljava.timer.TimerTest" From e0ac90831c0fc8c058cecb598724449d9b90211c Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 12 Nov 2021 15:02:16 -0300 Subject: [PATCH 13/14] configure logger in an android compatible way Signed-off-by: Ivan Santiago Paunovic --- .../src/test/java/org/ros2/rcljava/RCLJavaTest.java | 13 ++++++++++++- .../src/test/java/org/ros2/rcljava/SpinTest.java | 13 ++++++++++++- .../src/test/java/org/ros2/rcljava/TimeTest.java | 13 ++++++++++++- .../org/ros2/rcljava/node/NodeParametersTest.java | 13 ++++++++++++- .../test/java/org/ros2/rcljava/node/NodeTest.java | 13 ++++++++++++- .../rcljava/node/NodeUndeclaredParametersTest.java | 13 ++++++++++++- .../org/ros2/rcljava/publisher/PublisherTest.java | 13 ++++++++++++- .../ros2/rcljava/subscription/SubscriptionTest.java | 13 ++++++++++++- .../test/java/org/ros2/rcljava/timer/TimerTest.java | 13 ++++++++++++- 9 files changed, 108 insertions(+), 9 deletions(-) diff --git a/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java b/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java index 16a00dfe..ff98f632 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java @@ -25,7 +25,18 @@ public class RCLJavaTest { @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(); + } } @Test diff --git a/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java b/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java index f015d260..db2026eb 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java @@ -59,7 +59,18 @@ public int getCounter() { @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(); } diff --git a/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java b/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java index 5e8abf32..95940f12 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java @@ -30,7 +30,18 @@ public class TimeTest { @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(); + } } // @Test diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java index b5caf4a7..4564b65a 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java @@ -42,7 +42,18 @@ public class NodeParametersTest { @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(); } diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java index 6c5e45be..8df0cb7f 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeTest.java @@ -87,7 +87,18 @@ boolean checkPrimitives(rcljava.msg.Primitives primitives, boolean booleanValue, @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(); } 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 d4004873..c2e20f6a 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java @@ -41,7 +41,18 @@ public class NodeUndeclaredParametersTest { @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(); } diff --git a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java index 649c4aa8..0a5eb6e6 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java @@ -28,7 +28,18 @@ public class PublisherTest { @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(); + } } @Test diff --git a/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java b/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java index 6b01a634..0ca25c49 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java @@ -29,7 +29,18 @@ public class SubscriptionTest { @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(); + } } @Test diff --git a/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java b/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java index 2500f5cf..25140707 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java @@ -59,7 +59,18 @@ public int getCounter() { @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(); + } } @Test From a1d892c679a04e0e826f5689f89303166d283933 Mon Sep 17 00:00:00 2001 From: Ivan Santiago Paunovic Date: Fri, 12 Nov 2021 15:04:50 -0300 Subject: [PATCH 14/14] configure logger in an android compatible way Signed-off-by: Ivan Santiago Paunovic --- rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java | 2 ++ rcljava/src/test/java/org/ros2/rcljava/SpinTest.java | 1 + rcljava/src/test/java/org/ros2/rcljava/TimeTest.java | 1 + .../src/test/java/org/ros2/rcljava/node/NodeParametersTest.java | 1 + .../org/ros2/rcljava/node/NodeUndeclaredParametersTest.java | 1 + .../src/test/java/org/ros2/rcljava/publisher/PublisherTest.java | 2 ++ .../java/org/ros2/rcljava/subscription/SubscriptionTest.java | 2 ++ rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java | 1 + 8 files changed, 11 insertions(+) diff --git a/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java b/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java index ff98f632..a8dc2de3 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/RCLJavaTest.java @@ -18,6 +18,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertFalse; +import java.lang.reflect.Method; + import org.junit.BeforeClass; import org.junit.Test; diff --git a/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java b/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java index db2026eb..e8d97644 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/SpinTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import java.lang.reflect.Method; import java.util.concurrent.TimeUnit; import org.junit.AfterClass; diff --git a/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java b/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java index 95940f12..9bd0e01c 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/TimeTest.java @@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import java.lang.reflect.Method; import java.util.concurrent.TimeUnit; import org.junit.BeforeClass; diff --git a/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java b/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java index 4564b65a..718556b1 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java @@ -30,6 +30,7 @@ import org.ros2.rcljava.parameters.ParameterType; import org.ros2.rcljava.parameters.ParameterVariant; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.List; 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 c2e20f6a..9d9f3991 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeUndeclaredParametersTest.java @@ -30,6 +30,7 @@ import org.ros2.rcljava.parameters.ParameterType; import org.ros2.rcljava.parameters.ParameterVariant; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.List; diff --git a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java index 0a5eb6e6..336c6204 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/publisher/PublisherTest.java @@ -18,6 +18,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import java.lang.reflect.Method; + import org.junit.BeforeClass; import org.junit.Test; diff --git a/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java b/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java index 0ca25c49..61e9c448 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/subscription/SubscriptionTest.java @@ -18,6 +18,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; +import java.lang.reflect.Method; + import org.junit.BeforeClass; import org.junit.Test; diff --git a/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java b/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java index 25140707..9eff6b17 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/timer/TimerTest.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertFalse; import java.lang.ref.WeakReference; +import java.lang.reflect.Method; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit;