Skip to content

Conversation

@minggangw
Copy link
Member

@minggangw minggangw commented Aug 27, 2025

This PR adds a comprehensive tutorial on ROS 2 lifecycle nodes for the rclnodejs library. The tutorial explains the lifecycle state machine, provides practical examples, and covers best practices for implementing lifecycle nodes in JavaScript.

Key Changes:

  • New lifecycle nodes tutorial with detailed explanations, code examples, and troubleshooting guidance
  • README update to include a link to the new tutorials director

Fix: #1236

Copilot AI review requested due to automatic review settings August 27, 2025 09:11
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

This PR adds a comprehensive tutorial on ROS 2 lifecycle nodes for the rclnodejs library. The tutorial explains the lifecycle state machine, provides practical examples, and covers best practices for implementing lifecycle nodes in JavaScript.

Key Changes:

  • New lifecycle nodes tutorial with detailed explanations, code examples, and troubleshooting guidance
  • README update to include a link to the new tutorials directory

Reviewed Changes

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

File Description
tutorials/lifecycle-nodes.md Comprehensive tutorial covering lifecycle nodes concepts, implementation, and best practices
README.md Added link to tutorials directory in table of contents

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +106 to +110
// If camera fails, just deactivate and reactivate
onError() {
this.node.deactivate();
this.reinitializeCamera();
this.node.activate();
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The onError method shows calling deactivate() and activate() directly on the node, but lifecycle transitions should be handled through proper lifecycle service calls or state management, not direct method calls during error handling.

Suggested change
// If camera fails, just deactivate and reactivate
onError() {
this.node.deactivate();
this.reinitializeCamera();
this.node.activate();
// If camera fails, request lifecycle transitions via service calls
async onError() {
await this.node.changeState('deactivate');
await this.reinitializeCamera();
await this.node.changeState('activate');

Copilot uses AI. Check for mistakes.
switch (msg.data.toLowerCase()) {
case 'stop':
console.log('🛑 Stop command received, shutting down...');
this.shutdown();
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The handleCommand method calls this.shutdown() but the method is defined as shutdown() on the class, not this.node.shutdown(). This should be consistent with the lifecycle API pattern.

Suggested change
this.shutdown();
this.node.shutdown();

Copilot uses AI. Check for mistakes.

shutdown() {
console.log('🔚 Initiating shutdown sequence...');
this.node.deactivate();
Copy link

Copilot AI Aug 27, 2025

Choose a reason for hiding this comment

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

The shutdown method calls both this.node.deactivate() and this.node.shutdown() sequentially. According to the state machine diagram, shutdown can be called from any state, so the deactivate() call may be unnecessary and could cause issues if the node is not in active state.

Suggested change
this.node.deactivate();
// this.node.deactivate(); // Removed: shutdown can be called from any state

Copilot uses AI. Check for mistakes.
@coveralls
Copy link

coveralls commented Aug 27, 2025

Coverage Status

coverage: 84.555%. remained the same
when pulling da2f6ef on minggangw:fix-1236
into f1e8005 on RobotWebTools:develop.

@minggangw minggangw merged commit e54906b into RobotWebTools:develop Aug 27, 2025
19 checks passed
minggangw added a commit that referenced this pull request Sep 10, 2025
This PR adds a comprehensive tutorial on ROS 2 lifecycle nodes for the rclnodejs library. The tutorial explains the lifecycle state machine, provides practical examples, and covers best practices for implementing lifecycle nodes in JavaScript.

### Key Changes:
- **New lifecycle nodes tutorial** with detailed explanations, code examples, and troubleshooting guidance
- **README update** to include a link to the new tutorials director

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

Add tutorial for lifecycle nodes

2 participants