Skip to content

feat(canopen): lightweight CANopen client + DS402 drive helper - #730

Open
finger563 wants to merge 4 commits into
mainfrom
feat/canopen
Open

feat(canopen): lightweight CANopen client + DS402 drive helper#730
finger563 wants to merge 4 commits into
mainfrom
feat/canopen

Conversation

@finger563

Copy link
Copy Markdown
Contributor

Description

New components/canopen — a slim, standards-based CANopen (CiA 301) client/master with a DS402 (CiA 402) motion-profile helper. Phase 2 of Basicmicro MCP support (the MCPs' CAN interface is CANopen/DS402, not packet-serial-over-CAN), but deliberately device-agnostic: it drives any DS402-compliant node.

  • ESP-free wire core (detail/canopen_core.hpp): CanFrame, NMT/SYNC/PDO builders, heartbeat parser, SDO expedited upload/download builders + response parsing (correct ccs/scs/e/s/n command specifiers), toggle-checked segmented upload assembler (for strings like device name 0x1008), ~30 abort codes mapped to readable strings, DS402 object indices + controlword commands + statusword state decode (standard 0x4F/0x6F masks).
  • espp::CanopenClient (BaseComponent): send is a std::function over CanFrame (wire it to espp::Twai or anything else); received frames are fed via process_frame() from the transport's RX task; SDO transactions block on a condition variable with timeout, one in flight, mutex-serialized. Typed read_/write_{u8,u16,u32,i8,i16,i32}, read_string(), NMT/SYNC/RPDO helpers, per-COB-ID TPDO callbacks, cached heartbeat states.
  • espp::Ds402Drive: CiA 402 state decode, enable_operation() walking Shutdown→Switch On→Enable with statusword polling (resumes from intermediate states), disable() / quick_stop() / fault_reset() (rising-edge + poll), verified mode changes, profile-velocity targets and profile-position setpoints with the bit-4/5/12 handshake, identity/actual-value accessors.
  • esp32 example on espp::Twai (250 kbit NORMAL): NMT start → identity/device-name reads → DS402 detect → enable → gentle velocity ramp → stop.
  • Full registration: docs (doc/en/buses/canopen.rst), Doxyfile, build matrix, component upload, README.

Consciously deferred (documented): segmented/block SDO download, dynamic PDO-mapping config, EMCY parsing, heartbeat-time config, homing-status helpers, halt bit.

Testing

  • Host golden tests — every frame byte-verified against the CiA specs (NMT, expedited SDO u8/u16/u32 both directions, aborts, segmented upload with toggle-mismatch rejection, heartbeat, all eight DS402 statusword states): ALL PASSED (re-verified independently).
  • esp32 example builds clean; cppcheck (CI settings) clean.
  • Hardware validation against an MCP266 over CAN is the remaining gate — paired with the MCP web console's SDO browser for discovering the device's actual object dictionary.

🤖 Generated with Claude Code

Add a new `canopen` component: a slim, standards-based CANopen (CiA 301)
client / master with a CiA 402 (DS402) drive-profile helper, suitable for
driving devices like the Basicmicro MCP236/MCP266 motor controllers over
CAN (e.g. via the espp/twai component).

Architecture (modeled on odrive_native):

* include/detail/canopen_core.hpp -- host-buildable pure C++20 wire core
  with no ESP dependencies: a transport-agnostic CanFrame struct (mirrors
  espp::Twai::Message field-for-field), NMT command / SYNC / PDO frame
  builders, heartbeat parsing, SDO request builders and response parser
  (expedited up/download with correct ccs/scs/e/s/n command specifiers,
  segmented upload with a small toggle-checking assembler, abort frames),
  an abort-code -> human-readable-string map, little-endian helpers, and
  the CiA 402 object indices, controlword commands, and statusword state
  decoding (standard 0x4F / 0x6F masks).

* include/canopen_client.hpp -- espp::CanopenClient (BaseComponent):
  transport-agnostic via a user send function; the app feeds received
  frames to process_frame() (e.g. from the Twai on_receive callback).
  Blocking SDO transactions (condition variable + configurable timeout,
  one in flight per client, serialized by a mutex) with typed
  read_u8..read_i32 / write_u8..write_i32 accessors and read_string()
  (segmented upload, e.g. device name 0x1008). Non-blocking NMT master /
  SYNC / RPDO transmit helpers, per-COB-ID TPDO reception dispatch, and
  cached heartbeat NMT states with an optional callback.

* include/ds402.hpp -- espp::Ds402Drive layered on a CanopenClient:
  statusword state decoding, enable_operation() walking Shutdown (0x0006)
  -> Switch On (0x0007) -> Enable Operation (0x000F) with statusword
  polling + timeout, disable(), quick_stop(), fault_reset() (rising edge
  of controlword bit 7), set_mode() verified via 0x6061 (profile
  velocity / profile position / homing), set_target_velocity(), and
  set_target_position() with the new-set-point handshake (controlword
  bits 4/5/6 + statusword bit-12 acknowledge), plus accessors for the
  standard identification, profile, and actual-value objects.

* test/canopen_host_test.cpp -- golden-frame host unit tests (NMT, SYNC,
  PDO, heartbeat, SDO expedited up/download for u8/u16/u32, aborts,
  segmented upload of a 10-char string with toggle handling, LE helpers,
  and CiA 402 statusword decode vectors). Builds with
  `c++ -std=c++20 -I include test/canopen_host_test.cpp`.

* example/ -- esp32 example wiring espp::Twai to the client: NMT start,
  SDO identity / device type / device name reads, then (for CiA 402
  devices) profile-velocity enable + gentle ramp + stop.

Also registers the component in the docs (Doxyfile, doc/en/buses) and CI
(build matrix + component upload).

Verified: host test passes, example builds for esp32 (IDF v6.0.1), and
cppcheck is clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 18, 2026 21:42

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.

Pull request overview

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.

Adds a new CANopen (CiA 301) client component with a DS402 (CiA 402) drive helper, plus documentation, examples, CI wiring, and host-buildable golden tests.

Changes:

  • Introduces components/canopen with a transport-agnostic CANopen core, CanopenClient, and Ds402Drive.
  • Adds host golden tests for the wire core and DS402 statusword decoding, plus an ESP-IDF TWAI example.
  • Registers docs, Doxygen inputs/examples, CI build matrix, and component upload workflow entries.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
doc/en/buses/index.rst Adds CANopen docs page to the buses index.
doc/en/buses/canopen_example.md Includes the component example README in the docs.
doc/en/buses/canopen.rst New CANopen/DS402 API documentation + toctree link to example.
doc/Doxyfile Adds CANopen headers + example source to Doxygen inputs.
components/canopen/test/canopen_host_test.cpp Host-buildable golden tests for frame builders/parsers + DS402 decode.
components/canopen/include/ds402.hpp New DS402 helper API layered on CanopenClient.
components/canopen/include/detail/canopen_core.hpp New ESP-free CANopen wire core (builders/parsers, segmented upload assembler).
components/canopen/include/canopen_client.hpp New transport-agnostic CANopen client with blocking SDO and callbacks.
components/canopen/idf_component.yml Component Manager manifest for publishing canopen.
components/canopen/example/sdkconfig.defaults Example sdkconfig defaults for task stack sizing.
components/canopen/example/main/canopen_example.cpp ESP-IDF TWAI example wiring transport, reading identity, enabling DS402, ramping velocity.
components/canopen/example/main/CMakeLists.txt Registers example main component.
components/canopen/example/README.md Example instructions and expected output.
components/canopen/example/CMakeLists.txt Example top-level project CMake configuration.
components/canopen/README.md Component overview and feature list.
components/canopen/CMakeLists.txt Component registration (include dirs + dependency).
.github/workflows/upload_components.yml Adds components/canopen to upload list.
.github/workflows/build.yml Adds CANopen example to CI build matrix.

💡 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
Comment thread components/canopen/include/canopen_client.hpp
Comment thread components/canopen/include/canopen_client.hpp
Comment thread components/canopen/include/detail/canopen_core.hpp
Comment thread components/canopen/include/detail/canopen_core.hpp Outdated
Comment thread components/canopen/include/ds402.hpp
Comment thread components/canopen/idf_component.yml
@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

✅Static analysis result - no issues found! ✅

- SDO responses are now matched to the in-flight request: process_frame()
  rejects responses whose echoed index/subindex do not match (or, for
  upload segments which carry no address, whose phase does not match) —
  a late response from a timed-out transaction can no longer complete the
  wrong one. Stale/unexpected responses are logged and ignored.
- parse_sdo_response() requires dlc == 8 (CiA 301 SDO frames are exactly
  8 bytes); shorter frames parse as Unknown instead of decoding missing
  bytes. Regression test added.
- sdo_download() rejects payload sizes other than 1/2/4 (the only valid
  expedited sizes) with invalid_argument; builder precondition documented.
- Typed reads (read_u16/u32/...) now require the response length to equal
  the requested width — read_u32 on a u16 object is a protocol_error
  instead of a silently-wrong value.
- Profile-position handshake: after releasing the new-set-point bit, poll
  until set-point-acknowledge clears (bounded by the same timeout) so the
  next setpoint's rising edge is unambiguous.
- ds402.hpp includes <functional> explicitly; manifest uses the https://
  repository URL (github disabled the git:// protocol).

Host tests ALL PASSED (incl. new malformed-dlc vectors); esp32 example
builds; cppcheck clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

components/canopen/test/canopen_host_test.cpp:15

  • This test file uses std::equal (in bytes_equal) but does not include <algorithm>, which can break host builds depending on the standard library headers included transitively. Add #include <algorithm> to make the test self-contained.
#include <cstdint>
#include <cstdio>
#include <cstring>
#include <span>
#include <string>
#include <vector>

#include "detail/canopen_core.hpp"

components/canopen/include/canopen_client.hpp:128

  • This warning can become noisy in practice (e.g., late responses after timeouts, malformed frames, or segment responses where index/subindex are not meaningful). Consider lowering this to debug/trace level and/or improving the message for segment frames (which don’t carry index/subindex) to avoid misleading logs.
      if (delivered) {
        response_cv_.notify_all();
      } else {
        logger_.warn("Ignoring stale/unexpected SDO response (type {}, 0x{:04X}:{:02X})",
                     static_cast<int>(parsed.type), parsed.index, parsed.subindex);
      }

Comment thread components/canopen/include/detail/canopen_core.hpp Outdated
finger563 and others added 2 commits August 18, 2026 20:44
#730 review, cppcheck)

- parse_sdo_response(): reject RTR and extended-id frames (which merely
  collide with the COB-ID) as malformed — an in-flight transaction can no
  longer be satisfied by a frame that is not a CiA 301 SDO data frame.
  Regression vectors added.
- example: twai/client are function-local statics — the Twai receive task
  and the send lambda reference them and app_main() has early-return error
  paths, so static storage guarantees they outlive every callback (fixes
  the cppcheck danglingLifetime error on client_ptr).

Host tests ALL PASSED; esp32 example builds; cppcheck clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Follow-up to the danglingLifetime fix: capturing a static-storage variable
is ill-formed (-Werror), so the send / heartbeat lambdas are now
captureless and reference the static twai/logger directly (logger made
static for the same lifetime reason). esp32 example builds; cppcheck clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

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.

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (7)

components/canopen/include/canopen_client.hpp:419

  • Successful operations leave a pre-existing ec unchanged. Because callers reuse one error code (including the new example), a prior failure can make a later successful read appear to fail—for example, get_state() checks ec after the read. Clear ec before attempting each frame send, consistent with Twai::transmit() at components/twai/include/twai.hpp:366.
  bool send_frame(const CanFrame &frame, std::error_code &ec) {
    if (!send_ || !send_(frame)) {

components/canopen/include/canopen_client.hpp:115

  • An SDO abort is valid during any phase, including while waiting for an upload segment. Here expected_segment_ accepts only UploadSegment, so a server aborting a segmented read is ignored and reported as a timeout; last_abort_code() also never receives the abort. Accept a matching-address Abort in this branch as well.
          if (expected_segment_) {
            matches = is_segment;

components/canopen/include/detail/canopen_core.hpp:132

  • A CANopen heartbeat/boot-up message has exactly one data byte. Accepting larger DLCs lets malformed or unrelated frames at 0x700 + node_id overwrite the cached NMT state. Require dlc == 1, as the SDO parser already does for its fixed frame size.
inline std::optional<NmtState> parse_heartbeat(const CanFrame &frame, uint8_t &node_id) {
  if (frame.extended || frame.rtr || frame.dlc < 1) {
    return std::nullopt;

components/canopen/include/ds402.hpp:219

  • State::Unknown satisfies this predicate, so fault_reset() can report success on an unrecognized statusword even though there is no evidence the fault cleared. Continue polling (or fail) for Unknown and only succeed for a known non-fault state.
    return wait_for_state(
        [](State s) { return s != State::Fault && s != State::FaultReactionActive; },
        "fault cleared", ec);

components/canopen/include/ds402.hpp:299

  • After bit 4 is raised, this error path—and the acknowledge-timeout path below—returns without lowering it. The drive can therefore retain new set-point high, so a retry has no rising edge and may be silently ignored. Best-effort restore controlword on every exit after the raise, using a separate error code so the original failure is preserved.
      const auto statusword = get_statusword(ec);
      if (ec) {
        return false;

components/canopen/include/canopen_client.hpp:202

  • Payloads larger than eight bytes are silently truncated by make_pdo(), yet this public API returns success. For control PDOs that can send a valid-looking but incomplete command. Reject oversized spans with invalid_argument instead of transmitting partial data.
  bool send_rpdo(uint32_t cob_id, std::span<const uint8_t> data, std::error_code &ec) {
    return send_frame(detail::canopen::make_pdo(cob_id, data), ec);
  }

components/canopen/example/main/canopen_example.cpp:160

  • The return value from every target-velocity write is ignored, so an SDO timeout/abort leaves the previous motor setpoint active while the demo continues as though the ramp succeeded. Check each write and stop the motion sequence (preferably issuing a best-effort quick stop) on failure; apply the same handling to the descending loop and final zero command.
    for (int32_t v = step; v <= max_velocity; v += step) {
      drive.set_target_velocity(v, ec);
      std::this_thread::sleep_for(500ms);

Comment on lines +324 to +325
if (init.size_indicated) {
data_.reserve(init.total_size);
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