Dev#14
Conversation
Reviewer's GuideReferee.hpp is extended with richer launcher/chassis/sentry data structures and new helper methods for configuring and sending sentry decision commands, along with some minor refactors to data publishing and send/IO helpers; the CI-related build workflow is simplified by removing a debug print from the test harness. Sequence diagram for sending updated sentry decision commandsequenceDiagram
actor SentryController
participant Referee
participant SentryDecisionData
participant UART
SentryController->>Referee: SetNeedBullet(need_bullet)
Referee->>SentryDecisionData: increment buy_bullet_num
SentryController->>Referee: SetConfirmRevival(revival)
Referee->>SentryDecisionData: set confirm_resurrection
SentryController->>Referee: SetSwitchMode(state)
Referee->>SentryDecisionData: set current_state
SentryController->>Referee: SendSentryPack()
Referee->>Referee: SendStudentCmd(REF_STDNT_CMD_ID_SENTRY_CMD, GetRobotID(), 0x8080, sentry_dec_data)
Referee->>UART: SendFrame(..., payload)
UART-->>Referee: LibXR::ErrorCode
Referee-->>SentryController: LibXR::ErrorCode
Class diagram for updated Referee data packs and sentry helper methodsclassDiagram
class Referee {
+LauncherPack lp_
+ChassisPack cp_
+SentryPack sp_
+void SetNeedBullet(uint8_t need_bullet)
+void SetConfirmRevival(bool revival)
+void SetBulletRemote(uint8_t bullet_number)
+void SetHPRemote()
+void SetRevivalRemote(bool revival)
+void SetSwitchMode(State state)
+void SendSentryPack()
+LibXR::ErrorCode SendFrame(CommandID cmd_id, const void* payload, uint16_t PAYLOAD_LEN)
+LibXR::ErrorCode SendStudentCmd(CMDID data_cmd_id, uint16_t sender_id, uint16_t receiver_id, const PayloadType& payload)
}
class LauncherPack {
+RobotStatus rs
+RobotBuff rb
+LauncherData ld
+DartClient dc
}
class ChassisPack {
+RobotStatus rs
+uint16_t power_buffer
}
class SentryPack {
+Referee::RobotStatus rs
+Referee::GameStatus gs
}
class State {
<<enumeration>>
ATTACH
DEFEND
GUERRILLA
}
Referee o-- LauncherPack
Referee o-- ChassisPack
Referee o-- SentryPack
Referee .. State
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the new sentry setter APIs (e.g., SetNeedBullet, SetBulletRemote, SetHPRemote) you are incrementing fields (+= / ++-style semantics) rather than assigning, which may be surprising for 'Set*' functions; consider either renaming them to reflect incremental behavior or switching to assignment so the API is clearer.
- The member name romote_buy_hp_times in SetHPRemote looks like a typo; if possible, align the field name with the intended spelling (remote_buy_hp_times) to avoid confusion.
- You replaced LibXR::Memory::FastSet with a raw memset for robot_ineraction_data.user_data; consider using the same project-specific memory utility here for consistency with the rest of the codebase.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the new sentry setter APIs (e.g., SetNeedBullet, SetBulletRemote, SetHPRemote) you are incrementing fields (+= / ++-style semantics) rather than assigning, which may be surprising for 'Set*' functions; consider either renaming them to reflect incremental behavior or switching to assignment so the API is clearer.
- The member name romote_buy_hp_times in SetHPRemote looks like a typo; if possible, align the field name with the intended spelling (remote_buy_hp_times) to avoid confusion.
- You replaced LibXR::Memory::FastSet with a raw memset for robot_ineraction_data.user_data; consider using the same project-specific memory utility here for consistency with the rest of the codebase.
## Individual Comments
### Comment 1
<location path="Referee.hpp" line_range="1008-1009" />
<code_context>
+ * @brief 哨兵姿态
+ *
+ */
+ enum class State : uint8_t{
+ ATTACH = 0,
+ DEFEND = 1,
+ GUERRILLA = 2,
</code_context>
<issue_to_address>
**nitpick (typo):** State value `ATTACH` might be a typo for `ATTACK`.
Given the other states (`DEFEND`, `GUERRILLA`) and the context of 哨兵姿态, this looks like it was meant to be `ATTACK`. If it really means "attach" (e.g., follow mode), consider renaming to something clearer like `FOLLOW` to avoid confusion.
Suggested implementation:
```
enum class State : uint8_t{
ATTACK = 0,
DEFEND = 1,
GUERRILLA = 2,
```
1. Update all usages of `State::ATTACH` in the codebase to `State::ATTACK` to keep the build compiling and behavior consistent.
2. If there is any serialized protocol, documentation, or UI that references "ATTACH", update those references to "ATTACK" as well, unless the external protocol is fixed and requires the old literal (in which case, consider keeping the numeric value 0 but mapping it to `ATTACK`).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| enum class State : uint8_t{ | ||
| ATTACH = 0, |
There was a problem hiding this comment.
nitpick (typo): State value ATTACH might be a typo for ATTACK.
Given the other states (DEFEND, GUERRILLA) and the context of 哨兵姿态, this looks like it was meant to be ATTACK. If it really means "attach" (e.g., follow mode), consider renaming to something clearer like FOLLOW to avoid confusion.
Suggested implementation:
enum class State : uint8_t{
ATTACK = 0,
DEFEND = 1,
GUERRILLA = 2,
- Update all usages of
State::ATTACHin the codebase toState::ATTACKto keep the build compiling and behavior consistent. - If there is any serialized protocol, documentation, or UI that references "ATTACH", update those references to "ATTACK" as well, unless the external protocol is fixed and requires the old literal (in which case, consider keeping the numeric value 0 but mapping it to
ATTACK).
ci fix
Summary by Sourcery
Extend referee communication structures and add sentry control helpers while cleaning up minor formatting and CI build output.
New Features:
Enhancements:
CI: