Feat/feishu history - #39
Closed
heyeuu wants to merge 5 commits into
Closed
Conversation
Contributor
Walkthrough添加摄像头触发器与图像时间戳对齐逻辑；引入历史感知的共享内存通道（HistoryClient/Channel/ChannelTraits）与 Changes
Sequence Diagram(s)sequenceDiagram
participant Cap as Capturer
participant Hist as HistoryClient<br/>CameraTriggerEvent
participant Q as CaptureQueue
participant Img as Image
Cap->>Cap: capture_success(img)
activate Cap
Cap->>Hist: fetch_latest_matching(predicate: seq>last_bound && ts<=img.ts && age<=max_age)
activate Hist
alt 匹配到触发事件
Hist-->>Cap: CameraTriggerEvent(seq, timestamp)
Cap->>Img: 覆盖 image.timestamp = trigger.timestamp
Cap->>Cap: 更新 last_bound_trigger_seq_, reset rate limiter
else 未匹配
Hist-->>Cap: 无匹配
Cap->>Cap: 增加速率限制计数并可能记录警告
end
deactivate Hist
Cap->>Q: push(image) // 入队（可能已对齐）
deactivate Cap
sequenceDiagram
participant RT as Runtime
participant Auto as AutoAimComponent
participant Fei as Feishu<AutoAim>
participant HistS as HistoryClient<br/>ControlState
RT->>Auto: 传入 image_timestamp
activate Auto
Auto->>Fei: fetch_latest_before(image_timestamp)
activate Fei
Fei->>HistS: find_latest(state.timestamp <= image_timestamp)
activate HistS
alt 找到匹配 ControlState
HistS-->>Fei: 返回 ControlState
Fei-->>Auto: 返回对齐 ControlState
else 未找到或历史不可用
HistS-->>Fei: 返回空
Fei-->>Auto: std::nullopt
end
deactivate HistS
deactivate Fei
Auto->>Auto: 处理或提交 control_state（或警告/回退）
deactivate Auto
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
兔子诗
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/component.cpp`:
- Around line 146-151: publish_camera_trigger_event() is currently called before
feishu.commit(control_state), causing the trigger to be visible to AutoAim
before the ControlState with the same timestamp; move the commit so
feishu.commit(control_state) is executed before publish_camera_trigger_event()
inside publish_control_state(), and apply the same reorder to the other
occurrence around lines 176-179 (i.e., ensure update_gimbal_direction(),
update_control_state(), then feishu.commit(control_state), then
publish_camera_trigger_event()) so the trigger always follows the committed
ControlState.
- Around line 160-179: In publish_camera_trigger_event, avoid unguarded
dereferences of camera_trigger_seq_ and camera_trigger_timestamp_; first check
that the InputInterface/read handles are ready (e.g.,
camera_trigger_seq_.has_value() or an isReady() method) and only read
*camera_trigger_seq_ and *camera_trigger_timestamp_ when confirmed, otherwise
use a safe fallback (skip commit/return early or provide a default timestamp)
and ensure action_throttler logic still behaves correctly; update references to
camera_trigger_seq_, camera_trigger_timestamp_,
last_committed_camera_trigger_seq_, camera_trigger_channel.commit(...) and
action_throttler.dispatch/reset to operate only after readiness is verified.
In `@src/kernel/capturer.cpp`:
- Around line 115-123: 在 camera_trigger_channel.fetch_latest_matching
的匹配条件中加入对“上一帧原始图像时间戳”的下限检查，防止迟到触发被绑定到下一帧：新增并维护一个如 last_frame_raw_timestamp（初始为 0
或最小时间）并在每帧开始或绑定后更新它，然后在 lambda 中除了现有条件（candidate.seq > last_bound_trigger_seq_
&& candidate.timestamp <= capture_timestamp && capture_timestamp -
candidate.timestamp <= trigger_sync_max_age）再加上 candidate.timestamp >
last_frame_raw_timestamp；最后在成功绑定（在设置
image->set_timestamp/last_bound_trigger_seq_ 之前或之后按逻辑）更新
last_frame_raw_timestamp = previous capture_timestamp（即把当前帧的原始 capture_timestamp
保存为下一帧的下限）。
🪄 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: 07515262-70fb-496a-befb-918c3c5e18b1
📒 Files selected for processing (9)
config/config.yamlsrc/component.cppsrc/kernel/capturer.cppsrc/kernel/feishu.hppsrc/runtime.cppsrc/utility/shared/context.hppsrc/utility/shared/interprocess.hpptest/CMakeLists.txttest/timestamp_alignment.cpp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/kernel/capturer.cpp`:
- Around line 70-71: The code unconditionally calls
yaml["enable_trigger_sync"].as<bool>() which throws if the key is missing;
change logic in the initialization that sets enable_trigger_sync so you only
access yaml["enable_trigger_sync"] when source == "hikcamera" (e.g., evaluate
trigger_sync_config only inside the branch) or use yaml["enable_trigger_sync"]
with a safe default (check for existence with yaml[...].IsDefined() or use a
fallback) before calling .as<bool>(); update the assignment that uses
trigger_sync_config and ensure enable_trigger_sync remains false (or a safe
default) for non-"hikcamera" sources.
In `@test/timestamp_alignment.cpp`:
- Around line 27-35: The file uses std::move inside the ShmScope constructor
initializer (ShmScope::ShmScope and name_) but does not directly include
<utility>; add a direct include for <utility> at the top of the file so
std::move is properly declared and the dependency is explicit.
🪄 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: 96cae677-93bc-4e0a-9401-93f50c36d9de
📒 Files selected for processing (5)
src/component.cppsrc/kernel/capturer.cppsrc/module/predictor/outpost/robot_state.cppsrc/module/predictor/regular/robot_state.cpptest/timestamp_alignment.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- src/component.cpp
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/kernel/capturer.cpp (1)
70-70:⚠️ Potential issue | 🟠 Major给
enable_trigger_sync保留默认值，避免旧版 hikcamera 配置启动失败。Line 70 已经避免了非
hikcamera配置读取该字段，但hikcamera配置缺少新增键时仍会直接.as<bool>()并导致初始化失败；这个开关建议默认关闭以保持配置向后兼容。建议修改
- enable_trigger_sync = source == "hikcamera" && yaml["enable_trigger_sync"].as<bool>(); + enable_trigger_sync = + source == "hikcamera" && yaml["enable_trigger_sync"].as<bool>(false);可用下面的只读脚本检查仓库内是否还有
hikcamera配置未声明该新增键；预期结果是不输出缺失文件：#!/bin/bash set -euo pipefail fd -e yaml -e yml | while IFS= read -r file; do if rg -q -P 'source:\s*["'\''"]?hikcamera["'\''"]?\s*$' "$file" \ && ! rg -q -P 'enable_trigger_sync\s*:' "$file"; then printf 'hikcamera config missing enable_trigger_sync: %s\n' "$file" fi done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/kernel/capturer.cpp` at line 70, 当前代码直接对 hikcamera 配置做 yaml["enable_trigger_sync"].as<bool>()，如果该键缺失会抛异常，需保留默认关闭并只在键存在时读取； change the initialization of enable_trigger_sync to a safe default (false) and then, inside the existing source == "hikcamera" branch, check the YAML node existence before calling as<bool>() — e.g. use yaml["enable_trigger_sync"] && yaml["enable_trigger_sync"].IsDefined() (or yaml["enable_trigger_sync"].as<bool>(false) if yaml-cpp overload is preferred) to set enable_trigger_sync, referring to the symbol enable_trigger_sync in src/kernel/capturer.cpp.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/kernel/capturer.cpp`:
- Line 70: 当前代码直接对 hikcamera 配置做
yaml["enable_trigger_sync"].as<bool>()，如果该键缺失会抛异常，需保留默认关闭并只在键存在时读取； change the
initialization of enable_trigger_sync to a safe default (false) and then, inside
the existing source == "hikcamera" branch, check the YAML node existence before
calling as<bool>() — e.g. use yaml["enable_trigger_sync"] &&
yaml["enable_trigger_sync"].IsDefined() (or
yaml["enable_trigger_sync"].as<bool>(false) if yaml-cpp overload is preferred)
to set enable_trigger_sync, referring to the symbol enable_trigger_sync in
src/kernel/capturer.cpp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 241ee6a0-a603-4bce-be58-cdd955d45420
📒 Files selected for processing (2)
src/kernel/capturer.cpptest/timestamp_alignment.cpp
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
2 participants
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.
PR总结：飞书历史时间戳对齐功能
概述
本PR引入基于摄像机触发事件与控制状态历史的时间戳对齐机制，旨在提高图像帧与控制命令之间的时间同步精度，包含共享内存历史客户端、飞书通信层重构、摄像机触发同步、AutoAim 组件与运行时的若干适配以及对应单元测试。
主要改动
设计亮点
配置变更
影响与注意事项