Skip to content

Add enable_logger_service option for runtime log level control#1522

Open
minggangw wants to merge 3 commits into
RobotWebTools:developfrom
minggangw:fix-1521
Open

Add enable_logger_service option for runtime log level control#1522
minggangw wants to merge 3 commits into
RobotWebTools:developfrom
minggangw:fix-1521

Conversation

@minggangw
Copy link
Copy Markdown
Member

@minggangw minggangw commented May 27, 2026

Port rclpy's LoggingService to rclnodejs so a node can optionally expose <node>/get_logger_levels and <node>/set_logger_levels services for runtime log-level inspection and updates, and stabilize CI by wrapping flaky test steps with nick-fields/retry@v4.

Feature

  • New lib/logging_service.js (LoggingService) with idempotent start(), UNSET fallback on getLogger failure, and per-entry { successful, reason } for set requests.
  • New enableLoggerService option on NodeOptions (default false, matches rclpy) with backing field, getter/setter, and TypeScript declaration.
  • Wired into Node so the service starts after parameterService when the option is enabled.

Tests

  • Sinon-based unit tests for LoggingService (no ROS runtime needed).
  • Extended NodeOptions and type tests for the new flag.

CI

  • Wrap test invocations with nick-fields/retry@v4 (max_attempts: 2, retry_wait_seconds: 10, timeout_minutes: 60) in linux-x64, linux-arm64, windows, and asan workflows. Build-only, docs, publish, and orchestration workflows are intentionally left untouched.

Fix: #1521

Copilot AI review requested due to automatic review settings May 27, 2026 02:36
Copy link
Copy Markdown

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 NodeOptions.enableLoggerService option intended to let nodes start ROS 2 logger level services for runtime log level control.

Changes:

  • Adds enableLoggerService to JavaScript and TypeScript NodeOptions.
  • Wires Node initialization to start a LoggingService when enabled.
  • Adds unit/type tests for the new option and inferred logging service behavior.

Reviewed changes

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

Show a summary per file
File Description
lib/node.js Imports and starts LoggingService when enableLoggerService is true.
lib/node_options.js Adds storage, getter, setter, and constructor parameter for the new option.
types/node_options.d.ts Adds the TypeScript property declaration.
test/test-node-options.js Covers default and mutable option behavior.
test/types/index.test-d.ts Adds TypeScript type assertion for the new property.
test/test-logging-service.js Adds tests for logger service registration and handlers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/node.js
const GuardCondition = require('./guard_condition.js');
const loader = require('./interface_loader.js');
const Logging = require('./logging.js');
const LoggingService = require('./logging_service.js');
@coveralls
Copy link
Copy Markdown

coveralls commented May 27, 2026

Coverage Status

coverage: 85.492% (+0.02%) from 85.47% — minggangw:fix-1521 into RobotWebTools:develop

Copy link
Copy Markdown

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

Copilot reviewed 6 out of 7 changed files in this pull request and generated no new comments.

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 enable_logger_service option

3 participants