Skip to content

Use friend property to hide impl function#29

Merged
creeper5820 merged 1 commit intomainfrom
fix/hide-crtp-impl-function
Feb 8, 2026
Merged

Use friend property to hide impl function#29
creeper5820 merged 1 commit intomainfrom
fix/hide-crtp-impl-function

Conversation

@creeper5820
Copy link
Copy Markdown
Collaborator

@creeper5820 creeper5820 commented Feb 8, 2026

Pull Request 摘要

目的

使用友元属性隐藏CRTP实现函数,使基类 Movable 能够访问派生类的私有 impl_move() 方法。

主要变更

CMakeLists.txt

  • 注释掉 find_package(Ceres REQUIRED) 调用,禁用显式的Ceres依赖解析

src/utility/rclcpp/visual/armor.hpp

  • Armor 结构体中添加 friend Movable; 声明
  • 允许 Movable 基类访问 Armor 的私有成员,特别是 impl_move() 方法

src/utility/rclcpp/visual/posture.hpp

  • Posture 结构体中添加 friend Movable; 声明
  • 显式添加 public: 访问说明符,明确 Config 类型的公有地位
  • impl_move() 保持私有,由友元声明提供访问权限

src/utility/rclcpp/visual/transform.hpp

  • Transform 结构体中添加 friend Movable; 声明
  • 显式添加 public: 访问说明符,明确 Config 嵌套结构体的公有地位
  • impl_move() 保持私有,由友元声明提供访问权限

设计意图

通过友元声明,使得 Movable 基类能够调用派生类的私有 impl_move() 实现方法。这样用户只能通过基类公开的 move() 方法进行交互,而 impl_move() 作为实现细节保持隐藏和私有,符合CRTP设计模式的最佳实践。

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 8, 2026

Walkthrough

此拉取请求引入了对多个C++头文件的访问控制和友元声明修改,为Movable类添加了对Armor、Posture和Transform结构体的私有成员访问权限。同时在CMakeLists.txt中注释掉了显式的Ceres依赖查找调用。

Changes

内聚性 / 文件 变更摘要
CMake 配置
CMakeLists.txt
注释掉 find_package(Ceres REQUIRED) 调用,禁用对Ceres包的显式依赖解析。
友元访问授权
src/utility/rclcpp/visual/armor.hpp, src/utility/rclcpp/visual/transform.hpp
为Movable类添加友元声明,允许其访问这些结构体的私有成员。
访问控制重组
src/utility/rclcpp/visual/posture.hpp
添加Movable友元声明,显式添加public访问说明符以清晰定义Config类型的公开可见性,修改impl_move和数据成员的访问级别。

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 ✨
友元之链悄然结,Movable 展翅高飞,
访问权限重重开,Transform 与 Armor 相知,
CMake 簇拥聚一堂,代码之舞更绚烂!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题清晰准确地描述了本次变更的主要目的:使用友元属性来隐藏实现函数。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/hide-crtp-impl-function

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@CMakeLists.txt`:
- Line 14: The commented out change to the CMake dependency (the line containing
find_package(Ceres REQUIRED)) is unrelated to the PR topic and should be removed
or isolated; either restore the original find_package(Ceres REQUIRED) line in
CMakeLists.txt or move this change into its own commit/PR with a clear rationale
so the current PR remains focused on the friend/impl function visibility
changes.

find_package(OpenCV 4.5 REQUIRED)
find_package(OpenVINO REQUIRED)
find_package(Ceres REQUIRED)
# find_package(Ceres REQUIRED)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for Ceres includes and namespace usage
echo "=== Searching for Ceres includes ==="
rg -i 'include.*ceres' --type=cpp --type=c --type=h

echo -e "\n=== Searching for ceres:: namespace usage ==="
rg 'ceres::' --type=cpp --type=c --type=h

echo -e "\n=== Searching for any Ceres references ==="
rg -i 'ceres' --type=cpp --type=c --type=h -l

Repository: Alliance-Algorithm/rmcs_auto_aim_v2

Length of output: 206


注释掉 Ceres 依赖与 PR 主题(friend 隐藏 impl 函数)无关。

此更改虽然安全且有效(项目代码中未使用 Ceres),但与 PR 的主要目标无关,建议拆分到单独的提交或 PR 中。

🤖 Prompt for AI Agents
In `@CMakeLists.txt` at line 14, The commented out change to the CMake dependency
(the line containing find_package(Ceres REQUIRED)) is unrelated to the PR topic
and should be removed or isolated; either restore the original
find_package(Ceres REQUIRED) line in CMakeLists.txt or move this change into its
own commit/PR with a clear rationale so the current PR remains focused on the
friend/impl function visibility changes.

@creeper5820 creeper5820 merged commit 29fa855 into main Feb 8, 2026
2 checks passed
@creeper5820 creeper5820 deleted the fix/hide-crtp-impl-function branch February 8, 2026 18:22
@github-project-automation github-project-automation bot moved this from Todo to Done in RMCS Auto Aim V2 Feb 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant