Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ const {
} = require('./lib/parameter.js');
const path = require('path');
const QoS = require('./lib/qos.js');
const {
QoSPolicyKind,
QoSOverridingOptions,
} = require('./lib/qos_overriding_options.js');

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

index.js now exports QoSPolicyKind and QoSOverridingOptions, but the published TypeScript declarations (types/*.d.ts) don't currently declare these exports or the new qosOverridingOptions option on publisher/subscription creation. Please update the typings (e.g., types/index.d.ts and the Node publisher/subscription option types) to keep TS consumers in sync with the JS API.

Suggested change
} = require('./lib/qos_overriding_options.js');
} = require('./lib/qos_overriding_options.js');
module.exports.QoSPolicyKind = QoSPolicyKind;
module.exports.QoSOverridingOptions = QoSOverridingOptions;

Copilot uses AI. Check for mistakes.
const rclnodejs = require('./lib/native_loader.js');
const tsdGenerator = require('./rostsd_gen/index.js');
const validator = require('./lib/validator.js');
Expand Down Expand Up @@ -258,6 +262,12 @@ let rcl = {
/** {@link QoS} class */
QoS: QoS,

/** {@link QoSPolicyKind} enum */
QoSPolicyKind: QoSPolicyKind,

/** {@link QoSOverridingOptions} class */
QoSOverridingOptions: QoSOverridingOptions,

/** {@link RMWUtils} */
RMWUtils: RMWUtils,

Expand Down
34 changes: 34 additions & 0 deletions lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ const Service = require('./service.js');
const Subscription = require('./subscription.js');
const ObservableSubscription = require('./observable_subscription.js');
const MessageInfo = require('./message_info.js');
const {
declareQosParameters,
_resolveQoS,
} = require('./qos_overriding_options.js');
const TimeSource = require('./time_source.js');
const Timer = require('./timer.js');
const TypeDescriptionService = require('./type_description_service.js');
Expand Down Expand Up @@ -752,6 +756,21 @@ class Node extends rclnodejs.ShadowNode {
);
}

// Apply QoS overriding options if provided
if (options.qosOverridingOptions) {
const resolvedTopic = this.resolveTopicName(topic);
if (typeof options.qos === 'string' || !(options.qos instanceof QoS)) {
options.qos = _resolveQoS(options.qos);
Comment on lines +766 to +767

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

When qosOverridingOptions is provided, this branch resolves options.qos to a QoS instance whenever it is not a string and not instanceof QoS. That inadvertently discards valid QoS objects passed as plain JS objects (native bindings accept QoS as an object with history/depth/reliability/...), replacing them with the default profile before overrides are applied. Consider only resolving profile strings, and for non-QoS objects either convert them into a new QoS(...) based on their fields or reject with a clear validation error so user-supplied QoS values aren’t silently ignored.

Suggested change
if (typeof options.qos === 'string' || !(options.qos instanceof QoS)) {
options.qos = _resolveQoS(options.qos);
// Only resolve QoS profile *strings* (and undefined → default);
// for plain objects, construct a QoS instance so their fields are preserved.
if (typeof options.qos === 'string' || typeof options.qos === 'undefined') {
options.qos = _resolveQoS(options.qos);
} else if (options.qos && !(options.qos instanceof QoS)) {
options.qos = new QoS(options.qos);

Copilot uses AI. Check for mistakes.
}
declareQosParameters(
'publisher',
this,
resolvedTopic,
options.qos,
options.qosOverridingOptions
);
}
Comment on lines +763 to +776

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

The publisher creation path now supports options.qosOverridingOptions, but the JSDoc for createPublisher/_createPublisher options doesn't mention this new option or the parameter naming convention. Since this is a public API surface, please update the relevant docstring to document qosOverridingOptions and how/when overrides are applied (including that QoS profile strings will be resolved to a mutable QoS object).

Copilot uses AI. Check for mistakes.

let publisher = publisherClass.createPublisher(
this,
typeClass,
Expand Down Expand Up @@ -848,6 +867,21 @@ class Node extends rclnodejs.ShadowNode {
);
}

// Apply QoS overriding options if provided
if (options.qosOverridingOptions) {
const resolvedTopic = this.resolveTopicName(topic);
if (typeof options.qos === 'string' || !(options.qos instanceof QoS)) {
options.qos = _resolveQoS(options.qos);
Comment on lines +881 to +882

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

Same issue as in _createPublisher(): if qosOverridingOptions is set and the user passes a plain-object QoS (supported by the native layer), this condition will replace it with _resolveQoS(...) (default profile), silently ignoring the caller’s provided QoS fields. Limit _resolveQoS to profile strings, and handle non-QoS objects explicitly (convert or throw) so overrides apply to the intended base QoS.

Suggested change
if (typeof options.qos === 'string' || !(options.qos instanceof QoS)) {
options.qos = _resolveQoS(options.qos);
if (typeof options.qos === 'string') {
// Resolve QoS profile strings to QoS instances
options.qos = _resolveQoS(options.qos);
} else if (options.qos && !(options.qos instanceof QoS)) {
// Convert plain-object or non-QoS QoS values to QoS instances
options.qos = new QoS(options.qos);

Copilot uses AI. Check for mistakes.
}
declareQosParameters(
'subscription',
this,
resolvedTopic,
options.qos,
options.qosOverridingOptions
);
}
Comment on lines +878 to +891

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

Same as publisher: createSubscription now accepts options.qosOverridingOptions, but the createSubscription JSDoc options list doesn't document it. Please add it to the documentation so consumers discover the feature and understand the parameter naming/override behavior.

Copilot uses AI. Check for mistakes.

let subscription = Subscription.createSubscription(
this,
typeClass,
Expand Down
306 changes: 306 additions & 0 deletions lib/qos_overriding_options.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,306 @@
// Copyright (c) 2026 The Robot Web Tools Contributors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

'use strict';

const QoS = require('./qos.js');
const {
Parameter,
ParameterType,
ParameterDescriptor,
} = require('./parameter.js');

/**
* Enum of overridable QoS policy kinds.
* Each value corresponds to a QoS property on the {@link QoS} class.
* @enum {number}
*/
const QoSPolicyKind = Object.freeze({
HISTORY: 1,
DEPTH: 2,
RELIABILITY: 3,
DURABILITY: 4,
LIVELINESS: 5,
AVOID_ROS_NAMESPACE_CONVENTIONS: 6,
});

// Maps QoSPolicyKind -> { qosProp, paramType, toParam, fromParam }
const POLICY_MAP = {
[QoSPolicyKind.HISTORY]: {
qosProp: 'history',
enumObj: QoS.HistoryPolicy,
paramType: ParameterType.PARAMETER_STRING,
toParam: (val, enumObj) => _enumToString(val, enumObj),
fromParam: (val, enumObj) => _stringToEnum(val, enumObj),
},
[QoSPolicyKind.DEPTH]: {
qosProp: 'depth',
paramType: ParameterType.PARAMETER_INTEGER,
toParam: (val) => val,

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

For DEPTH overrides, Parameter stores PARAMETER_INTEGER values as BigInt, but POLICY_MAP.DEPTH.toParam returns a Number. That makes param.value !== paramValue always true (BigInt vs Number), so the code always treats the parameter as overridden and reassigns qos.depth. Consider normalizing types (e.g., make toParam return BigInt, or compare after converting both sides) to ensure override detection behaves as intended and avoids unintended type churn.

Suggested change
toParam: (val) => val,
toParam: (val) => BigInt(val),

Copilot uses AI. Check for mistakes.
fromParam: (val) => Number(val),
},
[QoSPolicyKind.RELIABILITY]: {
qosProp: 'reliability',
enumObj: QoS.ReliabilityPolicy,
paramType: ParameterType.PARAMETER_STRING,
toParam: (val, enumObj) => _enumToString(val, enumObj),
fromParam: (val, enumObj) => _stringToEnum(val, enumObj),
},
[QoSPolicyKind.DURABILITY]: {
qosProp: 'durability',
enumObj: QoS.DurabilityPolicy,
paramType: ParameterType.PARAMETER_STRING,
toParam: (val, enumObj) => _enumToString(val, enumObj),
fromParam: (val, enumObj) => _stringToEnum(val, enumObj),
},
[QoSPolicyKind.LIVELINESS]: {
qosProp: 'liveliness',
enumObj: QoS.LivelinessPolicy,
paramType: ParameterType.PARAMETER_STRING,
toParam: (val, enumObj) => _enumToString(val, enumObj),
fromParam: (val, enumObj) => _stringToEnum(val, enumObj),
},
[QoSPolicyKind.AVOID_ROS_NAMESPACE_CONVENTIONS]: {
qosProp: 'avoidRosNameSpaceConventions',
paramType: ParameterType.PARAMETER_BOOL,
toParam: (val) => val,
fromParam: (val) => Boolean(val),
},
Comment on lines +79 to +85

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

The parameter suffix is derived from mapping.qosProp. For AVOID_ROS_NAMESPACE_CONVENTIONS this yields a camelCase parameter name (...avoidRosNameSpaceConventions), unlike the other policy parameters (history, depth, ...). Since these parameter names are user-facing/stable, consider decoupling the parameter key from the QoS property name (e.g., use a consistent snake_case key like avoid_ros_namespace_conventions) while still mapping back to qos.avoidRosNameSpaceConventions.

Copilot uses AI. Check for mistakes.
};

/**
* Convert a numeric enum value to a lowercase string name.
* @param {number} val - enum numeric value
* @param {Object} enumObj - the enum object (e.g. QoS.HistoryPolicy)
* @returns {string}
*/
function _enumToString(val, enumObj) {
for (const [key, v] of Object.entries(enumObj)) {
if (v === val) {
// Strip the RMW prefix: "RMW_QOS_POLICY_HISTORY_KEEP_LAST" -> "keep_last"
const parts = key.split('_');
// Find the index after the policy name (HISTORY, RELIABILITY, etc.)
// Pattern: RMW_QOS_POLICY_<POLICY>_<VALUE>
const policyNames = [
'HISTORY',
'RELIABILITY',
'DURABILITY',
'LIVELINESS',
];
let valueStart = 4; // default: skip RMW_QOS_POLICY_<X>_
for (let i = 3; i < parts.length; i++) {
if (policyNames.includes(parts[i])) {
valueStart = i + 1;
break;
}
}
return parts.slice(valueStart).join('_').toLowerCase();
}
}
return String(val);
}

/**
* Convert a lowercase string back to a numeric enum value.
* @param {string} str - the string value (e.g. "keep_last")
* @param {Object} enumObj - the enum object
* @returns {number}
*/
function _stringToEnum(str, enumObj) {
const upper = str.toUpperCase();
for (const [key, val] of Object.entries(enumObj)) {
if (key.endsWith('_' + upper)) {
return val;
}
}
throw new Error(`Unknown QoS policy value: "${str}"`);
}

/**
* Options for overriding QoS policies via ROS parameters.
*
* When passed to `createPublisher()` or `createSubscription()`, the node
* will declare read-only parameters for each specified policy kind. These
* parameters can be set via command-line arguments, launch files, or
* parameter files to override the QoS profile at startup.
*
* Parameter naming convention:
* `qos_overrides.<topic>.<publisher|subscription>[_<entityId>].<policy>`
*
* @example
* // Override history, depth, and reliability via parameters
* const sub = node.createSubscription(
* 'std_msgs/msg/String', '/chatter',
* { qos: rclnodejs.QoS.profileDefault,
* qosOverridingOptions: QoSOverridingOptions.withDefaultPolicies() },
* (msg) => console.log(msg.data)
* );
* // Now you can override via CLI:
* // --ros-args -p "qos_overrides./chatter.subscription.depth:=20"
*/
class QoSOverridingOptions {
/**
* @param {Array<QoSPolicyKind>} policyKinds - Which QoS policies to expose as parameters.
* @param {Object} [opts]
* @param {function} [opts.callback] - Optional validation callback. Receives the
* final QoS profile after overrides are applied. Should return
* `{successful: true}` or `{successful: false, reason: '...'}`.
* @param {string} [opts.entityId] - Optional suffix to disambiguate multiple
* publishers/subscriptions on the same topic.
*/
constructor(policyKinds, opts = {}) {
this._policyKinds = Array.from(policyKinds);
this._callback = opts.callback || null;
this._entityId = opts.entityId || null;
}

get policyKinds() {
return this._policyKinds;
}

get callback() {
return this._callback;
}

get entityId() {
return this._entityId;
}

/**
* Create options that override history, depth, and reliability —
* the most commonly tuned policies.
* @param {Object} [opts]
* @param {function} [opts.callback] - Validation callback.
* @param {string} [opts.entityId] - Entity disambiguation suffix.
* @returns {QoSOverridingOptions}
*/
static withDefaultPolicies(opts = {}) {
return new QoSOverridingOptions(
[QoSPolicyKind.HISTORY, QoSPolicyKind.DEPTH, QoSPolicyKind.RELIABILITY],
opts
);
}
}

/**
* Resolve QoS profile string to a mutable QoS object.
* If already a QoS instance, return as-is.
* @param {QoS|string} qos
* @returns {QoS}
*/
function _resolveQoS(qos) {
if (qos instanceof QoS) {
return qos;
}
// Profile string: create a QoS with the corresponding defaults
switch (qos) {
case QoS.profileSensorData:
return new QoS(
QoS.HistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_LAST,
5,
QoS.ReliabilityPolicy.RMW_QOS_POLICY_RELIABILITY_BEST_EFFORT,
QoS.DurabilityPolicy.RMW_QOS_POLICY_DURABILITY_VOLATILE
);
case QoS.profileServicesDefault:
return new QoS(
QoS.HistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_LAST,
10,
QoS.ReliabilityPolicy.RMW_QOS_POLICY_RELIABILITY_RELIABLE,
QoS.DurabilityPolicy.RMW_QOS_POLICY_DURABILITY_VOLATILE
);
case QoS.profileDefault:
case QoS.profileSystemDefault:
default:
return new QoS(
QoS.HistoryPolicy.RMW_QOS_POLICY_HISTORY_KEEP_LAST,
10,
QoS.ReliabilityPolicy.RMW_QOS_POLICY_RELIABILITY_RELIABLE,
QoS.DurabilityPolicy.RMW_QOS_POLICY_DURABILITY_VOLATILE
);
}

Copilot AI Apr 2, 2026

Copy link

Choose a reason for hiding this comment

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

_resolveQoS() currently collapses several supported QoS profile strings into the same hard-coded defaults. In particular, QoS.profileSystemDefault should preserve "system default" semantics (history/reliability/durability/liveliness = SYSTEM_DEFAULT, depth = 0), and profiles like profileParameters/profileParameterEvents/profileActionStatusDefault are not handled at all (native layer supports them). Please add explicit cases for all QoS.profile* strings so using qosOverridingOptions doesn't silently change the effective QoS when a profile string is provided.

Copilot uses AI. Check for mistakes.
}

/**
* Declare QoS override parameters on the node and apply any overrides
* to the QoS profile in-place.
*
* @param {'publisher'|'subscription'} entityType
* @param {Node} node
* @param {string} topic - Fully resolved topic name.
* @param {QoS} qos - Mutable QoS object (will be modified in-place).
* @param {QoSOverridingOptions} options
*/
function declareQosParameters(entityType, node, topic, qos, options) {
if (!options || options.policyKinds.length === 0) {
return;
}

const idSuffix = options.entityId ? `_${options.entityId}` : '';
const namePrefix = `qos_overrides.${topic}.${entityType}${idSuffix}`;

for (const policyKind of options.policyKinds) {
const mapping = POLICY_MAP[policyKind];
if (!mapping) {
continue;
}

const paramName = `${namePrefix}.${mapping.qosProp}`;
const currentValue = qos[mapping.qosProp];
const paramValue = mapping.toParam(currentValue, mapping.enumObj);

const descriptor = new ParameterDescriptor(
paramName,
mapping.paramType,
`QoS override for ${mapping.qosProp}`,
true // readOnly
);

let param;
try {
param = node.declareParameter(
new Parameter(paramName, mapping.paramType, paramValue),
descriptor
);
} catch (e) {
// Already declared (e.g. multiple entities on same topic) — reuse
if (node.hasParameter(paramName)) {
param = node.getParameter(paramName);
} else {
throw e;
}
}

// Apply the (possibly overridden) parameter value back to QoS
if (param && param.value !== paramValue) {
qos[mapping.qosProp] = mapping.fromParam(param.value, mapping.enumObj);
}
}

// Run validation callback if provided
if (options.callback) {
const result = options.callback(qos);
if (result && !result.successful) {
throw new Error(
`QoS override validation failed: ${result.reason || 'unknown reason'}`
);
}
}
}

module.exports = {
QoSPolicyKind,
QoSOverridingOptions,
declareQosParameters,
_resolveQoS,
};
Loading
Loading