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

Routable metadata #545

Merged
merged 6 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
15 changes: 9 additions & 6 deletions ext/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,17 @@
# exit on errors
set -e

# Find number of cores available
numCPU=$(nproc)

# An OPENAPI based Qt webservice is needed by the plugins for http requests processing. A custom generated code using OPENAPI framework is located in GitHub.
pushd /tmp
QHTTPENGINE_VERSION=1.0.1
wget -O qhttpengine-${QHTTPENGINE_VERSION}.tar.gz https://github.com/nitroshare/qhttpengine/archive/refs/tags/${QHTTPENGINE_VERSION}.tar.gz
tar xvf qhttpengine-${QHTTPENGINE_VERSION}.tar.gz
cd qhttpengine-${QHTTPENGINE_VERSION}/
cmake .
make
make -j${numCPU}
make install
popd

Expand All @@ -20,7 +23,7 @@ wget -O date-${DATELIB_VERSION}.tar.gz https://github.com/HowardHinnant/date/arc
tar xvf date-${DATELIB_VERSION}.tar.gz
cd date-${DATELIB_VERSION}/
cmake .
make
make -j${numCPU}
make install
popd

Expand All @@ -29,19 +32,19 @@ ldconfig
# Server for the Qt webservice
pushd server
cmake .
make
make -j${numCPU}
make install
popd

pushd ccserver
cmake .
make
make -j${numCPU}
make install
popd

pushd pdclient
cmake .
make
make -j${numCPU}
make install
popd

Expand All @@ -50,6 +53,6 @@ pushd /tmp
git clone https://github.com/ckgt/NemaTode.git
cd NemaTode
cmake .
make
make -j${numCPU}
make install
popd
3 changes: 3 additions & 0 deletions scripts/install_dependencies.sh
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,12 @@ LIBRARY_DEPENDENCIES=" \
# install all things needed for deployment, always done
apt-get install -y $DEPENDENCIES ${LIBRARY_DEPENDENCIES}

numCPU=$(nproc)

# install gtest
cd /usr/src/googletest/
mkdir -p build/
cd build
cmake ..
make -j${numCPU}
make install
5 changes: 4 additions & 1 deletion src/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ set -e
# script executes all tmx and v2i build and coverage steps so that they can be singularly
# wrapped by the sonarcloud build-wrapper

numCPU=$(nproc)

RELEASE_BUILD=0
if [ "$1" = "release" ]; then
RELEASE_BUILD=1
Expand All @@ -36,14 +38,15 @@ fi
pushd tmx
cmake -Bbuild -DCMAKE_PREFIX_PATH=\"/usr/local/share/tmx\;\/opt/carma/cmake\;\" -DCMAKE_CXX_FLAGS="${COVERAGE_FLAGS}" -DCMAKE_C_FLAGS="${COVERAGE_FLAGS}" -DCMAKE_BUILD_TYPE="${BUILD_TYPE}" .
pushd build
make
make -j${numCPU}
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}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@jwillmartin jwillmartin Jun 26, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@jwillmartin jwillmartin Jun 26, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@dan-du-car dan-du-car Jun 26, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, numCPU set by numproc

make install
popd
popd
4 changes: 2 additions & 2 deletions src/v2i-hub/MessageReceiverPlugin/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@
"description":"Port for the incoming message network connection."
},
{
"key":"RouteDSRC",
"key":"RouteMessage",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why renaming this boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

make sense to me

"default":"false",
"description":"Set the flag to route a received J2735 message."
"description":"Set the flag to route/broadcast a received J2735 message to TMX Core."
},
{
"key":"EnableSimulatedBSM",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ void MessageReceiverPlugin::OnMessageReceived(routeable_message &msg)
BsmEncodedMessage encodedBsm;
SrmEncodedMessage encodedSrm;


uint16_t msgPSID = NULL;

if (msg.get_type() == "Unknown" && msg.get_subtype() == "Unknown")
{
Expand Down Expand Up @@ -263,6 +263,7 @@ void MessageReceiverPlugin::OnMessageReceived(routeable_message &msg)
}

sendMsg = encode(encodedBsm, bsm);
msgPSID = 0x20;
Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

yep, I think that is a good location

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dan-du-car Changes made

if (!simBSM) return;
}
break;
Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@paulbourelly999 Addressed


}
break;
Expand Down Expand Up @@ -330,6 +332,7 @@ void MessageReceiverPlugin::OnMessageReceived(routeable_message &msg)
if (routeDsrc)
{
sendMsg->set_flags(IvpMsgFlags_RouteDSRC);
sendMsg->addDsrcMetadata(msgPSID);
}
else
{
Expand All @@ -345,7 +348,7 @@ void MessageReceiverPlugin::UpdateConfigSettings()
lock_guard<mutex> lock(syncLock);

// Atomic flags
GetConfigValue("RouteDSRC", routeDsrc);
GetConfigValue("RouteMessage", routeDsrc);
GetConfigValue("EnableSimulatedBSM", simBSM);
GetConfigValue("EnableSimulatedSRM", simSRM);
GetConfigValue("EnableSimulatedLocation", simLoc);
Expand Down