Skip to content

Conversation

@minggangw
Copy link
Member

@minggangw minggangw commented Jun 17, 2025

Adds support for logging the lifecycle state machine data.

  • Introduces a new C++ binding (print) that calls rcl_print_state_machine
  • Exposes a print() method on the JavaScript LifecycleNode class
  • Adds TypeScript and JS tests to verify the new print() functionality

Fix: #1167

@minggangw minggangw requested a review from Copilot June 17, 2025 09:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for logging the lifecycle state machine data.

  • Introduces a new C++ binding (print) that calls rcl_print_state_machine
  • Exposes a print() method on the JavaScript LifecycleNode class
  • Adds TypeScript and JS tests to verify the new print() functionality

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
test/types/index.test-d.ts Added a type-level test for lifecycleNode.print()
test/test-lifecycle.js New unit test to ensure print() does not throw
src/rcl_lifecycle_bindings.cpp Implemented the Print N-API binding
lib/lifecycle.js Exposed print() method in LifecycleNode with JSDoc
Comments suppressed due to low confidence (4)

lib/lifecycle.js:606

  • [nitpick] JSDoc @returns tag can be simplified to @returns {void} to better convey that this method returns nothing.
* @returns {undefined} void.

test/test-lifecycle.js:361

  • This test only asserts that print() does not throw; consider capturing and asserting on the output (e.g., stubbing console.log) to verify that the state machine data is actually logged.
assert.doesNotThrow(() => {

test/test-lifecycle.js:362

  • The test references node.print() but the lifecycle node instance used elsewhere is named testNode. It should be testNode.print() to avoid a ReferenceError.
      node.print();

test/types/index.test-d.ts:130

  • Ensure the type definition for print is added to the LifecycleNode interface in the public .d.ts so this test passes.
expectType<void>(lifecycleNode.print());

@minggangw minggangw merged commit 8da31c9 into RobotWebTools:develop Jun 17, 2025
23 of 25 checks passed
@coveralls
Copy link

Coverage Status

coverage: 84.731%. first build
when pulling 64b7466 on minggangw:fix-1167
into 3c4492b on RobotWebTools:develop.

minggangw added a commit that referenced this pull request Jun 18, 2025
Adds support for logging the lifecycle state machine data.

- Introduces a new C++ binding (`print`) that calls `rcl_print_state_machine`
- Exposes a `print()` method on the JavaScript `LifecycleNode` class
- Adds TypeScript and JS tests to verify the new `print()` functionality

Fix: #1167
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.

Support to log the state machine data

2 participants