Skip to content

feat(canopen): configurable axis object-offset in Ds402Drive for multi-axis drives - #757

Merged
finger563 merged 1 commit into
mainfrom
pr/ds402-multi-axis-offset
Sep 2, 2026
Merged

feat(canopen): configurable axis object-offset in Ds402Drive for multi-axis drives#757
finger563 merged 1 commit into
mainfrom
pr/ds402-multi-axis-offset

Conversation

@finger563

Copy link
Copy Markdown
Contributor

Summary

Ds402Drive addressed a single axis at the standard device-profile indices (controlword 0x6040, statusword 0x6041, ...), so it could not drive the second axis of a multi-axis controller.

Basicmicro's MCP266, for example, mirrors every M1 object at +0x800 for M2 (0x6840, 0x6841, 0x687A, ...). This adds Config::object_offset, applied via a new axis_object() helper.

Details

  • The offset is applied only to device-profile objects (0x6000-0x6FFF). Communication and identity objects (< 0x6000, e.g. 0x1000 device type, 0x1008 device name, 0x1018 identity) are shared by the whole device and are never offset.
  • object_offset defaults to 0, so existing single-axis behavior is unchanged.
  • Two Ds402Drive instances (offset 0 and 0x800) can share one CanopenClient to drive both channels of a dual-axis node.
espp::Ds402Drive m1(client, {.object_offset = 0x000});
espp::Ds402Drive m2(client, {.object_offset = 0x800});

Testing

  • Existing canopen host test still passes (ALL TESTS PASSED).
  • Exercised on real MCP266 hardware driving both channels' profile-position moves through two offset instances over one client.

🤖 Generated with Claude Code

Copilot AI lite review requested due to automatic review settings September 2, 2026 16:51

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 configurable per-axis object dictionary index offset to Ds402Drive so a single CANopen node can be controlled as multiple DS402 axes (e.g., second axis mirrored at +0x800).

Changes:

  • Introduces Config::object_offset and stores it in Ds402Drive.
  • Applies the offset to DS402 device-profile SDO accesses via a new axis_object() helper.
  • Updates DS402 read/write helpers (controlword/statusword/modes/targets/profile params) to use the offset mapping.

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

Comment thread components/canopen/include/ds402.hpp Outdated
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown

✅Static analysis result - no issues found! ✅

@finger563
finger563 force-pushed the pr/ds402-multi-axis-offset branch from d4a04a1 to 3ddd224 Compare September 2, 2026 18:01
finger563 added a commit that referenced this pull request Sep 2, 2026
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
@finger563

Copy link
Copy Markdown
Contributor Author

Addressed the review comment:

  • Offset range: axis_object() now restricts the offset to the inclusive CiA 402 device-profile range 0x60000x6FFF. Objects at 0x7000+ (e.g. manufacturer-specific ranges) are no longer offset, matching the documented behavior; comment clarified.

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 1 out of 1 changed files in this pull request and generated 2 comments.

Comment thread components/canopen/include/ds402.hpp Outdated
Comment thread components/canopen/include/ds402.hpp Outdated
@finger563
finger563 force-pushed the pr/ds402-multi-axis-offset branch from 3ddd224 to dc9d9e4 Compare September 2, 2026 19:03
finger563 added a commit that referenced this pull request Sep 2, 2026
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
@finger563

Copy link
Copy Markdown
Contributor Author

Second round addressed:

  • Comment overclaim: reworded — the helper no longer asserts that >= 0x7000 objects are "shared / not axis-relative" (which can be false per device); it now just says those indices "are not offset by this helper and returned unchanged."
  • uint16_t wraparound: the offset addition is done in uint32_t, and the offset result is only used when it still fits in uint16_t (<= 0xFFFF); otherwise the original index is returned rather than silently wrapping.

@finger563
finger563 requested a balanced review from Copilot September 2, 2026 19:10
finger563 added a commit that referenced this pull request Sep 2, 2026
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
@finger563

Copy link
Copy Markdown
Contributor Author

Self-reviewed the full diff — no further issues. Every CiA 402 device-profile object accessor is wrapped with axis_object() while the identity/communication objects (0x1000/0x1001/0x1008/0x1018) are correctly left unoffset; the range is inclusive 0x6000-0x6FFF and the widened-add guards against a uint16_t wrap. Existing host test still 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 1 out of 1 changed files in this pull request and generated 3 comments.

Comment thread components/canopen/include/ds402.hpp
Comment thread components/canopen/include/ds402.hpp
Comment thread components/canopen/include/ds402.hpp Outdated
…i-axis drives

Ds402Drive addressed a single axis at the standard device-profile
indices (controlword 0x6040, statusword 0x6041, ...), so it could not
drive the second axis of a multi-axis controller. Basicmicro's MCP266,
for example, mirrors every M1 object at +0x800 for M2 (0x6840, 0x6841,
...). Add Config::object_offset, applied via a new axis_object() helper
to device-profile objects (0x6000-0x6FFF) only; communication and
identity objects (< 0x6000, e.g. 0x1000/0x1008/0x1018) are shared by the
whole device and are never offset. Default 0 keeps existing single-axis
behavior unchanged, so two Ds402Drive instances (offset 0 and 0x800) can
share one CanopenClient to drive both channels.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0153z8MvyCu6YT47myGDn4rK
@finger563
finger563 force-pushed the pr/ds402-multi-axis-offset branch from dc9d9e4 to 6cf6776 Compare September 2, 2026 20:51
finger563 added a commit that referenced this pull request Sep 2, 2026
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
@finger563

Copy link
Copy Markdown
Contributor Author

Addressed the latest round:

  • Magic numbers → named constants: added OBJ_DEVICE_PROFILE_MIN (0x6000), OBJ_DEVICE_PROFILE_MAX (0x6FFF), OBJ_INDEX_MAX (0xFFFF) and MAX_AXIS_OBJECT_OFFSET (= OBJ_INDEX_MAX - OBJ_DEVICE_PROFILE_MAX) in detail::ds402, and the offset logic uses them.

  • Overflow no longer silently uses the wrong axis: the offset is now validated once, in the Ds402Drive constructor. An object_offset greater than MAX_AXIS_OBJECT_OFFSET (i.e. one that could push a device-profile index past the 16-bit index space) is rejected with an error log and offsetting is disabled — rather than silently falling back per-call. With a valid offset, axis_object() can never overflow.

  • Host-testable + tested: extracted the mapping into a pure constexpr detail::ds402::apply_axis_offset(index, offset) (which Ds402Drive::axis_object() now delegates to), and added test_axis_offset to the canopen host test covering (1) device-profile indices in 0x6000–0x6FFF are offset (incl. the inclusive boundaries), (2) indices outside the range (identity/comm objects, 0x5FFF, 0x7000) are not offset, and (3) an offset that would overflow returns the index unchanged. ALL TESTS PASSED.

@finger563
finger563 merged commit 26d162f into main Sep 2, 2026
151 of 152 checks passed
@finger563
finger563 deleted the pr/ds402-multi-axis-offset branch September 2, 2026 21:37
finger563 added a commit that referenced this pull request Sep 2, 2026
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
finger563 added a commit that referenced this pull request Sep 2, 2026
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
finger563 added a commit that referenced this pull request Sep 3, 2026
…758)

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).


Claude-Session: https://claude.ai/code/session_0153z8MvyCu6YT47myGDn4rK

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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