Add enable_logger_service option for runtime log level control#1522
Open
minggangw wants to merge 3 commits into
Open
Add enable_logger_service option for runtime log level control#1522minggangw wants to merge 3 commits into
minggangw wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
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
enableLoggerServiceto JavaScript and TypeScriptNodeOptions. - Wires
Nodeinitialization to start aLoggingServicewhen 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.
| const GuardCondition = require('./guard_condition.js'); | ||
| const loader = require('./interface_loader.js'); | ||
| const Logging = require('./logging.js'); | ||
| const LoggingService = require('./logging_service.js'); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Port rclpy's
LoggingServiceto rclnodejs so a node can optionally expose<node>/get_logger_levelsand<node>/set_logger_levelsservices for runtime log-level inspection and updates, and stabilize CI by wrapping flaky test steps withnick-fields/retry@v4.Feature
lib/logging_service.js(LoggingService) with idempotentstart(), UNSET fallback ongetLoggerfailure, and per-entry{ successful, reason }for set requests.enableLoggerServiceoption onNodeOptions(defaultfalse, matches rclpy) with backing field, getter/setter, and TypeScript declaration.Nodeso the service starts afterparameterServicewhen the option is enabled.Tests
LoggingService(no ROS runtime needed).NodeOptionsand type tests for the new flag.CI
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