diff --git a/rcljava/src/main/java/org/ros2/rcljava/action/ActionServerImpl.java b/rcljava/src/main/java/org/ros2/rcljava/action/ActionServerImpl.java index 31369ede..ed379c6b 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/action/ActionServerImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/action/ActionServerImpl.java @@ -176,9 +176,9 @@ public synchronized void publishFeedback(FeedbackDefinition feedback) { ActionServerImpl.this.actionTypeInstance.getFeedbackMessageType(); FeedbackMessageDefinition feedbackMessage = null; try { - feedbackMessage = feedbackMessageType.newInstance(); - } catch (Exception ex) { - throw new IllegalArgumentException("Failed to instantiate feedback message", ex); + feedbackMessage = feedbackMessageType.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException ex) { + throw new IllegalArgumentException("Failed to instantiate feedback message: ", ex); } feedbackMessage.setFeedback(feedback); feedbackMessage.setGoalUuid(this.goalInfo.getGoalId().getUuidAsList()); @@ -202,7 +202,7 @@ public final long getHandle() { private synchronized final void toTerminalState(byte status, ResultDefinition result) { nativeUpdateGoalState(this.handle, status); - ResultResponseDefinition resultResponse = ActionServerImpl.this.createResultResponse(); + ResultResponseDefinition resultResponse = ActionServerImpl.this.createResultResponse(); resultResponse.setGoalStatus(status); resultResponse.setResult(result); ActionServerImpl.this.sendResult(goalInfo.getGoalId().getUuidAsList(), resultResponse); @@ -265,9 +265,9 @@ public ActionServerImpl( final Consumer> acceptedCallback) throws IllegalArgumentException { this.nodeReference = nodeReference; try { - this.actionTypeInstance = actionType.newInstance(); - } catch (Exception ex) { - throw new IllegalArgumentException("Failed to instantiate provided action type", ex); + this.actionTypeInstance = actionType.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException ex) { + throw new IllegalArgumentException("Failed to instantiate provided action type: ", ex); } this.actionName = actionName; this.goalCallback = goalCallback; @@ -545,9 +545,10 @@ private action_msgs.msg.GoalInfo createGoalInfo(List goalUuid) { private ResultResponseDefinition createResultResponse() { ResultResponseDefinition resultResponse; try { - resultResponse = this.actionTypeInstance.getGetResultResponseType().newInstance(); - } catch (Exception ex) { - throw new IllegalArgumentException("Failed to instantiate provided action type", ex); + resultResponse = + this.actionTypeInstance.getGetResultResponseType().getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException ex) { + throw new IllegalStateException("Failed to instantiate provided action type: ", ex); } return resultResponse; } @@ -582,12 +583,10 @@ public void execute() { GoalResponseDefinition responseMessage = null; try { - requestMessage = requestType.newInstance(); - responseMessage = responseType.newInstance(); - } catch (InstantiationException ie) { - ie.printStackTrace(); - } catch (IllegalAccessException iae) { - iae.printStackTrace(); + requestMessage = requestType.getDeclaredConstructor().newInstance(); + responseMessage = responseType.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException ex) { + throw new IllegalStateException("Failed to instantiate request or responce: ", ex); } if (requestMessage != null && responseMessage != null) { @@ -648,9 +647,9 @@ public void execute() { ResultRequestDefinition requestMessage = null; try { - requestMessage = requestType.newInstance(); - } catch (Exception ex) { - throw new IllegalArgumentException("Failed to instantiate action result request", ex); + requestMessage = requestType.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException ex) { + throw new IllegalArgumentException("Failed to instantiate action result request: ", ex); } if (requestMessage != null) { diff --git a/rcljava/src/main/java/org/ros2/rcljava/executors/BaseExecutor.java b/rcljava/src/main/java/org/ros2/rcljava/executors/BaseExecutor.java index f6ed8c02..bd86e63a 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/executors/BaseExecutor.java +++ b/rcljava/src/main/java/org/ros2/rcljava/executors/BaseExecutor.java @@ -15,17 +15,18 @@ package org.ros2.rcljava.executors; +import java.lang.SuppressWarnings; import java.util.AbstractMap; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; - import java.util.Collection; import java.util.concurrent.BlockingQueue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.LinkedBlockingQueue; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,6 +82,23 @@ protected void removeNode(ComposableNode node) { this.nodes.remove(node); } + @SuppressWarnings("unchecked") + protected static void executeSubscriptionCallbackUnchecked( + Subscription subscription, + MessageDefinition message) + { + subscription.executeCallback(message); + } + + @SuppressWarnings("unchecked") + protected static void clientHandleResponseUnchecked( + Client client, + RMWRequestId rmwRequestId, + MessageDefinition response) + { + client.handleResponse(rmwRequestId, response); + } + protected void executeAnyExecutable(AnyExecutable anyExecutable) { if (anyExecutable.timer != null) { anyExecutable.timer.callTimer(); @@ -92,25 +110,25 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) { MessageDefinition message = nativeTake( anyExecutable.subscription.getHandle(), anyExecutable.subscription.getMessageType()); if (message != null) { - anyExecutable.subscription.executeCallback(message); + // Safety: nativeTake() will return the correct type here. + // We can't do much better here, as subscriptions are type erased. + executeSubscriptionCallbackUnchecked(anyExecutable.subscription, message); } subscriptionHandles.remove(anyExecutable.subscription.getHandle()); } if (anyExecutable.service != null) { - Class requestType = anyExecutable.service.getRequestType(); - Class responseType = anyExecutable.service.getResponseType(); + Class requestType = anyExecutable.service.getRequestType(); + Class responseType = anyExecutable.service.getResponseType(); MessageDefinition requestMessage = null; MessageDefinition responseMessage = null; try { - requestMessage = requestType.newInstance(); - responseMessage = responseType.newInstance(); - } catch (InstantiationException ie) { - ie.printStackTrace(); - } catch (IllegalAccessException iae) { - iae.printStackTrace(); + requestMessage = requestType.getDeclaredConstructor().newInstance(); + responseMessage = responseType.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException("Failed to instantiate service request/response"); } if (requestMessage != null && responseMessage != null) { @@ -134,24 +152,18 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) { } if (anyExecutable.client != null) { - Class requestType = anyExecutable.client.getRequestType(); - Class responseType = anyExecutable.client.getResponseType(); + Class requestType = anyExecutable.client.getRequestType(); + Class responseType = anyExecutable.client.getResponseType(); - MessageDefinition requestMessage = null; MessageDefinition responseMessage = null; try { - requestMessage = requestType.newInstance(); - responseMessage = responseType.newInstance(); - } catch (InstantiationException ie) { - ie.printStackTrace(); - } catch (IllegalAccessException iae) { - iae.printStackTrace(); + responseMessage = responseType.getDeclaredConstructor().newInstance(); + } catch (ReflectiveOperationException ie) { + throw new IllegalStateException("Failed to instantiate service response"); } - if (requestMessage != null && responseMessage != null) { - long requestFromJavaConverterHandle = requestMessage.getFromJavaConverterInstance(); - long requestToJavaConverterHandle = requestMessage.getToJavaConverterInstance(); + if (responseMessage != null) { long responseFromJavaConverterHandle = responseMessage.getFromJavaConverterInstance(); long responseToJavaConverterHandle = responseMessage.getToJavaConverterInstance(); long responseDestructorHandle = responseMessage.getDestructorInstance(); @@ -161,7 +173,9 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) { responseToJavaConverterHandle, responseDestructorHandle, responseMessage); if (rmwRequestId != null) { - anyExecutable.client.handleResponse(rmwRequestId, responseMessage); + // Safety: nativeTakeResponse() will return the correct type here. + // We can't do much better here, as subscriptions are type erased. + clientHandleResponseUnchecked(anyExecutable.client, rmwRequestId, responseMessage); } } clientHandles.remove(anyExecutable.client.getHandle()); @@ -187,7 +201,7 @@ protected void waitForWork(long timeout) { this.actionServerHandles.clear(); for (ComposableNode node : this.nodes) { - for (Subscription subscription : node.getNode().getSubscriptions()) { + for (Subscription subscription : node.getNode().getSubscriptions()) { this.subscriptionHandles.add(new AbstractMap.SimpleEntry( subscription.getHandle(), subscription)); Collection eventHandlers = subscription.getEventHandlers(); @@ -209,17 +223,17 @@ protected void waitForWork(long timeout) { this.timerHandles.add(new AbstractMap.SimpleEntry(timer.getHandle(), timer)); } - for (Service service : node.getNode().getServices()) { + for (Service service : node.getNode().getServices()) { this.serviceHandles.add( new AbstractMap.SimpleEntry(service.getHandle(), service)); } - for (Client client : node.getNode().getClients()) { + for (Client client : node.getNode().getClients()) { this.clientHandles.add( new AbstractMap.SimpleEntry(client.getHandle(), client)); } - for (ActionServer actionServer : node.getNode().getActionServers()) { + for (ActionServer actionServer : node.getNode().getActionServers()) { this.actionServerHandles.add( new AbstractMap.SimpleEntry(actionServer.getHandle(), actionServer)); } @@ -498,7 +512,7 @@ private static native void nativeWaitSetAddSubscription( private static native void nativeWait(long waitSetHandle, long timeout); private static native MessageDefinition nativeTake( - long subscriptionHandle, Class messageType); + long subscriptionHandle, Class messageType); private static native void nativeWaitSetAddService(long waitSetHandle, long serviceHandle); diff --git a/rcljava/src/main/java/org/ros2/rcljava/graph/NameAndTypes.java b/rcljava/src/main/java/org/ros2/rcljava/graph/NameAndTypes.java index 580ad329..12ec3d78 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/graph/NameAndTypes.java +++ b/rcljava/src/main/java/org/ros2/rcljava/graph/NameAndTypes.java @@ -45,12 +45,12 @@ public class NameAndTypes { */ public NameAndTypes(final String name, final Collection types) { this.name = name; - this.types = new ArrayList(types); + this.types = new ArrayList(types); } /// @internal Default constructor, only used from jni code. private NameAndTypes() { - this.types = new ArrayList(); + this.types = new ArrayList(); } @Override 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 d1ba5d03..63dc6b24 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/Node.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/Node.java @@ -233,6 +233,7 @@ ActionServer createActionServer(final Class a * @param callback Function that is called when the timer expires. * @return The created timer. */ + @SuppressWarnings("deprecation") WallTimer createWallTimer(final long period, final TimeUnit unit, final Callback callback); /** 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 7c6dd4b4..3bfb0e4b 100644 --- a/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java +++ b/rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java @@ -482,10 +482,11 @@ public final long getHandle() { private static native long nativeCreateTimerHandle(long clockHandle, long contextHandle, long timerPeriod); + @SuppressWarnings("deprecation") private Timer createTimer(Clock clock, final long period, final TimeUnit unit, final Callback callback) { long timerPeriodNS = TimeUnit.NANOSECONDS.convert(period, unit); long timerHandle = nativeCreateTimerHandle(clock.getHandle(), this.context.getHandle(), timerPeriodNS); - WallTimer timer = new WallTimerImpl(new WeakReference(this), timerHandle, callback, timerPeriodNS); + Timer timer = new WallTimerImpl(new WeakReference(this), timerHandle, callback, timerPeriodNS); this.timers.add(timer); return timer; } @@ -493,6 +494,7 @@ private Timer createTimer(Clock clock, final long period, final TimeUnit unit, f /** * {@inheritDoc} */ + @SuppressWarnings("deprecation") public WallTimer createWallTimer(final long period, final TimeUnit unit, final Callback callback) { return (WallTimer) this.createTimer(this.wall_clock, period, unit, callback); } diff --git a/rcljava/src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java b/rcljava/src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java index 818314f8..21d91688 100644 --- a/rcljava/src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java +++ b/rcljava/src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java @@ -136,7 +136,7 @@ public final void testListParameters() throws Exception { assertArrayEquals( parametersAndPrefixes.getNames(), new String[] {"foo.first", "foo.second"}); - assertEquals(parametersAndPrefixes.getPrefixes(), new String[] {"foo"}); + assertArrayEquals(parametersAndPrefixes.getPrefixes(), new String[] {"foo"}); } @Test