Skip to content

Conversation

minggangw
Copy link
Member

@minggangw minggangw commented Aug 12, 2025

This pull request adds support for getting the logger name for ROS 2 entities (subscription, publisher, service, and client) by exposing a loggerName getter property on each entity class.

  • Adds loggerName getter property to Publisher, Subscription, Service, and Client classes
  • Stores node references in Publisher and Subscription constructors to enable logger name access
  • Adds comprehensive test coverage for the new loggerName property across all entity types

Fix: #1219

@minggangw minggangw marked this pull request as ready for review August 12, 2025 07:36
@Copilot Copilot AI review requested due to automatic review settings August 12, 2025 07:36
Copilot

This comment was marked as outdated.

@minggangw minggangw requested a review from Copilot August 12, 2025 08:03
Copy link

@Copilot 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 pull request adds support for getting the logger name for ROS 2 entities (subscription, publisher, service, and client) by exposing a loggerName getter property on each entity class.

  • Adds loggerName getter property to Publisher, Subscription, Service, and Client classes
  • Stores node references in Publisher and Subscription constructors to enable logger name access
  • Adds comprehensive test coverage for the new loggerName property across all entity types

Reviewed Changes

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

Show a summary per file
File Description
lib/publisher.js Adds node reference storage and loggerName getter using rclnodejs.getNodeLoggerName()
lib/subscription.js Adds node reference storage and loggerName getter using rclnodejs.getNodeLoggerName()
lib/service.js Adds loggerName getter using existing _nodeHandle property
lib/client.js Adds loggerName getter using existing _nodeHandle property
test/test-publisher.js Adds test verifying loggerName returns a string for publishers
test/test-subscription.js Adds test verifying loggerName returns a string for subscriptions
test/test-service.js Adds test verifying loggerName returns a string for both services and clients
test/types/index.test-d.ts Adds TypeScript type assertions for loggerName property on all entity types

result.sum = request.a + request.b;
}
);
const client = node.createClient(AddTwoInts, 'single_ps_channel2');
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

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

The variable AddTwoInts is not defined in this scope. It should be defined as a string like 'example_interfaces/srv/AddTwoInts' to match the service type used above.

Suggested change
const client = node.createClient(AddTwoInts, 'single_ps_channel2');
const client = node.createClient('example_interfaces/srv/AddTwoInts', 'single_ps_channel2');

Copilot uses AI. Check for mistakes.

@coveralls
Copy link

Coverage Status

coverage: 84.555% (+0.03%) from 84.526%
when pulling f3fead1 on minggangw:fix-1219
into a5f8aec on RobotWebTools:develop.

@minggangw minggangw merged commit 0c7b9af into RobotWebTools:develop Aug 12, 2025
19 checks passed
minggangw added a commit that referenced this pull request Aug 18, 2025
…client (#1224)

This pull request adds support for getting the logger name for ROS 2 entities (subscription, publisher, service, and client) by exposing a `loggerName` getter property on each entity class.

- Adds `loggerName` getter property to Publisher, Subscription, Service, and Client classes
- Stores node references in Publisher and Subscription constructors to enable logger name access
- Adds comprehensive test coverage for the new `loggerName` property across all entity types

Fix: #1219
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 getting logger name for subscription, publisher, service and client

2 participants