Add Sensor/Detector Registration to Infrastructure Ambassador #152

paulbourelly999 · 2023-07-27T18:44:29Z

PR Details

Description

Added additional information to the Infrastructure Registration Message including :

Port for transmitting relevant interactions
List of sensors/detectors to register for infrastructure instance

Added functionality to process new registration message, trigger sensors/detectors registration interactions to prompt creation of sensors/detectors.
Added functionality to process incoming Detected Object interactions from sensors and forward them via the new interactions port to registered infrastructure instances based on the sensor that reports the detection.

Related Issue

#211

Motivation and Context

New functionality will be used to create sensor in CARLA and retrieve detection information for the VRU use case

How Has This Been Tested?

Unit tested and Integration testing with XIL Cloud setup
Only tested that CDASim properly processes Infrastructure registration and still forwards time sync messages. Further testing for detection interaction processing is advised once CARLA Ambassador work completes.

Types of changes

Defect fix (non-breaking change that fixes an issue)
New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that cause existing functionality to change)

Checklist:

I have added any new packages to the sonar-scanner.properties file
My change requires a change to the documentation.
[] I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
CARMA Contributing Guide
I have added tests to cover my changes.
All new and existing tests passed.

Added DetectedObject and Sensor Updated Infrastructure Registration to include Sensors Added interact for both DetectedObject and Sensor registration Removed Infrastructure Time interface and moved functionality to Infrastructure Instance manager

replace List with ArrayList in method signature/ class members

…thod

Renamed Sensor to detector

Moved unit tests for mosiac objects into object lib

Added unit test for DetectedObject Fixed DetectionType label

…yncMessage to InfrastructureInstanceManager

+ Fixed stop call for Registration and Message Receiver

paulbourelly999 · 2023-08-01T03:26:45Z

...lation/lib/mosaic-carma-utils/src/main/java/gov/dot/fhwa/saxton/CarmaV2xMessageReceiver.java

@@ -41,7 +42,7 @@ public class CarmaV2xMessageReceiver implements Runnable {
    private Queue<Tuple<InetAddress, CarmaV2xMessage>> rxQueue = new LinkedList<>();
    private DatagramSocket listenSocket = null;
    private int listenPort;
-    private boolean running = true;
+    private AtomicBoolean running = new AtomicBoolean(false);


Same comment as with the InfrastructureMessageReceiver.

…tration

...e/src/main/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstance.java

co-simulation/fed/mosaic-infrastructure/pom.xml

...lation/lib/mosaic-carma-utils/src/main/java/gov/dot/fhwa/saxton/CarmaV2xMessageReceiver.java

...lib/mosaic-objects/src/main/java/org/eclipse/mosaic/lib/objects/detector/DetectedObject.java

.../lib/mosaic-objects/src/main/java/org/eclipse/mosaic/lib/objects/detector/DetectionType.java

...ation/lib/mosaic-objects/src/main/java/org/eclipse/mosaic/lib/objects/detector/Detector.java

...e/src/main/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstance.java

...ain/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstanceManager.java

...n/lib/mosaic-objects/src/main/java/org/eclipse/mosaic/lib/objects/detector/DetectorType.java

...on/lib/mosaic-objects/src/main/java/org/eclipse/mosaic/lib/objects/detector/Orientation.java

...imulation/lib/mosaic-objects/src/main/java/org/eclipse/mosaic/lib/objects/detector/Size.java

...-objects/src/main/java/org/eclipse/mosaic/lib/objects/detector/gson/DetectorTypeAdapter.java

...b/mosaic-objects/src/test/java/org/eclipse/mosaic/lib/objects/detector/DetectorTypeTest.java

...e/src/main/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstance.java

kjrush

Few minor things, but overall looks good to me!

kjrush · 2023-08-07T01:22:13Z

...e/src/main/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstance.java

@@ -156,7 +223,7 @@ public void setTimeSyncPort(int timeSyncPort) {
     *                     methods
     */
    public void bind() throws IOException {
-        rxMsgsSocket = new DatagramSocket();
+        socket = new DatagramSocket();


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.

kjrush · 2023-08-07T01:24:59Z

...e/src/main/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstance.java

+     * @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 {


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.

...ain/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstanceManager.java

kjrush · 2023-08-07T01:29:47Z

...ain/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstanceManager.java

+     * @param message java object containing message information
+     * @return bytes encoded from JSON string representation of object.
+     */
+    private byte[] encodeMsg(Object message) {


To echo above, it's not a big deal, but I'm not a huge fan of how broad this function signature is. If I'm tinkering in this class in the future, this function signature doesn't do much to inform me what type of stuff we're usually encoding or need to encode. I'd generally prefer narrow function signatures where possible.

kjrush · 2023-08-07T01:31:22Z

...ain/java/org/eclipse/mosaic/fed/infrastructure/ambassador/InfrastructureInstanceManager.java

+        }
+
+        for (InfrastructureInstance currentInstance : managedInstances.values()) {
+            currentInstance.sendTimeSyncMsg(encodeMsg(message));


This is somewhere I think we could just make sendTimeSyncMsg consume InfrastructureTimeMessage like I noted prior, then the entire encodeMsg function is no longer needed (or at least not needed here) and then I think the API is a little clearer in terms of intended use.

…nd send appropriate messages

sonarcloud · 2023-08-08T14:48:17Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

73.5% Coverage
1.1% Duplication

The version of Java (11.0.19) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

paulbourelly999 added 16 commits July 27, 2023 14:32

Initial Commit

69a8ca8

Added DetectedObject and Sensor Updated Infrastructure Registration to include Sensors Added interact for both DetectedObject and Sensor registration Removed Infrastructure Time interface and moved functionality to Infrastructure Instance manager

Added logic to trigger sensor registration interactions

2f201f4

replace List with ArrayList in method signature/ class members

Removed duplication in InfrastructureInstance by adding sendPacket me…

be88743

…thod

Addressed Sonar scan report issues

a5d58ec

Addressed final sonar code smell issues.

364fcae

Moved non-interactions to mosiac lib objects

c699981

Renamed Sensor to detector

Added unit test for DetectedObjectInteraction

d552372

Moved unit tests for mosiac objects into object lib

Removed unnecessary setters from DetectedObject

b6811ec

Added unit test for DetectedObject Fixed DetectionType label

Added unit test for DetectorRegistration

ebeb5b2

Fixed unit test and added hash method for sonar scanner code smells

8337c64

Fix unit test

bbd2003

Added unit test for processInteraction

4c88de3

Added unit test for InfrastructureInstance

1cec564

Added unit test coverage for DetectedObjectInteraction

8a78b85

Adding Unit test coverage for onDetectedObjectInteraction and onTimeS…

0dc36d8

…yncMessage to InfrastructureInstanceManager

Added unit test for Infrastructure Instance Manager

70dbb62

paulbourelly999 changed the title ~~Initial Commit~~ Add Sensor/Detector Registration to Infrastructure Ambassador Jul 31, 2023

paulbourelly999 added 11 commits July 31, 2023 08:51

Added unit test for size and orientation

beef07d

Added test for Detector setters

da3bf45

Added unit test for DetectionType enum

5b7cf1b

Added unit testing to InfrastructureMessageManager

e0b789a

+ Fixed stop call for Registration and Message Receiver

Added setters and unit test coverage to detected object

a1b82d4

Fixed sonar code smells

4bb37a4

Increase unit test coverage

f94bf76

Increase code coverage

07799c0

Increase code coverage

5d347d7

Added unit test coverage for InfrastructureInstance

8d0c6ea

Added unit test coverage

f2c5463

paulbourelly999 requested review from kjrush and dan-du-car July 31, 2023 21:40

paulbourelly999 commented Aug 1, 2023

View reviewed changes

paulbourelly999 added 2 commits July 31, 2023 23:30

Added comments and updated log statements

7c9b453

Added javadoc comments to DetectedObjectInteraction and DetectorRegis…

35c45a5

…tration