Add reBot DevArm leader arm plugin (Damiao dm-serial backend)#729
Open
johnnynunez wants to merge 2 commits into
Open
Add reBot DevArm leader arm plugin (Damiao dm-serial backend)#729johnnynunez wants to merge 2 commits into
johnnynunez wants to merge 2 commits into
Conversation
Streams the Seeed reBot DevArm (6-DOF + gripper, 7 Damiao DM-series motors: DM4340P on joints 1-3, DM4310 on joints 4-6 and the gripper) as JointStateOutput over the OpenXR tensor transport, on the same generic joint-space device path as the SO-101 leader plugin (JointStateTracker / JointStateSource / JointStateRetargeter). DamiaoBus speaks the Damiao USB-to-CAN adapter's fixed-size CDC framing directly (no SDK dependency) and implements the leader-arm subset: disable torque so the arm can be back-driven by hand, then request one feedback frame per motor per cycle (command 0xCC via CAN id 0x7FF) and decode the fixed-point position/velocity feedback, which lands directly in radians (no tick conversion; optional per-joint sign / zero-offset calibration file). Includes a synthetic fallback backend (no hardware; CI / headless bring-up) and a 'probe' subcommand that verifies the bus, motor ids, and decode path without an OpenXR runtime. Verified on real hardware (reBot Arm B601-DM, /dev/ttyACM0): probe decodes all 7 motors while disabled and exits 0; decoded positions match the motorbridge reference reader on the same bus. Signed-off-by: Johnny <johnnync13@gmail.com>
Contributor
|
📝 Docs preview is not auto-deployed for fork PRs. A maintainer with write access to |
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a new plugin, Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Main
participant Plugin as RebotDevarmLeaderPlugin
participant Bus as DamiaoBus
participant Serial as Serial Port
participant Pusher as SchemaPusher
Main->>Plugin: construct(device_path, collection_id, calibration_path)
Plugin->>Bus: open serial port, disable motors
loop 90 Hz update loop
Main->>Plugin: update()
alt hardware mode
Plugin->>Bus: request_feedback(motor_id)
Bus->>Serial: send_frame(...)
Plugin->>Bus: read_frame(out, timeout_ms)
Bus->>Serial: fill_rx_buffer / parse_frame
Bus-->>Plugin: decoded CanFrame
else synthetic mode
Plugin->>Plugin: generate sinusoidal joint state
end
Plugin->>Pusher: push JointStateOutput (FlatBuffers)
end
Related Issues: None referenced in provided context. Related PRs: None referenced in provided context. Suggested labels: plugin, new-feature, hardware Suggested reviewers: Reviewers familiar with plugin CMake wiring, POSIX serial/termios handling, and CAN frame protocol decoding. PoemA rabbit taps a CAN bus wire, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/plugins/rebot_devarm_leader/rebot_devarm_leader_plugin.cpp`:
- Around line 230-267: The reply loop in
RebotDevarmLeaderPlugin::read_hardware() is counting matching CAN frames with a
raw replies counter, so duplicate or stale frames can satisfy the loop before
every joint has actually responded. Replace the count-based exit condition with
per-joint seen tracking keyed by calibration_[i].feedback_id, and only stop once
each distinct joint has been observed in this cycle. Keep the existing
bus_->request_feedback(), bus_->read_frame(), and calibration_ matching logic,
but ensure each joint is marked once and duplicates do not advance completion.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ad041b18-1fb1-4b77-bc88-39664ef50d7f
📒 Files selected for processing (9)
CMakeLists.txtsrc/plugins/rebot_devarm_leader/CMakeLists.txtsrc/plugins/rebot_devarm_leader/README.mdsrc/plugins/rebot_devarm_leader/damiao_bus.cppsrc/plugins/rebot_devarm_leader/damiao_bus.hppsrc/plugins/rebot_devarm_leader/main.cppsrc/plugins/rebot_devarm_leader/plugin.yamlsrc/plugins/rebot_devarm_leader/rebot_devarm_leader_plugin.cppsrc/plugins/rebot_devarm_leader/rebot_devarm_leader_plugin.hpp
Comment on lines
+230
to
+267
| void RebotDevarmLeaderPlugin::read_hardware() | ||
| { | ||
| // Request one feedback frame per motor (command 0xCC via id 0x7FF), then collect the replies | ||
| // that arrive on each motor's MST id. A motor that doesn't reply this cycle holds its last | ||
| // value so a transient bus hiccup never faults. | ||
| for (int i = 0; i < kNumJoints; ++i) | ||
| { | ||
| bus_->request_feedback(calibration_[i].motor_id); | ||
| } | ||
|
|
||
| int replies = 0; | ||
| const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(kCollectTimeoutMs); | ||
| CanFrame frame; | ||
| while (replies < kNumJoints && std::chrono::steady_clock::now() < deadline) | ||
| { | ||
| if (!bus_->read_frame(frame, 1)) | ||
| { | ||
| continue; | ||
| } | ||
| for (int i = 0; i < kNumJoints; ++i) | ||
| { | ||
| if (frame.arbitration_id != calibration_[i].feedback_id || frame.dlc < 8) | ||
| { | ||
| continue; | ||
| } | ||
| // Feedback payload: [status<<4 | id_low, pos_hi, pos_lo, vel_hi, vel_lo<<4 | torq_hi, | ||
| // torq_lo, t_mos, t_rotor]; pos is 16-bit and vel 12-bit over the model limits. | ||
| const uint32_t pos_u = (static_cast<uint32_t>(frame.data[1]) << 8) | frame.data[2]; | ||
| const uint32_t vel_u = (static_cast<uint32_t>(frame.data[3]) << 4) | (frame.data[4] >> 4); | ||
| const double raw_pos = uint_to_float(pos_u, calibration_[i].p_max, kPositionBits); | ||
| const double raw_vel = uint_to_float(vel_u, calibration_[i].v_max, kVelocityBits); | ||
| positions_[i] = calibration_[i].sign * (raw_pos - calibration_[i].offset_rad); | ||
| velocities_[i] = calibration_[i].sign * raw_vel; | ||
| ++replies; | ||
| break; | ||
| } | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reply counter can be satisfied by a duplicate/stale frame instead of a genuinely missing joint.
replies is incremented on every matching frame regardless of which joint it belongs to. If two frames match the same feedback_id within the window (e.g. a leftover reply from a previous cycle that overshot its 5 ms deadline plus this cycle's real reply), replies reaches kNumJoints while a different, genuinely-unanswered joint never gets read this cycle — silently falling back to stale data without the "missed reply" path being what actually triggered it. Track per-joint seen flags instead of a raw count so the loop only exits once every distinct joint has answered.
🐛 Proposed fix
- int replies = 0;
+ bool seen[kNumJoints] = { false };
+ int replies = 0;
const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(kCollectTimeoutMs);
CanFrame frame;
while (replies < kNumJoints && std::chrono::steady_clock::now() < deadline)
{
if (!bus_->read_frame(frame, 1))
{
continue;
}
for (int i = 0; i < kNumJoints; ++i)
{
if (frame.arbitration_id != calibration_[i].feedback_id || frame.dlc < 8)
{
continue;
}
...
positions_[i] = calibration_[i].sign * (raw_pos - calibration_[i].offset_rad);
velocities_[i] = calibration_[i].sign * raw_vel;
- ++replies;
+ if (!seen[i])
+ {
+ seen[i] = true;
+ ++replies;
+ }
break;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| void RebotDevarmLeaderPlugin::read_hardware() | |
| { | |
| // Request one feedback frame per motor (command 0xCC via id 0x7FF), then collect the replies | |
| // that arrive on each motor's MST id. A motor that doesn't reply this cycle holds its last | |
| // value so a transient bus hiccup never faults. | |
| for (int i = 0; i < kNumJoints; ++i) | |
| { | |
| bus_->request_feedback(calibration_[i].motor_id); | |
| } | |
| int replies = 0; | |
| const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(kCollectTimeoutMs); | |
| CanFrame frame; | |
| while (replies < kNumJoints && std::chrono::steady_clock::now() < deadline) | |
| { | |
| if (!bus_->read_frame(frame, 1)) | |
| { | |
| continue; | |
| } | |
| for (int i = 0; i < kNumJoints; ++i) | |
| { | |
| if (frame.arbitration_id != calibration_[i].feedback_id || frame.dlc < 8) | |
| { | |
| continue; | |
| } | |
| // Feedback payload: [status<<4 | id_low, pos_hi, pos_lo, vel_hi, vel_lo<<4 | torq_hi, | |
| // torq_lo, t_mos, t_rotor]; pos is 16-bit and vel 12-bit over the model limits. | |
| const uint32_t pos_u = (static_cast<uint32_t>(frame.data[1]) << 8) | frame.data[2]; | |
| const uint32_t vel_u = (static_cast<uint32_t>(frame.data[3]) << 4) | (frame.data[4] >> 4); | |
| const double raw_pos = uint_to_float(pos_u, calibration_[i].p_max, kPositionBits); | |
| const double raw_vel = uint_to_float(vel_u, calibration_[i].v_max, kVelocityBits); | |
| positions_[i] = calibration_[i].sign * (raw_pos - calibration_[i].offset_rad); | |
| velocities_[i] = calibration_[i].sign * raw_vel; | |
| ++replies; | |
| break; | |
| } | |
| } | |
| } | |
| void RebotDevarmLeaderPlugin::read_hardware() | |
| { | |
| // Request one feedback frame per motor (command 0xCC via id 0x7FF), then collect the replies | |
| // that arrive on each motor's MST id. A motor that doesn't reply this cycle holds its last | |
| // value so a transient bus hiccup never faults. | |
| for (int i = 0; i < kNumJoints; ++i) | |
| { | |
| bus_->request_feedback(calibration_[i].motor_id); | |
| } | |
| bool seen[kNumJoints] = { false }; | |
| int replies = 0; | |
| const auto deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(kCollectTimeoutMs); | |
| CanFrame frame; | |
| while (replies < kNumJoints && std::chrono::steady_clock::now() < deadline) | |
| { | |
| if (!bus_->read_frame(frame, 1)) | |
| { | |
| continue; | |
| } | |
| for (int i = 0; i < kNumJoints; ++i) | |
| { | |
| if (frame.arbitration_id != calibration_[i].feedback_id || frame.dlc < 8) | |
| { | |
| continue; | |
| } | |
| // Feedback payload: [status<<4 | id_low, pos_hi, pos_lo, vel_hi, vel_lo<<4 | torq_hi, | |
| // torq_lo, t_mos, t_rotor]; pos is 16-bit and vel 12-bit over the model limits. | |
| const uint32_t pos_u = (static_cast<uint32_t>(frame.data[1]) << 8) | frame.data[2]; | |
| const uint32_t vel_u = (static_cast<uint32_t>(frame.data[3]) << 4) | (frame.data[4] >> 4); | |
| const double raw_pos = uint_to_float(pos_u, calibration_[i].p_max, kPositionBits); | |
| const double raw_vel = uint_to_float(vel_u, calibration_[i].v_max, kVelocityBits); | |
| positions_[i] = calibration_[i].sign * (raw_pos - calibration_[i].offset_rad); | |
| velocities_[i] = calibration_[i].sign * raw_vel; | |
| if (!seen[i]) | |
| { | |
| seen[i] = true; | |
| +replies; | |
| } | |
| 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 `@src/plugins/rebot_devarm_leader/rebot_devarm_leader_plugin.cpp` around lines
230 - 267, The reply loop in RebotDevarmLeaderPlugin::read_hardware() is
counting matching CAN frames with a raw replies counter, so duplicate or stale
frames can satisfy the loop before every joint has actually responded. Replace
the count-based exit condition with per-joint seen tracking keyed by
calibration_[i].feedback_id, and only stop once each distinct joint has been
observed in this cycle. Keep the existing bus_->request_feedback(),
bus_->read_frame(), and calibration_ matching logic, but ensure each joint is
marked once and duplicates do not advance completion.
5 tasks
…gs instead of streaming them The Damiao multi-turn counter is volatile across power cycles: the single-turn absolute zero survives but the turn count does not. The reBot DevArm gripper's geared travel (~6.8 rad) exceeds one turn, so after a repower it can wake up reading physical + 2*pi*k (verified on a B601-DM: a physically closed gripper read +6.227 rad = -0.056 + 2*pi while the six arm joints, whose travel is under one turn, all kept their zero). Such a reading is not a pose, and streaming it as-is makes a downstream follower clip it into a full-travel slam on the first teleop frame. - read_hardware(): detect a gripper reading outside its physical travel window and warn once on the rising edge (not at 90 Hz). - push_current_state(): stream the gripper joint with valid=false while out of travel, so consumers can hold/ignore it instead of executing it. - probe: return new exit code 3 (motors all replied, but the gripper is out of travel) with a re-homing hint, so wiring checks catch the wrapped state before any teleop session. - README: document the wrap failure mode and exit code 3. No static calibration offset can fix this class of failure (travel > 2*pi makes the branch ambiguous), so the plugin's job is detection and honest signaling; re-homing against the mechanical stop is the recovery. Signed-off-by: Johnny <johnnync13@gmail.com>
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.
Description
Adds a reBot DevArm leader arm plugin (
src/plugins/rebot_devarm_leader), streaming the Seeed reBot DevArm (6-DOF arm + gripper) joint angles asJointStateOutputover the OpenXR tensor transport — on the same generic joint-space device path (JointStateTracker/JointStateSource/JointStateRetargeter) as the existingso101_leaderplugin, and mirroring its structure and CLI shape.The reBot DevArm is 7 Damiao DM-series MIT-protocol motors (DM4340P on joints 1–3, DM4310 on joints 4–6 and the gripper) on a CAN bus behind a Damiao USB-to-CAN serial adapter (USB CDC-ACM).
DamiaoBusimplements the adapter's fixed-size binary framing and the leader-arm subset of the Damiao motor protocol directly — no SDK dependency:FF..FF FD) so the arm can be back-driven by hand; Damiao motors keep answering feedback requests while disabled,0xCCaddressed via CAN id0x7FF) and decode the 16-bit position / 12-bit velocity fixed-point feedback, which lands directly in radians (no tick conversion; optional per-joint sign / zero-offset via a plain-text calibration file).Like
so101_leader, an empty device path selects a synthetic trajectory backend so the device → tracker → retargeter pipeline runs with no hardware (CI / headless bring-up), and aprobesubcommand verifies the bus, motor ids, and decode path with no OpenXR runtime.Changes
src/plugins/rebot_devarm_leader/damiao_bus.{hpp,cpp}— minimal POSIX serial client for the Damiao USB-to-CAN adapter (TX 30-byte / RX 16-byte framing, resync on garbage, Windows stub throws).src/plugins/rebot_devarm_leader/rebot_devarm_leader_plugin.{hpp,cpp}— plugin: defaults for the factory id layout (command ids 1..7, MST feedback ids 0x11..0x17) and motor models, calibration file parsing, hardware/synthetic read paths,JointStateOutputpush (positions + velocities), and theprobehelper.src/plugins/rebot_devarm_leader/main.cpp— entry point (90 Hz loop, positional args matchingso101_leader).src/plugins/rebot_devarm_leader/{CMakeLists.txt,plugin.yaml,README.md}— build wiring, plugin manifest, docs (run, probe, calibration format, Python consumption).CMakeLists.txt— add the plugin underBUILD_PLUGINS.Verification (real hardware)
Tested against a physical reBot Arm B601-DM (7 Damiao motors, factory ids,
/dev/ttyACM0):Decoded positions match a reference reader built on the vendor-protocol
motorbridgestack on the same bus to ~1e-3 rad (e.g.joint6=0.2401on both). The disabled-motors-still-reply behavior (the basis of the back-drivable leader design) was verified the same way: 7/7 motors answer0xCCwhile torque is off.Synthetic backend starts and pushes frames (verified up to the expected CloudXR runtime-dir warning in a bare environment).
Checklist
rebot_devarm_leader_plugintarget)clang-format14 clean (--dry-run --Werror)SKIP=check-copyright-year pre-commit runclean on all touched files (REUSE/SPDX included)Summary by CodeRabbit
New Features
Documentation