Routable metadata #545

jwillmartin · 2023-06-26T15:00:49Z

PR Details

Description

Adds PSID metadata to BSMs and SRMs that are sent to the Message Receiver plugin. If "RouteMessage" is enabled, the messages will be available for the Immediate Forward plugin to use.

Additional changes were made to build process to speed up process.

Related Issue

Motivation and Context

How Has This Been Tested?

Sent BSMs and SRMs to Message Receiver plugin. Messages were transmitted by Immediate Forward plugin when "RouteMessage" was enabled and not routed when disabled. Previously, messages were skipped or not routed, respectively.

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.
V2XHUB Contributing Guide
I have added tests to cover my changes.
All new and existing tests passed.

modified: src/build.sh modified: src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugin.cpp

…n.cpp

modified: src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugin.cpp

dan-du-car · 2023-06-26T15:23:48Z

src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugin.cpp

@@ -263,6 +263,7 @@ void MessageReceiverPlugin::OnMessageReceived(routeable_message &msg)
 								}

 								sendMsg = encode(encodedBsm, bsm);
+								msgPSID = 0x20;


Do different messages have different msgPSID? Does a PSID ever change for a message type? I am not sure if we want to hardcode these values for different msgPSID?

These two I added were under the cases for BSMs and SRMs that are detected by the plugin. Others are kept at null.
If we want to add more messages to be routable, we can think about where to add their PSIDs. Right now it's added within the specific plugins that generate the messages.

Do we have a enum or struct to consolidate all the PSID? Then use it here in this class? It might be easy to read.

We don't, but we could make one. What do you think about adding it here? https://github.com/usdot-fhwa-OPS/V2X-Hub/blob/develop/src/tmx/TmxApi/tmx/TmxApiMessages.h

yep, I think that is a good location

@dan-du-car Changes made

dan-du-car · 2023-06-26T15:24:32Z

src/build.sh

 make install
 popd
 popd

 pushd v2i-hub
 cmake -Bbuild  -DCMAKE_PREFIX_PATH=\"/usr/local/share/tmx\;\/opt/carma/cmake\;\" -DqserverPedestrian_DIR=/usr/local/share/qserverPedestrian/cmake -Dv2xhubWebAPI_DIR=/usr/local/share/v2xhubWebAPI/cmake/ -DCMAKE_CXX_FLAGS="${COVERAGE_FLAGS}" -DCMAKE_C_FLAGS="${COVERAGE_FLAGS}" -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" .
 pushd build
+make -j${numCPU}


Why do we need this ${numCPU}? Whese is this env variable from?

The env is for using the number of cores available on the local machine. This allows for building in parallel. Since we're using Ubuntu as the docker image, this command to get the env variable will always work.

Do you have a link explain how this command "make -j${numCPU}" allow building in parallel. Sorry, I have not seen this before and am curious how this works. Also if we are running this inside a base image ubuntu, how does that get the host machine hardware specs, like num of CPUs?

Yep! Section 5.4 here: https://www.gnu.org/software/make/manual/make.html#Parallel
or run: make --help and you'll see a short description there

Yeah, it's still linked to the host machine, so it's able to find the number of cores available. Then this would run one job per core.

I see, in section 5.4: If there is nothing looking like an integer after the ‘-j’ option, there is no limit on the number of job slots.

Oh, numCPU set by numproc

dan-du-car · 2023-06-26T15:28:39Z

src/v2i-hub/MessageReceiverPlugin/manifest.json

@@ -29,9 +29,9 @@
 			"description":"Port for the incoming message network connection."
 		},
 		{
-			"key":"RouteDSRC",
+			"key":"RouteMessage",


Why renaming this boolean?

Only changing the user-facing name. Idea is to remove "DSRC" to avoid confusion with communication hardware used

Is this boolean only for enable route for J2735Message? Some other internal v2xhub message like State, log on the UI are not controlled by this boolean? Maybe narrow down the Message to a specific type?

Nope, just here. It's specific to some logic for broadcasting messages received by this plugin.
I could change it to RouteJ2735

make sense to me

paulbourelly999 · 2023-06-27T16:33:25Z

src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugin.cpp

@@ -280,6 +281,7 @@ void MessageReceiverPlugin::OnMessageReceived(routeable_message &msg)
 										ntohl(*((uint32_t*)&(bytes.data()[24]))),
 										ntohl(*((uint32_t*)&(bytes.data()[28]))));
 								sendMsg = encode(encodedSrm, srm);
+								msgPSID = 0xE0000016;


There is a code smell on this line about an implicit conversion. Please check this out in the sonar scanner report.

@paulbourelly999 Addressed

modified: src/tmx/TmxApi/tmx/TmxApiMessages.h modified: src/v2i-hub/ImmediateForwardPlugin/src/ImmediateForwardPlugin.cpp modified: src/v2i-hub/MessageReceiverPlugin/manifest.json modified: src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugin.cpp

…n.cpp

paulbourelly999 · 2023-07-05T13:38:32Z

src/tmx/TmxApi/tmx/IvpMessage.h

@@ -64,7 +64,7 @@ typedef enum {
 */
 IvpMessage *ivpMsg_create(const char *type, const char *subtype, const char *encoding, IvpMsgFlags flags, cJSON *payload);

-IvpMessage *ivpMsg_addDsrcMetadata(IvpMessage *msg, int channel, int psid);
+IvpMessage *ivpMsg_addDsrcMetadata(IvpMessage *msg, int psid, int channel);


Is there a reason we reversed these and can we ensure this does not have any unintentional impacts.

@paulbourelly999 Did it to match the changes made here:
https://github.com/usdot-fhwa-OPS/V2X-Hub/pull/433/files#diff-ea7aa329ba2d3d9e2dbe5e75039c3eadbb3886943f4596c9e0fbacb5e8af8b0c

Did some quick tests with SPaT, MAP, PSM, and TIM to make sure they were still getting the right PSIDs set

paulbourelly999

Looks good.

jwillmartin added 4 commits June 23, 2023 17:10

modified: ext/build.sh

711080c

modified: src/build.sh modified: src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugin.cpp

modified: scripts/install_dependencies.sh

cf28cae

modified: src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugi…

63ae453

…n.cpp

modified: src/v2i-hub/MessageReceiverPlugin/manifest.json

32b1c05

modified: src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugin.cpp

jwillmartin requested review from dan-du-car and paulbourelly999 June 26, 2023 15:00

dan-du-car reviewed Jun 26, 2023

View reviewed changes

paulbourelly999 reviewed Jun 27, 2023

View reviewed changes

jwillmartin added 2 commits June 30, 2023 15:33

modified: src/v2i-hub/MessageReceiverPlugin/src/MessageReceiverPlugi…

a705342

…n.cpp

paulbourelly999 reviewed Jul 5, 2023

View reviewed changes

paulbourelly999 approved these changes Jul 5, 2023

View reviewed changes

paulbourelly999 merged commit 37eeb9b into develop Jul 5, 2023
8 checks passed

paulbourelly999 deleted the routable-metadata branch July 5, 2023 15:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Routable metadata #545

Routable metadata #545

jwillmartin commented Jun 26, 2023

dan-du-car Jun 26, 2023

jwillmartin Jun 26, 2023

dan-du-car Jun 26, 2023

jwillmartin Jun 26, 2023

dan-du-car Jun 26, 2023

jwillmartin Jun 30, 2023

dan-du-car Jun 26, 2023

jwillmartin Jun 26, 2023 •

edited

Loading

dan-du-car Jun 26, 2023

jwillmartin Jun 26, 2023 •

edited

Loading

dan-du-car Jun 26, 2023 •

edited

Loading

dan-du-car Jun 26, 2023

jwillmartin Jun 26, 2023

dan-du-car Jun 26, 2023

jwillmartin Jun 26, 2023

dan-du-car Jun 26, 2023

jwillmartin Jun 26, 2023

dan-du-car Jun 26, 2023

paulbourelly999 Jun 27, 2023

jwillmartin Jun 30, 2023

paulbourelly999 Jul 5, 2023

jwillmartin Jul 5, 2023

paulbourelly999 left a comment

Routable metadata #545

Routable metadata #545

Conversation

jwillmartin commented Jun 26, 2023

PR Details

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwillmartin Jun 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwillmartin Jun 26, 2023 • edited Loading

Choose a reason for hiding this comment

dan-du-car Jun 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulbourelly999 left a comment

Choose a reason for hiding this comment

jwillmartin Jun 26, 2023 •

edited

Loading

jwillmartin Jun 26, 2023 •

edited

Loading

dan-du-car Jun 26, 2023 •

edited

Loading