Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Sensor/Detector Registration to Infrastructure Ambassador #152

Merged
merged 44 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from 41 commits
Commits
Show all changes
44 commits
Select commit Hold shift + click to select a range
69a8ca8
Initial Commit
paulbourelly999 Jul 27, 2023
2f201f4
Added logic to trigger sensor registration interactions
paulbourelly999 Jul 27, 2023
be88743
Removed duplication in InfrastructureInstance by adding sendPacket me…
paulbourelly999 Jul 27, 2023
a5d58ec
Addressed Sonar scan report issues
paulbourelly999 Jul 27, 2023
364fcae
Addressed final sonar code smell issues.
paulbourelly999 Jul 27, 2023
c699981
Moved non-interactions to mosiac lib objects
paulbourelly999 Jul 28, 2023
d552372
Added unit test for DetectedObjectInteraction
paulbourelly999 Jul 28, 2023
b6811ec
Removed unnecessary setters from DetectedObject
paulbourelly999 Jul 28, 2023
ebeb5b2
Added unit test for DetectorRegistration
paulbourelly999 Jul 28, 2023
8337c64
Fixed unit test and added hash method for sonar scanner code smells
paulbourelly999 Jul 28, 2023
bbd2003
Fix unit test
paulbourelly999 Jul 28, 2023
4c88de3
Added unit test for processInteraction
paulbourelly999 Jul 28, 2023
1cec564
Added unit test for InfrastructureInstance
paulbourelly999 Jul 28, 2023
8a78b85
Added unit test coverage for DetectedObjectInteraction
paulbourelly999 Jul 28, 2023
0dc36d8
Adding Unit test coverage for onDetectedObjectInteraction and onTimeS…
paulbourelly999 Jul 28, 2023
70dbb62
Added unit test for Infrastructure Instance Manager
paulbourelly999 Jul 31, 2023
beef07d
Added unit test for size and orientation
paulbourelly999 Jul 31, 2023
da3bf45
Added test for Detector setters
paulbourelly999 Jul 31, 2023
5b7cf1b
Added unit test for DetectionType enum
paulbourelly999 Jul 31, 2023
e0b789a
Added unit testing to InfrastructureMessageManager
paulbourelly999 Jul 31, 2023
a1b82d4
Added setters and unit test coverage to detected object
paulbourelly999 Jul 31, 2023
4bb37a4
Fixed sonar code smells
paulbourelly999 Jul 31, 2023
f94bf76
Increase unit test coverage
paulbourelly999 Jul 31, 2023
07799c0
Increase code coverage
paulbourelly999 Jul 31, 2023
5d347d7
Increase code coverage
paulbourelly999 Jul 31, 2023
8d0c6ea
Added unit test coverage for InfrastructureInstance
paulbourelly999 Jul 31, 2023
f2c5463
Added unit test coverage
paulbourelly999 Jul 31, 2023
0f9e37c
Added Javadoc comments
paulbourelly999 Aug 1, 2023
8fb2676
Added Javadoc comments to InfrastructureInstanceManager
paulbourelly999 Aug 1, 2023
b444173
Added Javadoc comments for InfrastructureMessageAmbassador
paulbourelly999 Aug 1, 2023
483fc99
Added Javadoc comments to infrastructure registration message
paulbourelly999 Aug 1, 2023
7c9b453
Added comments and updated log statements
paulbourelly999 Aug 1, 2023
35c45a5
Added javadoc comments to DetectedObjectInteraction and DetectorRegis…
paulbourelly999 Aug 1, 2023
0eaef9e
Added javadoc comments
paulbourelly999 Aug 1, 2023
90675be
Addressing PR comments
paulbourelly999 Aug 1, 2023
522233f
Adding Javadoc comment
paulbourelly999 Aug 2, 2023
e6ad177
Added dependency on mosiac-utils test jar
paulbourelly999 Aug 2, 2023
cee7b84
Fixed pom.xml
paulbourelly999 Aug 2, 2023
cd0f635
Added comments/formating/organize imports for InfrastructureInstanceTest
paulbourelly999 Aug 3, 2023
9a0fe43
Update enum fromLabel to ignore case
paulbourelly999 Aug 3, 2023
a837403
Updated InfrastructureMessageAmbassadorTest with inline comments
paulbourelly999 Aug 3, 2023
d3276a9
Rework InfrastructureInstance API to expose typed methods to encode a…
paulbourelly999 Aug 7, 2023
99038aa
Added null check for null pointer exception
paulbourelly999 Aug 8, 2023
66769eb
Added toString methods for logging
paulbourelly999 Aug 8, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions co-simulation/fed/mosaic-infrastructure/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
<artifactId>mosaic-objects</artifactId>
<version>${mosaic.version}</version>
</dependency>
<dependency>
<groupId>org.eclipse.mosaic</groupId>
<artifactId>mosaic-objects</artifactId>
<version>${mosaic.version}</version>
<type>test-jar</type>
Comment on lines +29 to +32
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This package is already included in the infrastructure ambassador pom as a dependency but I had to add the test-jar to gain access to Rules used to initialize singletons for unit testing. See GeoProjectionRule and IpResolverRule in the InfrastructureMessageAdapterTest.

<scope>test</scope>
</dependency>
<dependency>
<groupId>gov.dot.fhwa.saxton</groupId>
<artifactId>mosaic-carma-utils</artifactId>
dan-du-car marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,19 @@

package org.eclipse.mosaic.fed.infrastructure.ambassador;

import org.eclipse.mosaic.lib.geo.CartesianPoint;

import java.io.IOException;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.InetAddress;
import java.util.List;

import org.eclipse.mosaic.lib.geo.CartesianPoint;
import org.eclipse.mosaic.lib.objects.detector.Detector;

/**
* InfrastructureInstance class represents a physical instance of an
* infrastructure node in the simulated environment.
* It contains information about the infrastructure node such as its ID,
* infrastructure instance in the simulated environment.
* It contains information about the infrastructure instance such as its ID,
* location, target address, and ports.
dan-du-car marked this conversation as resolved.
Show resolved Hide resolved
*/
public class InfrastructureInstance {
Expand All @@ -35,40 +37,48 @@ public class InfrastructureInstance {
private InetAddress targetAddress;
private int rxMessagePort;
private int timeSyncPort;
private int simulatedInteractionPort;
dan-du-car marked this conversation as resolved.
Show resolved Hide resolved
private CartesianPoint location = null;
private DatagramSocket rxMsgsSocket = null;
private DatagramSocket socket = null;
private List<Detector> sensors;

/**
* Constructor for InfrastructureInstance
*
* @param infrastructureId the ID of the infrastructure node
* @param targetAddress the target IP address of the infrastructure node
* @param rxMessagePort the receive message port of the infrastructure node
* @param timeSyncPort the time synchronization port of the infrastructure
* node
* @param location the location of the infrastructure node in the
* simulated environment
*/
public InfrastructureInstance(String infrastructureId, InetAddress targetAddress,
int rxMessagePort, int timeSyncPort, CartesianPoint location) {
* @param infrastructureId the ID of the infrastructure instance.
* @param targetAddress the target IP address of the infrastructure instance.
* @param rxMessagePort the receive V2X message port of the infrastructure instance.
dan-du-car marked this conversation as resolved.
Show resolved Hide resolved
* @param timeSyncPort the receive time synchronization message port of the infrastructure.
*
* @param simulatedInteractionPort the receive simulated interaction message port of the infrastructure
* instance.
* @param location the location of the infrastructure instance in the
* simulated environment
*/
public InfrastructureInstance(String infrastructureId, InetAddress targetAddress, int rxMessagePort,
int timeSyncPort, int simulatedInteractionPort, CartesianPoint location, List<Detector> sensors) {
this.infrastructureId = infrastructureId;
this.targetAddress = targetAddress;
this.rxMessagePort = rxMessagePort;
this.timeSyncPort = timeSyncPort;
this.simulatedInteractionPort = simulatedInteractionPort;
this.location = location;
this.sensors = sensors;
}



/**
* Returns the target IP address of the infrastructure node
* Returns the target IP address of the infrastructure instance
*
* @return InetAddress the target IP address of the infrastructure node
* @return InetAddress the target IP address of the infrastructure instance
*/
public InetAddress getTargetAddress() {
return targetAddress;
}

/**
* Sets the target IP address of the infrastructure node
* Sets the target IP address of the infrastructure instance
*
* @param targetAddress the target IP address to set
*/
Expand All @@ -77,16 +87,16 @@ public void setTargetAddress(InetAddress targetAddress) {
}

/**
* Returns the location of the infrastructure node in the simulated environment
* Returns the location of the infrastructure instance in the simulated environment
*
* @return CartesianPoint the location of the infrastructure node
* @return CartesianPoint the location of the infrastructure instance
*/
public CartesianPoint getLocation() {
return this.location;
}

/**
* Sets the location of the infrastructure node in the simulated environment
* Sets the location of the infrastructure instance in the simulated environment
*
* @param location the location to set
*/
Expand All @@ -95,16 +105,16 @@ public void setLocation(CartesianPoint location) {
}

/**
* Returns the ID of the infrastructure node
* Returns the ID of the infrastructure instance
*
* @return String the ID of the infrastructure node
* @return String the ID of the infrastructure instance
*/
public String getInfrastructureId() {
return infrastructureId;
}

/**
* Sets the ID of the infrastructure node
* Sets the ID of the infrastructure instance
*
* @param infrastructureId the ID to set
*/
Expand All @@ -113,16 +123,16 @@ public void setInfrastructureId(String infrastructureId) {
}

/**
* Returns the receive message port of the infrastructure node
* Returns the receive message port of the infrastructure instance
*
* @return int the receive message port of the infrastructure node
* @return int the receive message port of the infrastructure instance
*/
public int getRxMessagePort() {
return rxMessagePort;
}

/**
* Sets the receive message port of the infrastructure node
* Sets the receive message port of the infrastructure instance
*
* @param rxMessagePort the port to set
*/
Expand All @@ -131,22 +141,79 @@ public void setRxMessagePort(int rxMessagePort) {
}

/**
* Returns the time synchronization port of the infrastructure node
* Returns the time synchronization port of the infrastructure instance
*
* @return int the time synchronization port of the infrastructure node
* @return int the time synchronization port of the infrastructure instance
*/
public int getTimeSyncPort() {
return timeSyncPort;
}

/**
* Sets the time synchronization port of the infrastructure node
* Sets the time synchronization port of the infrastructure instance
*
* @param timeSyncPort the port to set
*/
public void setTimeSyncPort(int timeSyncPort) {
this.timeSyncPort = timeSyncPort;
}

/**
* Returns the simulated interaction port of the infrastructure instance.
*
* @return int simulated interaction port of the infrastructure instance.
*/
public int getSimulatedInteractionPort() {
return simulatedInteractionPort;
}


/**
* Sets the simulated interaction port of the infrastructure instance.
*
* @param simulatedInteractionPort int simulated interaction port of the infrastructure
* instance.
*/
public void setSimulatedInteractionPort(int simulatedInteractionPort) {
dan-du-car marked this conversation as resolved.
Show resolved Hide resolved
this.simulatedInteractionPort = simulatedInteractionPort;
}


/**
* Returns list of Sensor/Detectors registered to this infrastructure instance.
*
* @return list of detectors registered to infrastructure instance.
*/
public List<Detector> getSensors() {
return sensors;
}


/**
* Sets list of Sensors/Detectors registered to infrastructure instance.
*
* @param sensors list of detectors registered to infrastructure instance.
*/
public void setSensors(List<Detector> sensors) {
this.sensors = sensors;
}


/**
* Method that returns boolean value based on whether any registered Sensor/Detector
* in the sensor list has a sensorId equivalent to the passed parameter.
*
* @param sensorId sensor ID to search for
* @return true if sensor with sensor ID exists in sensor list. False otherwise.
*/
public boolean containsSensor(String sensorId) {
for (Detector sensor : sensors) {
if (sensor.getSensorId().equals(sensorId) ) {
return true;
}
}
return false;
}

/**
* Creates a DatagramSocket object and binds it to this infrastructure
Expand All @@ -156,7 +223,7 @@ public void setTimeSyncPort(int timeSyncPort) {
* methods
*/
public void bind() throws IOException {
rxMsgsSocket = new DatagramSocket();
socket = new DatagramSocket();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renamed to socket since this Datagramsocket is used now to transmit Time Sync, V2X and Simulated Interactions. RxMsgsSocket indicates it relates the only to the rxMessagePort used for V2X messages.

Copy link

@dan-du-car dan-du-car Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bind() gives the vibe that we are creating a UDP server socket and trying to bind it to a particular port. But this infrastructure instance in fact acts as a series of UDP clients and send packet to the remote infrastructure identified by the targetAddress . Maybe we can just call this socket = new DatagramSocket inside the infrastructure instance initialization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not really part of the scope for this PR but I can look into a better naming convention for this method. It is a single client though, just different ports which can be assigned at the datagrampacket level not the socket level.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe connect is a better name or something along those lines? the primary reason - if I recall correctly - for having a separate method to init the DatagramSocket was to avoid a possible failure in the constructor (e.g. the ports are already in use) which is just a weird state to have. Not strongly attached to this implementation though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to connect

}

/**
Expand All @@ -167,27 +234,40 @@ public void bind() throws IOException {
* @throws IOException If there is an issue with the underlying socket object or
* methods
*/
public void sendMsgs(byte[] data) throws IOException {
if (rxMsgsSocket == null) {
throw new IllegalStateException("Attempted to send data before opening socket");
}
DatagramPacket packet = new DatagramPacket(data, data.length, targetAddress, rxMessagePort);
rxMsgsSocket.send(packet);

public void sendV2xMsg(byte[] data) throws IOException {
sendPacket(data, rxMessagePort);
}

/**
* Sends time sync data to the Infrastructure Device communications interface configured at construction time.
* Sends time sync data to the Infrastrucutre Instance Time Sync port.
*
* @param data The binary data to transmit
* @throws IOException If there is an issue with the underlying socket object or methods
*/
public void sendTimeSyncMsg(byte[] data) throws IOException {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we defined a class for the Time Sync message? I feel like it might be cleaner to have this (and the interaction one below) take in some type more meaningful than a byte[]. I think it make sense in the v2x message case because byte[] is actually the real type of a V2XMessage, but the time data and (I imagine) the interaction data are more higher-level than that.

sendPacket(data, timeSyncPort);
}
/**
* Sends time sync data to the Infrastrucutre Instance Simulated Interaction port.
*
* @param data The binary data to transmit
* @throws IOException If there is an issue with the underlying socket object or methods
*/
public void sendTimeSyncMsgs(byte[] data) throws IOException {
if (rxMsgsSocket == null) {
public void sendInteraction(byte[] data) throws IOException {
sendPacket(data, simulatedInteractionPort);
}
/**
* Method to send byte data to specified port using the infrastructure instance Datagramsocket.
*
* @param data byte data to send using Datagramsocket.
* @param port in integer format to send Datagram to.
* @throws IOException
*/
public void sendPacket(byte[] data, int port) throws IOException {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New method created to avoid code duplication between sendTimeSyncMsg sendV2xMsg and sendInteraction

if (socket == null) {
dan-du-car marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalStateException("Attempted to send data before opening socket");
}

DatagramPacket packet = new DatagramPacket(data, data.length, targetAddress, timeSyncPort);
rxMsgsSocket.send(packet);

DatagramPacket packet = new DatagramPacket(data, data.length, targetAddress, port);
socket.send(packet);
}
}
Loading