From ceae53f01a6b658062ff728336c02d4a3be697ad Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 17:54:10 +0300 Subject: [PATCH 1/9] Add guava dependency and maven local --- build.gradle | 1 + rosjava_geometry/build.gradle | 1 + rosjava_helpers/build.gradle | 1 + 3 files changed, 3 insertions(+) diff --git a/build.gradle b/build.gradle index df8fad18..cf753b9d 100644 --- a/build.gradle +++ b/build.gradle @@ -29,6 +29,7 @@ subprojects { targetCompatibility = JavaVersion.VERSION_17 repositories{ + mavenLocal() mavenCentral() jcenter() maven { diff --git a/rosjava_geometry/build.gradle b/rosjava_geometry/build.gradle index 42beff0b..fd90fabe 100644 --- a/rosjava_geometry/build.gradle +++ b/rosjava_geometry/build.gradle @@ -18,6 +18,7 @@ plugins{ } dependencies { api 'org.ros.rosjava_messages:geometry_msgs:[1.12,1.13)' + implementation 'com.google.guava:guava:31.1-jre' implementation project(':rosjava') testImplementation 'junit:junit:4.8.2' } diff --git a/rosjava_helpers/build.gradle b/rosjava_helpers/build.gradle index 7d17701d..40f4fd3b 100644 --- a/rosjava_helpers/build.gradle +++ b/rosjava_helpers/build.gradle @@ -1,5 +1,6 @@ dependencies { implementation project(':rosjava') + implementation 'com.google.guava:guava:31.1-jre' implementation 'org.yaml:snakeyaml:1.30' testImplementation 'junit:junit:4.8.2' testImplementation project(':rosjava').sourceSets.test.output From 15dbb5a6186003ae751bef085e05f1175e7c03a0 Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 17:54:34 +0300 Subject: [PATCH 2/9] use method reference for service implementation --- .../org/ros/rosjava_tutorial_services/Server.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/rosjava_tutorial_services/src/main/java/org/ros/rosjava_tutorial_services/Server.java b/rosjava_tutorial_services/src/main/java/org/ros/rosjava_tutorial_services/Server.java index 5a313bfb..e80ccd71 100644 --- a/rosjava_tutorial_services/src/main/java/org/ros/rosjava_tutorial_services/Server.java +++ b/rosjava_tutorial_services/src/main/java/org/ros/rosjava_tutorial_services/Server.java @@ -20,7 +20,7 @@ import org.ros.node.AbstractNodeMain; import org.ros.node.ConnectedNode; import org.ros.node.NodeMain; -import org.ros.node.service.ServiceResponseBuilder; +import org.ros.node.service.*; import org.ros.node.service.ServiceServer; /** @@ -35,15 +35,12 @@ public GraphName getDefaultNodeName() { return GraphName.of("rosjava_tutorial_services/server"); } + + private final void createResponse(final rosjava_test_msgs.AddTwoIntsRequest request,final rosjava_test_msgs.AddTwoIntsResponse response) { + response.setSum(request.getA() + request.getB()); + } @Override public void onStart(ConnectedNode connectedNode) { - connectedNode.newServiceServer("add_two_ints", rosjava_test_msgs.AddTwoInts._TYPE, - new ServiceResponseBuilder() { - @Override - public void - build(rosjava_test_msgs.AddTwoIntsRequest request, rosjava_test_msgs.AddTwoIntsResponse response) { - response.setSum(request.getA() + request.getB()); - } - }); + connectedNode.newServiceServer("add_two_ints", rosjava_test_msgs.AddTwoInts._TYPE,this::createResponse); } } From 1b5c874b2677dac9b7d1f0efb2c697912a00adf5 Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 17:55:04 +0300 Subject: [PATCH 3/9] Easy to use simple service client --- .../java/org/ros/node/ServiceClientNode.java | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 rosjava/src/main/java/org/ros/node/ServiceClientNode.java diff --git a/rosjava/src/main/java/org/ros/node/ServiceClientNode.java b/rosjava/src/main/java/org/ros/node/ServiceClientNode.java new file mode 100644 index 00000000..708680a7 --- /dev/null +++ b/rosjava/src/main/java/org/ros/node/ServiceClientNode.java @@ -0,0 +1,111 @@ +/* + * Copyright (C) 2012 Google 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.ros.node; + +import com.google.common.base.Preconditions; +import org.apache.commons.lang3.StringUtils; +import org.ros.exception.ServiceNotFoundException; +import org.ros.internal.message.Message; +import org.ros.namespace.GraphName; +import org.ros.node.service.ServiceClient; + +/** + * A {@link NodeMain} which provides a service client + * + * @author Spyros Koukas + */ +public class ServiceClientNode extends AbstractNodeMain { + private final GraphName graphName; + + /** + * Getter for serviceClient + * + * @return serviceClient + **/ + public final ServiceClient getServiceClient() { + return serviceClient; + } + + private ServiceClient serviceClient; + private final String serviceType; + private final String serviceName; + + public ServiceClientNode(final String name, final String serviceName, final String serviceType) { + Preconditions.checkArgument(StringUtils.isNotBlank(name)); + Preconditions.checkArgument(StringUtils.isNotBlank(serviceName)); + Preconditions.checkArgument(StringUtils.isNotBlank(serviceType)); + this.graphName = GraphName.of(name); + this.serviceName = serviceName; + this.serviceType = serviceType; + } + + @Override + public void onStart(final ConnectedNode connectedNode) { + try { + this.serviceClient = connectedNode.newServiceClient(serviceName, serviceType); + } catch (final ServiceNotFoundException exception) { + throw new RuntimeException(exception); + } + } + + @Override + public void onShutdown(Node node) { + if (this.serviceClient != null) { + try { + this.serviceClient.shutdown(); + } catch (final Exception ignore) { + } + this.serviceClient = null; + } + } + + @Override + public void onShutdownComplete(Node node) { + } + + @Override + public void onError(Node node, Throwable throwable) { + } + + /** + * Getter for serviceType + * + * @return serviceType + **/ + public final String getServiceType() { + return serviceType; + } + + /** + * Getter for serviceName + * + * @return serviceName + **/ + public final String getServiceName() { + return serviceName; + } + + /** + * @return the name of the {@link Node} that will be used if a name was not + * specified in the {@link Node}'s associated + * {@link NodeConfiguration} + */ + @Override + public GraphName getDefaultNodeName() { + return graphName; + } +} From b9dd1b94b2984c85e42e14a6c8dea4605adb1500 Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 17:56:02 +0300 Subject: [PATCH 4/9] remove apache logging imports --- .../main/java/org/ros/internal/transport/ConnectionHeader.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/rosjava/src/main/java/org/ros/internal/transport/ConnectionHeader.java b/rosjava/src/main/java/org/ros/internal/transport/ConnectionHeader.java index a8b94a8a..fc2e47db 100644 --- a/rosjava/src/main/java/org/ros/internal/transport/ConnectionHeader.java +++ b/rosjava/src/main/java/org/ros/internal/transport/ConnectionHeader.java @@ -18,9 +18,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.Maps; - -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.jboss.netty.buffer.ChannelBuffer; import org.ros.exception.RosRuntimeException; import org.ros.internal.message.MessageBuffers; From 2237e86ea3ae53c7ef893bdb453d9e1243597552 Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 17:56:53 +0300 Subject: [PATCH 5/9] Fixing references, adding final and this. --- .../java/org/ros/internal/node/RosoutLogger.java | 4 ++-- .../java/org/ros/internal/node/client/Client.java | 3 ++- .../server/master/MasterRegistrationManagerImpl.java | 12 ++++++------ .../ros/internal/node/service/ServiceFactory.java | 2 +- .../ros/internal/node/service/ServiceIdentifier.java | 2 +- .../ros/internal/node/topic/PublisherFactory.java | 8 ++++---- 6 files changed, 16 insertions(+), 15 deletions(-) diff --git a/rosjava/src/main/java/org/ros/internal/node/RosoutLogger.java b/rosjava/src/main/java/org/ros/internal/node/RosoutLogger.java index 246381e6..530a7d71 100644 --- a/rosjava/src/main/java/org/ros/internal/node/RosoutLogger.java +++ b/rosjava/src/main/java/org/ros/internal/node/RosoutLogger.java @@ -17,7 +17,7 @@ package org.ros.internal.node; import com.google.common.base.Preconditions; -import org.apache.commons.lang.exception.ExceptionUtils; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.ros.Topics; import org.ros.node.ConnectedNode; import org.ros.node.topic.Publisher; @@ -64,7 +64,7 @@ public RosoutLogger() { /** * Starts logging connected to ROS, if usePublisher is not null it is applied * - * @param defaultNode + * @param connectedNode * @param usePublisher */ RosoutLogger(final ConnectedNode connectedNode, final Consumer> usePublisher) { diff --git a/rosjava/src/main/java/org/ros/internal/node/client/Client.java b/rosjava/src/main/java/org/ros/internal/node/client/Client.java index 94314268..3bdee8f1 100644 --- a/rosjava/src/main/java/org/ros/internal/node/client/Client.java +++ b/rosjava/src/main/java/org/ros/internal/node/client/Client.java @@ -17,7 +17,8 @@ package org.ros.internal.node.client; import com.google.common.base.Preconditions; -import org.apache.commons.lang.StringUtils; + +import org.apache.commons.lang3.StringUtils; import org.apache.xmlrpc.client.XmlRpcClient; import org.apache.xmlrpc.client.XmlRpcClientConfigImpl; import org.apache.xmlrpc.client.XmlRpcCommonsTransportFactory; diff --git a/rosjava/src/main/java/org/ros/internal/node/server/master/MasterRegistrationManagerImpl.java b/rosjava/src/main/java/org/ros/internal/node/server/master/MasterRegistrationManagerImpl.java index 8f77507b..9f1e23d6 100644 --- a/rosjava/src/main/java/org/ros/internal/node/server/master/MasterRegistrationManagerImpl.java +++ b/rosjava/src/main/java/org/ros/internal/node/server/master/MasterRegistrationManagerImpl.java @@ -245,11 +245,11 @@ public ServiceRegistrationInfo registerService(GraphName nodeName, URI nodeSlave serviceUri, nodeName, nodeSlaveUri)); } - NodeRegistrationInfo node = obtainNodeRegistrationInfo(nodeName, nodeSlaveUri); + final NodeRegistrationInfo node = obtainNodeRegistrationInfo(nodeName, nodeSlaveUri); - ServiceRegistrationInfo service = services.get(serviceName); + ServiceRegistrationInfo service = services.get(serviceName); if (service != null) { - NodeRegistrationInfo previousServiceNode = service.getNode(); + final NodeRegistrationInfo previousServiceNode = service.getNode(); if (previousServiceNode == node) { // If node is the same, no need to do anything if (LOGGER.isWarnEnabled()) { @@ -452,15 +452,15 @@ private NodeRegistrationInfo obtainNodeRegistrationInfo(GraphName nodeName, URI * the node being replaced */ private void cleanupNode(NodeRegistrationInfo node) { - for (TopicRegistrationInfo topic : node.getPublishers()) { + for (final TopicRegistrationInfo topic : node.getPublishers()) { topic.removePublisher(node); } - for (TopicRegistrationInfo topic : node.getSubscribers()) { + for (final TopicRegistrationInfo topic : node.getSubscribers()) { topic.removeSubscriber(node); } - for (ServiceRegistrationInfo service : node.getServices()) { + for (final ServiceRegistrationInfo service : node.getServices()) { services.remove(service.getServiceName()); } } diff --git a/rosjava/src/main/java/org/ros/internal/node/service/ServiceFactory.java b/rosjava/src/main/java/org/ros/internal/node/service/ServiceFactory.java index 7f53e4d2..472c475a 100644 --- a/rosjava/src/main/java/org/ros/internal/node/service/ServiceFactory.java +++ b/rosjava/src/main/java/org/ros/internal/node/service/ServiceFactory.java @@ -76,7 +76,7 @@ public DefaultServiceServer newSer final GraphName name = serviceDeclaration.getName(); synchronized (this.mutex) { - if (serviceManager.hasServer(name)) { + if (this.serviceManager.hasServer(name)) { throw new DuplicateServiceException(String.format("ServiceServer %s already exists.", name)); } else { final DefaultServiceServer serviceServer = diff --git a/rosjava/src/main/java/org/ros/internal/node/service/ServiceIdentifier.java b/rosjava/src/main/java/org/ros/internal/node/service/ServiceIdentifier.java index 1e4040fd..e2d8294c 100644 --- a/rosjava/src/main/java/org/ros/internal/node/service/ServiceIdentifier.java +++ b/rosjava/src/main/java/org/ros/internal/node/service/ServiceIdentifier.java @@ -25,7 +25,7 @@ /** * @author damonkohler@google.com (Damon Kohler) */ -public class ServiceIdentifier { +public final class ServiceIdentifier { private final GraphName name; private final URI uri; diff --git a/rosjava/src/main/java/org/ros/internal/node/topic/PublisherFactory.java b/rosjava/src/main/java/org/ros/internal/node/topic/PublisherFactory.java index 2b105d78..ffeccb33 100644 --- a/rosjava/src/main/java/org/ros/internal/node/topic/PublisherFactory.java +++ b/rosjava/src/main/java/org/ros/internal/node/topic/PublisherFactory.java @@ -65,12 +65,12 @@ public PublisherFactory(NodeIdentifier nodeIdentifier, @SuppressWarnings("unchecked") public Publisher newOrExisting(TopicDeclaration topicDeclaration, MessageSerializer messageSerializer) { - GraphName topicName = topicDeclaration.getName(); + final GraphName topicName = topicDeclaration.getName(); synchronized (mutex) { - if (topicParticipantManager.hasPublisher(topicName)) { - return (DefaultPublisher) topicParticipantManager.getPublisher(topicName); + if (this.topicParticipantManager.hasPublisher(topicName)) { + return (DefaultPublisher) this.topicParticipantManager.getPublisher(topicName); } else { - DefaultPublisher publisher = + final DefaultPublisher publisher = new DefaultPublisher(nodeIdentifier, topicDeclaration, messageSerializer, messageFactory, executorService); publisher.addListener(new DefaultPublisherListener() { From f955fdd12b26b620d555e47544d9969294377e81 Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 17:57:53 +0300 Subject: [PATCH 6/9] lamda implementation on onServiceServerAdded --- .../ros/internal/node/client/Registrar.java | 48 ++++++++----------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/rosjava/src/main/java/org/ros/internal/node/client/Registrar.java b/rosjava/src/main/java/org/ros/internal/node/client/Registrar.java index 36e0fb67..96233347 100644 --- a/rosjava/src/main/java/org/ros/internal/node/client/Registrar.java +++ b/rosjava/src/main/java/org/ros/internal/node/client/Registrar.java @@ -1,12 +1,12 @@ /* * Copyright (C) 2011 Google 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 @@ -33,6 +33,7 @@ import org.ros.internal.node.topic.DefaultSubscriber; import org.ros.internal.node.topic.PublisherIdentifier; import org.ros.internal.node.topic.TopicParticipantManagerListener; +import org.ros.namespace.GraphName; import org.ros.node.service.ServiceServer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,7 +48,7 @@ /** * Manages topic, and service registrations of a {@link SlaveServer} with the * {@link MasterServer}. - * + * * @author kwc@willowgarage.com (Ken Conley) * @author damonkohler@google.com (Damon Kohler) */ @@ -88,7 +89,7 @@ public Registrar(MasterClient masterClient, ScheduledExecutorService executorSer * Failed registration actions are retried periodically until they succeed. * This method adjusts the delay between successive retry attempts for any * particular registration action. - * + * * @param delay * the delay in units of {@code unit} between retries * @param unit @@ -258,35 +259,24 @@ public void run() { } } + + @Override public void onServiceServerAdded(final ServiceServer serviceServer) { if (DEBUG) { LOGGER.info("Registering service: " + serviceServer); } - boolean submitted = submit(new Callable() { - @Override - public Boolean call() throws Exception { - boolean success = callMaster(new Callable>() { - @Override - public Response call() throws Exception { - return masterClient.registerService(nodeIdentifier, serviceServer); - } - }); - if (success) { - serviceServer.onMasterRegistrationSuccess(); - } else { - serviceServer.onMasterRegistrationFailure(); - } - return !success; + boolean submitted = submit(() -> { + boolean success = callMaster(() -> masterClient.registerService(nodeIdentifier, serviceServer)); + if (success) { + serviceServer.onMasterRegistrationSuccess(); + } else { + serviceServer.onMasterRegistrationFailure(); } + return !success; }); if (!submitted) { - executorService.execute(new Runnable() { - @Override - public void run() { - serviceServer.onMasterRegistrationFailure(); - } - }); + executorService.execute(() -> serviceServer.onMasterRegistrationFailure()); } } @@ -325,7 +315,7 @@ public void run() { /** * Starts the {@link Registrar} for the {@link SlaveServer} identified by the * given {@link NodeIdentifier}. - * + * * @param nodeIdentifier * the {@link NodeIdentifier} for the {@link SlaveServer} this * {@link Registrar} is responsible for @@ -339,12 +329,12 @@ public void start(NodeIdentifier nodeIdentifier) { /** * Shuts down the {@link Registrar}. - * + * *

* No further registration requests will be accepted. All queued registration * jobs have up to {@link #SHUTDOWN_TIMEOUT} {@link #SHUTDOWN_TIMEOUT_UNITS} * to complete before being canceled. - * + * *

* Calling {@link #shutdown()} more than once has no effect. */ From c1d5b73de1a53caf6bff720ab6afada38c5dbd0d Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 19:45:11 +0300 Subject: [PATCH 7/9] Removed unused methods, added preconditions --- .../org/ros/internal/node/DefaultNode.java | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/rosjava/src/main/java/org/ros/internal/node/DefaultNode.java b/rosjava/src/main/java/org/ros/internal/node/DefaultNode.java index ef054ced..26617657 100644 --- a/rosjava/src/main/java/org/ros/internal/node/DefaultNode.java +++ b/rosjava/src/main/java/org/ros/internal/node/DefaultNode.java @@ -17,10 +17,13 @@ package org.ros.internal.node; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import org.apache.commons.lang3.StringUtils; import org.ros.Parameters; import org.ros.concurrent.CancellableLoop; import org.ros.concurrent.ListenerGroup; import org.ros.concurrent.SignalRunnable; +import org.ros.exception.DuplicateServiceException; import org.ros.exception.RemoteException; import org.ros.exception.ServiceNotFoundException; import org.ros.internal.message.Message; @@ -184,7 +187,7 @@ public void onMasterRegistrationSuccess(Publisher registrant) { return; } - boolean useSimTime = false; + final boolean useSimTime; try { useSimTime = parameterTree.has(Parameters.USE_SIM_TIME) @@ -244,7 +247,7 @@ private MessageSerializer newServiceResponseSerializer(St @SuppressWarnings("unchecked") private MessageDeserializer newServiceResponseDeserializer(String serviceType) { - return this.nodeConfiguration.getMessageSerializationFactory() + return this.nodeConfiguration.getMessageSerializationFactory() .newServiceResponseDeserializer(serviceType); } @@ -265,8 +268,7 @@ public Publisher newPublisher(GraphName topicName, String final GraphName resolvedTopicName = resolveName(topicName); final TopicDescription topicDescription = this.nodeConfiguration.getTopicDescriptionFactory().newFromType(messageType); - final TopicDeclaration topicDeclaration = - TopicDeclaration.newFromTopicName(resolvedTopicName, topicDescription, null); + final TopicDeclaration topicDeclaration = TopicDeclaration.newFromTopicName(resolvedTopicName, topicDescription, null); final org.ros.message.MessageSerializer serializer = newMessageSerializer(messageType); return this.publisherFactory.newOrExisting(topicDeclaration, serializer); } @@ -284,10 +286,8 @@ public Subscriber newSubscriber(GraphName topicName, Stri @Override public Subscriber newSubscriber(GraphName topicName, String messageType, TransportHints transportHints) { final GraphName resolvedTopicName = resolveName(topicName); - final TopicDescription topicDescription = - this.nodeConfiguration.getTopicDescriptionFactory().newFromType(messageType); - final TopicDeclaration topicDeclaration = - TopicDeclaration.newFromTopicName(resolvedTopicName, topicDescription, transportHints); + final TopicDescription topicDescription = this.nodeConfiguration.getTopicDescriptionFactory().newFromType(messageType); + final TopicDeclaration topicDeclaration = TopicDeclaration.newFromTopicName(resolvedTopicName, topicDescription, transportHints); final MessageDeserializer deserializer = newMessageDeserializer(messageType); final Subscriber subscriber = this.subscriberFactory.newOrExisting(topicDeclaration, deserializer); return subscriber; @@ -305,8 +305,17 @@ public Subscriber newSubscriber(String topicName, String @Override public ServiceServer newServiceServer(GraphName serviceName, String serviceType, - ServiceResponseBuilder responseBuilder) { + ServiceResponseBuilder responseBuilder) { + Preconditions.checkNotNull(responseBuilder, "ResponseBuilder should not be null"); + Preconditions.checkNotNull(serviceName, "serviceName should not be null"); + Preconditions.checkArgument(StringUtils.isNotBlank(serviceType), "serviceType should not be blank"); + + final GraphName resolvedServiceName = resolveName(serviceName); +// final URI uri = lookupServiceUri(resolvedServiceName); +// if (uri != null) { +// throw new DuplicateServiceException("Service " + resolvedServiceName + " is already registered with URI: " + uri); +// } // TODO(damonkohler): It's rather non-obvious that the URI will be // created later on the fly. final ServiceIdentifier identifier = new ServiceIdentifier(resolvedServiceName, null); @@ -321,13 +330,15 @@ public ServiceServer newServiceServ @Override public ServiceServer newServiceServer(String serviceName, String serviceType, - ServiceResponseBuilder responseBuilder) { + ServiceResponseBuilder responseBuilder) { + Preconditions.checkArgument(StringUtils.isNotBlank(serviceName), "serviceName should not be blank"); + return newServiceServer(GraphName.of(serviceName), serviceType, responseBuilder); } @SuppressWarnings("unchecked") @Override - public ServiceServer getServiceServer(GraphName serviceName) { + public ServiceServer getServiceServer(GraphName serviceName) { return (ServiceServer) serviceManager.getServer(serviceName); } From 7b12f346435c25d5884e5b801148c0a78517035b Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 19:45:49 +0300 Subject: [PATCH 8/9] Can wait for start --- .../java/org/ros/node/ServiceClientNode.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/rosjava/src/main/java/org/ros/node/ServiceClientNode.java b/rosjava/src/main/java/org/ros/node/ServiceClientNode.java index 708680a7..1eace99b 100644 --- a/rosjava/src/main/java/org/ros/node/ServiceClientNode.java +++ b/rosjava/src/main/java/org/ros/node/ServiceClientNode.java @@ -23,6 +23,9 @@ import org.ros.namespace.GraphName; import org.ros.node.service.ServiceClient; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + /** * A {@link NodeMain} which provides a service client * @@ -30,6 +33,7 @@ */ public class ServiceClientNode extends AbstractNodeMain { private final GraphName graphName; + private CountDownLatch countDownLatch = new CountDownLatch(1); /** * Getter for serviceClient @@ -60,6 +64,7 @@ public void onStart(final ConnectedNode connectedNode) { } catch (final ServiceNotFoundException exception) { throw new RuntimeException(exception); } + this.countDownLatch.countDown(); } @Override @@ -73,6 +78,35 @@ public void onShutdown(Node node) { } } + /** + * Awaits for the Node to start. + * The thread however can be interrupted or it can return if the Node has shutdown. + * + * @return true if the Node is started and the service client is connected + * + * @throws InterruptedException + */ + public boolean awaitConnection() throws InterruptedException { + this.countDownLatch.await(); + return this.serviceClient != null && this.serviceClient.isConnected(); + } + + /** + * Awaits for the Node to start. + * The thread however can be interrupted or it can return if the Node has shutdown. + * + * @param time + * @param unit + * + * @return true if the Node is started and the service client is connected + * + * @throws InterruptedException + */ + public boolean awaitConnection(final long time, final TimeUnit unit) throws InterruptedException { + return this.countDownLatch.await(time, unit) && + this.serviceClient != null && this.serviceClient.isConnected(); + } + @Override public void onShutdownComplete(Node node) { } From 6572fe7272d628953621a2555b4a8a65f6b69db8 Mon Sep 17 00:00:00 2001 From: Spyros <21125056+SpyrosKou@users.noreply.github.com> Date: Sat, 18 Jun 2022 20:03:04 +0300 Subject: [PATCH 9/9] Added tests for issue #272 See https://github.com/rosjava/rosjava_core/issues/272 --- .../node/service/ServiceIntegrationTest.java | 212 +++++++++++++++++- 1 file changed, 204 insertions(+), 8 deletions(-) diff --git a/rosjava/src/test/java/org/ros/node/service/ServiceIntegrationTest.java b/rosjava/src/test/java/org/ros/node/service/ServiceIntegrationTest.java index df4f7040..188516c1 100644 --- a/rosjava/src/test/java/org/ros/node/service/ServiceIntegrationTest.java +++ b/rosjava/src/test/java/org/ros/node/service/ServiceIntegrationTest.java @@ -16,15 +16,19 @@ package org.ros.node.service; +import org.junit.Assert; import org.junit.Test; import org.ros.RosTest; import org.ros.exception.*; import org.ros.namespace.GraphName; import org.ros.node.AbstractNodeMain; import org.ros.node.ConnectedNode; +import org.ros.node.ServiceClientNode; +import rosjava_test_msgs.AddTwoIntsResponse; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.*; @@ -34,6 +38,8 @@ public class ServiceIntegrationTest extends RosTest { private static final String SERVICE_NAME = "/add_two_ints"; + private static final String SERVER_NAME = "server"; + private static final String CLIENT = "client"; @Test public void testPesistentServiceConnection() throws Exception { @@ -42,7 +48,7 @@ public void testPesistentServiceConnection() throws Exception { nodeMainExecutor.execute(new AbstractNodeMain() { @Override public GraphName getDefaultNodeName() { - return GraphName.of("server"); + return GraphName.of(SERVER_NAME); } @Override @@ -54,7 +60,8 @@ public void onStart(final ConnectedNode connectedNode) { rosjava_test_msgs.AddTwoInts._TYPE, (request, response) -> response.setSum(request.getA() + request.getB())); try { - connectedNode.newServiceServer(SERVICE_NAME, rosjava_test_msgs.AddTwoInts._TYPE, null); + connectedNode.newServiceServer(SERVICE_NAME, rosjava_test_msgs.AddTwoInts._TYPE, (a, b) -> { + }); fail(); } catch (DuplicateServiceException e) { // Only one ServiceServer with a given name can be created. @@ -69,7 +76,7 @@ public void onStart(final ConnectedNode connectedNode) { nodeMainExecutor.execute(new AbstractNodeMain() { @Override public GraphName getDefaultNodeName() { - return GraphName.of("client"); + return GraphName.of(CLIENT); } @Override @@ -85,12 +92,12 @@ public void onStart(ConnectedNode connectedNode) { } catch (ServiceNotFoundException e) { throw new RosRuntimeException(e); } - final rosjava_test_msgs.AddTwoIntsRequest request = serviceClient.newMessage(); + final rosjava_test_msgs.AddTwoIntsRequest request = serviceClient.newMessage(); { // final rosjava_test_msgs.AddTwoIntsRequest request = serviceClient.newMessage(); request.setA(2); request.setB(2); - serviceClient.call(request, new ServiceResponseListener() { + serviceClient.call(request, new ServiceResponseListener<>() { @Override public void onSuccess(rosjava_test_msgs.AddTwoIntsResponse response) { assertEquals(response.getSum(), 4); @@ -108,7 +115,7 @@ public void onFailure(RemoteException e) { // final rosjava_test_msgs.AddTwoIntsRequest request = serviceClient.newMessage(); request.setA(3); request.setB(3); - serviceClient.call(request, new ServiceResponseListener() { + serviceClient.call(request, new ServiceResponseListener<>() { @Override public void onSuccess(rosjava_test_msgs.AddTwoIntsResponse response) { assertEquals(response.getSum(), 6); @@ -127,6 +134,195 @@ public void onFailure(RemoteException e) { assertTrue(latch.await(1, TimeUnit.SECONDS)); } + /** + * Test the behaviour discussed in https://github.com/rosjava/rosjava_core/issues/272 + * Creating two service servers with the same name from the same nodes is prevented with a {@link DuplicateServiceException} + * + * See also this comment https://github.com/rosjava/rosjava_core/issues/272#issuecomment-1159455438 + * @throws Exception + */ + @Test + public void testMultipleServiceDeclarationSameNode() throws Exception { + final AtomicInteger dualServiceDeclarationDetected = new AtomicInteger(0); + final CountDownServiceServerListener countDownServiceServerListener = + CountDownServiceServerListener.newFromCounts(1, 0, 0, 2); + + final AbstractNodeMain node = new AbstractNodeMain() { + @Override + public GraphName getDefaultNodeName() { + return GraphName.of(SERVER_NAME); + } + + @Override + public void onStart(final ConnectedNode connectedNode) { + + try { + final ServiceServer serviceServer1 = + connectedNode.newServiceServer( + SERVICE_NAME, + rosjava_test_msgs.AddTwoInts._TYPE, + (request, response) -> response.setSum(request.getA() + request.getB())); + serviceServer1.addListener(countDownServiceServerListener); + } catch (DuplicateServiceException e) { + // Only one ServiceServer with a given name can be created. + dualServiceDeclarationDetected.incrementAndGet(); + } + try { + final ServiceServer serviceServer1 = + connectedNode.newServiceServer( + SERVICE_NAME, + rosjava_test_msgs.AddTwoInts._TYPE, + (request, response) -> response.setSum(request.getA() + request.getB())); + serviceServer1.addListener(countDownServiceServerListener); + } catch (DuplicateServiceException e) { + // Only one ServiceServer with a given name can be created. + dualServiceDeclarationDetected.incrementAndGet(); + } + + + } + }; + this.nodeMainExecutor.execute(node, nodeConfiguration); + + assertTrue(countDownServiceServerListener.awaitMasterRegistrationSuccess(2, TimeUnit.SECONDS)); + if (dualServiceDeclarationDetected.get() != 1) { + fail("Dual registration not detected"); + } + + } + + + /** + * Test the behaviour discussed in https://github.com/rosjava/rosjava_core/issues/272 + * Creating two service servers with the same name from two different nodes results in the first service server being called even from new clients. + * + * See also this comment https://github.com/rosjava/rosjava_core/issues/272#issuecomment-1159455438 + * @throws Exception + */ + @Test + public void testMultipleServiceDeclarationDifferentNodes() throws Exception { + final CountDownLatch clientsCompleted = new CountDownLatch(4); + final CountDownLatch server1started=new CountDownLatch(1); + final CountDownLatch server2started=new CountDownLatch(1); + final CountDownServiceServerListener countDownServiceServerListener = + CountDownServiceServerListener.newFromCounts(1, 0, 0, 2); + + final AbstractNodeMain node1 = new AbstractNodeMain() { + @Override + public GraphName getDefaultNodeName() { + return GraphName.of(SERVER_NAME + 1); + } + + @Override + public void onStart(final ConnectedNode connectedNode) { + try { + final ServiceServer serviceServer1 = + connectedNode.newServiceServer( + SERVICE_NAME, + rosjava_test_msgs.AddTwoInts._TYPE, + (request, response) -> response.setSum(1)); + serviceServer1.addListener(countDownServiceServerListener); + } catch (DuplicateServiceException e) { + // Only one ServiceServer with a given name can be created. + + } + server1started.countDown(); + } + + + }; + this.nodeMainExecutor.execute(node1, nodeConfiguration); + server1started.await(); + final ServiceClientNode client1 = new ServiceClientNode<>(SERVER_NAME + CLIENT + 1, SERVICE_NAME, rosjava_test_msgs.AddTwoInts._TYPE); + + this.nodeMainExecutor.execute(client1, nodeConfiguration); + client1.awaitConnection(); + client1.getServiceClient().call(client1.getServiceClient().newMessage(), new ServiceResponseListener<>() { + @Override + public void onSuccess(AddTwoIntsResponse response) { + Assert.assertEquals(response.getSum(), 1); + clientsCompleted.countDown(); + } + + @Override + public void onFailure(RemoteException e) { + + } + }); + + + final AbstractNodeMain node2 = new AbstractNodeMain() { + @Override + public GraphName getDefaultNodeName() { + return GraphName.of(SERVER_NAME + 2); + } + + @Override + public void onStart(final ConnectedNode connectedNode) { + + try { + final ServiceServer serviceServer1 = + connectedNode.newServiceServer( + SERVICE_NAME, + rosjava_test_msgs.AddTwoInts._TYPE, + (request, response) -> response.setSum(2)); + serviceServer1.addListener(countDownServiceServerListener); + } catch (DuplicateServiceException e) { + // Only one ServiceServer with a given name can be created. + + } + server2started.countDown(); + } + }; + + this.nodeMainExecutor.execute(node2, nodeConfiguration); + server2started.await(); + client1.getServiceClient().call(client1.getServiceClient().newMessage(), new ServiceResponseListener<>() { + @Override + public void onSuccess(AddTwoIntsResponse response) { + Assert.assertEquals(response.getSum(), 1); + clientsCompleted.countDown(); + } + + @Override + public void onFailure(RemoteException e) { + + } + }); + + + final ServiceClientNode client2 = new ServiceClientNode<>(SERVER_NAME + CLIENT + 2, SERVICE_NAME, rosjava_test_msgs.AddTwoInts._TYPE); + + this.nodeMainExecutor.execute(client2, nodeConfiguration); + client2.awaitConnection(); + client2.getServiceClient().call(client2.getServiceClient().newMessage(), new ServiceResponseListener<>() { + @Override + public void onSuccess(AddTwoIntsResponse response) { + Assert.assertEquals(response.getSum(), 2); + clientsCompleted.countDown(); + } + + @Override + public void onFailure(RemoteException e) { + + } + }); + client1.getServiceClient().call(client1.getServiceClient().newMessage(), new ServiceResponseListener<>() { + @Override + public void onSuccess(AddTwoIntsResponse response) { + Assert.assertEquals(response.getSum(), 1); + clientsCompleted.countDown(); + } + + @Override + public void onFailure(RemoteException e) { + + } + }); + clientsCompleted.await(); + } + + @Test public void testRequestFailure() throws Exception { final String errorMessage = "Error!"; @@ -135,7 +331,7 @@ public void testRequestFailure() throws Exception { nodeMainExecutor.execute(new AbstractNodeMain() { @Override public GraphName getDefaultNodeName() { - return GraphName.of("server"); + return GraphName.of(SERVER_NAME); } @Override @@ -158,7 +354,7 @@ public void onStart(ConnectedNode connectedNode) { nodeMainExecutor.execute(new AbstractNodeMain() { @Override public GraphName getDefaultNodeName() { - return GraphName.of("client"); + return GraphName.of(CLIENT); } @Override