Dev#13
Closed
Gcgc4514 wants to merge 5 commits into
Closed
Conversation
Reviewer's GuideExtends the referee interface to provide structured data packs for launcher, chassis, and sentry modules (including new sentry decision helpers), and performs minor refactors around data copying, UART read handling, and send helper formatting. Sequence diagram for SendSentryPack and referee transmissionsequenceDiagram
actor SentryModule
participant Referee
participant UART
SentryModule->>Referee: SetNeedBullet(need_bullet)
SentryModule->>Referee: SetConfirmRevival(revival)
SentryModule->>Referee: SetBulletRemote(bullet_number)
SentryModule->>Referee: SetHPRemote()
SentryModule->>Referee: SetRevivalRemote(revival)
SentryModule->>Referee: SetSwitchMode(state)
SentryModule->>Referee: SendSentryPack()
Referee->>Referee: Prepare sentry_dec_data
Referee->>Referee: SendStudentCmd(REF_STDNT_CMD_ID_SENTRY_CMD, GetRobotID(), 0x8080, sentry_dec_data)
Referee->>Referee: SendFrame(cmd_id, payload, PAYLOAD_LEN)
Referee->>UART: Write(frame_bytes)
UART-->>Referee: LibXR_ErrorCode OK
Referee-->>SentryModule: transmission result
Class diagram for updated Referee packs and sentry helpersclassDiagram
class Referee {
+LauncherPack lp_
+ChassisPack cp_
+SentryPack sp_
+Data data_
+Topic launcherpack_topic_
+Topic chassispack_topic_
+Topic sentrypack_topic_
+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 {
+RobotStatus rs
+GameStatus gs
}
class State {
<<enumeration>>
ATTACH
DEFEND
GUERRILLA
}
class Data {
+RobotStatus robot_status
+RobotBuff robot_buff
+LauncherData launcher_data
+DartClient dart_client
+PowerHeat power_heat
+GameStatus game_status
+SentryDecisionData sentry_dec_data
+RobotInteractionData robot_ineraction_data
}
class SentryDecisionData {
+uint8_t buy_bullet_num
+bool confirm_resurrection
+uint8_t remote_buy_bullet_times
+uint8_t romote_buy_hp_times
+bool buy_resurrection
+uint32_t current_state
}
class RobotStatus
class RobotBuff
class LauncherData
class DartClient
class GameStatus
class PowerHeat {
+uint16_t chassis_pwr_buff
+uint16_t launcher_id1_17_heat
}
class RobotInteractionData {
+uint8_t user_data
}
class Topic {
+void Publish(T payload)
}
class CommandID
class CMDID
class LibXR_ErrorCode
Referee --> LauncherPack : uses
Referee --> ChassisPack : uses
Referee --> SentryPack : uses
Referee --> Data : aggregates
Referee --> State : uses
Referee --> SentryDecisionData : modifies
Data --> RobotStatus
Data --> RobotBuff
Data --> LauncherData
Data --> DartClient
Data --> PowerHeat
Data --> GameStatus
Data --> SentryDecisionData
Data --> RobotInteractionData
Referee --> Topic : publishes packs
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The setter methods for bullet quantities (
SetNeedBullet,SetBulletRemote) use+=semantics even though the comments describe them as setting values; if these APIs are intended to overwrite rather than accumulate, consider switching to assignment to avoid unexpected state growth across calls. - When writing the sentinel state in
SetSwitchMode,Stateis cast touint32_t; if the protocol field is not actually 32-bit or the enum values must match a specific wire format, it would be safer to defineStatewith an explicit underlying type that matches the field and store it directly. - You replaced
LibXR::Memory::FastSetwithmemsetforrobot_ineraction_data.user_data; if the project relies on the abstraction (e.g., for portability or instrumentation), consider keepingFastSetfor consistency with the rest of the codebase.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The setter methods for bullet quantities (`SetNeedBullet`, `SetBulletRemote`) use `+=` semantics even though the comments describe them as setting values; if these APIs are intended to overwrite rather than accumulate, consider switching to assignment to avoid unexpected state growth across calls.
- When writing the sentinel state in `SetSwitchMode`, `State` is cast to `uint32_t`; if the protocol field is not actually 32-bit or the enum values must match a specific wire format, it would be safer to define `State` with an explicit underlying type that matches the field and store it directly.
- You replaced `LibXR::Memory::FastSet` with `memset` for `robot_ineraction_data.user_data`; if the project relies on the abstraction (e.g., for portability or instrumentation), consider keeping `FastSet` for consistency with the rest of the codebase.
## Individual Comments
### Comment 1
<location path="Referee.hpp" line_range="1029-1030" />
<code_context>
- RobotStatus rs; /* 等级和功率上限 */
- uint16_t power_buffer; /* 底盘缓冲能量，单位 J */
- };
+ void SetNeedBullet(uint8_t need_bullet){
+ this->data_.sentry_dec_data.buy_bullet_num += need_bullet;
+ }
+
</code_context>
<issue_to_address>
**question (bug_risk):** Method name/doc suggest setting an absolute value but implementation accumulates, which can be confusing or unintended.
Using `+=` means each call adds to `buy_bullet_num` rather than setting it. If the protocol expects an absolute value per frame, this will lead to over-redemption. If accumulation is correct, please rename the method or update the doc so it’s clearly not a simple setter.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1029
to
+1030
| void SetNeedBullet(uint8_t need_bullet){ | ||
| this->data_.sentry_dec_data.buy_bullet_num += need_bullet; |
There was a problem hiding this comment.
question (bug_risk): Method name/doc suggest setting an absolute value but implementation accumulates, which can be confusing or unintended.
Using += means each call adds to buy_bullet_num rather than setting it. If the protocol expects an absolute value per frame, this will lead to over-redemption. If accumulation is correct, please rename the method or update the doc so it’s clearly not a simple setter.
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.
哨兵包定义及launcher pack
Summary by Sourcery
Define and expose new data packs and control interfaces for the sentry, chassis, and launcher subsystems in the referee module, and hook them into existing publish/send flows.
New Features:
Enhancements: