Skip to content

Add VT13 remote control and dual control arbitration#93

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
creeper5820 merged 1 commit into from
Jul 19, 2026
Merged

Add VT13 remote control and dual control arbitration #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rmcs_ws/src/rmcs_bringup/config/steering-hero-little-six-friction.yaml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -8,7 +8,6 @@ rmcs_executor:
- rmcs_core::referee::command::Interaction -> referee_interaction
- rmcs_core::referee::command::interaction::Ui -> referee_ui
- rmcs_core::referee::app::ui::Hero -> referee_ui_hero
- rmcs_core::referee::app::ui::AutoAimUi -> auto_aim_ui
- rmcs_core::referee::Command -> referee_command

- rmcs_core::controller::gimbal::HeroGimbalController -> gimbal_controller
Expand DownExpand Up@@ -38,6 +37,7 @@ rmcs_executor:

- rmcs::AutoAimCapturerComponent -> auto_aim_capturer
- rmcs::AutoAimComponent -> auto_aim_component
- rmcs_core::referee::app::ui::AutoAimUi -> auto_aim_ui

# - rmcs_core::broadcaster::ValueBroadcaster -> value_broadcaster
# - rmcs_core::broadcaster::TfBroadcaster -> tf_broadcaster
Expand Down
27 changes: 23 additions & 4 deletions rmcs_ws/src/rmcs_core/src/hardware/deformable-infantry-omni-b.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,9 @@
#include "hardware/device/dji_motor.hpp"
#include "hardware/device/dr16.hpp"
#include "hardware/device/lk_motor.hpp"
#include "hardware/device/remote_control.hpp"
#include "hardware/device/supercap.hpp"
#include "hardware/device/vt13.hpp"
#include "hardware/util/status_monitor.hpp"

namespace rmcs_core::hardware {
Expand All@@ -58,6 +60,8 @@ class DeformableInfantryOmniB

tf_->set_transform<PitchLink, CameraLink>(Eigen::Translation3d{0.058, -0.08, 0.0});

remote_control_ = std::make_unique<device::RemoteControl>(*this);

bottom_board_ = std::make_unique<BottomBoard>(
*this, *command_, get_parameter("serial_filter_bottom_board").as_string());
top_board_ = std::make_unique<TopBoard>(
Expand All@@ -79,6 +83,7 @@ class DeformableInfantryOmniB
void update() override {
bottom_board_->update();
top_board_->update();
remote_control_->update();

using namespace rmcs_description;
*camera_transform_ = fast_tf::lookup_transform<OdomImu, CameraLink>(*tf_);
Expand DownExpand Up@@ -121,7 +126,8 @@ class DeformableInfantryOmniB
explicit TopBoard(
DeformableInfantryOmniB& status, Component& command,
const std::string& serial_filter = {})
: tf_{status.tf_}
: status_{status}
, tf_{status.tf_}
, bmi088_{device::Bmi088Ekf::Config{
.body_to_sensor =
Eigen::AngleAxisd{std::numbers::pi / 2.0, Eigen::Vector3d::UnitX()}
Expand DownExpand Up@@ -163,8 +169,11 @@ class DeformableInfantryOmniB
.capture_timestamp = true,
.pull = librmcs::data::GpioPull::kUp,
});
}

board_->start_transmit().uart_config(Spec::kUarts.kUart0, {.baudrate = 921600});

status_.remote_control_->register_vt13(&vt13_);
}
~TopBoard() override = default;

[[nodiscard]] auto gimbal_yaw_velocity() const -> double {
Expand All@@ -177,6 +186,7 @@ class DeformableInfantryOmniB
}

void update() {
vt13_.update_status();
gimbal_pitch_motor_.update_status();
gimbal_left_friction_.update_status();
gimbal_right_friction_.update_status();
Expand DownExpand Up@@ -243,6 +253,11 @@ class DeformableInfantryOmniB
}
}

void uart_receive_callback(const Spec::Uart& uart, const View::Uart& data) override {
if (uart == Spec::kUarts.kUart0)
vt13_.store_status(data.uart_data);
}

void accelerometer_receive_callback(const View::ImuAccelerometer& data) override {
const auto timestamp = board_clock_lifter_.advance_timebase(data.timestamp_quarter_us);
bmi088_.push_accelerometer_sample(data.x, data.y, data.z, timestamp);
Expand DownExpand Up@@ -277,6 +292,7 @@ class DeformableInfantryOmniB

auto status() const -> std::vector<std::string> { return monitor_.text(); }

DeformableInfantryOmniB& status_;
OutputInterface<rmcs_description::Tf>& tf_;
OutputInterface<double> gimbal_yaw_velocity_bmi088_;
OutputInterface<double> gimbal_pitch_velocity_bmi088_;
Expand All@@ -286,6 +302,7 @@ class DeformableInfantryOmniB

device::Bmi088Ekf bmi088_;
device::BoardClockLifter board_clock_lifter_;
device::Vt13 vt13_;
device::LkMotor gimbal_pitch_motor_;
device::DjiMotor gimbal_left_friction_;
device::DjiMotor gimbal_right_friction_;
Expand DownExpand Up@@ -371,8 +388,9 @@ class DeformableInfantryOmniB
auto options = librmcs::board::AdvancedOptions{};
options.dangerously_skip_version_checks = true;
board_ = std::make_unique<librmcs::board::RmcsBoardLite>(*this, serial_filter, options);
}

status_.remote_control_->register_dr16(&dr16_);
}
void update() {
imu_.update_status();
*chassis_yaw_velocity_imu_ = imu_.gz();
Expand DownExpand Up@@ -584,7 +602,7 @@ class DeformableInfantryOmniB

device::Bmi088 imu_{1000, 0.2, 0.0};
device::LkMotor gimbal_yaw_motor_{status_, command_, "/gimbal/yaw"};
device::Dr16 dr16_{status_};
device::Dr16 dr16_;

device::DjiMotor chassis_wheel_motors_[4]{
device::DjiMotor{status_, command_, "/chassis/left_front_wheel"},
Expand DownExpand Up@@ -844,6 +862,7 @@ class DeformableInfantryOmniB

std::unique_ptr<BottomBoard> bottom_board_;
std::unique_ptr<TopBoard> top_board_;
std::unique_ptr<device::RemoteControl> remote_control_;

std::shared_ptr<Command> command_;
uint32_t cmd_tick_ = 0;
Expand Down
9 changes: 8 additions & 1 deletion rmcs_ws/src/rmcs_core/src/hardware/deformable-infantry-omni.cpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -32,6 +32,7 @@
#include "hardware/device/dji_motor.hpp"
#include "hardware/device/dr16.hpp"
#include "hardware/device/lk_motor.hpp"
#include "hardware/device/remote_control.hpp"
#include "hardware/device/supercap.hpp"
#include "hardware/util/status_monitor.hpp"

Expand DownExpand Up@@ -59,6 +60,8 @@ class DeformableInfantryOmni

tf_->set_transform<PitchLink, CameraLink>(Eigen::Translation3d{0.058, -0.08, 0.0});

remote_control_ = std::make_unique<device::RemoteControl>(*this);

bottom_board_ = std::make_unique<BottomBoard>(
*this, *command_, get_parameter("serial_filter_bottom_board").as_string());
top_board_ = std::make_unique<TopBoard>(
Expand All@@ -80,6 +83,7 @@ class DeformableInfantryOmni
void update() override {
bottom_board_->update();
top_board_->update();
remote_control_->update();

using namespace rmcs_description;
*camera_transform_ = fast_tf::lookup_transform<OdomImu, CameraLink>(*tf_);
Expand DownExpand Up@@ -193,6 +197,8 @@ class DeformableInfantryOmni
auto options = librmcs::board::AdvancedOptions{};
options.dangerously_skip_version_checks = true;
board_ = std::make_unique<librmcs::board::RmcsBoardLite>(*this, serial_filter, options);

status_.remote_control_->register_dr16(&dr16_);
}

void update() {
Expand DownExpand Up@@ -406,7 +412,7 @@ class DeformableInfantryOmni

device::Bmi088 imu_{1000, 0.2, 0.0};
device::LkMotor gimbal_yaw_motor_{status_, command_, "/gimbal/yaw"};
device::Dr16 dr16_{status_};
device::Dr16 dr16_{};

device::DjiMotor chassis_wheel_motors_[4]{
device::DjiMotor{status_, command_, "/chassis/left_front_wheel"},
Expand DownExpand Up@@ -848,6 +854,7 @@ class DeformableInfantryOmni

std::unique_ptr<BottomBoard> bottom_board_;
std::unique_ptr<TopBoard> top_board_;
std::unique_ptr<device::RemoteControl> remote_control_;

std::shared_ptr<Command> command_;
uint32_t cmd_tick_ = 0;
Expand Down
107 changes: 47 additions & 60 deletions rmcs_ws/src/rmcs_core/src/hardware/device/dr16.hpp
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,11 +6,9 @@

#include <atomic>
#include <bit>
#include <chrono>

#include <eigen3/Eigen/Dense>
#include <rclcpp/logger.hpp>
#include <rclcpp/logging.hpp>
#include <rmcs_executor/component.hpp>
#include <rmcs_msgs/keyboard.hpp>
#include <rmcs_msgs/mouse.hpp>
#include <rmcs_msgs/switch.hpp>
Expand All@@ -19,32 +17,7 @@ namespace rmcs_core::hardware::device {

class Dr16 {
public:
explicit Dr16(rmcs_executor::Component& component) {
component.register_output(
"/remote/joystick/right", joystick_right_output_, Eigen::Vector2d::Zero());
component.register_output(
"/remote/joystick/left", joystick_left_output_, Eigen::Vector2d::Zero());

component.register_output(
"/remote/switch/right", switch_right_output_, rmcs_msgs::Switch::UNKNOWN);
component.register_output(
"/remote/switch/left", switch_left_output_, rmcs_msgs::Switch::UNKNOWN);

component.register_output(
"/remote/mouse/velocity", mouse_velocity_output_, Eigen::Vector2d::Zero());
component.register_output("/remote/mouse/mouse_wheel", mouse_wheel_output_);

component.register_output("/remote/mouse", mouse_output_);
std::memset(&*mouse_output_, 0, sizeof(*mouse_output_));
component.register_output("/remote/keyboard", keyboard_output_);
std::memset(&*keyboard_output_, 0, sizeof(*keyboard_output_));

component.register_output("/remote/rotary_knob", rotary_knob_output_);

// Simulate the rotary knob as a switch, with anti-shake algorithm.
component.register_output(
"/remote/rotary_knob_switch", rotary_knob_switch_output_, rmcs_msgs::Switch::UNKNOWN);
}
Dr16() = default;

void store_status(const std::byte* uart_data, size_t uart_data_length) {
if (uart_data_length != 6 + 8 + 4)
Expand DownExpand Up@@ -73,9 +46,17 @@ class Dr16 {
std::memcpy(&part3, uart_data, 4);
uart_data += 4;
data_part3_.store(part3, std::memory_order::relaxed);

last_remote_control_received_at_ = Clock::now();
valid_ = true;
}

void update_status() {
const auto now = Clock::now();
refresh_validity(now);
if (!valid_)
return;
Comment on lines +49 to +58

@coderabbitaicoderabbitaiBotJul 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

valid_ / last_remote_control_received_at_ 存在跨线程数据竞争。

store_status()（第 49-51 行）在 UART 回调线程中以普通（非原子）方式写入 last_remote_control_received_at_ 与 valid_；而 update_status()/refresh_validity()（第 55-58、192-198 行）以及 valid()（第 155 行）在组件主更新线程读取/写入这两个字段，全程没有任何同步（不像 data_part1_/data_part2_/data_part3_ 已经是 std::atomic）。这是本 PR 新引入的未同步跨线程访问，属于未定义行为的数据竞争。

对比 vt13.hpp：其 valid_/last_remote_control_received_at_ 只在 update_status()（主线程）内部写入，store_status() 只写入环形缓冲区，因此没有这个问题——这恰好印证了 dr16.hpp 这里的不对称是个真实缺陷。

该竞争会直接影响 RemoteControl::update() 在全部 6 个硬件文件（deformable-infantry-omni-b.cpp、deformable-infantry-omni.cpp、flight.cpp、omni_infantry.cpp、sentry.cpp、steering-hero-little-six-friction.cpp）中基于 dr16_->valid() 的仲裁判断，因此在此处统一说明，不在各硬件文件重复。

🔒 建议修复：将 valid_ 改为原子变量并使用 acquire/release 语义
- Vector mouse_velocity_ = Vector::zero();- double mouse_wheel_ = 0.0;+ Vector mouse_velocity_ = Vector::zero();+ double mouse_wheel_ = 0.0;
@@
- double rotary_knob_ = 0.0;- rmcs_msgs::Switch rotary_knob_switch_ = rmcs_msgs::Switch::UNKNOWN;- TimePoint last_remote_control_received_at_ = TimePoint::min();- bool valid_ = false;+ double rotary_knob_ = 0.0;+ rmcs_msgs::Switch rotary_knob_switch_ = rmcs_msgs::Switch::UNKNOWN;+ TimePoint last_remote_control_received_at_ = TimePoint::min();+ std::atomic<bool> valid_ = false;
- last_remote_control_received_at_ = Clock::now();- valid_ = true;+ last_remote_control_received_at_ = Clock::now();+ valid_.store(true, std::memory_order::release);
 void update_status() {
const auto now = Clock::now();
refresh_validity(now);
- if (!valid_)+ if (!valid_.load(std::memory_order::acquire))
return;
 void refresh_validity(const TimePoint now) {
- if (!valid_ || now - last_remote_control_received_at_ <= kFreshTimeout)+ if (!valid_.load(std::memory_order::acquire)+ || now - last_remote_control_received_at_ <= kFreshTimeout)
return;
reset_remote_control_state();
- valid_ = false;+ valid_.store(false, std::memory_order::relaxed);
}
- bool valid() const noexcept { return valid_; }+ bool valid() const noexcept { return valid_.load(std::memory_order::acquire); }

Also applies to: 192-198, 279-280, 155-156

🤖 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/hardware/device/dr16.hpp` around lines 49 - 58,
Resolve the cross-thread race in the DR16 status state by making valid_ atomic
and using acquire/release semantics for its writes and reads. Update
store_status(), refresh_validity(), update_status(), and valid() consistently,
and synchronize last_remote_control_received_at_ as well—either protect both
fields with the same mutex or use an atomic time representation—while preserving
the existing validity and RemoteControl::update() behavior.


auto part1 alignas(uint64_t) =
std::bit_cast<Dr16DataPart1>(data_part1_.load(std::memory_order::relaxed));

Expand DownExpand Up@@ -110,19 +91,6 @@ class Dr16 {
keyboard_ = part3.keyboard;
rotary_knob_ = channel_to_double(part3.rotary_knob);

*joystick_right_output_ = joystick_right();
*joystick_left_output_ = joystick_left();

*switch_right_output_ = switch_right();
*switch_left_output_ = switch_left();

*mouse_velocity_output_ = mouse_velocity();
*mouse_wheel_output_ = mouse_wheel();

*mouse_output_ = mouse();
*keyboard_output_ = keyboard();

*rotary_knob_output_ = rotary_knob();
update_rotary_knob_switch();
}

Expand DownExpand Up@@ -182,6 +150,10 @@ class Dr16 {
rmcs_msgs::Mouse mouse() const { return std::bit_cast<rmcs_msgs::Mouse>(mouse_); }
rmcs_msgs::Keyboard keyboard() const { return std::bit_cast<rmcs_msgs::Keyboard>(keyboard_); }

rmcs_msgs::Switch rotary_knob_switch() const { return rotary_knob_switch_; }

bool valid() const noexcept { return valid_; }

double rotary_knob() const { return rotary_knob_; }

double mouse_wheel() const { return mouse_wheel_; }
Expand All@@ -193,22 +165,49 @@ class Dr16 {
constexpr double divider = 0.7, anti_shake_shift = 0.05;
double upper_divider = divider, lower_divider = -divider;

auto& switch_value = *rotary_knob_switch_output_;
auto switch_value = rotary_knob_switch_;
if (switch_value == rmcs_msgs::Switch::UP)
upper_divider -= anti_shake_shift, lower_divider -= anti_shake_shift;
else if (switch_value == rmcs_msgs::Switch::MIDDLE)
upper_divider += anti_shake_shift, lower_divider -= anti_shake_shift;
else if (switch_value == rmcs_msgs::Switch::DOWN)
upper_divider += anti_shake_shift, lower_divider += anti_shake_shift;

const auto knob_value = -*rotary_knob_output_;
const auto knob_value = -rotary_knob_;
if (knob_value > upper_divider) {
switch_value = rmcs_msgs::Switch::UP;
} else if (knob_value < lower_divider) {
switch_value = rmcs_msgs::Switch::DOWN;
} else {
switch_value = rmcs_msgs::Switch::MIDDLE;
}
rotary_knob_switch_ = switch_value;
}

using Clock = std::chrono::steady_clock;
using TimePoint = Clock::time_point;

static constexpr auto kFreshTimeout = std::chrono::milliseconds(500);

void refresh_validity(const TimePoint now) {
if (!valid_ || now - last_remote_control_received_at_ <= kFreshTimeout)
return;

reset_remote_control_state();
valid_ = false;
}

void reset_remote_control_state() {
joystick_right_ = Vector::zero();
joystick_left_ = Vector::zero();
switch_right_ = Switch::kUnknown;
switch_left_ = Switch::kUnknown;
mouse_velocity_ = Vector::zero();
mouse_wheel_ = 0.0;
mouse_ = Mouse::zero();
keyboard_ = Keyboard::zero();
rotary_knob_ = 0.0;
rotary_knob_switch_ = rmcs_msgs::Switch::UNKNOWN;
}

struct [[gnu::packed]] Dr16DataPart1 {
Expand DownExpand Up@@ -270,27 +269,15 @@ class Dr16 {
Switch switch_left_ = Switch::kUnknown;

Vector mouse_velocity_ = Vector::zero();
double mouse_wheel_ = 0.0;

Mouse mouse_ = Mouse::zero();
Keyboard keyboard_ = Keyboard::zero();

double rotary_knob_ = 0.0;
double mouse_wheel_ = 0.0;

rmcs_executor::Component::OutputInterface<Eigen::Vector2d> joystick_right_output_;
rmcs_executor::Component::OutputInterface<Eigen::Vector2d> joystick_left_output_;

rmcs_executor::Component::OutputInterface<rmcs_msgs::Switch> switch_right_output_;
rmcs_executor::Component::OutputInterface<rmcs_msgs::Switch> switch_left_output_;

rmcs_executor::Component::OutputInterface<Eigen::Vector2d> mouse_velocity_output_;
rmcs_executor::Component::OutputInterface<double> mouse_wheel_output_;

rmcs_executor::Component::OutputInterface<rmcs_msgs::Mouse> mouse_output_;
rmcs_executor::Component::OutputInterface<rmcs_msgs::Keyboard> keyboard_output_;

rmcs_executor::Component::OutputInterface<double> rotary_knob_output_;
rmcs_executor::Component::OutputInterface<rmcs_msgs::Switch> rotary_knob_switch_output_;
rmcs_msgs::Switch rotary_knob_switch_ = rmcs_msgs::Switch::UNKNOWN;
TimePoint last_remote_control_received_at_ = TimePoint::min();
bool valid_ = false;
};

} // namespace rmcs_core::hardware::device
Loading