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

Update DSRC driver to support C1T V2X use on ARM #117

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
14 changes: 6 additions & 8 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
# Copyright (C) 2018-2021 LEIDOS.
#
Copy link
Contributor

Choose a reason for hiding this comment

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

(Note: This if for Line 1 of the file) Let's update the copyright here (i.e. Copyright (C) 2018-2024 LEIDOS.) and in other updated files.

#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not
# use this file except in compliance with the License. You may obtain a copy of
# the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations under
# the License.

FROM usdotfhwastoldev/carma-base:develop as setup
FROM usdotfhwastoldev/carma-base:develop-arm as setup
Copy link

Choose a reason for hiding this comment

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

Unrelated to this PR, but we should look at multi-platform images to avoid having this *-arm designators.

Copy link
Author

Choose a reason for hiding this comment

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

agreed, blame @JonSmet on this namespace
this can just be a stand-in name while we work something better out. Our Docker image tags in general should be reviewed and C1T may be another decent place to trial options

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed as well, I didn't realize docker images supported this but it looks like a good approach to consider.


ARG ROS1_PACKAGES=""
ENV ROS1_PACKAGES=${ROS1_PACKAGES}
ARG ROS2_PACKAGES=""
ENV ROS2_PACKAGES=${ROS2_PACKAGES}

Expand All @@ -24,15 +22,15 @@ COPY --chown=carma . /home/carma/src/
RUN ~/src/docker/checkout.bash
RUN ~/src/docker/install.sh

FROM usdotfhwastoldev/carma-base:develop
FROM usdotfhwastoldev/carma-base:develop-arm

ARG BUILD_DATE="NULL"
ARG VERSION="NULL"
ARG VCS_REF="NULL"

LABEL org.label-schema.schema-version="1.0"
LABEL org.label-schema.name="carma-cohda-dsrc-driver"
LABEL org.label-schema.description="Cohda DSRC On-Board Unit comms driver for the CARMA Platform"
LABEL org.label-schema.description="Cohda DSRC On-Board Unit comms driver for ARM processors"
LABEL org.label-schema.vendor="Leidos"
LABEL org.label-schema.version=${VERSION}
LABEL org.label-schema.url="https://highways.dot.gov/research/research-programs/operations/CARMA"
Expand Down
16 changes: 8 additions & 8 deletions docker/build-image.sh
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#!/bin/bash

# Copyright (C) 2018-2021 LEIDOS.
#
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not
# use this file except in compliance with the License. You may obtain a copy of
# the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
Expand All @@ -17,7 +17,7 @@
USERNAME=usdotfhwastol

cd "$(dirname "$0")"
IMAGE=$(basename `git rev-parse --show-toplevel`)
IMAGE=$USERNAME/carma-cohda-dsrc-driver:develop-arm

echo ""
echo "##### $IMAGE Docker Image Build Script #####"
Expand Down Expand Up @@ -84,7 +84,7 @@ while [[ $# -gt 0 ]]; do
else
echo "Unknown argument $arg..."
exit -1
fi
fi
shift
;;
esac
Expand All @@ -94,7 +94,7 @@ if [[ ! -z "$ROS1_PACKAGES$ROS2_PACKAGES" ]]; then
echo "Performing incremental build of image to rebuild packages: ROS1>> $ROS1_PACKAGES ROS2>> $ROS2_PACKAGES..."

echo "Updating Dockerfile references to use most recent image as base image"
# Trim of docker image LS command sourced from
# Trim of docker image LS command sourced from
# https://stackoverflow.com/questions/50625619/why-doesnt-the-cut-command-work-for-a-docker-image-ls-command
# Question Asker: Chris F
# Question Answerer: Arount
Expand Down Expand Up @@ -140,8 +140,8 @@ elif [[ $COMPONENT_VERSION_STRING = "SNAPSHOT" ]]; then
--build-arg VCS_REF=`git rev-parse --short HEAD` \
--build-arg BUILD_DATE=`date -u +”%Y-%m-%dT%H:%M:%SZ”` .
else
#The addition of --network=host was a fix for a DNS resolution error that occured
#when running the platform inside an Ubuntu 20.04 virtual machine. The error and possible soliutions are
#The addition of --network=host was a fix for a DNS resolution error that occured
#when running the platform inside an Ubuntu 20.04 virtual machine. The error and possible soliutions are
# discussed here: https://github.com/moby/moby/issues/41003
docker build --network=host --no-cache -t $USERNAME/$IMAGE:$COMPONENT_VERSION_STRING \
--build-arg VERSION="$COMPONENT_VERSION_STRING" \
Expand Down
15 changes: 5 additions & 10 deletions docker/checkout.bash
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
#!/bin/bash

# Copyright (C) 2018-2021 LEIDOS.
#
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not
# use this file except in compliance with the License. You may obtain a copy of
# the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
Expand Down Expand Up @@ -35,10 +35,5 @@ while [[ $# -gt 0 ]]; do
esac
done

if [[ "$BRANCH" = "develop" ]]; then
git clone https://github.com/usdot-fhwa-stol/carma-msgs.git ${dir}/src/CARMAMsgs --branch $BRANCH
git clone https://github.com/usdot-fhwa-stol/carma-utils.git ${dir}/src/CARMAUtils --branch $BRANCH
else
git clone https://github.com/usdot-fhwa-stol/carma-msgs.git ${dir}/src/CARMAMsgs --branch develop
git clone https://github.com/usdot-fhwa-stol/carma-utils.git ${dir}/src/CARMAUtils --branch develop
fi
git clone https://github.com/usdot-fhwa-stol/carma-msgs.git ${dir}/src/CARMAMsgs --branch develop
git clone https://github.com/usdot-fhwa-stol/carma-utils.git ${dir}/src/CARMAUtils --branch develop
20 changes: 5 additions & 15 deletions docker/install.sh
Original file line number Diff line number Diff line change
@@ -1,30 +1,20 @@
#!/bin/bash

# Copyright (C) 2018-2021 LEIDOS.
#
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may not
# use this file except in compliance with the License. You may obtain a copy of
# the License at
#
#
# http://www.apache.org/licenses/LICENSE-2.0
#
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations under
# the License.

if [[ ! -z "$ROS2_PACKAGES" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Were these code blocks (original lines 17-23 and 26-30) removed because issues were found with the incremental build process, or if it was to simplify install.sh? I could still see a use for supporting the incremental build approach if we'd like to test a small update to the dsrc_driver package without needing to rebuild the packages in carma-utils.

echo "Sourcing previous build for incremental build start point..."
source /opt/carma/install/setup.bash
else
echo "Sourcing base image for full build..."
source /opt/ros/foxy/setup.bash
fi
source /opt/ros/humble/install/setup.bash

cd ~/
if [[ ! -z "$ROS2_PACKAGES" ]]; then
colcon build --cmake-args -DCMAKE_BUILD_TYPE=Release --packages-above $ROS2_PACKAGES
else
colcon build --cmake-args -DCMAKE_BUILD_TYPE=Release
fi
colcon build --cmake-args -DCMAKE_BUILD_TYPE=Release --packages-up-to dsrc_driver
willjohnsonk marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 1 addition & 1 deletion dsrc_driver/config/DSRC.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ from dynamic_reconfigure.parameter_generator_catkin import *

gen = ParameterGenerator()

gen.add("dsrc_address", str_t, 0, "DSRC Address", "192.168.88.40")
gen.add("dsrc_address", str_t, 0, "DSRC Address", "192.168.217.16")
gen.add("dsrc_listening_port", int_t, 0, "DSRC Port", 1516)
gen.add("listening_port", int_t, 0, "Local Port", 5398)

Expand Down
4 changes: 2 additions & 2 deletions dsrc_driver/config/params.yaml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
dsrc_address: "192.168.88.40"
dsrc_address: "192.168.217.16"
dsrc_listening_port: 1516
listening_port: 5398
listening_port: 5398
94 changes: 94 additions & 0 deletions dsrc_driver/launch/drivers.launch.py
Copy link

Choose a reason for hiding this comment

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

Is this intended to replace the existing dsrc_driver.py?

Copy link
Author

Choose a reason for hiding this comment

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

I think it should probably be compiled into a single launch file. Right now the new drivers.launch.py is just used to configure and activate the dsrc node with a group action that targets dsrc_driver.launch.py. This is more of a proof of concept so something existing was used. I expect this driver will go through a major refactor as we move forward with the new CDA implementation anyways

Copy link
Contributor

@JonSmet JonSmet Mar 29, 2024

Choose a reason for hiding this comment

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

[UPDATE - I missed that drivers.launch.py calls dsrc_driver.py. The below comment can be disregarded]
I'm leaning towards the following. Thoughts on this?

  • Remove dsrc_driver.py from this directory
  • Rename this drivers.launch.py to dsrc_driver.launch.py
  • Add a TODO comment (maybe above process_configure_dsrc_driver_node) stating that we may want to consider standalone node(s) to manage the lifecycle of nodes in the C1T system, rather than configuring and activating directly from the launch file? Probably best to create a basic Jira story (we can add detail later) to mention in the comment if we do this

Copy link
Author

Choose a reason for hiding this comment

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

@JonSmet I assume you switched the names here but with that assumption it seems like this PR is mostly unnecessary. The only real addition here besides cleaning up some unused Docker code was to target the carma-base arm image in that case, and that name update still is in the air. I figured we won't be merging this anyways since the changed image names will break develop. All the actual code changes were in the msgs/utils dependencies that @adamlm worked on. DSRC probably would have worked out of the box if it wasn't for the ROS issues the Orin was having (which Anish and I think was caused by ROS2 DDS collisions on saxtonlab)

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch, I had an incorrect assumption and my previous comment can be disregarded (added a note at the beginning stating this as well).

Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# Copyright (C) 2024 LEIDOS.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

from launch import LaunchDescription
from launch.actions import IncludeLaunchDescription
from launch.launch_description_sources import PythonLaunchDescriptionSource
from launch.actions import DeclareLaunchArgument
from launch.substitutions import LaunchConfiguration
from launch_ros.substitutions import FindPackageShare
from launch.substitutions import EnvironmentVariable
from launch.substitutions import PythonExpression
from launch.conditions import IfCondition
from launch.actions import GroupAction
from launch_ros.actions import PushRosNamespace
from carma_ros2_utils.launch.get_log_level import GetLogLevel

import launch.actions
import launch.events

import launch_ros.actions
import launch_ros.events
import launch_ros.events.lifecycle
import lifecycle_msgs.msg

def generate_launch_description():
"""
Launch desired CARMA Messenger drivers
"""

env_log_levels = EnvironmentVariable('CARMA_ROS_LOGGING_CONFIG', default_value='{ "default_level" : "WARN" }')

configuration_delay = LaunchConfiguration('configuration_delay')
declare_configuration_delay_arg = DeclareLaunchArgument(
name ='configuration_delay', default_value='4.0')

drivers = LaunchConfiguration('drivers')
declare_drivers_arg = DeclareLaunchArgument(
name = 'drivers', default_value = 'dsrc_driver', description = "Desired drivers to launch specified by package name."
)

dsrc_group = GroupAction(
condition=IfCondition(PythonExpression(["'dsrc_driver' in '", drivers, "'.split()"])),
actions=[
PushRosNamespace(EnvironmentVariable('CARMA_INTR_NS', default_value='hardware_interface')),
IncludeLaunchDescription(
PythonLaunchDescriptionSource([ FindPackageShare('dsrc_driver'), '/launch/dsrc_driver.py']),
launch_arguments = {
'log_level' : GetLogLevel('dsrc_driver', env_log_levels),
}.items()
),
]
)

ros2_cmd = launch.substitutions.FindExecutable(name='ros2')

process_configure_dsrc_driver_node = launch.actions.ExecuteProcess(
cmd=[ros2_cmd, "lifecycle", "set", [ EnvironmentVariable('CARMA_INTR_NS', default_value='hardware_interface'), "/dsrc_driver_node" ], "configure"],
)

configuration_trigger = launch.actions.TimerAction(
period=configuration_delay,
actions=[
process_configure_dsrc_driver_node
]
)

configured_event_handler_dsrc_driver_node = launch.actions.RegisterEventHandler(launch.event_handlers.OnExecutionComplete(
target_action=process_configure_dsrc_driver_node,
on_completion=[
launch.actions.ExecuteProcess(
cmd=[ros2_cmd, "lifecycle", "set", [ EnvironmentVariable('CARMA_INTR_NS', default_value='hardware_interface'), "/dsrc_driver_node" ], "activate"],
)
]
)
)

return LaunchDescription([
declare_configuration_delay_arg,
declare_drivers_arg,
dsrc_group,
configuration_trigger,
configured_event_handler_dsrc_driver_node
])
Loading