Skip to content

wip(framework): lua wrap and lua decision framework implementation - #4

Merged
creeper5820 merged 9 commits into
mainfrom
dev/lua-wrap
Apr 9, 2026
Merged

wip(framework): lua wrap and lua decision framework implementation#4
creeper5820 merged 9 commits into
mainfrom
dev/lua-wrap

Conversation

@creeper5820

Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai

coderabbitaiBot commented Apr 9, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3cb2a328-b5d6-48c5-9262-7b600634e4eb

📥 Commits

Reviewing files that changed from the base of the PR and between e4097d7 and 2f87cca.

📒 Files selected for processing (3)
  • src/cxx/component.cc
  • src/lua/blackboard.lua
  • src/lua/main.lua
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/lua/blackboard.lua
  • src/lua/main.lua
  • src/cxx/component.cc

步行指南

该变更把导航组件从纯C++实现迁移为混合 C++/Lua 运行时:移除旧的决策状态机与多种工具类,新增基于 sol2 的 Lua VM、Lua 核心模块与工具库,更新构建以依赖 Lua 5.4 并安装 Lua 脚本到安装树。

变更

Cohort / File(s)Summary
构建与格式
\.luarc.json, \.stylua.toml, CMakeLists.txt, cmake/export.cmake
新增 Lua/格式化 配置;CMake 切换为 cxx_std_23、引入 Lua 5.4、FetchContent 拉取 sol2,并将 src/lua/ 安装到 share 下。
文档
AGENTS.md, README.md
重写/补充实现原则与 Lua 运行时上下文;README 改为描述 C++/Lua 组件模型、ROS 接口、mock 模式与运行说明。
移除:旧 C++ 实现与工具
src/component/..., src/component/decision/..., src/component/util/*
删除原导航组件、PlanBox、Config、TimedTask、各类检测/队列/混入/屏幕管理等辅助实现及对应测试。
新增:C++ 组件与工具
src/cxx/component.cc, src/cxx/util/logger_mixin.hh, src/cxx/util/rmcs_msgs_format.hh
新增基于 rmcs_executor 的 C++ 组件实现(嵌入 Lua VM、黑板同步、mock context、command_vel 回调)、新的 LoggerMixin 与消息格式化辅助。
新增:Lua 核心模块
src/lua/main.lua, src/lua/api.lua, src/lua/blackboard.lua, src/lua/option.lua
添加 Lua 入口(on_init/on_tick/control_speed_callback)、api stub 与 restart_navigation、黑板单例与默认字段,空的 option 文件。
新增:Lua 工具库
src/lua/util/clock.lua, src/lua/util/edge.lua, src/lua/util/fsm.lua, src/lua/util/scheduler.lua, src/lua/util/native.lua
新增时钟、边缘触发集合、FSM 库、协程调度器与本地命令/环境发现等工具模块。
移除:旧 C++ 测试配置
test/CMakeLists.txt, test/*_*.cc
删除旧的 GoogleTest 配置与多个 C++ 单元测试文件。
新增:Lua 测试与工具
test/lua/util.lua, test/lua/clock.lua, test/lua/fsm.lua, test/lua/scheduler.lua, test/lua/restart_navigation.lua, test/lua/runable.lua
新增 Lua 测试工具与对 clock/fsm/scheduler/restart/runable 的单元与集成测试脚本。

序列图

sequenceDiagram
participant ROS2 as ROS2
participant Cpp as Navigation C++ (Component)
participant LuaVM as Lua VM
participant Blackboard as Blackboard (Lua)
participant FSM as FSM / Scheduler
participant API as Lua API
participant CmdTopic as Command Topic
Note over Cpp,LuaVM: 初始化
ROS2->>Cpp: 加载组件并调用 initialize
Cpp->>LuaVM: 创建 sol::state, 加载 `main`, `api`, `blackboard`
LuaVM->>Blackboard: 初始化黑板单例
LuaVM->>FSM: 注册任务与 FSM 状态
Cpp->>LuaVM: 调用 on_init()
Note over ROS2,Cpp: 运行时循环(每次 update)
ROS2->>Cpp: update()
Cpp->>Blackboard: 同步输入数据(health, ammo, stage, switches, timestamp)
Cpp->>LuaVM: 调用 on_tick()
LuaVM->>FSM: spin_once / scheduler:spin_once()
FSM->>API: 触发 api 调用(apply_navigation_goal / restart 等)
API->>Cpp: 返回/影响(或通过 topic 发布)
LuaVM->>CmdTopic: 通过 control_speed_callback 发布底盘命令
Loading

预估代码审查工作量

🎯 4 (复杂) | ⏱️ ~60 分钟

诗歌

🐇 旧瓦片拆尽,Lua 新芽生长,
黑板悄声记录,调度轻步上场。
C++ 携手小语,决策在夜里唱,
导航有了节拍,兔子为你鼓掌。

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 25.68% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedPR标题准确反映了主要变更:将C++组件迁移到Lua框架驱动的决策系统,以及引入Lua运行时包装。

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev/lua-wrap

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.

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/cxx/util/rmcs_msgs_format.hh (1)

88-93: ⚠️ Potential issue | 🔴 Critical

修正 GameStage 格式化器中的命名空间

第91行调用 rmcs_msgs::to_string(stage) 有误。to_string(GameStage) 函数定义在 rmcs::navigation::detail 命名空间(第19-30行),而非 rmcs_msgs 命名空间。此处会导致编译错误。注意同一文件中其他格式化器(第98行和第105行)已正确使用 rmcs::navigation::detail::to_string()

建议修复
 template <>
struct std::formatter<rmcs_msgs::GameStage> : std::formatter<const char*> {
auto format(rmcs_msgs::GameStage stage, std::format_context& ctx) const {
- return std::formatter<const char*>::format(rmcs_msgs::to_string(stage), ctx);+ return std::formatter<const char*>::format(rmcs::navigation::detail::to_string(stage), ctx);
}
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cxx/util/rmcs_msgs_format.hh` around lines 88 - 93, The formatter
specialization std::formatter<rmcs_msgs::GameStage>::format is calling
rmcs_msgs::to_string(stage), which is in the rmcs::navigation::detail namespace;
change the call to rmcs::navigation::detail::to_string(stage) so it matches the
actual function definition (consistent with the other formatters in the file)
inside the format method of std::formatter<rmcs_msgs::GameStage>.
🧹 Nitpick comments (15)
.stylua.toml (2)

1-1: 可考虑添加更完整的 Stylua 配置

当前配置仅包含一个选项。可以考虑添加其他常用的格式化选项以确保代码风格的完整性和一致性,例如:

  • column_width - 行宽限制
  • indent_type - 缩进类型(Spaces/Tabs)
  • indent_width - 缩进宽度
  • quote_style - 引号风格
📝 更完整的配置示例
column_width = 120indent_type = "Spaces"indent_width = 4quote_style = "AutoPreferDouble"call_parentheses = "Always"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.stylua.toml at line 1, Add more Stylua formatting options to .stylua.toml
beyond the existing call_parentheses setting: include keys such as column_width,
indent_type, indent_width, and quote_style to enforce consistent line length,
indentation style/size, and quote preferences; update the existing
call_parentheses entry (currently "call_parentheses") to the desired policy and
add the new keys with project-preferred values so the formatter enforces a
complete, consistent style.

1-1: 建议使用更明确的格式化规则以确保代码一致性

当前配置 call_parentheses = "Input" 会保留输入代码的括号风格,这可能导致代码库中括号使用不一致。对于代码格式化工具,通常应该强制统一的风格规则。

建议考虑以下更明确的选项:

  • "Always" - 始终使用括号
  • "NoSingleString" - 单个字符串字面量调用时省略括号
  • "NoSingleTable" - 单个表调用时省略括号
📋 建议的配置示例
-call_parentheses = "Input"+call_parentheses = "Always"

或根据团队偏好选择其他选项。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.stylua.toml at line 1, The current Stylua setting call_parentheses =
"Input" allows inconsistent parentheses styles; update the call_parentheses
value in the .stylua.toml to a concrete enforced policy (for example use
"Always" to always include parentheses, or "NoSingleString"/"NoSingleTable" to
omit them only in those specific cases) so the formatter enforces a consistent
style across the codebase; modify the call_parentheses entry accordingly in
.stylua.toml (referencing the call_parentheses key) to the chosen option and
commit the change.
src/lua/blackboard.lua (1)

31-50: 考虑保护内部函数表

bb.conditionbb.getter 可被外部代码意外覆盖。如果需要防止误操作,可以考虑使用 __newindex 元方法保护这些表,或在文档中明确标注这些字段为只读。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lua/blackboard.lua` around lines 31 - 50, Protect bb.condition and
bb.getter from accidental external overwrites by making them read-only via a
metatable: create a metatable for bb that implements __newindex to block writes
to the keys "condition" and "getter" (or to their subtables) and/or set
metatables on bb.condition and bb.getter that forbid new keys or reassignments;
update any initialization code that currently assigns directly to bb.condition
or bb.getter to set their contents before applying the read-only metatable;
reference the symbols bb.condition, bb.getter and the __newindex metamethod to
locate where to apply the protection.
src/lua/util/edge.lua (1)

35-42: 缺少参数类型验证

Edges:on() 未验证 gettercallback 是否为函数。如果传入非函数类型,错误会延迟到 spin() 执行时才暴露,增加调试难度。

♻️ 建议添加断言
 function Edges:on(getter, signal, callback)
+	assert(type(getter) == "function", "getter must be a function")+	assert(type(callback) == "function", "callback must be a function")
self._entries[`#self._entries` + 1] = {
getter = getter,
edge = new_edge(signal),
callback = callback,
}
return self
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lua/util/edge.lua` around lines 35 - 42, Edges:on currently appends
entries without validating types — add early assertions to validate that the
getter and callback arguments are functions (e.g., inside Edges:on check that
type(getter) == "function" and type(callback) == "function"), and raise a clear
error if not; mention the offending parameter in the message so misuse is caught
at call time rather than later in Edges:spin or when new_edge/edge callbacks
run.
test/lua/util.lua (2)

50-57: write_file 未检查写入结果

file:write(content) 可能因磁盘满等原因失败,但返回值未被检查。在测试场景中这通常不是问题,但为健壮性考虑可添加检查。

♻️ 建议修改
 function M.write_file(path, content)
local file, err = io.open(path, "w")
if file == nil then
error(string.format("failed to open %s for writing: %s", path, tostring(err)))
end
-	file:write(content)+	local ok, write_err = file:write(content)+	if not ok then+ file:close()+ error(string.format("failed to write to %s: %s", path, tostring(write_err)))+	end
file:close()
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/lua/util.lua` around lines 50 - 57, M.write_file does not check the
result of file:write so write failures (e.g., disk full) are ignored; update
M.write_file to capture the return values from file:write, close the file on
error, and raise an error that includes the path and the write error message
(e.g., use local ok, err = file:write(content); if not ok then file:close();
error(string.format("failed to write %s: %s", path, tostring(err))) end). Ensure
file is still closed on both success and failure.

34-39: assert_table_eq 仅比较数组部分

当前实现只比较数字索引的元素(#actual#expectedfor i = 1, #expected``),不会检查非数字键。如果需要比较完整的表结构,需要扩展实现。对于当前测试用例(如 scheduler 测试中的数组),这已足够。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/lua/util.lua` around lines 34 - 39, M.assert_table_eq currently only
compares array-like numeric indices using `#actual/`#expected and a numeric
for-loop; update it to also iterate non-numeric keys so full table structure is
compared. Modify M.assert_table_eq to (1) verify lengths for array part as now,
(2) iterate pairs(expected) and assert actual[k] == v for every key (to catch
string/other keys), and (3) iterate pairs(actual) to ensure there are no extra
keys missing from expected; use the existing M.assert_eq for comparisons and
reference the function name M.assert_table_eq and its parameters
actual/expected/message to locate and update the logic.
src/lua/util/fsm.lua (1)

23-33: FsmHandle 仅暴露 last_state,无法查询当前状态

handle 提供 set_next()last_state(),但没有提供获取当前状态的方法。如果状态处理函数需要知道当前状态名,需要通过其他方式传递。这可能是有意的设计(强制状态处理函数只关注自身逻辑),但值得确认是否满足使用场景。

♻️ 如需要,可添加 current_state 查询
 function handle:last_state()
local fsm = self.details.fsm
return fsm.details.last_state
end
++--- `@return` string+function handle:current_state()+	local fsm = self.details.fsm+	return fsm.details.current_state+end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lua/util/fsm.lua` around lines 23 - 33, Add a current-state accessor to
FsmHandle so callers can read the current state; implement a new method
handle:current_state() that retrieves the FSM via self.details.fsm and returns
the current state field (use the same field name used by the FSM, e.g.
fsm.details.state or fsm.details.current_state) and update its annotation to
`--- `@return` string|nil`; place it alongside handle:set_next and
handle:last_state so state handlers can query the active state when needed.
src/cxx/util/rmcs_msgs_format.hh (1)

19-30: 返回类型声明风格不一致

to_string(GameStage) 使用 auto 推导返回类型,而同文件中其他 to_string 重载(包括新增的 to_string(Switch) 在第31行)都显式声明 -> const char*。建议统一风格。

♻️ 建议修改
-constexpr auto to_string(rmcs_msgs::GameStage stage) noexcept {+constexpr auto to_string(rmcs_msgs::GameStage stage) noexcept -> const char* {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cxx/util/rmcs_msgs_format.hh` around lines 19 - 30, The to_string
overload for rmcs_msgs::GameStage uses return type deduction (auto) while other
overloads (e.g., to_string(Switch)) use an explicit -> const char*; please
change the GameStage function signature to match the explicit style (e.g., add
"-> const char*" while keeping constexpr and noexcept) so the declaration style
is consistent across the file, and ensure the function still returns the same
string literals.
CMakeLists.txt (1)

44-48: cxx_std_23 更适合设成 PRIVATE

这个 target 目前没有导出 public header,PUBLIC 会把 C++23 要求继续传给下游链接方,等于把实现细节变成构建约束。

💡 建议修改
 target_compile_features(
${PROJECT_NAME}
- PUBLIC+ PRIVATE
cxx_std_23
)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CMakeLists.txt` around lines 44 - 48, 将 target_compile_features 调用中把
cxx_std_23 的可见性从 PUBLIC 改为 PRIVATE:定位到 target_compile_features(${PROJECT_NAME}
PUBLIC cxx_std_23) 并将 PUBLIC 替换为 PRIVATE,以避免把 C++23 要求传播给下游消费者,只在当前目标实现上强制使用
C++23。
test/lua/scheduler.lua (1)

138-159: 补一条 wait_until() 命中 deadline 的边界用例。

当前只验证了 1.1 > 1.0 的超时路径,没有覆盖 clock == deadline。这里正好是调度器最容易回归的边界,建议再补一个 step(ctx, 1.0) 直接断言 "timeout" 的 case。As per coding guidelines, "若修改了 Lua 运行时、行为树或入口逻辑,优先补充/更新对应 Lua 测试。"

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/lua/scheduler.lua` around lines 138 - 159, 在测试块中补一个命中 deadline
的边界用例:在已有任务(通过 ctx:append_task 使用
task_of、scheduler.request:wait_until)和现有断言前后插入一个 step(ctx, 1.0) 并断言 trace 已包含
"timeout";也就是说在调用 step(ctx, 0.5) 后保留现有 assert_table_eq(trace,
{"before_wait"}),然后执行 step(ctx, 1.0) 并断言 trace 为 {"before_wait",
"timeout"},最后可保留或调整后续 step(ctx, 1.1) 断言以确保不变。确保引用到的符号包括
wait_until、scheduler.request、ctx:append_task、task_of、step 和 trace 来定位修改位置。
src/lua/main.lua (2)

130-134: control_speed_callback 忽略第三个参数的处理方式可改进

当前使用 local _ = qx 来标记未使用的参数。Lua 中更惯用的做法是在参数列表中直接使用下划线。

♻️ 建议的改进
 --- 由 NAV2 发布的目标速度值,在此处理回调
-control_speed_callback = function(vx, vy, qx)-	local _ = qx+control_speed_callback = function(vx, vy, _)
api.update_chassis_vel(vx, vy)
end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lua/main.lua` around lines 130 - 134, The function control_speed_callback
currently declares qx then marks it unused with local _ = qx; change the
parameter list to use an underscore placeholder and remove the local assignment
(i.e., define control_speed_callback = function(vx, vy, _) and delete the local
_ = qx line) so the unused third argument is idiomatically ignored in Lua while
leaving api.update_chassis_vel(vx, vy) unchanged.

32-32: 全局变量 blackboard 的设计意图明确

此处创建全局变量 blackboard 是有意为之,C++ 组件通过 (*lua)["blackboard"] 访问此表进行数据同步(见 component.cc 第 232 行)。

建议添加注释说明这是故意暴露的全局变量,以避免后续维护时被误删。

💡 建议添加注释
+-- 注意: blackboard 故意设为全局变量,供 C++ 组件访问和同步
blackboard = require("blackboard").singleton()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lua/main.lua` at line 32, The global variable blackboard is intentionally
exposed for C++ to access via the Lua state (e.g. (*lua)["blackboard"]), so add
a clear comment above the line blackboard = require("blackboard").singleton()
stating that this global is intentionally exported for C++ components and must
not be removed or renamed; include brief notes on how C++ accesses it (e.g. via
the Lua global "blackboard") to help future maintainers.
src/cxx/component.cc (1)

262-262: get_parameter 无默认值可能导致构造失败

get_parameter("command_vel_name") 在参数未设置时会抛出异常。考虑到 mock_context 使用了 get_parameter_or 提供默认值,建议对 command_vel_name 也采用相同模式以提高健壮性。

♻️ 建议提供默认值
- const auto command_vel_name = get_parameter("command_vel_name").as_string();+ const auto command_vel_name = get_parameter_or<std::string>("command_vel_name", "/cmd_vel");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cxx/component.cc` at line 262, The code calls
get_parameter("command_vel_name").as_string() which throws if the parameter is
absent; change this to use get_parameter_or("command_vel_name", "<default>")
(matching how mock_context uses get_parameter_or) so the component constructor
won't throw when the parameter is missing—update the use site (the variable
command_vel_name in component.cc) to call get_parameter_or with an appropriate
default string and remove the direct get_parameter usage.
src/lua/api.lua (1)

7-7: 函数名 fuck 不够专业

建议使用更正式的命名,如 errorfatal,以保持代码库的专业性。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lua/api.lua` at line 7, 注释中暴露的函数名 `fuck` 不够专业,请将其重命名为更正式的标识(例如 `error` 或
`fatal`)并同步更新所有相关位置:修改 api 注释中的 `--- `@field` fuck fun(message: string)`
为所选新名,重命名实现该接口的函数和所有对该函数的引用(调用处、导出、文档与测试),确保签名保持一致并运行现有测试以验证没有遗漏引用。
README.md (1)

1-1: 文档标题中的免责声明

标题 "Ai Generated, 不保真" 表明这是 AI 生成的文档且不保证准确性。建议在正式发布前人工审核关键技术细节,或在稳定后移除该免责声明。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` at line 1, Update the README title line "# rmcs-navigation (Ai
Generated, 不保真)" to remove or move the disclaimer: either drop the parenthetical
from the header and place a short, prominent disclaimer paragraph below the
title (e.g., "This documentation was AI-generated; please review technical
details before publishing."), or revise the parenthetical to a more formal note;
edit the README.md header line and the immediate following section so the title
stays clean while keeping the review disclaimer in the document body.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CMakeLists.txt`:
- Around line 34-40: The CMake FetchContent use for sol2 (FetchContent_Declare
with GIT_REPOSITORY and GIT_TAG v3.5.0 followed by FetchContent_MakeAvailable)
pulls from the network at configure time which breaks reproducibility and
offline/CI builds; fix by replacing the mutable tag with a fixed commit hash in
the GIT_TAG, or vendor sol2 into the repo and adjust the CMake to use the local
copy instead of FetchContent, or prefer the system package (find_package for
sol2) and remove the FetchContent_Declare/FetchContent_MakeAvailable usage for
sol2.
In `@src/cxx/component.cc`:
- Around line 97-99: The code contains dead code after a return (the standalone
vector creation and vector.emplace_back(1)) which will never execute; remove the
unreachable statements (the auto vector = std::vector<int>{}; and
vector.emplace_back(1)) or move any intended logic before the return in the same
function (search for the return {} and the following vector usage to locate the
unreachable block in component.cc).
- Around line 267-271: The code is using msg->angular.x for the yaw rate but
should use msg->angular.z; update the extraction so qx is assigned from
msg->angular.z (where qx is passed into lua_control_speed_callback) to reflect
ROS conventions and ground-robot yaw rate usage, ensuring the call to
lua_control_speed_callback(vx, vy, qx) supplies the correct angular component.
In `@src/cxx/util/rmcs_msgs_format.hh`:
- Around line 31-39: Missing std::formatter specialization for
rmcs_msgs::Switch: add a template<> struct std::formatter<rmcs_msgs::Switch>
(deriving from std::formatter<const char*>) that calls the existing
to_string(rmcs_msgs::Switch) inside its format(...) method so std::format works
for Switch; place this new specialization alongside the other formatter
specializations (the GameStage/ArmorID/RobotId block) so it is discovered by the
compiler.
In `@src/lua/option.lua`:
- Line 1: 当前模块为空,会导致 require 后无可用接口;请在模块中添加最小稳定导出:定义模块表(例如 option 或
M)并导出至少一个入口函数(如 setup(config) 或 new(config))和一个查询函数(如 get(name) 或 defaults()),并在
setup 中做基本的参数校验/合并到默认配置,最后返回模块表;在代码里定位并修改空的 option.lua,添加这些符号(option, setup/new,
get/defaults)以确保上层调用契约明确且可演进。
In `@src/lua/util/scheduler.lua`:
- Around line 101-112: scheduler_request:wait_until currently asserts only
type(args.monitor) and uses >= when computing timeout inside resume_request but
returns clock:now() > deadline, causing a false negative when resumed exactly at
deadline and also risks a nil args access; fix by validating args is a table and
args.monitor is a function, compute a single timed_out boolean (e.g. local
timed_out = clock:now() >= deadline) inside the resume closure and return that
same timed_out value (or capture and return the same comparison) so the resume
condition and the function return use the exact same >= comparison, keeping the
resume_request, deadline and args.monitor symbols intact.
In `@test/lua/restart_navigation.lua`:
- Around line 35-88: 该测试会直接调用真实的 api.restart_navigation("rmul") 并干预已有 screen
会话,需改为显式 opt-in 才能运行:在 restart_navigation.lua 的顶端(处理
read_screen_output/print_screen_output/kill_screen/cleanup 的同一文件)先检测一个环境变量或标志(例如
RUN_RESTART_NAV 或 RUN_INTEGRATION_TESTS),如果未设置则跳过/返回而非执行测试;仅在该变量为真时才调用
api.restart_navigation("rmul"),并保持现有 cleanup/kill_screen/hardcopy_path
行为不变以确保资源清理;同时在说明里提示要在受控环境下运行或用可注入的 screen_label/mock 替代真实调用以避免破坏开发机(参考函数名:
read_screen_output, kill_screen, cleanup, api.restart_navigation)。
---
Outside diff comments:
In `@src/cxx/util/rmcs_msgs_format.hh`:
- Around line 88-93: The formatter specialization
std::formatter<rmcs_msgs::GameStage>::format is calling
rmcs_msgs::to_string(stage), which is in the rmcs::navigation::detail namespace;
change the call to rmcs::navigation::detail::to_string(stage) so it matches the
actual function definition (consistent with the other formatters in the file)
inside the format method of std::formatter<rmcs_msgs::GameStage>.
---
Nitpick comments:
In @.stylua.toml:
- Line 1: Add more Stylua formatting options to .stylua.toml beyond the existing
call_parentheses setting: include keys such as column_width, indent_type,
indent_width, and quote_style to enforce consistent line length, indentation
style/size, and quote preferences; update the existing call_parentheses entry
(currently "call_parentheses") to the desired policy and add the new keys with
project-preferred values so the formatter enforces a complete, consistent style.
- Line 1: The current Stylua setting call_parentheses = "Input" allows
inconsistent parentheses styles; update the call_parentheses value in the
.stylua.toml to a concrete enforced policy (for example use "Always" to always
include parentheses, or "NoSingleString"/"NoSingleTable" to omit them only in
those specific cases) so the formatter enforces a consistent style across the
codebase; modify the call_parentheses entry accordingly in .stylua.toml
(referencing the call_parentheses key) to the chosen option and commit the
change.
In `@CMakeLists.txt`:
- Around line 44-48: 将 target_compile_features 调用中把 cxx_std_23 的可见性从 PUBLIC 改为
PRIVATE:定位到 target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_23) 并将 PUBLIC
替换为 PRIVATE,以避免把 C++23 要求传播给下游消费者,只在当前目标实现上强制使用 C++23。
In `@README.md`:
- Line 1: Update the README title line "# rmcs-navigation (Ai Generated, 不保真)"
to remove or move the disclaimer: either drop the parenthetical from the header
and place a short, prominent disclaimer paragraph below the title (e.g., "This
documentation was AI-generated; please review technical details before
publishing."), or revise the parenthetical to a more formal note; edit the
README.md header line and the immediate following section so the title stays
clean while keeping the review disclaimer in the document body.
In `@src/cxx/component.cc`:
- Line 262: The code calls get_parameter("command_vel_name").as_string() which
throws if the parameter is absent; change this to use
get_parameter_or("command_vel_name", "<default>") (matching how mock_context
uses get_parameter_or) so the component constructor won't throw when the
parameter is missing—update the use site (the variable command_vel_name in
component.cc) to call get_parameter_or with an appropriate default string and
remove the direct get_parameter usage.
In `@src/cxx/util/rmcs_msgs_format.hh`:
- Around line 19-30: The to_string overload for rmcs_msgs::GameStage uses return
type deduction (auto) while other overloads (e.g., to_string(Switch)) use an
explicit -> const char*; please change the GameStage function signature to match
the explicit style (e.g., add "-> const char*" while keeping constexpr and
noexcept) so the declaration style is consistent across the file, and ensure the
function still returns the same string literals.
In `@src/lua/api.lua`:
- Line 7: 注释中暴露的函数名 `fuck` 不够专业,请将其重命名为更正式的标识(例如 `error` 或
`fatal`)并同步更新所有相关位置:修改 api 注释中的 `--- `@field` fuck fun(message: string)`
为所选新名,重命名实现该接口的函数和所有对该函数的引用(调用处、导出、文档与测试),确保签名保持一致并运行现有测试以验证没有遗漏引用。
In `@src/lua/blackboard.lua`:
- Around line 31-50: Protect bb.condition and bb.getter from accidental external
overwrites by making them read-only via a metatable: create a metatable for bb
that implements __newindex to block writes to the keys "condition" and "getter"
(or to their subtables) and/or set metatables on bb.condition and bb.getter that
forbid new keys or reassignments; update any initialization code that currently
assigns directly to bb.condition or bb.getter to set their contents before
applying the read-only metatable; reference the symbols bb.condition, bb.getter
and the __newindex metamethod to locate where to apply the protection.
In `@src/lua/main.lua`:
- Around line 130-134: The function control_speed_callback currently declares qx
then marks it unused with local _ = qx; change the parameter list to use an
underscore placeholder and remove the local assignment (i.e., define
control_speed_callback = function(vx, vy, _) and delete the local _ = qx line)
so the unused third argument is idiomatically ignored in Lua while leaving
api.update_chassis_vel(vx, vy) unchanged.
- Line 32: The global variable blackboard is intentionally exposed for C++ to
access via the Lua state (e.g. (*lua)["blackboard"]), so add a clear comment
above the line blackboard = require("blackboard").singleton() stating that this
global is intentionally exported for C++ components and must not be removed or
renamed; include brief notes on how C++ accesses it (e.g. via the Lua global
"blackboard") to help future maintainers.
In `@src/lua/util/edge.lua`:
- Around line 35-42: Edges:on currently appends entries without validating types
— add early assertions to validate that the getter and callback arguments are
functions (e.g., inside Edges:on check that type(getter) == "function" and
type(callback) == "function"), and raise a clear error if not; mention the
offending parameter in the message so misuse is caught at call time rather than
later in Edges:spin or when new_edge/edge callbacks run.
In `@src/lua/util/fsm.lua`:
- Around line 23-33: Add a current-state accessor to FsmHandle so callers can
read the current state; implement a new method handle:current_state() that
retrieves the FSM via self.details.fsm and returns the current state field (use
the same field name used by the FSM, e.g. fsm.details.state or
fsm.details.current_state) and update its annotation to `--- `@return`
string|nil`; place it alongside handle:set_next and handle:last_state so state
handlers can query the active state when needed.
In `@test/lua/scheduler.lua`:
- Around line 138-159: 在测试块中补一个命中 deadline 的边界用例:在已有任务(通过 ctx:append_task 使用
task_of、scheduler.request:wait_until)和现有断言前后插入一个 step(ctx, 1.0) 并断言 trace 已包含
"timeout";也就是说在调用 step(ctx, 0.5) 后保留现有 assert_table_eq(trace,
{"before_wait"}),然后执行 step(ctx, 1.0) 并断言 trace 为 {"before_wait",
"timeout"},最后可保留或调整后续 step(ctx, 1.1) 断言以确保不变。确保引用到的符号包括
wait_until、scheduler.request、ctx:append_task、task_of、step 和 trace 来定位修改位置。
In `@test/lua/util.lua`:
- Around line 50-57: M.write_file does not check the result of file:write so
write failures (e.g., disk full) are ignored; update M.write_file to capture the
return values from file:write, close the file on error, and raise an error that
includes the path and the write error message (e.g., use local ok, err =
file:write(content); if not ok then file:close(); error(string.format("failed to
write %s: %s", path, tostring(err))) end). Ensure file is still closed on both
success and failure.
- Around line 34-39: M.assert_table_eq currently only compares array-like
numeric indices using `#actual/`#expected and a numeric for-loop; update it to
also iterate non-numeric keys so full table structure is compared. Modify
M.assert_table_eq to (1) verify lengths for array part as now, (2) iterate
pairs(expected) and assert actual[k] == v for every key (to catch string/other
keys), and (3) iterate pairs(actual) to ensure there are no extra keys missing
from expected; use the existing M.assert_eq for comparisons and reference the
function name M.assert_table_eq and its parameters actual/expected/message to
locate and update the logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a465b0be-4830-443e-90db-4de297c3f697

📥 Commits

Reviewing files that changed from the base of the PR and between bd3b3e1 and e4097d7.

📒 Files selected for processing (45)
  • .luarc.json
  • .stylua.toml
  • AGENTS.md
  • CMakeLists.txt
  • README.md
  • cmake/export.cmake
  • src/component/component.cc
  • src/component/decision/config.hh
  • src/component/decision/plan.cc
  • src/component/decision/plan.hh
  • src/component/util/bool_edge_trigger.hh
  • src/component/util/delayed_task_queue.hh
  • src/component/util/logger_mixin.hh
  • src/component/util/navigation_screen.hh
  • src/component/util/nod_task_queue.hh
  • src/component/util/switch_event_detector.hh
  • src/component/util/timed_task.hh
  • src/component/util/value_enter_detector.hh
  • src/cxx/component.cc
  • src/cxx/util/fsm.hh
  • src/cxx/util/logger_mixin.hh
  • src/cxx/util/pimpl.hh
  • src/cxx/util/rmcs_msgs_format.hh
  • src/cxx/util/tie.hh
  • src/lua/api.lua
  • src/lua/blackboard.lua
  • src/lua/main.lua
  • src/lua/option.lua
  • src/lua/util/clock.lua
  • src/lua/util/edge.lua
  • src/lua/util/fsm.lua
  • src/lua/util/native.lua
  • src/lua/util/scheduler.lua
  • test/CMakeLists.txt
  • test/bool_edge_trigger.cc
  • test/cxx/runable.cc
  • test/fsm.cc
  • test/lua/clock.lua
  • test/lua/fsm.lua
  • test/lua/restart_navigation.lua
  • test/lua/runable.lua
  • test/lua/scheduler.lua
  • test/lua/util.lua
  • test/map.cc
  • test/plan_box.cc
💤 Files with no reviewable changes (17)
  • test/map.cc
  • test/bool_edge_trigger.cc
  • test/fsm.cc
  • src/component/component.cc
  • src/component/util/value_enter_detector.hh
  • test/CMakeLists.txt
  • src/component/util/nod_task_queue.hh
  • src/component/util/bool_edge_trigger.hh
  • src/component/util/delayed_task_queue.hh
  • src/component/util/switch_event_detector.hh
  • src/component/util/logger_mixin.hh
  • src/component/decision/plan.hh
  • src/component/decision/config.hh
  • src/component/util/navigation_screen.hh
  • src/component/util/timed_task.hh
  • src/component/decision/plan.cc
  • test/plan_box.cc

Comment threadCMakeLists.txt
Comment threadsrc/cxx/component.cc Outdated
Comment threadsrc/cxx/component.cc
Comment threadsrc/cxx/util/rmcs_msgs_format.hh
Comment threadsrc/lua/option.lua
Comment threadsrc/lua/util/scheduler.lua
Comment threadtest/lua/restart_navigation.lua
@creeper5820
creeper5820 merged commit 740c30a into mainApr 9, 2026
1 check passed
@creeper5820
creeper5820 deleted the dev/lua-wrap branch April 9, 2026 05:13
@coderabbitaicoderabbitaiBot mentioned this pull request Apr 14, 2026
This was referenced May 6, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@creeper5820