feat: include micro-xrce-dds in runtime container#58
Closed
Embers-of-the-Fire wants to merge 1 commit intoAlliance-Algorithm:mainfrom
Closed
feat: include micro-xrce-dds in runtime container#58Embers-of-the-Fire wants to merge 1 commit intoAlliance-Algorithm:mainfrom
Embers-of-the-Fire wants to merge 1 commit intoAlliance-Algorithm:mainfrom
Conversation
Walkthrough在 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 诗
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
63-104: 构建流程实现合理。整体实现正确：
- 使用静态链接（
BUILD_SHARED_LIBS=OFF）确保生成独立可执行文件- 构建顺序正确：Fast-CDR → Fast-DDS → Micro-XRCE-DDS-Agent
- 清理逻辑完整，移除了构建依赖和临时文件
几点建议：
日志功能被禁用（Line 94）：
UAGENT_LOGGER_PROFILE=OFF会使运行时调试变得困难。如果是有意为之请忽略，否则建议保留日志功能。清理命令轻微冗余（Lines 101-102）：
apt-get purge -y --auto-remove已经包含了 autoremove 功能。♻️ 可选：简化清理命令
- apt-get purge -y --auto-remove libasio-dev && \ - apt-get autoremove -y && apt-get clean && \ + apt-get purge -y --auto-remove libasio-dev && \ + apt-get clean && \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Dockerfile` around lines 63 - 104, The build disables agent logging by setting CMake flag UAGENT_LOGGER_PROFILE=OFF; change it to ON (or make it configurable via a build ARG like MICROXRCE_LOGGER_PROFILE and pass -DUAGENT_LOGGER_PROFILE=${MICROXRCE_LOGGER_PROFILE}) so runtime logs are available, and simplify cleanup by removing the redundant apt-get autoremove call after apt-get purge (keep either purge --auto-remove or a single autoremove), updating the Dockerfile lines that set UAGENT_LOGGER_PROFILE and the apt cleanup commands accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 16-18: The Dockerfile currently pins FASTCDR_TAG to v1.1.1 which
is incompatible with FASTDDS_TAG v2.12.2; update the FASTCDR_TAG argument to a
2.x release (for example change FASTCDR_TAG to v2.3.0 or a newer v2.y.z) so
Fast-DDS requirements are met, then rebuild to verify the image; locate the ARG
FASTCDR_TAG declaration near ARG FASTDDS_TAG and ARG MICROXRCE_AGENT_TAG and
replace the tag string accordingly.
---
Nitpick comments:
In `@Dockerfile`:
- Around line 63-104: The build disables agent logging by setting CMake flag
UAGENT_LOGGER_PROFILE=OFF; change it to ON (or make it configurable via a build
ARG like MICROXRCE_LOGGER_PROFILE and pass
-DUAGENT_LOGGER_PROFILE=${MICROXRCE_LOGGER_PROFILE}) so runtime logs are
available, and simplify cleanup by removing the redundant apt-get autoremove
call after apt-get purge (keep either purge --auto-remove or a single
autoremove), updating the Dockerfile lines that set UAGENT_LOGGER_PROFILE and
the apt cleanup commands accordingly.
Embers-of-the-Fire
force-pushed
the
feat/micro-xrce-dds
branch
from
ffb6d12 to
c45238a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Dockerfile`:
- Around line 16-17: The Dockerfile currently only patches _fastdds_tag but not
the upstream hardcoded _fastdds_version, which can lead to version mismatches
and silent failures because sed can succeed without changing anything; update
the patch step (the lines that reference FASTDDS_TAG, _fastdds_tag and
UAGENT_SUPERBUILD) to replace both _fastdds_tag and _fastdds_version to match
ARG FASTDDS_TAG and derive the version portion (e.g., strip the leading "v" and
minor patch) and then immediately verify both substitutions succeeded (check
that both expected strings are present) and exit non‑zero if either verification
fails so the build stops on a failed patch.
Member
Author
|
Closed because of solution changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
功能特性
在运行时容器的 rmcs-base 阶段中直接从源码构建并包含 Micro XRCE-DDS Agent，可在容器内以可执行文件形式提供 MicroXRCEAgent。
变更详情
Dockerfile
新增构建参数：
在 rmcs-base 阶段新增一个 RUN 块以作为独立依赖构建步骤，主要操作：
在 rmcs-base 中，在挂载并安装 rmcs_ws 依赖（rosdep）之前完成上述 Micro XRCE-DDS Agent 的源码构建，使运行时镜像包含已编译的 agent 可执行文件。
导出/公共声明变更
其他说明