feat(referee): adapt to latest referee protocol and add sentry interaction#72
Closed
gqsdjhh wants to merge 4 commits into
Closed
feat(referee): adapt to latest referee protocol and add sentry interaction#72gqsdjhh wants to merge 4 commits into
gqsdjhh wants to merge 4 commits into
Conversation
Walkthrough新增 GameRobotPosition、OpponentMapRobotData 与 SentryInfo；扩展 Status 注册与解码以发布更多游戏/盟军/对手/哨兵字段与地图事件子字段；新增 SentryDecision ROS2 Component 实现命令打包与重传，并在 plugins.xml 注册该组件。 变更内容Referee 系统协议与哨兵扩展
🎯 4 (Complex) | ⏱️ ~60 分钟
🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 (3)
rmcs_ws/src/rmcs_core/src/referee/status.cpp (2)
171-199: ⚡ Quick win
process_frame的分支结构存在两条独立if链,建议改成单一 dispatch。第 173-174 行
if (command_id == 0x0001)是独立if,而第 175 行起的if (command_id == 0x0003) ... else if ...是另一条链。本 PR 在该链中又新增了0x0101/0x0105/0x020D/0x0303等多个分支,后续如果有人在第二条链中漏写else或者补充0x0001同源处理,会立刻引入分支被吞掉的隐藏 bug。建议统一成单一else if链或switch (command_id)。♻️ 建议改造
- if (command_id == 0x0001) - update_game_status(); - if (command_id == 0x0003) - update_game_robot_hp(); - else if (command_id == 0x0101) - update_event_data(); - else if (command_id == 0x0105) - update_dart_info(); - else if (command_id == 0x0201) - update_robot_status(); - else if (command_id == 0x0202) - update_power_heat_data(); - else if (command_id == 0x0203) - update_robot_position(); - else if (command_id == 0x0206) - update_hurt_data(); - else if (command_id == 0x0207) - update_shoot_data(); - else if (command_id == 0x0208) - update_bullet_allowance(); - else if (command_id == 0x020B) - update_game_robot_position(); - else if (command_id == 0x020D) - update_sentry_info(); - else if (command_id == 0x0303) - update_map_command(); + switch (command_id) { + case 0x0001: update_game_status(); break; + case 0x0003: update_game_robot_hp(); break; + case 0x0101: update_event_data(); break; + case 0x0105: update_dart_info(); break; + case 0x0201: update_robot_status(); break; + case 0x0202: update_power_heat_data(); break; + case 0x0203: update_robot_position(); break; + case 0x0206: update_hurt_data(); break; + case 0x0207: update_shoot_data(); break; + case 0x0208: update_bullet_allowance(); break; + case 0x020B: update_game_robot_position(); break; + case 0x020D: update_sentry_info(); break; + case 0x0303: update_map_command(); break; + default: break; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/referee/status.cpp` around lines 171 - 199, The current process_frame() has two separate if chains (a lone if for command_id == 0x0001 and a separate if/else-if chain for other command_id values) which can cause missed branches; refactor process_frame() to use a single dispatch (either a unified else-if chain or a switch(command_id)) covering all handlers (update_game_status, update_game_robot_hp, update_event_data, update_dart_info, update_robot_status, update_power_heat_data, update_robot_position, update_hurt_data, update_shoot_data, update_bullet_allowance, update_game_robot_position, update_sentry_info, update_map_command) so each command_id is handled exclusively and no branch can be inadvertently bypassed.
256-258: 💤 Low value
bool赋值建议显式比较,避免隐式窄化。
*chassis_output_status_ = data.power_management_status & (1u << 1);把unsigned结果赋给OutputInterface<bool>,虽然行为正确但部分编译器/静态扫描会警告隐式窄化,且其他位状态(底盘/枪口供电)未来若并列处理时容易写错。♻️ 建议改造
- *chassis_output_status_ = data.power_management_status & (1u << 1); + *chassis_output_status_ = (data.power_management_status & (1u << 1)) != 0;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/referee/status.cpp` around lines 256 - 258, The assignment to OutputInterface<bool> chassis_output_status_ uses a raw unsigned bitmask expression which can trigger implicit-narrowing warnings; change the assignment in status.cpp so that *chassis_output_status_ is set to an explicit boolean expression, e.g. evaluate (data.power_management_status & (1u << 1)) != 0 (or use a named bit-test helper) instead of assigning the unsigned result directly; update the nearby robot_chassis_power_limit_ usage only if needed but ensure you reference chassis_output_status_, data.power_management_status, and robot_chassis_power_limit_ when making the fix.rmcs_ws/src/rmcs_core/src/referee/status/field.hpp (1)
7-104: ⚡ Quick win建议为所有 packed 结构体补
static_assert(sizeof(...) == N)。
MapCommand已经有static_assert(sizeof(MapCommand) == 12);(line 99),这种保护非常有效。建议把同样的 size 守卫加到GameStatus、GameRobotHp、EventData、DartInfo、RobotStatus、PowerHeatData、BulletAllowance、GameRobotPosition、SentryInfo等所有用reinterpret_cast<...&>(frame_.body.data)解码的结构体上,后续协议升级时一旦字节数对不上能在编译期立即暴露,避免线上调试。♻️ 示例
struct __attribute__((packed)) SentryInfo { uint32_t sentry_info; uint16_t sentry_info_2; }; +static_assert(sizeof(SentryInfo) == 6);对其他结构体按官方协议手册标定的字节数同步补充即可。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/referee/status/field.hpp` around lines 7 - 104, The packed POD structs used for reinterpret_cast decoding (GameStatus, GameRobotHp, EventData, DartInfo, RobotStatus, PowerHeatData, BulletAllowance, GameRobotPosition, SentryInfo, etc.) lack compile-time size guards; add static_assert(sizeof(StructName) == N) for each of those structs (using the protocol-specified byte sizes) like the existing static_assert for MapCommand so any size mismatch is caught at compile time; update each struct declaration to include a corresponding static_assert with the correct expected size.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/referee/status.cpp`:
- Around line 214-227: The sentry-bit fields in update_sentry_info() are
misaligned with make_command(): adjust the bit shifts and masks in
update_sentry_info() so they exactly match make_command()'s layout
(bullet_exchange_value bits 2-12, remote_bullet_count bits 13-16,
remote_hp_count bits 17-20, mode bits 21-22, activate_energy_mechanism bit 23),
and add extraction for remote_hp_count, confirm_revive/exchange_instant_revive
at their proper bit positions instead of reading mode/activate from
sentry_info_2; update the variables sentry_exchanged_bullet,
sentry_remote_bullet_exchange_count, remote_hp_count, confirm_revive,
exchange_instant_revive, mode, and activate_energy_mechanism to use the
corrected shifts/masks and verify against the protocol doc.
---
Nitpick comments:
In `@rmcs_ws/src/rmcs_core/src/referee/status.cpp`:
- Around line 171-199: The current process_frame() has two separate if chains (a
lone if for command_id == 0x0001 and a separate if/else-if chain for other
command_id values) which can cause missed branches; refactor process_frame() to
use a single dispatch (either a unified else-if chain or a switch(command_id))
covering all handlers (update_game_status, update_game_robot_hp,
update_event_data, update_dart_info, update_robot_status,
update_power_heat_data, update_robot_position, update_hurt_data,
update_shoot_data, update_bullet_allowance, update_game_robot_position,
update_sentry_info, update_map_command) so each command_id is handled
exclusively and no branch can be inadvertently bypassed.
- Around line 256-258: The assignment to OutputInterface<bool>
chassis_output_status_ uses a raw unsigned bitmask expression which can trigger
implicit-narrowing warnings; change the assignment in status.cpp so that
*chassis_output_status_ is set to an explicit boolean expression, e.g. evaluate
(data.power_management_status & (1u << 1)) != 0 (or use a named bit-test helper)
instead of assigning the unsigned result directly; update the nearby
robot_chassis_power_limit_ usage only if needed but ensure you reference
chassis_output_status_, data.power_management_status, and
robot_chassis_power_limit_ when making the fix.
In `@rmcs_ws/src/rmcs_core/src/referee/status/field.hpp`:
- Around line 7-104: The packed POD structs used for reinterpret_cast decoding
(GameStatus, GameRobotHp, EventData, DartInfo, RobotStatus, PowerHeatData,
BulletAllowance, GameRobotPosition, SentryInfo, etc.) lack compile-time size
guards; add static_assert(sizeof(StructName) == N) for each of those structs
(using the protocol-specified byte sizes) like the existing static_assert for
MapCommand so any size mismatch is caught at compile time; update each struct
declaration to include a corresponding static_assert with the correct expected
size.
🪄 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: a364eb20-8452-435d-8c9d-6ddb3513aa90
📒 Files selected for processing (4)
rmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/referee/command/interaction/sentry_decision.cpprmcs_ws/src/rmcs_core/src/referee/status.cpprmcs_ws/src/rmcs_core/src/referee/status/field.hpp
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
rmcs_ws/src/rmcs_core/src/referee/status.cpp (1)
360-378:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win删除
update_sentry_info()后的孤立代码块。第 360-378 行存在孤立代码，这些是
update_map_command()的重复语句但不在任何函数体内，而是直接位于类定义作用域，会导致此文件成为无效的 C++。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/referee/status.cpp` around lines 360 - 378, There is an orphaned block of code (appearing to be part of update_map_command()) left outside any function after update_sentry_info(), causing invalid C++; remove this stray block or reattach its logic into the proper function (e.g. update_map_command()) so it is inside a method body, ensuring uses of last_map_command_, has_last_map_command_, map_command_received_timestamp_, map_command_event_* and related members remain valid and compiled; after moving or deleting, run a build to confirm no unused/duplicate logic remains.
🧹 Nitpick comments (1)
rmcs_ws/src/rmcs_core/src/referee/status/field.hpp (1)
79-105: ⚡ Quick win把重复的
MapCommand断言改成新增协议结构体的尺寸断言。Line 105 又断言了一次
MapCommand，但这次新增的GameRobotPosition/SentryInfo反而没有被尺寸锁定。status.cpp里这两个 payload 都是按二进制结构体直接解析的，后续一旦有填充或字段调整，就会静默把协议解析搞偏。可直接改成这样
struct __attribute__((packed)) MapCommand { float target_position_x; float target_position_y; uint8_t cmd_keyboard; uint8_t target_robot_id; uint16_t cmd_source; }; static_assert(sizeof(MapCommand) == 12); struct __attribute__((packed)) SentryInfo { uint32_t sentry_info; uint16_t sentry_info_2; }; -static_assert(sizeof(MapCommand) == 12); +static_assert(sizeof(GameRobotPosition) == 40); +static_assert(sizeof(SentryInfo) == 6);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rmcs_ws/src/rmcs_core/src/referee/status/field.hpp` around lines 79 - 105, 文件里重复写了 MapCommand 的静态断言，应该改为为新加的结构体也加上尺寸断言；在当前代码中保留 static_assert(sizeof(MapCommand) == 12) 并把第二个重复断言替换为 static_assert(sizeof(GameRobotPosition) == 40) 和 static_assert(sizeof(SentryInfo) == 6)，以确保 GameRobotPosition、SentryInfo 和 MapCommand 在被按二进制解析时尺寸不发生静默变化（参考类型名 GameRobotPosition、SentryInfo、MapCommand 和现有 static_assert 使用位置）。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/referee/status.cpp`:
- Around line 344-358: The update_sentry_info function currently
reinterpret_casts frame_.body.data into SentryInfo and unpacks fields without
validating the received payload length; add a length check against
sizeof(SentryInfo) (e.g., verify frame_.body.size/length >= sizeof(SentryInfo))
before doing the reinterpret_cast and bit extraction, and if the check fails,
avoid writing the sentry outputs (or set them to safe defaults) so that
short/invalid 0x020D packets cannot write dirty bitfields into
*sentry_exchanged_bullet_, *sentry_remote_bullet_exchange_count_,
*sentry_can_confirm_free_revive_, *sentry_can_exchange_instant_revive_,
*sentry_instant_revive_cost_, *sentry_exchangeable_bullet_, *sentry_mode_, and
*sentry_energy_mechanism_activatable_; keep all decoding logic (shifts/masks)
only inside the guarded branch.
- Around line 293-304: update_game_robot_position() currently reinterpret_casts
frame_.body.data to GameRobotPosition without checking payload length for
message 0x020B; add the same payload-length guard used in update_map_command():
verify frame_.body.data_length >= sizeof(GameRobotPosition) (or >= 40) before
doing the reinterpret_cast and assigning to ally_hero_position_x_,
ally_hero_position_y_, ally_engineer_position_x_, ally_engineer_position_y_,
ally_infantry_1_position_x_, ally_infantry_1_position_y_,
ally_infantry_2_position_x_, and ally_infantry_2_position_y_; if the check
fails, skip updates (or set safe defaults) to avoid using leftover buffer data.
---
Outside diff comments:
In `@rmcs_ws/src/rmcs_core/src/referee/status.cpp`:
- Around line 360-378: There is an orphaned block of code (appearing to be part
of update_map_command()) left outside any function after update_sentry_info(),
causing invalid C++; remove this stray block or reattach its logic into the
proper function (e.g. update_map_command()) so it is inside a method body,
ensuring uses of last_map_command_, has_last_map_command_,
map_command_received_timestamp_, map_command_event_* and related members remain
valid and compiled; after moving or deleting, run a build to confirm no
unused/duplicate logic remains.
---
Nitpick comments:
In `@rmcs_ws/src/rmcs_core/src/referee/status/field.hpp`:
- Around line 79-105: 文件里重复写了 MapCommand 的静态断言，应该改为为新加的结构体也加上尺寸断言；在当前代码中保留
static_assert(sizeof(MapCommand) == 12) 并把第二个重复断言替换为
static_assert(sizeof(GameRobotPosition) == 40) 和
static_assert(sizeof(SentryInfo) == 6)，以确保 GameRobotPosition、SentryInfo 和
MapCommand 在被按二进制解析时尺寸不发生静默变化（参考类型名 GameRobotPosition、SentryInfo、MapCommand 和现有
static_assert 使用位置）。
🪄 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: 3f7bec07-e224-43d3-933e-ebf8f50189ab
📒 Files selected for processing (3)
rmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/referee/status.cpprmcs_ws/src/rmcs_core/src/referee/status/field.hpp
🚧 Files skipped from review as they are similar to previous changes (1)
- rmcs_ws/src/rmcs_core/plugins.xml
…_commands and sentry data
…bot decision commands
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rmcs_ws/src/rmcs_core/src/referee/command/interaction/sentry_decision.cpp`:
- Around line 120-123: The method is_sentry_robot() and similar accessors
currently dereference robot_id_ (and sentry_mode_) without checking readiness;
before dereferencing *robot_id_ or *sentry_mode_ call the input handle's ready()
(or equivalent) and return a safe default (false for is_sentry_robot(), and a
neutral/default enum value for sentry_mode_) when not ready; update
is_sentry_robot(), any accessor in the same block (lines around 137–153), and
any place that uses *robot_id_ or *sentry_mode_ to first check ready() and only
then dereference to avoid startup null/invalid access.
In `@rmcs_ws/src/rmcs_core/src/referee/status/field.hpp`:
- Around line 79-118: 为防止打包结构随编译器/ABI变化导致协议错位，在这些结构定义后加编译期尺寸校验：为
GameRobotPosition、MapCommand、OpponentMapRobotData 和 SentryInfo 分别添加
static_assert(sizeof(GameRobotPosition) == 40, "..."),
static_assert(sizeof(MapCommand) == 12, "..."),
static_assert(sizeof(OpponentMapRobotData) == 24, "...") 和
static_assert(sizeof(SentryInfo) == 6, "...")（将断言放在各 struct
定义之后或同一头文件合适位置，并提供简短错误信息以便诊断）。
🪄 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: 6c37f34a-987e-4160-9376-8caa1ef78249
📒 Files selected for processing (4)
rmcs_ws/src/rmcs_core/plugins.xmlrmcs_ws/src/rmcs_core/src/referee/command/interaction/sentry_decision.cpprmcs_ws/src/rmcs_core/src/referee/status.cpprmcs_ws/src/rmcs_core/src/referee/status/field.hpp
✅ Files skipped from review due to trivial changes (1)
- rmcs_ws/src/rmcs_core/plugins.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- rmcs_ws/src/rmcs_core/src/referee/status.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
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.
This PR updates RMCS referee communication to match the latest referee system protocol, and adds sentry-referee interaction support.
Changes
SentryDecisioninteraction component for sentry decision commandsplugins.xmlValidation
费更新 Referee 通信协议并添加哨兵交互支持
此 PR 将 RMCS 与最新 referee 协议对齐，扩展了 referee->本地 的状态解析与发布项，并新增哨兵（sentry）相关的交互组件与主题。
主要改动
协议字段与打包结构（field.hpp）
状态处理扩展（status.cpp）
哨兵交互组件（sentry_decision.cpp）
插件注册（plugins.xml）
验证