Update DSRC driver to support C1T V2X use on ARM #117
Conversation
…ig and activation of node
|
Likely not refactoring all Docker code since it is out of the scope of this story but significant portions, such as all code related to ROS1, can be removed. |
|
Docker image names are liable to change and are not currently represented in Docker Hub |
| # 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 |
There was a problem hiding this comment.
Unrelated to this PR, but we should look at multi-platform images to avoid having this *-arm designators.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Agreed as well, I didn't realize docker images supported this but it looks like a good approach to consider.
dsrc_driver/launch/drivers.launch.py
Outdated
There was a problem hiding this comment.
Is this intended to replace the existing dsrc_driver.py?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
[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.pyfrom this directory - Rename this
drivers.launch.pytodsrc_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
There was a problem hiding this comment.
@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)
There was a problem hiding this comment.
Good catch, I had an incorrect assumption and my previous comment can be disregarded (added a note at the beginning stating this as well).
| @@ -1,21 +1,19 @@ | |||
| # Copyright (C) 2018-2021 LEIDOS. | |||
| # | |||
There was a problem hiding this comment.
(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.
| # 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 |
There was a problem hiding this comment.
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.
dsrc_driver/launch/drivers.launch.py
Outdated
There was a problem hiding this comment.
[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.pyfrom this directory - Rename this
drivers.launch.pytodsrc_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
|
|
Currently inactive; originally developed as part of the CARMA Platform approach on the scale C1T vehicles. Branch needed for future dockerization efforts of the system. Not included in C1T releases. |
PR Details
Description
Updated files relating to building the Docker image of carma-cohda-dsrc-driver for ARM since a new carma-base image is being used. A new launch file is also needed to correctly configure and active the node. Otherwise, these changes mirror the original functionality.
Related GitHub Issue
Related Jira Key
CF-789
Motivation and Context
Added support for new ARM Docker images.
How Has This Been Tested?
Successfully builds on ARM device and can forward messages to a connected Raspberry Pi
Types of changes
Checklist: