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] 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