Skip to content

Commit 7b00bbf

Browse files
ivanpaunoivanpauno
authored
Fix some java compiler warnings (#72)
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
1 parent c15a9f8 commit 7b00bbf

6 files changed

Lines changed: 67 additions & 51 deletions

File tree

‎rcljava/src/main/java/org/ros2/rcljava/action/ActionServerImpl.java‎

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -176,9 +176,9 @@ public synchronized void publishFeedback(FeedbackDefinition<T> feedback) {
176176
ActionServerImpl.this.actionTypeInstance.getFeedbackMessageType();
177177
FeedbackMessageDefinition<T> feedbackMessage = null;
178178
try {
179-
feedbackMessage = feedbackMessageType.newInstance();
180-
} catch (Exception ex) {
181-
throw new IllegalArgumentException("Failed to instantiate feedback message", ex);
179+
feedbackMessage = feedbackMessageType.getDeclaredConstructor().newInstance();
180+
} catch (ReflectiveOperationException ex) {
181+
throw new IllegalArgumentException("Failed to instantiate feedback message: ", ex);
182182
}
183183
feedbackMessage.setFeedback(feedback);
184184
feedbackMessage.setGoalUuid(this.goalInfo.getGoalId().getUuidAsList());
@@ -202,7 +202,7 @@ public final long getHandle() {
202202

203203
private synchronized final void toTerminalState(byte status, ResultDefinition<T> result) {
204204
nativeUpdateGoalState(this.handle, status);
205-
ResultResponseDefinition resultResponse = ActionServerImpl.this.createResultResponse();
205+
ResultResponseDefinition<T> resultResponse = ActionServerImpl.this.createResultResponse();
206206
resultResponse.setGoalStatus(status);
207207
resultResponse.setResult(result);
208208
ActionServerImpl.this.sendResult(goalInfo.getGoalId().getUuidAsList(), resultResponse);
@@ -265,9 +265,9 @@ public ActionServerImpl(
265265
final Consumer<ActionServerGoalHandle<T>> acceptedCallback) throws IllegalArgumentException {
266266
this.nodeReference = nodeReference;
267267
try {
268-
this.actionTypeInstance = actionType.newInstance();
269-
} catch (Exception ex) {
270-
throw new IllegalArgumentException("Failed to instantiate provided action type", ex);
268+
this.actionTypeInstance = actionType.getDeclaredConstructor().newInstance();
269+
} catch (ReflectiveOperationException ex) {
270+
throw new IllegalArgumentException("Failed to instantiate provided action type: ", ex);
271271
}
272272
this.actionName = actionName;
273273
this.goalCallback = goalCallback;
@@ -545,9 +545,10 @@ private action_msgs.msg.GoalInfo createGoalInfo(List<Byte> goalUuid) {
545545
private ResultResponseDefinition<T> createResultResponse() {
546546
ResultResponseDefinition<T> resultResponse;
547547
try {
548-
resultResponse = this.actionTypeInstance.getGetResultResponseType().newInstance();
549-
} catch (Exception ex) {
550-
throw new IllegalArgumentException("Failed to instantiate provided action type", ex);
548+
resultResponse =
549+
this.actionTypeInstance.getGetResultResponseType().getDeclaredConstructor().newInstance();
550+
} catch (ReflectiveOperationException ex) {
551+
throw new IllegalStateException("Failed to instantiate provided action type: ", ex);
551552
}
552553
return resultResponse;
553554
}
@@ -582,12 +583,10 @@ public void execute() {
582583
GoalResponseDefinition<T> responseMessage = null;
583584

584585
try {
585-
requestMessage = requestType.newInstance();
586-
responseMessage = responseType.newInstance();
587-
} catch (InstantiationException ie) {
588-
ie.printStackTrace();
589-
} catch (IllegalAccessException iae) {
590-
iae.printStackTrace();
586+
requestMessage = requestType.getDeclaredConstructor().newInstance();
587+
responseMessage = responseType.getDeclaredConstructor().newInstance();
588+
} catch (ReflectiveOperationException ex) {
589+
throw new IllegalStateException("Failed to instantiate request or responce: ", ex);
591590
}
592591

593592
if (requestMessage != null && responseMessage != null) {
@@ -648,9 +647,9 @@ public void execute() {
648647

649648
ResultRequestDefinition<T> requestMessage = null;
650649
try {
651-
requestMessage = requestType.newInstance();
652-
} catch (Exception ex) {
653-
throw new IllegalArgumentException("Failed to instantiate action result request", ex);
650+
requestMessage = requestType.getDeclaredConstructor().newInstance();
651+
} catch (ReflectiveOperationException ex) {
652+
throw new IllegalArgumentException("Failed to instantiate action result request: ", ex);
654653
}
655654

656655
if (requestMessage != null) {

‎rcljava/src/main/java/org/ros2/rcljava/executors/BaseExecutor.java‎

Lines changed: 42 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -15,17 +15,18 @@
1515

1616
package org.ros2.rcljava.executors;
1717

18+
import java.lang.SuppressWarnings;
1819
import java.util.AbstractMap;
1920
import java.util.ArrayList;
2021
import java.util.Iterator;
2122
import java.util.List;
2223
import java.util.Map;
23-
2424
import java.util.Collection;
2525
import java.util.concurrent.BlockingQueue;
2626
import java.util.concurrent.ConcurrentHashMap;
2727
import java.util.concurrent.LinkedBlockingQueue;
2828

29+
2930
import org.slf4j.Logger;
3031
import org.slf4j.LoggerFactory;
3132

@@ -81,6 +82,23 @@ protected void removeNode(ComposableNode node) {
8182
this.nodes.remove(node);
8283
}
8384

85+
@SuppressWarnings("unchecked")
86+
protected static void executeSubscriptionCallbackUnchecked(
87+
Subscription subscription,
88+
MessageDefinition message)
89+
{
90+
subscription.executeCallback(message);
91+
}
92+
93+
@SuppressWarnings("unchecked")
94+
protected static void clientHandleResponseUnchecked(
95+
Client client,
96+
RMWRequestId rmwRequestId,
97+
MessageDefinition response)
98+
{
99+
client.handleResponse(rmwRequestId, response);
100+
}
101+
84102
protected void executeAnyExecutable(AnyExecutable anyExecutable) {
85103
if (anyExecutable.timer != null) {
86104
anyExecutable.timer.callTimer();
@@ -92,25 +110,25 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) {
92110
MessageDefinition message = nativeTake(
93111
anyExecutable.subscription.getHandle(), anyExecutable.subscription.getMessageType());
94112
if (message != null) {
95-
anyExecutable.subscription.executeCallback(message);
113+
// Safety: nativeTake() will return the correct type here.
114+
// We can't do much better here, as subscriptions are type erased.
115+
executeSubscriptionCallbackUnchecked(anyExecutable.subscription, message);
96116
}
97117
subscriptionHandles.remove(anyExecutable.subscription.getHandle());
98118
}
99119

100120
if (anyExecutable.service != null) {
101-
Class<MessageDefinition> requestType = anyExecutable.service.getRequestType();
102-
Class<MessageDefinition> responseType = anyExecutable.service.getResponseType();
121+
Class<? extends MessageDefinition> requestType = anyExecutable.service.getRequestType();
122+
Class<? extends MessageDefinition> responseType = anyExecutable.service.getResponseType();
103123

104124
MessageDefinition requestMessage = null;
105125
MessageDefinition responseMessage = null;
106126

107127
try {
108-
requestMessage = requestType.newInstance();
109-
responseMessage = responseType.newInstance();
110-
} catch (InstantiationException ie) {
111-
ie.printStackTrace();
112-
} catch (IllegalAccessException iae) {
113-
iae.printStackTrace();
128+
requestMessage = requestType.getDeclaredConstructor().newInstance();
129+
responseMessage = responseType.getDeclaredConstructor().newInstance();
130+
} catch (ReflectiveOperationException e) {
131+
throw new IllegalStateException("Failed to instantiate service request/response");
114132
}
115133

116134
if (requestMessage != null && responseMessage != null) {
@@ -134,24 +152,18 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) {
134152
}
135153

136154
if (anyExecutable.client != null) {
137-
Class<MessageDefinition> requestType = anyExecutable.client.getRequestType();
138-
Class<MessageDefinition> responseType = anyExecutable.client.getResponseType();
155+
Class<? extends MessageDefinition> requestType = anyExecutable.client.getRequestType();
156+
Class<? extends MessageDefinition> responseType = anyExecutable.client.getResponseType();
139157

140-
MessageDefinition requestMessage = null;
141158
MessageDefinition responseMessage = null;
142159

143160
try {
144-
requestMessage = requestType.newInstance();
145-
responseMessage = responseType.newInstance();
146-
} catch (InstantiationException ie) {
147-
ie.printStackTrace();
148-
} catch (IllegalAccessException iae) {
149-
iae.printStackTrace();
161+
responseMessage = responseType.getDeclaredConstructor().newInstance();
162+
} catch (ReflectiveOperationException ie) {
163+
throw new IllegalStateException("Failed to instantiate service response");
150164
}
151165

152-
if (requestMessage != null && responseMessage != null) {
153-
long requestFromJavaConverterHandle = requestMessage.getFromJavaConverterInstance();
154-
long requestToJavaConverterHandle = requestMessage.getToJavaConverterInstance();
166+
if (responseMessage != null) {
155167
long responseFromJavaConverterHandle = responseMessage.getFromJavaConverterInstance();
156168
long responseToJavaConverterHandle = responseMessage.getToJavaConverterInstance();
157169
long responseDestructorHandle = responseMessage.getDestructorInstance();
@@ -161,7 +173,9 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) {
161173
responseToJavaConverterHandle, responseDestructorHandle, responseMessage);
162174

163175
if (rmwRequestId != null) {
164-
anyExecutable.client.handleResponse(rmwRequestId, responseMessage);
176+
// Safety: nativeTakeResponse() will return the correct type here.
177+
// We can't do much better here, as subscriptions are type erased.
178+
clientHandleResponseUnchecked(anyExecutable.client, rmwRequestId, responseMessage);
165179
}
166180
}
167181
clientHandles.remove(anyExecutable.client.getHandle());
@@ -187,7 +201,7 @@ protected void waitForWork(long timeout) {
187201
this.actionServerHandles.clear();
188202

189203
for (ComposableNode node : this.nodes) {
190-
for (Subscription<MessageDefinition> subscription : node.getNode().getSubscriptions()) {
204+
for (Subscription subscription : node.getNode().getSubscriptions()) {
191205
this.subscriptionHandles.add(new AbstractMap.SimpleEntry<Long, Subscription>(
192206
subscription.getHandle(), subscription));
193207
Collection<EventHandler> eventHandlers = subscription.getEventHandlers();
@@ -209,17 +223,17 @@ protected void waitForWork(long timeout) {
209223
this.timerHandles.add(new AbstractMap.SimpleEntry<Long, Timer>(timer.getHandle(), timer));
210224
}
211225

212-
for (Service<ServiceDefinition> service : node.getNode().getServices()) {
226+
for (Service service : node.getNode().getServices()) {
213227
this.serviceHandles.add(
214228
new AbstractMap.SimpleEntry<Long, Service>(service.getHandle(), service));
215229
}
216230

217-
for (Client<ServiceDefinition> client : node.getNode().getClients()) {
231+
for (Client client : node.getNode().getClients()) {
218232
this.clientHandles.add(
219233
new AbstractMap.SimpleEntry<Long, Client>(client.getHandle(), client));
220234
}
221235

222-
for (ActionServer<ActionDefinition> actionServer : node.getNode().getActionServers()) {
236+
for (ActionServer actionServer : node.getNode().getActionServers()) {
223237
this.actionServerHandles.add(
224238
new AbstractMap.SimpleEntry<Long, ActionServer>(actionServer.getHandle(), actionServer));
225239
}
@@ -498,7 +512,7 @@ private static native void nativeWaitSetAddSubscription(
498512
private static native void nativeWait(long waitSetHandle, long timeout);
499513

500514
private static native MessageDefinition nativeTake(
501-
long subscriptionHandle, Class<MessageDefinition> messageType);
515+
long subscriptionHandle, Class<? extends MessageDefinition> messageType);
502516

503517
private static native void nativeWaitSetAddService(long waitSetHandle, long serviceHandle);
504518

‎rcljava/src/main/java/org/ros2/rcljava/graph/NameAndTypes.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,12 @@ public class NameAndTypes {
4545
*/
4646
public NameAndTypes(final String name, final Collection<String> types) {
4747
this.name = name;
48-
this.types = new ArrayList(types);
48+
this.types = new ArrayList<String>(types);
4949
}
5050

5151
/// @internal Default constructor, only used from jni code.
5252
private NameAndTypes() {
53-
this.types = new ArrayList();
53+
this.types = new ArrayList<String>();
5454
}
5555

5656
@Override

‎rcljava/src/main/java/org/ros2/rcljava/node/Node.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,7 @@ <T extends ActionDefinition> ActionServer<T> createActionServer(final Class<T> a
233233
* @param callback Function that is called when the timer expires.
234234
* @return The created timer.
235235
*/
236+
@SuppressWarnings("deprecation")
236237
WallTimer createWallTimer(final long period, final TimeUnit unit, final Callback callback);
237238

238239
/**

‎rcljava/src/main/java/org/ros2/rcljava/node/NodeImpl.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,17 +482,19 @@ public final long getHandle() {
482482

483483
private static native long nativeCreateTimerHandle(long clockHandle, long contextHandle, long timerPeriod);
484484

485+
@SuppressWarnings("deprecation")
485486
private Timer createTimer(Clock clock, final long period, final TimeUnit unit, final Callback callback) {
486487
long timerPeriodNS = TimeUnit.NANOSECONDS.convert(period, unit);
487488
long timerHandle = nativeCreateTimerHandle(clock.getHandle(), this.context.getHandle(), timerPeriodNS);
488-
WallTimer timer = new WallTimerImpl(new WeakReference<Node>(this), timerHandle, callback, timerPeriodNS);
489+
Timer timer = new WallTimerImpl(new WeakReference<Node>(this), timerHandle, callback, timerPeriodNS);
489490
this.timers.add(timer);
490491
return timer;
491492
}
492493

493494
/**
494495
* {@inheritDoc}
495496
*/
497+
@SuppressWarnings("deprecation")
496498
public WallTimer createWallTimer(final long period, final TimeUnit unit, final Callback callback) {
497499
return (WallTimer) this.createTimer(this.wall_clock, period, unit, callback);
498500
}

‎rcljava/src/test/java/org/ros2/rcljava/parameters/SyncParametersClientTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ public final void testListParameters() throws Exception {
136136

137137
assertArrayEquals(
138138
parametersAndPrefixes.getNames(), new String[] {"foo.first", "foo.second"});
139-
assertEquals(parametersAndPrefixes.getPrefixes(), new String[] {"foo"});
139+
assertArrayEquals(parametersAndPrefixes.getPrefixes(), new String[] {"foo"});
140140
}
141141

142142
@Test

0 commit comments

Comments
 (0)