Skip to content

Commit f6108a4

Browse files
authored
Cleanup service/client creation code (#74)
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
1 parent 94ce734 commit f6108a4

12 files changed

Lines changed: 124 additions & 125 deletions

File tree

rcljava/src/main/java/org/ros2/rcljava/client/Client.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,7 @@
2626
import org.ros2.rcljava.service.RMWRequestId;
2727

2828
public interface Client<T extends ServiceDefinition> extends Disposable {
29-
<U extends MessageDefinition> Class<U> getRequestType();
30-
31-
<U extends MessageDefinition> Class<U> getResponseType();
29+
ServiceDefinition getServiceDefinition();
3230

3331
<U extends MessageDefinition> void handleResponse(RMWRequestId header, U response);
3432

rcljava/src/main/java/org/ros2/rcljava/client/ClientImpl.java

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -55,20 +55,25 @@ public class ClientImpl<T extends ServiceDefinition> implements Client<T> {
5555
private final String serviceName;
5656
private Map<Long, Map.Entry<Consumer, RCLFuture>> pendingRequests;
5757

58-
private final Class<MessageDefinition> requestType;
59-
private final Class<MessageDefinition> responseType;
60-
61-
public ClientImpl(final WeakReference<Node> nodeReference, final long handle,
62-
final String serviceName, final Class<MessageDefinition> requestType,
63-
final Class<MessageDefinition> responseType) {
58+
private final ServiceDefinition serviceDefinition;
59+
60+
public ClientImpl(
61+
final ServiceDefinition serviceDefinition,
62+
final WeakReference<Node> nodeReference,
63+
final long handle,
64+
final String serviceName)
65+
{
6466
this.nodeReference = nodeReference;
6567
this.handle = handle;
6668
this.serviceName = serviceName;
67-
this.requestType = requestType;
68-
this.responseType = responseType;
69+
this.serviceDefinition = serviceDefinition;
6970
this.pendingRequests = new HashMap<Long, Map.Entry<Consumer, RCLFuture>>();
7071
}
7172

73+
public ServiceDefinition getServiceDefinition() {
74+
return this.serviceDefinition;
75+
}
76+
7277
public final <U extends MessageDefinition, V extends MessageDefinition> Future<V>
7378
asyncSendRequest(final U request) {
7479
return asyncSendRequest(request, new Consumer<Future<V>>() {
@@ -112,14 +117,6 @@ private static native long nativeSendClientRequest(
112117
long handle, long requestFromJavaConverterHandle, long requestDestructorHandle,
113118
MessageDefinition requestMessage);
114119

115-
public final Class<MessageDefinition> getRequestType() {
116-
return this.requestType;
117-
}
118-
119-
public final Class<MessageDefinition> getResponseType() {
120-
return this.responseType;
121-
}
122-
123120
/**
124121
* Destroy a ROS2 client (rcl_client_t).
125122
*

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

Lines changed: 10 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -118,18 +118,9 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) {
118118
}
119119

120120
if (anyExecutable.service != null) {
121-
Class<? extends MessageDefinition> requestType = anyExecutable.service.getRequestType();
122-
Class<? extends MessageDefinition> responseType = anyExecutable.service.getResponseType();
123-
124-
MessageDefinition requestMessage = null;
125-
MessageDefinition responseMessage = null;
126-
127-
try {
128-
requestMessage = requestType.getDeclaredConstructor().newInstance();
129-
responseMessage = responseType.getDeclaredConstructor().newInstance();
130-
} catch (ReflectiveOperationException e) {
131-
throw new IllegalStateException("Failed to instantiate service request/response");
132-
}
121+
ServiceDefinition serviceDefinition = anyExecutable.service.getServiceDefinition();
122+
MessageDefinition requestMessage = serviceDefinition.newRequestInstance();
123+
MessageDefinition responseMessage = serviceDefinition.newResponseInstance();
133124

134125
if (requestMessage != null && responseMessage != null) {
135126
long requestFromJavaConverterHandle = requestMessage.getFromJavaConverterInstance();
@@ -140,28 +131,21 @@ protected void executeAnyExecutable(AnyExecutable anyExecutable) {
140131
long responseDestructorHandle = responseMessage.getDestructorInstance();
141132

142133
RMWRequestId rmwRequestId =
143-
nativeTakeRequest(anyExecutable.service.getHandle(), requestFromJavaConverterHandle,
144-
requestToJavaConverterHandle, requestDestructorHandle, requestMessage);
134+
nativeTakeRequest(anyExecutable.service.getHandle(), requestFromJavaConverterHandle,
135+
requestToJavaConverterHandle, requestDestructorHandle, requestMessage);
145136
if (rmwRequestId != null) {
146137
anyExecutable.service.executeCallback(rmwRequestId, requestMessage, responseMessage);
147-
nativeSendServiceResponse(anyExecutable.service.getHandle(), rmwRequestId,
148-
responseFromJavaConverterHandle, responseDestructorHandle, responseMessage);
138+
nativeSendServiceResponse(
139+
anyExecutable.service.getHandle(), rmwRequestId,
140+
responseFromJavaConverterHandle, responseDestructorHandle, responseMessage);
149141
}
150142
}
151143
serviceHandles.remove(anyExecutable.service.getHandle());
152144
}
153145

154146
if (anyExecutable.client != null) {
155-
Class<? extends MessageDefinition> requestType = anyExecutable.client.getRequestType();
156-
Class<? extends MessageDefinition> responseType = anyExecutable.client.getResponseType();
157-
158-
MessageDefinition responseMessage = null;
159-
160-
try {
161-
responseMessage = responseType.getDeclaredConstructor().newInstance();
162-
} catch (ReflectiveOperationException ie) {
163-
throw new IllegalStateException("Failed to instantiate service response");
164-
}
147+
ServiceDefinition serviceDefinition = anyExecutable.client.getServiceDefinition();
148+
MessageDefinition responseMessage = serviceDefinition.newResponseInstance();
165149

166150
if (responseMessage != null) {
167151
long responseFromJavaConverterHandle = responseMessage.getFromJavaConverterInstance();

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -126,23 +126,24 @@ <T extends MessageDefinition> Publisher<T> createPublisher(
126126
<T extends MessageDefinition> Publisher<T> createPublisher(
127127
final Class<T> messageType, final String topic);
128128

129-
<T extends ServiceDefinition> Service<T> createService(final Class<T> serviceType,
129+
<T extends ServiceDefinition> Service<T> createService(
130+
final Class<T> serviceType,
130131
final String serviceName,
131132
final TriConsumer<RMWRequestId, ? extends MessageDefinition, ? extends MessageDefinition>
132133
callback,
133-
final QoSProfile qosProfile) throws NoSuchFieldException, IllegalAccessException;
134+
final QoSProfile qosProfile);
134135

135-
<T extends ServiceDefinition> Service<T> createService(final Class<T> serviceType,
136+
<T extends ServiceDefinition> Service<T> createService(
137+
final Class<T> serviceType,
136138
final String serviceName,
137139
final TriConsumer<RMWRequestId, ? extends MessageDefinition, ? extends MessageDefinition>
138-
callback) throws NoSuchFieldException, IllegalAccessException;
140+
callback);
139141

140142
<T extends ServiceDefinition> Client<T> createClient(
141-
final Class<T> serviceType, final String serviceName, final QoSProfile qosProfile)
142-
throws NoSuchFieldException, IllegalAccessException;
143+
final Class<T> serviceType, final String serviceName, final QoSProfile qosProfile);
143144

144-
<T extends ServiceDefinition> Client<T> createClient(final Class<T> serviceType,
145-
final String serviceName) throws NoSuchFieldException, IllegalAccessException;
145+
<T extends ServiceDefinition> Client<T> createClient(
146+
final Class<T> serviceType, final String serviceName);
146147

147148
/**
148149
* Create an ActionServer&lt;T&gt;.

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

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -191,18 +191,8 @@ public NodeImpl(final long handle, final NodeOptions nodeOptions) {
191191
this.wall_clock = new Clock(ClockType.STEADY_TIME);
192192
this.timeSource = new TimeSource(this);
193193
this.timeSource.attachClock(this.clock);
194-
try {
195-
this.parameterService = nodeOptions.getStartParameterServices() ?
196-
new ParameterServiceImpl(this) : null;
197-
// TODO(ivanpauno): Modify createService, createClient so they don't declare
198-
// NoSuchFieldException and IllegalAccessException checked exceptions.
199-
// That only happens if the user passed the wrong Class object, and the exception should
200-
// be rethrown as an unchecked IllegalArgumentException.
201-
} catch (NoSuchFieldException ex) {
202-
throw new IllegalArgumentException(ex.getMessage());
203-
} catch (IllegalAccessException ex) {
204-
throw new IllegalArgumentException(ex.getMessage());
205-
}
194+
this.parameterService = nodeOptions.getStartParameterServices() ?
195+
new ParameterServiceImpl(this) : null;
206196
}
207197

208198
/**
@@ -340,30 +330,39 @@ private static native <T extends ServiceDefinition> long nativeCreateServiceHand
340330
long handle, Class<T> cls, String serviceName, long qosProfileHandle);
341331

342332
public final <T extends ServiceDefinition> Service<T> createService(final Class<T> serviceType,
343-
final String serviceName,
344-
final TriConsumer<RMWRequestId, ? extends MessageDefinition, ? extends MessageDefinition>
345-
callback,
346-
final QoSProfile qosProfile) throws NoSuchFieldException, IllegalAccessException {
347-
Class<MessageDefinition> requestType = (Class) serviceType.getField("RequestType").get(null);
348-
349-
Class<MessageDefinition> responseType = (Class) serviceType.getField("ResponseType").get(null);
350-
333+
final String serviceName,
334+
final TriConsumer<RMWRequestId, ? extends MessageDefinition, ? extends MessageDefinition>
335+
callback,
336+
final QoSProfile qosProfile)
337+
{
338+
T serviceDefinition;
339+
try {
340+
serviceDefinition = serviceType.getDeclaredConstructor().newInstance();
341+
} catch (ReflectiveOperationException e) {
342+
throw new IllegalStateException("Failed to instantiate service definition");
343+
}
351344
long qosProfileHandle = RCLJava.convertQoSProfileToHandle(qosProfile);
352345
long serviceHandle =
353346
nativeCreateServiceHandle(this.handle, serviceType, serviceName, qosProfileHandle);
354347
RCLJava.disposeQoSProfile(qosProfileHandle);
355348

356-
Service<T> service = new ServiceImpl<T>(new WeakReference<Node>(this), serviceHandle,
357-
serviceName, callback, requestType, responseType);
349+
Service<T> service = new ServiceImpl<T>(
350+
serviceDefinition,
351+
new WeakReference<Node>(this),
352+
serviceHandle,
353+
serviceName,
354+
callback);
358355
this.services.add(service);
359356

360357
return service;
361358
}
362359

363-
public <T extends ServiceDefinition> Service<T> createService(final Class<T> serviceType,
364-
final String serviceName,
365-
final TriConsumer<RMWRequestId, ? extends MessageDefinition, ? extends MessageDefinition>
366-
callback) throws NoSuchFieldException, IllegalAccessException {
360+
public <T extends ServiceDefinition> Service<T> createService(
361+
final Class<T> serviceType,
362+
final String serviceName,
363+
final TriConsumer<RMWRequestId, ? extends MessageDefinition, ? extends MessageDefinition>
364+
callback)
365+
{
367366
return this.<T>createService(serviceType, serviceName, callback, QoSProfile.SERVICES_DEFAULT);
368367
}
369368

@@ -375,26 +374,29 @@ public final Collection<Service> getServices() {
375374
}
376375

377376
public final <T extends ServiceDefinition> Client<T> createClient(
378-
final Class<T> serviceType, final String serviceName, final QoSProfile qosProfile)
379-
throws NoSuchFieldException, IllegalAccessException {
380-
Class<MessageDefinition> requestType = (Class) serviceType.getField("RequestType").get(null);
381-
382-
Class<MessageDefinition> responseType = (Class) serviceType.getField("ResponseType").get(null);
383-
377+
final Class<T> serviceType, final String serviceName, final QoSProfile qosProfile)
378+
{
384379
long qosProfileHandle = RCLJava.convertQoSProfileToHandle(qosProfile);
385380
long clientHandle =
386381
nativeCreateClientHandle(this.handle, serviceType, serviceName, qosProfileHandle);
387382
RCLJava.disposeQoSProfile(qosProfileHandle);
388383

384+
T serviceDefinition;
385+
try {
386+
serviceDefinition = serviceType.getDeclaredConstructor().newInstance();
387+
} catch (ReflectiveOperationException e) {
388+
throw new IllegalStateException("Failed to instantiate service definition");
389+
}
390+
389391
Client<T> client = new ClientImpl<T>(
390-
new WeakReference<Node>(this), clientHandle, serviceName, requestType, responseType);
392+
serviceDefinition, new WeakReference<Node>(this), clientHandle, serviceName);
391393
this.clients.add(client);
392394

393395
return client;
394396
}
395397

396398
public <T extends ServiceDefinition> Client<T> createClient(final Class<T> serviceType,
397-
final String serviceName) throws NoSuchFieldException, IllegalAccessException {
399+
final String serviceName) {
398400
return this.<T>createClient(serviceType, serviceName, QoSProfile.SERVICES_DEFAULT);
399401
}
400402

rcljava/src/main/java/org/ros2/rcljava/parameters/client/AsyncParametersClientImpl.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public class AsyncParametersClientImpl implements AsyncParametersClient {
4949
private final Client<rcl_interfaces.srv.DescribeParameters> describeParametersClient;
5050

5151
public AsyncParametersClientImpl(final Node node, final String remoteName,
52-
final QoSProfile qosProfile) throws NoSuchFieldException, IllegalAccessException {
52+
final QoSProfile qosProfile) {
5353
this.node = node;
5454
if (remoteName != "") {
5555
this.remoteName = remoteName;
@@ -84,17 +84,17 @@ public AsyncParametersClientImpl(final Node node, final String remoteName,
8484
}
8585

8686
public AsyncParametersClientImpl(final Node node, final QoSProfile qosProfile)
87-
throws NoSuchFieldException, IllegalAccessException {
87+
{
8888
this(node, "", qosProfile);
8989
}
9090

9191
public AsyncParametersClientImpl(final Node node, final String remoteName)
92-
throws NoSuchFieldException, IllegalAccessException {
92+
{
9393
this(node, remoteName, QoSProfile.PARAMETERS);
9494
}
9595

9696
public AsyncParametersClientImpl(final Node node)
97-
throws NoSuchFieldException, IllegalAccessException {
97+
{
9898
this(node, "", QoSProfile.PARAMETERS);
9999
}
100100

rcljava/src/main/java/org/ros2/rcljava/parameters/client/SyncParametersClientImpl.java

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -55,44 +55,55 @@ public ConsumerHelper(RCLFuture<T> resultFuture) {
5555

5656
public AsyncParametersClient asyncParametersClient;
5757

58-
public SyncParametersClientImpl(final Node node, final String remoteName,
59-
final QoSProfile qosProfile) throws NoSuchFieldException, IllegalAccessException {
58+
public SyncParametersClientImpl(
59+
final Node node,
60+
final String remoteName,
61+
final QoSProfile qosProfile)
62+
{
6063
this.asyncParametersClient = new AsyncParametersClientImpl(node, remoteName, qosProfile);
6164
}
6265

6366
public SyncParametersClientImpl(final Node node, final QoSProfile qosProfile)
64-
throws NoSuchFieldException, IllegalAccessException {
67+
{
6568
this(node, "", qosProfile);
6669
}
6770

6871
public SyncParametersClientImpl(final Node node, final String remoteName)
69-
throws NoSuchFieldException, IllegalAccessException {
72+
{
7073
this(node, remoteName, QoSProfile.PARAMETERS);
7174
}
7275

7376
public SyncParametersClientImpl(final Node node)
74-
throws NoSuchFieldException, IllegalAccessException {
77+
{
7578
this(node, "", QoSProfile.PARAMETERS);
7679
}
7780

78-
public SyncParametersClientImpl(final Executor executor, final Node node, final String remoteName,
79-
final QoSProfile qosProfile) throws NoSuchFieldException, IllegalAccessException {
81+
public SyncParametersClientImpl(
82+
final Executor executor,
83+
final Node node,
84+
final String remoteName,
85+
final QoSProfile qosProfile)
86+
{
8087
this.executor = executor;
8188
this.asyncParametersClient = new AsyncParametersClientImpl(node, remoteName, qosProfile);
8289
}
8390

84-
public SyncParametersClientImpl(final Executor executor, final Node node,
85-
final QoSProfile qosProfile) throws NoSuchFieldException, IllegalAccessException {
91+
public SyncParametersClientImpl(
92+
final Executor executor,
93+
final Node node,
94+
final QoSProfile qosProfile)
95+
{
8696
this(executor, node, "", qosProfile);
8797
}
8898

89-
public SyncParametersClientImpl(final Executor executor, final Node node, final String remoteName)
90-
throws NoSuchFieldException, IllegalAccessException {
99+
public SyncParametersClientImpl(
100+
final Executor executor, final Node node, final String remoteName)
101+
{
91102
this(executor, node, remoteName, QoSProfile.PARAMETERS);
92103
}
93104

94105
public SyncParametersClientImpl(final Executor executor, final Node node)
95-
throws NoSuchFieldException, IllegalAccessException {
106+
{
96107
this(executor, node, "", QoSProfile.PARAMETERS);
97108
}
98109

rcljava/src/main/java/org/ros2/rcljava/parameters/service/ParameterServiceImpl.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public class ParameterServiceImpl implements ParameterService {
3838
Service<rcl_interfaces.srv.ListParameters> listParametersService;
3939

4040
public ParameterServiceImpl(final Node node, final QoSProfile qosProfile)
41-
throws NoSuchFieldException, IllegalAccessException {
41+
{
4242
this.node = node;
4343
this.getParametersService = node.<rcl_interfaces.srv.GetParameters>createService(
4444
rcl_interfaces.srv.GetParameters.class,
@@ -146,7 +146,7 @@ public void accept(RMWRequestId rmwRequestId,
146146
qosProfile);
147147
}
148148

149-
public ParameterServiceImpl(final Node node) throws NoSuchFieldException, IllegalAccessException {
149+
public ParameterServiceImpl(final Node node) {
150150
this(node, QoSProfile.PARAMETERS);
151151
}
152152
}

rcljava/src/main/java/org/ros2/rcljava/service/Service.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import org.ros2.rcljava.interfaces.ServiceDefinition;
2222

2323
public interface Service<T extends ServiceDefinition> extends Disposable {
24-
<U extends MessageDefinition> Class<U> getRequestType();
25-
26-
<U extends MessageDefinition> Class<U> getResponseType();
24+
ServiceDefinition getServiceDefinition();
2725

2826
void executeCallback(RMWRequestId rmwRequestId, MessageDefinition request, MessageDefinition response);
2927

0 commit comments

Comments
 (0)