Skip to content

feat(mcp266): CANopen dual-channel MCP266 motor controller component - #758

Merged
finger563 merged 1 commit into
mainfrom
pr/mcp266-component
Sep 3, 2026
Merged

feat(mcp266): CANopen dual-channel MCP266 motor controller component#758
finger563 merged 1 commit into
mainfrom
pr/mcp266-component

Conversation

@finger563

Copy link
Copy Markdown
Contributor

Summary

A new component, espp/mcp266: a dual-channel controller for a Basicmicro MCP266 (RoboClaw family) brushed-DC motor driver over CANopen. Layered on espp::CanopenClient like espp::Ds402Drive, so it's transport-agnostic — the app owns the CAN transport and feeds frames to the client.

Both motor channels (M1, M2) are driven symmetrically; M2's CiA 402 objects mirror M1's at +0x800 via Ds402Drive's object offset.

What it provides

  • Position control (CiA 402 profile position mode) for both axes — the supported, validated capability.
  • configure_position_loop() handling two device quirks: the position PID's MinPos/MaxPos clamp defaults to [0, 0] (forces every target to zero), and the setter (cmds 61/62) uses field order D, P, I while the readback (63/64) uses P, I, D.
  • Feedback (encoder / speed / statusword) and telemetry (battery, temperature) per axis.
  • The manufacturer speed/duty command mirror (drive_speed / drive_duty) — implemented but documented as inert on the MCP266 firmware tested (the drive advertises only cyclic-sync modes; velocity likely needs csv + SYNC/PDO, undocumented for this device).

Device mapping

The MCP266 mirrors its packet-serial command set into the manufacturer OD region at index 0x2000 + command number. That reverse-engineered mapping (command objects, per-axis addresses, position-PID field remap) lives in a host-buildable detail/mcp266_core.hpp with a standalone unit test (test/mcp266_host_test.cpp, ALL PASSED).

Included

  • Component (include/mcp266.hpp + detail/mcp266_core.hpp), README, idf_component.yml.
  • Example (Twai + CanopenClient + Mcp266 position sequence) — builds for esp32 and esp32p4.
  • Docs: doc/en/motor_control/mcp266.rst + example md + toctree + Doxyfile entries.
  • CI: example-build matrix (esp32) and component-upload registrations.

Dependency

Stacked on #757 (Ds402Drive object-offset) — the component uses Ds402Drive::Config::object_offset to address M2. Base will retarget to main once #757 merges.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 2, 2026 17:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Adds a new espp/mcp266 component to control a Basicmicro MCP266 dual-channel brushed DC motor controller over CANopen, including docs, an ESP-IDF example, and host-buildable mapping tests.

Changes:

  • Introduces espp::Mcp266 controller + constexpr mapping core (detail/mcp266_core.hpp).
  • Adds host unit tests for the mapping/PID-field remap and an ESP-IDF TWAI+CANopen example.
  • Wires the component into docs (Sphinx/MyST + Doxygen) and CI (build + component upload).

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
doc/en/motor_control/mcp266_example.md Includes the component example README into the docs.
doc/en/motor_control/mcp266.rst Adds MCP266 component documentation and toctree entry.
doc/en/motor_control/index.rst Adds MCP266 page to the motor control docs index.
doc/Doxyfile Adds MCP266 headers and example to Doxygen inputs.
components/mcp266/test/mcp266_host_test.cpp Adds host-buildable unit test for mapping core.
components/mcp266/include/mcp266.hpp Implements the espp::Mcp266 dual-axis controller API.
components/mcp266/include/detail/mcp266_core.hpp Adds constexpr mapping helpers and PID field-order remap.
components/mcp266/idf_component.yml Declares the component for IDF Component Manager.
components/mcp266/example/sdkconfig.defaults Sets example task stack defaults.
components/mcp266/example/main/mcp266_example.cpp Adds ESP-IDF example using TWAI + CanopenClient + Mcp266.
components/mcp266/example/main/CMakeLists.txt Registers example main component.
components/mcp266/example/README.md Documents how to build/run the example.
components/mcp266/example/CMakeLists.txt Adds example project CMake configuration.
components/mcp266/README.md Adds component README with capabilities/limitations and device quirks.
components/mcp266/CMakeLists.txt Registers the new component and include paths.
.github/workflows/upload_components.yml Adds MCP266 to component upload workflow.
.github/workflows/build.yml Adds MCP266 example to CI build matrix.
Suppressed comments (1)

doc/en/motor_control/mcp266_example.md:3

  • The MyST {include} fence should start at column 1. If there is any leading indentation in the actual file (the diff view here suggests it may be indented), Sphinx/MyST can treat it as a nested code block and fail to process the directive. Ensure the opening/closing fences are not indented.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread components/mcp266/include/mcp266.hpp Outdated
Comment thread components/mcp266/example/main/mcp266_example.cpp
Comment thread components/mcp266/include/mcp266.hpp
@finger563

Copy link
Copy Markdown
Contributor Author

Addressed the review comments:

  • start() masking NMT failure: it now fails fast — if nmt_start() fails it logs and returns false (previously the failure was only logged and reset_faults() cleared ec, so the call could report success while the node was never started).
  • reset_estop() retry leaking ec: ec is now cleared before the write_u32 fallback, so the first attempt's error doesn't leak into the retry; on success ec is clear, on failure it holds the last error.
  • std::abs include in the example: added <cstdlib>.

(Verified the example still compiles for esp32 and esp32p4, and the host mapping test passes.)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.

Comment thread components/mcp266/include/mcp266.hpp
Comment thread components/mcp266/include/mcp266.hpp
Comment thread components/mcp266/include/mcp266.hpp
Comment thread components/mcp266/include/mcp266.hpp
Comment thread components/mcp266/include/mcp266.hpp
Comment thread components/mcp266/include/mcp266.hpp Outdated
@finger563
finger563 force-pushed the pr/mcp266-component branch 2 times, most recently from 0d4d55b to e71cd43 Compare September 2, 2026 19:14
@finger563

Copy link
Copy Markdown
Contributor Author

Self-review — one fix applied, two notes:

  • Fixed (footgun in a reusable component): configure_position_loop() used to seed a hardcoded, motor-specific P gain (0x3C83, the value from the one motor this was developed against) whenever the drive's stored P read back as zero. That is fine for the original board but wrong to bake into a generic component. It is now a defaulted fallback_p parameter, only used when the stored gain is zero, documented as a coarse non-tuned starting point (with a warn-level log when it fires), and a drive that already has a non-zero P keeps its stored gains untouched.

  • Note — M2 software position limits: set_position_limits() addresses 0x607D + object_offset, i.e. 0x6E7D for M2, on the assumption the MCP mirrors every 0x60xx object at +0x800 (verified for the profile-position objects, not specifically for 0x607D). Consistent with the documented mirror; flagging since only M1 has been exercised on hardware.

  • Note — synchronization: like Ds402Drive, this class holds no internal lock, so a compound operation (e.g. configure_position_loop's read-7 / write-7 / verify) is not atomic against concurrent callers on the same CanopenClient. Individual SDOs still serialize inside the client. Callers that share an instance across tasks should serialize compound calls themselves.

Host mapping test passes; example builds for esp32 and esp32p4.

Base automatically changed from pr/ds402-multi-axis-offset to main September 2, 2026 21:37
New component espp/mcp266: a dual-channel controller for a Basicmicro
MCP266 (RoboClaw family) over CANopen, layered on CanopenClient like
Ds402Drive (transport-agnostic). Both axes are driven symmetrically; M2's
CiA 402 objects mirror M1's at +0x800 via Ds402Drive's object offset.

Position control uses standard CiA 402 profile position mode and is the
supported capability; configure_position_loop() handles the two device
quirks (the [0,0] MinPos/MaxPos clamp that zeroes every target, and the
setter's D,P,I vs readback's P,I,D field order). The manufacturer speed/
duty command mirror is implemented but documented as inert on the tested
firmware. Telemetry (battery, temperature) reads via the mirrored command
objects.

The reverse-engineered object mapping (0x2000 + command mirror, per-axis
objects, position-PID field remap) lives in a host-buildable detail core
with a standalone unit test. Includes an example (Twai + CanopenClient +
Mcp266 position sequence), README, rst docs + Doxyfile/toctree entries,
and CI example-build + component-upload registrations.

Depends on the Ds402Drive object-offset feature (#757).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0153z8MvyCu6YT47myGDn4rK
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅Static analysis result - no issues found! ✅

@finger563
finger563 merged commit cefbe1b into main Sep 3, 2026
155 checks passed
@finger563
finger563 deleted the pr/mcp266-component branch September 3, 2026 00:44
@finger563

Copy link
Copy Markdown
Contributor Author

Hardware validationespp::Mcp266 has now been run end-to-end on a real Basicmicro MCP266 over CAN (ESP32-P4 + 3.3 V transceiver, CANopen node 10, 1 Mbit/s), driving M1 through the standard CiA 402 profile-position path:

  • start() → NMT start + fault clear, reset_estop(), and configure_position_loop() (which correctly kept the drive's stored P=15491 and did not seed the fallback, since the stored gain was non-zero) — the [0,0] clamp was widened as intended.
  • A profile-position setpoint sequence (10000 → -10000 → 5000 → 0) reached each target (e.g. reached -10000 in 8059 ms), with statusword transitioning to 0x0637 (target reached) at each hold.
  • A continuous ±10000 ping-pong ran indefinitely.
  • Telemetry over the mirrored command objects read correctly (main battery 20.1 V, board temperature 25.1 C).

This was exercised by building the MIB firmware against the component via override_path behind a build flag, so the exact component code in this PR ran on the motor. (Velocity/duty over CAN remains inert on this firmware, as documented.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants