适配VT13#4
Conversation
Reviewer's GuideAdd VT13 as an additional RC input source and introduce logic to select and track the active RC source while refactoring CMD to store RC inputs per source and updating processing/publish logic accordingly. Sequence diagram for RC multi-source input selection and publishingsequenceDiagram
participant RC_DR16
participant RC_VT13
participant CMD
participant LibXR_Event as Event
participant Topic_Chassis as ChassisTopic
participant Topic_Gimbal as GimbalTopic
participant Topic_Fire as FireTopic
RC_DR16->>CMD: FeedRC(Data rc_dr16)
activate CMD
CMD->>CMD: FeedRC(RCInputSource_RC_INPUT_DR16, rc_dr16)
CMD->>CMD: rc_input_data_[DR16] = rc_dr16
CMD->>CMD: rc_input_seq_[DR16] = ++rc_update_seq_
CMD->>CMD: IsRCInputActive(rc_dr16)
alt DR16 data is active and online
CMD->>CMD: active_rc_input_ = RC_INPUT_DR16
end
CMD->>CMD: ProcessAndPublish()
deactivate CMD
RC_VT13->>CMD: FeedRC(RCInputSource_RC_INPUT_VT13, Data rc_vt13)
activate CMD
CMD->>CMD: rc_input_data_[VT13] = rc_vt13
CMD->>CMD: rc_input_seq_[VT13] = ++rc_update_seq_
CMD->>CMD: IsRCInputActive(rc_vt13)
alt VT13 data is active and online
CMD->>CMD: active_rc_input_ = RC_INPUT_VT13
end
CMD->>CMD: ProcessAndPublish()
CMD->>CMD: SelectRCData()
alt active_rc_input_ online
CMD->>CMD: use rc_input_data_[active]
else active source offline
alt other source online
CMD->>CMD: switch active_rc_input_ to other source
CMD->>CMD: use rc_input_data_[new_active]
else both offline
CMD->>CMD: MakeOfflineRCData()
end
end
CMD->>CMD: update data_[CTRL_SOURCE_RC]
alt RC offline and was online
CMD->>Event: Active(CMD_EVENT_LOST_CTRL)
else RC online and was offline
CMD->>Event: Active(CMD_EVENT_START_CTRL)
end
alt mode_ == CMD_OP_CTRL
CMD->>GimbalTopic: Publish(rc_selected.gimbal)
CMD->>ChassisTopic: Publish(rc_selected.chassis)
CMD->>FireTopic: Publish(rc_selected.launcher)
else mode_ == CMD_AUTO_CTRL
CMD->>CMD: choose ai or rc for chassis/gimbal/launcher
CMD->>GimbalTopic: Publish(out_gimbal)
CMD->>ChassisTopic: Publish(out_chassis)
CMD->>FireTopic: Publish(out_launcher)
end
deactivate CMD
Updated class diagram for CMD RC multi-input handlingclassDiagram
class LibXR_Application
class Data
Data : bool chassis_online
Data : bool gimbal_online
Data : ControlSource ctrl_source
Data : ChassisCMD chassis
Data : GimbalCMD gimbal
Data : LauncherCMD launcher
class ChassisCMD
class GimbalCMD
class LauncherCMD
class LibXR_Event
LibXR_Event : Active(uint32_t event_id)
LibXR_Event : Register(uint32_t event_id, Callback_uint32_t callback)
class LibXR_Topic
LibXR_Topic : Publish(ChassisCMD chassis)
LibXR_Topic : Publish(GimbalCMD gimbal)
LibXR_Topic : Publish(LauncherCMD launcher)
class CMD
CMD : Mode mode_
CMD : bool online_
CMD : LibXR_Event cmd_event_
CMD : array~Data~ data_
CMD : array~Data~ rc_input_data_
CMD : array~uint32_t~ rc_input_seq_
CMD : LibXR_Topic chassis_data_tp_
CMD : LibXR_Topic gimbal_data_tp_
CMD : LibXR_Topic fire_data_tp_
CMD : LibXR_Topic host_euler_data_tp_
CMD : RCInputSource active_rc_input_
CMD : uint32_t rc_update_seq_
CMD : FeedRC(Data rc_data)
CMD : FeedRC(RCInputSource source, Data rc_data)
CMD : FeedAI(Data ai_data)
CMD : Init(void* hw, void* app)
CMD : EventHandler(uint32_t event_id)
CMD : ProcessAndPublish()
CMD : Data SelectRCData()
CMD : static bool IsRCInputOnline(Data rc_data)
CMD : static bool IsRCInputActive(Data rc_data)
CMD : static Data MakeOfflineRCData()
class ControlSource {
<<enum>>
ControlSource : CTRL_SOURCE_RC
ControlSource : CTRL_SOURCE_AI
ControlSource : CTRL_SOURCE_NUM
}
class RCInputSource {
<<enum>>
RCInputSource : RC_INPUT_DR16
RCInputSource : RC_INPUT_VT13
RCInputSource : RC_INPUT_NUM
}
class Mode {
<<enum>>
Mode : CMD_OP_CTRL
Mode : CMD_AUTO_CTRL
}
CMD --|> LibXR_Application
CMD --> LibXR_Event : uses
CMD --> LibXR_Topic : publishes
CMD --> Data : processes
Data --> ControlSource
CMD --> RCInputSource
CMD --> Mode
Data --> ChassisCMD
Data --> GimbalCMD
Data --> LauncherCMD
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:
- The
rc_update_seq_counter andrc_input_seq_array are written but never read, which suggests incomplete or dead logic for choosing between multiple RC sources; either remove them or complete the selection logic to use the sequence information. FeedRC(RCInputSource source, ...)silently returns on an invalidsource_index; consider asserting or logging in this path so configuration or call-site errors are easier to detect.- In
MakeOfflineRCDatayou always setctrl_sourcetoCTRL_SOURCE_RCeven when the last active input might have beenVT13; ifctrl_sourceis used downstream for telemetry or behavior decisions, you may want to reflect the specific RC input or explicitly mark it as offline/unknown instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `rc_update_seq_` counter and `rc_input_seq_` array are written but never read, which suggests incomplete or dead logic for choosing between multiple RC sources; either remove them or complete the selection logic to use the sequence information.
- `FeedRC(RCInputSource source, ...)` silently returns on an invalid `source_index`; consider asserting or logging in this path so configuration or call-site errors are easier to detect.
- In `MakeOfflineRCData` you always set `ctrl_source` to `CTRL_SOURCE_RC` even when the last active input might have been `VT13`; if `ctrl_source` is used downstream for telemetry or behavior decisions, you may want to reflect the specific RC input or explicitly mark it as offline/unknown instead.
## Individual Comments
### Comment 1
<location path="CMD.hpp" line_range="164" />
<code_context>
+ }
+
+ this->rc_input_data_[source_index] = rc_data;
+ this->rc_input_seq_[source_index] = ++this->rc_update_seq_;
+
+ if (rc_data.chassis_online && this->IsRCInputActive(rc_data)) {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** rc_input_seq_/rc_update_seq_ are written here but not used in selection, which may indicate either dead state or a missed use in RC source arbitration.
In this header, rc_input_seq_ and rc_update_seq_ are only written in FeedRC and never read (including in SelectRCData). If recency is meant to influence RC source arbitration (e.g., for tie‑breaking), that logic is missing. If recency isn’t needed, consider removing this sequence tracking to avoid dead state and confusion about intent.
Suggested implementation:
```
/**
* @brief 按遥控输入源写入控制数据
*/
void FeedRC(RCInputSource source, const Data& rc_data) {
const auto source_index = static_cast<size_t>(source);
if (source_index >= static_cast<size_t>(RCInputSource::RC_INPUT_NUM)) {
return;
}
this->rc_input_data_[source_index] = rc_data;
if (rc_data.chassis_online && this->IsRCInputActive(rc_data)) {
this->active_rc_input_ = source;
}
this->ProcessAndPublish();
}
/**
* @brief 直接写入 AI 控制数据
*/
void FeedAI(const Data& ai_data) {
```
1. Remove the `rc_input_seq_` and `rc_update_seq_` member variables from the class definition in CMD.hpp (or its corresponding header), including any initialization in the constructor.
2. Remove any other writes to `rc_input_seq_` / `rc_update_seq_` if they exist elsewhere.
3. If there are comments or documentation referring to RC input sequencing/recency, update or remove them to reflect that recency is no longer tracked.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } | ||
|
|
||
| this->rc_input_data_[source_index] = rc_data; | ||
| this->rc_input_seq_[source_index] = ++this->rc_update_seq_; |
There was a problem hiding this comment.
suggestion (bug_risk): rc_input_seq_/rc_update_seq_ are written here but not used in selection, which may indicate either dead state or a missed use in RC source arbitration.
In this header, rc_input_seq_ and rc_update_seq_ are only written in FeedRC and never read (including in SelectRCData). If recency is meant to influence RC source arbitration (e.g., for tie‑breaking), that logic is missing. If recency isn’t needed, consider removing this sequence tracking to avoid dead state and confusion about intent.
Suggested implementation:
/**
* @brief 按遥控输入源写入控制数据
*/
void FeedRC(RCInputSource source, const Data& rc_data) {
const auto source_index = static_cast<size_t>(source);
if (source_index >= static_cast<size_t>(RCInputSource::RC_INPUT_NUM)) {
return;
}
this->rc_input_data_[source_index] = rc_data;
if (rc_data.chassis_online && this->IsRCInputActive(rc_data)) {
this->active_rc_input_ = source;
}
this->ProcessAndPublish();
}
/**
* @brief 直接写入 AI 控制数据
*/
void FeedAI(const Data& ai_data) {
- Remove the
rc_input_seq_andrc_update_seq_member variables from the class definition in CMD.hpp (or its corresponding header), including any initialization in the constructor. - Remove any other writes to
rc_input_seq_/rc_update_seq_if they exist elsewhere. - If there are comments or documentation referring to RC input sequencing/recency, update or remove them to reflect that recency is no longer tracked.
Summary by Sourcery
Add support for multiple RC input sources and centralize selection logic for command processing.
New Features:
Enhancements: