diff --git a/rcljava/CMakeLists.txt b/rcljava/CMakeLists.txt index 7dd71e37..7f667541 100644 --- a/rcljava/CMakeLists.txt +++ b/rcljava/CMakeLists.txt @@ -138,8 +138,12 @@ 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/InvalidParametersException.java" + "src/main/java/org/ros2/rcljava/parameters/InvalidParameterValueException.java" + "src/main/java/org/ros2/rcljava/parameters/ParameterAlreadyDeclaredException.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/ParameterNotDeclaredException.java" "src/main/java/org/ros2/rcljava/parameters/ParameterType.java" "src/main/java/org/ros2/rcljava/parameters/ParameterVariant.java" "src/main/java/org/ros2/rcljava/parameters/service/ParameterService.java" 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 4f276c82..e5d875cb 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/Node.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/Node.java @@ -177,13 +177,6 @@ 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. * @@ -208,7 +201,7 @@ Client createClient(final Class serviceType, * 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. + * set then an InvalidParameterValueException is thrown. * * The returned reference will remain valid until the parameter is * undeclared. @@ -216,10 +209,9 @@ Client createClient(final Class serviceType, * @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. + * @throws InvalidParametersException if the parameter name is invalid. + * @throws ParameterAlreadyDeclaredException if parameter has already been declared. + * @throws InvalidParameterValueException if initial value fails to be set. */ ParameterVariant declareParameter(ParameterVariant parameter, rcl_interfaces.msg.ParameterDescriptor descriptor); @@ -235,15 +227,17 @@ Client createClient(final Class serviceType, * 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. + * set then an InvalidParameterValueException 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 + * @throws InvalidParametersException if any of the parameter names is invalid. + * @throws ParameterAlreadyDeclaredException if any parameter in the list has already * been declared. + * @throws InvalidParameterValueException if any of the initial values fail to be set. */ List declareParameters(List parameters, List descriptors); @@ -254,7 +248,7 @@ Client createClient(final Class serviceType, * addOnSetParametersCallback to be called. * * @param name The name of the parameter to be undeclared. - * @throws IllegalArgumentException if the parameter + * @throws ParameterNotDeclaredException if the parameter * has not been declared. */ void undeclareParameter(String name); @@ -270,17 +264,17 @@ Client createClient(final Class serviceType, /** * Return a list of parameters by the given list of parameter names. * - * This method will throw an IllegalArgumentException if any of the requested + * This method will throw a ParameterNotDeclaredException 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 + * declared, then the returned ParameterVariant 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 + * @throws ParameterNotDeclaredException if any of the parameters have not been * declared and undeclared parameters are not allowed. */ List getParameters(List names); @@ -292,7 +286,7 @@ Client createClient(final Class serviceType, * * @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 + * @throws ParameterNotDeclaredException if the parameter has not been declared * and undeclared parameters are not allowed. */ ParameterVariant getParameter(String name); @@ -300,7 +294,7 @@ Client createClient(final Class serviceType, /** * Return the named parameter value, or the "alternativeValue" if not set. * - * This method will not throw IllegalArgumentException if a parameter is + * This method will not throw ParameterNotDeclaredException if a parameter is * not declared. * Instead, it will return the alternativeValue. * @@ -336,7 +330,7 @@ Client createClient(final Class serviceType, * 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. + * allowed, this function will throw a ParameterNotDeclaredException. * * If undeclared parameters are allowed, then the parameter is implicitly * declared with the default parameter meta data before being set. @@ -354,7 +348,8 @@ Client createClient(final Class serviceType, * * @param parameter The parameter to be set. * @return The result of the set action. - * @throws IllegalArgumentException if the parameter + * @throws InvalidParametersException if parameter name is invalid. + * @throws ParameterNotDeclaredException if the parameter * has not been declared and undeclared parameters are not allowed. */ rcl_interfaces.msg.SetParametersResult setParameter(ParameterVariant parameter); @@ -367,7 +362,7 @@ Client createClient(final Class serviceType, * * 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. + * this method will throw a ParameterNotDeclaredException. * * 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 @@ -389,7 +384,8 @@ Client createClient(final Class serviceType, * * @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 + * @throws InvalidParametersException if any parameter name is invalid. + * @throws ParameterNotDeclaredException if any parameter has not been declared * and undeclared parameters are not allowed. */ List setParameters(List parameters); @@ -400,8 +396,8 @@ Client createClient(final Class serviceType, * 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 + * Like setParameter and setParameters, this method will throw a + * ParameterNotDeclaredException 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. * @@ -419,7 +415,8 @@ Client createClient(final Class serviceType, * * @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 + * @throws InvalidParametersException if any parameter name is invalid. + * @throws ParameterNotDeclaredException if any parameter has not been declared * and undeclared parameters are not allowed. */ rcl_interfaces.msg.SetParametersResult setParametersAtomically(List parameters); @@ -467,7 +464,7 @@ Client createClient(final Class serviceType, /** * Return the parameter descriptor for the given parameter name. * - * This method will throw an IllegalArgumentException if the requested + * This method will throw an ParameterNotDeclaredException if the requested * parameter has not been declared and undeclared parameters are not allowed. * * If undeclared parameters are allowed, then a default initialized @@ -475,7 +472,7 @@ Client createClient(final Class serviceType, * * @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 + * @throws ParameterNotDeclaredException if the parameter has not been declared * and undeclared parameters are not allowed. */ rcl_interfaces.msg.ParameterDescriptor describeParameter(String name); @@ -483,7 +480,7 @@ Client createClient(final Class serviceType, /** * Return a List of parameter descriptors, one for each of the given names. * - * This method will throw an IllegalArgumentException if any of the requested + * This method will throw an ParameterNotDeclaredException 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 @@ -493,7 +490,7 @@ Client createClient(final Class serviceType, * * @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 + * @throws ParameterNotDeclaredException if any of the parameters have not been * declared and undeclared parameters are not allowed. */ List describeParameters(List names); @@ -501,15 +498,15 @@ Client createClient(final Class serviceType, /** * Return a list of parameter types, one for each of the given names. * - * This method will throw an IllegalArgumentException if any of the requested + * This method will throw an ParameterNotDeclaredException 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. + * 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 + * @throws ParameterNotDeclaredException if any of the parameters have not been * declared and undeclared parameters are not allowed. */ List getParameterTypes(List names); 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 dffbb707..36affa63 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -27,7 +27,11 @@ import org.ros2.rcljava.interfaces.Disposable; import org.ros2.rcljava.interfaces.MessageDefinition; import org.ros2.rcljava.interfaces.ServiceDefinition; +import org.ros2.rcljava.parameters.InvalidParametersException; +import org.ros2.rcljava.parameters.InvalidParameterValueException; +import org.ros2.rcljava.parameters.ParameterAlreadyDeclaredException; import org.ros2.rcljava.parameters.ParameterCallback; +import org.ros2.rcljava.parameters.ParameterNotDeclaredException; import org.ros2.rcljava.parameters.ParameterType; import org.ros2.rcljava.parameters.ParameterVariant; import org.ros2.rcljava.publisher.Publisher; @@ -429,11 +433,17 @@ public List declareParameters(List parameter throw new IllegalArgumentException("Parameters length must be equal to descriptors length"); } + for (ParameterVariant parameter : parameters) { + if (parameter.getName().length() == 0) { + throw new InvalidParametersException("Parameter name must not be empty"); + } + } + 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"); + throw new InvalidParameterValueException(String.format("Parameters were rejected by callback: %s", result.getReason())); } } } @@ -447,7 +457,7 @@ public List declareParameters(List parameter 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())); + throw new ParameterAlreadyDeclaredException(String.format("Parameter '%s' is already declared", parameter.getName())); } ParameterAndDescriptor pandd = new ParameterAndDescriptor(); @@ -465,7 +475,7 @@ public List declareParameters(List parameter public void undeclareParameter(String name) { synchronized (parametersMutex) { if (!this.parameters.containsKey(name)) { - throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", name)); + throw new ParameterNotDeclaredException(String.format("Parameter '%s' is not declared", name)); } this.parameters.remove(name); @@ -488,7 +498,7 @@ public List getParameters(List names) { } else if (this.allowUndeclaredParameters) { results.add(new ParameterVariant(name)); } else { - throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", name)); + throw new ParameterNotDeclaredException(String.format("Parameter '%s' is not declared", name)); } } } @@ -534,6 +544,12 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically( List parameters) { rcl_interfaces.msg.SetParametersResult result = new rcl_interfaces.msg.SetParametersResult(); + for (ParameterVariant parameter : parameters) { + if (parameter.getName().length() == 0) { + throw new InvalidParametersException("Parameter name must not be empty"); + } + } + synchronized (parameterCallbacksMutex) { for (ParameterCallback cb : this.parameterCallbacks) { result = cb.callback(parameters); @@ -561,7 +577,7 @@ private rcl_interfaces.msg.SetParametersResult internalSetParametersAtomically( if (this.allowUndeclaredParameters) { parametersToDeclare.add(parameter.getName()); } else { - throw new IllegalArgumentException(String.format("Parameter '%s' is not declared", parameter.getName())); + throw new ParameterNotDeclaredException(String.format("Parameter '%s' is not declared", parameter.getName())); } } } @@ -654,7 +670,7 @@ public List describeParameters( } 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)); + throw new ParameterNotDeclaredException(String.format("Parameter '%s' is not declared", name)); } } } @@ -672,7 +688,7 @@ public List getParameterTypes(List names) { } 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)); + throw new ParameterNotDeclaredException(String.format("Parameter '%s' is not declared", name)); } } } diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/InvalidParameterValueException.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/InvalidParameterValueException.java new file mode 100644 index 00000000..fe65b212 --- /dev/null +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/InvalidParameterValueException.java @@ -0,0 +1,22 @@ +/* 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; + +public class InvalidParameterValueException extends RuntimeException { + public InvalidParameterValueException(String message) { + super(message); + } +} diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/InvalidParametersException.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/InvalidParametersException.java new file mode 100644 index 00000000..95f2a577 --- /dev/null +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/InvalidParametersException.java @@ -0,0 +1,22 @@ +/* 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; + +public class InvalidParametersException extends RuntimeException { + public InvalidParametersException(String message) { + super(message); + } +} diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterAlreadyDeclaredException.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterAlreadyDeclaredException.java new file mode 100644 index 00000000..a47846a4 --- /dev/null +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterAlreadyDeclaredException.java @@ -0,0 +1,22 @@ +/* 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; + +public class ParameterAlreadyDeclaredException extends RuntimeException { + public ParameterAlreadyDeclaredException(String message) { + super(message); + } +} diff --git a/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterNotDeclaredException.java b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterNotDeclaredException.java new file mode 100644 index 00000000..a4ece64a --- /dev/null +++ b/rcljava/src/main/java/org/ros2/rcljava/parameters/ParameterNotDeclaredException.java @@ -0,0 +1,22 @@ +/* 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; + +public class ParameterNotDeclaredException extends RuntimeException { + public ParameterNotDeclaredException(String message) { + super(message); + } +} 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 718556b1..72af9f8d 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/node/NodeParametersTest.java @@ -26,7 +26,11 @@ import org.junit.Test; import org.ros2.rcljava.RCLJava; +import org.ros2.rcljava.parameters.InvalidParametersException; +import org.ros2.rcljava.parameters.InvalidParameterValueException; +import org.ros2.rcljava.parameters.ParameterAlreadyDeclaredException; import org.ros2.rcljava.parameters.ParameterCallback; +import org.ros2.rcljava.parameters.ParameterNotDeclaredException; import org.ros2.rcljava.parameters.ParameterType; import org.ros2.rcljava.parameters.ParameterVariant; @@ -83,18 +87,18 @@ public final void testDeclareParameter() { assertTrue(returnp.asBool()); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = ParameterAlreadyDeclaredException.class) public final void testDeclareParameterTwice() { ParameterVariant p = new ParameterVariant("foo", true); node.declareParameter(p); node.declareParameter(p); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = InvalidParameterValueException.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); + return new rcl_interfaces.msg.SetParametersResult().setSuccessful(false).setReason("reason"); } } @@ -102,6 +106,11 @@ public rcl_interfaces.msg.SetParametersResult callback(List pa node.declareParameter(new ParameterVariant("foo.bar", true)); } + @Test(expected = InvalidParametersException.class) + public final void testDeclareParameterInvalidName() { + node.declareParameter(new ParameterVariant()); + } + @Test public final void testDeclareParameters() { List plist = new ArrayList(); @@ -133,7 +142,7 @@ public final void testUndeclareParameter() { assertFalse(node.hasParameter("foo")); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = ParameterNotDeclaredException.class) public final void testUndeclareParameterDoesNotExist() { node.undeclareParameter("foo"); } @@ -158,7 +167,7 @@ public final void testGetParameter() { assertEquals("foo", param.getName()); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = ParameterNotDeclaredException.class) public final void testGetParameterNotDeclared() { ParameterVariant param = node.getParameter("foo"); } @@ -199,7 +208,7 @@ public final void testSetParameter() { assertFalse(node.getParameter("foo.bar").asBool()); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = ParameterNotDeclaredException.class) public final void testSetParameterNotDeclared() { node.setParameter(new ParameterVariant("foo.bar", false)); } @@ -214,7 +223,12 @@ public final void testSetParameterNotSet() { assertFalse(node.hasParameter("foo.bar")); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = InvalidParametersException.class) + public final void testSetParameterInvalidName() { + node.setParameter(new ParameterVariant()); + } + + @Test(expected = ParameterNotDeclaredException.class) public final void testSetParameterNotSetInvalidName() { node.setParameter(new ParameterVariant("unset")); } @@ -250,7 +264,7 @@ public final void testSetParametersPartialUpdate() { try { List result = node.setParameters(params); - } catch (IllegalArgumentException e) { + } catch (ParameterNotDeclaredException e) { } assertTrue(node.hasParameter("foo.bar")); assertFalse(node.getParameter("foo.bar").asBool()); // Got updated @@ -275,7 +289,7 @@ public final void testSetParametersAtomicallyNoUpdate() { try { node.setParametersAtomically(params); - } catch (IllegalArgumentException e) { + } catch (ParameterNotDeclaredException e) { } assertTrue(node.hasParameter("foo.bar")); assertTrue(node.getParameter("foo.bar").asBool()); // Didn't get updated because of failure @@ -294,7 +308,7 @@ public final void testDescribeParameter() { assertEquals("description", newdesc.getDescription()); } - @Test(expected = IllegalArgumentException.class) + @Test(expected = ParameterNotDeclaredException.class) public final void testDescribeParameterNotDeclared() { node.describeParameter("foo.bar"); } @@ -323,8 +337,8 @@ public final void testGetParameterTypes() { assertEquals(ParameterType.PARAMETER_BOOL, types.get(0)); } - @Test(expected = IllegalArgumentException.class) - public final void testGetParameterTypesUndeclared() { + @Test(expected = ParameterNotDeclaredException.class) + public final void testGetParameterTypesNotDeclared() { node.declareParameter(new ParameterVariant("foo.bar", true)); node.getParameterTypes(new ArrayList(Arrays.asList("foo.bar", "unset"))); }