From 293f20449b5f0ce036fb742a13d23bc797f38ea2 Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 03:45:38 -0500 Subject: [PATCH 1/9] refactor(protocol): Create an exhaustive typing system with documentation based on the Rosbridge Protocol Signed-off-by: Drew Hoener --- src/types/protocol.ts | 677 ++++++++++++++++++++++++++++------------- src/util/type-utils.ts | 12 + 2 files changed, 476 insertions(+), 213 deletions(-) create mode 100644 src/util/type-utils.ts diff --git a/src/types/protocol.ts b/src/types/protocol.ts index 390fc79e3..43ea6096a 100644 --- a/src/types/protocol.ts +++ b/src/types/protocol.ts @@ -1,323 +1,574 @@ -/** - * https://github.com/RobotWebTools/rosbridge_suite/blob/ros2/ROSBRIDGE_PROTOCOL.md - */ +// Base interface for common fields across all operations -export interface RosbridgeMessage { - op: string; -} +import { hasOwn } from "../util/type-utils.ts"; +import type { GoalStatus } from "../core/GoalStatus.ts"; -export function isRosbridgeMessage( - message: unknown, -): message is RosbridgeMessage { - return ( - message instanceof Object && - "op" in message && - typeof message.op === "string" - ); +type DeepPartial = T extends object + ? { + [P in keyof T]?: DeepPartial; + } + : T; +export type BridgeStatusLevel = "info" | "warning" | "error" | "none"; +export type BridgeCompressionType = "none" | "png" | "cbor" | "cbor-raw"; + +export function isValidBridgeCompression( + compression: string, +): compression is BridgeCompressionType { + return ["none", "png", "cbor", "cbor-raw"].includes(compression); } -export interface RosbridgeStatusMessage extends RosbridgeMessage { - op: "status"; +export interface BaseOp { + op: T; id?: string; - level: string; - msg: string; } -export function isRosbridgeStatusMessage( - message: RosbridgeMessage, -): message is RosbridgeStatusMessage { - return message.op === "status"; +export interface QoSOp extends BaseOp { + /** + * ROS1: Passed to publisher + * ROS2: Sets QoS to infinite lifespan and depth of 1 + */ + latch?: boolean; + /** + * ROS1: Passed to publisher + * ROS2: Sets the QoS Queue Depth + */ + queue_size?: number; } -export interface RosbridgeSetStatusLevelMessage extends RosbridgeMessage { - op: "set_level"; - id?: string; +export interface AuthOp extends BaseOp<"auth"> { + mac: string; + client: string; + dest: string; + rand: string; + t: number; level: string; + end: number; } -export function isRosbridgeSetStatusLevelMessage( - message: RosbridgeMessage, -): message is RosbridgeSetStatusLevelMessage { - return message.op === "set_level"; -} - -export interface RosbridgeFragmentMessage extends RosbridgeMessage { - op: "fragment"; +//region Data transformation operations +export interface FragmentOp extends BaseOp<"fragment"> { + /** + * An id is required for fragmented messages, to identify corresponding fragments for the fragmented message. + */ id: string; + /** + * A fragment of data that, when combined with other fragments of data, makes up another message + */ data: string; + /** + * The index of the fragment in the message + */ num: number; + /** + * The total number of fragments + */ total: number; } -export function isRosbridgeFragmentMessage( - message: RosbridgeMessage, -): message is RosbridgeFragmentMessage { - return message.op === "fragment"; -} - -export interface RosbridgePngMessage extends RosbridgeMessage { - op: "png"; +export interface PngOp extends BaseOp<"png"> { + /** + * Only required if the message is fragmented. Identifies the fragments for the fragmented message. + */ id?: string; + /** + * A fragment of a PNG-encoded message or an entire message. + */ data: string; + /** + * Only required if the message is fragmented. The index of the fragment. + */ num?: number; + /** + * Only required if the message is fragmented. The total number of fragments. + */ total?: number; } -export function isRosbridgePngMessage( - message: RosbridgeMessage, -): message is RosbridgePngMessage { - return message.op === "png"; +//endregion + +//region Status operations +export interface SetStatusLevelOp extends BaseOp<"set_level"> { + level: BridgeStatusLevel; } -export interface RosbridgeAdvertiseMessage extends RosbridgeMessage { - op: "advertise"; +export interface StatusOp extends BaseOp<"status"> { + /** + * If the status message was the result of some operation that had an id, then that id is included + */ id?: string; - type: string; - topic: string; - latch?: boolean; - queue_size?: number; + /** + * The level of this status message + */ + level: BridgeStatusLevel; + /** + * The string message being logged + */ + msg: string; } -export function isRosbridgeAdvertiseMessage( - message: RosbridgeMessage, -): message is RosbridgeAdvertiseMessage { - return message.op === "advertise"; -} +//endregion -export interface RosbridgeUnadvertiseMessage extends RosbridgeMessage { - op: "unadvertise"; - id?: string; - topic: string; -} +//region Topic operations -export function isRosbridgeUnadvertiseMessage( - message: RosbridgeMessage, -): message is RosbridgeUnadvertiseMessage { - return message.op === "unadvertise"; +/** + * + * If the topic does not already exist, and the type specified is a valid type, then the topic will be established with this type. + * + * If the topic already exists with a different type, an error status message is sent and this message is dropped. + * + * If the topic already exists with the same type, the sender of this message is registered as another publisher. + * + * If the topic doesn't already exist but the type cannot be resolved, then an error status message is sent and this message is dropped. + */ +export interface AdvertiseOp extends QoSOp<"advertise"> { + /** + * The string name of the topic to advertise + */ + topic: string; + /** + * The string type to advertise for the topic + */ + type: string; } -export interface RosbridgePublishMessage - extends RosbridgeMessage { - op: "publish"; - id?: string; +/** + * If the topic does not exist, a warning status message is sent and this message is dropped + * + * If the topic exists and there are still clients left advertising it, rosbridge will continue to advertise it until all of them have unadvertised + * + * If the topic exists but rosbridge is not advertising it, a warning status message is sent and this message is dropped + */ +export interface UnadvertiseOp extends BaseOp<"unadvertise"> { + /** + * The string name of the topic being unadvertised + */ topic: string; - msg: TMessage; } -export function isRosbridgePublishMessage( - message: RosbridgeMessage, -): message is RosbridgePublishMessage { - return message.op === "publish"; +/** + * The publish command publishes a message on a topic. + * + * If the topic does not exist, then an error status message is sent and this message is dropped + * + * If the msg does not conform to the type of the topic, then an error status message is sent and this message is dropped + * + * If the msg is a subset of the type of the topic, then a warning status message is sent and the unspecified fields are filled in with defaults + * + * Special case: if the type being published has a 'header' field, then the client can optionally omit the header from the msg. + * If this happens, rosbridge will automatically populate the header with a frame id of "" and the timestamp as the current time. + * Alternatively, just the timestamp field can be omitted, and then the current time will be automatically inserted. + */ +export interface PublishOp extends QoSOp<"publish"> { + topic: string; + msg: T; } -export interface RosbridgeSubscribeMessage extends RosbridgeMessage { - op: "subscribe"; +/** + * This command subscribes the client to the specified topic. + * It is recommended that if the client has multiple components subscribing to the same topic, that each component + * makes its own subscription request providing an ID. That way, each can individually unsubscribe and rosbridge can select the correct rate at which to send messages. + * + * If queue_length is specified, then messages are placed into the queue before being sent. + * Messages are sent from the head of the queue. If the queue gets full, the oldest message is removed and + * replaced by the newest message. + * + * If a client has multiple subscriptions to the same topic, then messages are sent at the lowest throttle_rate, + * with the lowest fragmentation size, and highest queue_length. + * It is recommended that the client provides IDs for its subscriptions to enable rosbridge to effectively + * choose the appropriate fragmentation size and publishing rate. + */ +export interface SubscribeOp extends BaseOp<"subscribe"> { + /** + * If specified, then this specific subscription can be unsubscribed by referencing the ID. + */ id?: string; + /** + * The (expected) type of the topic to subscribe to. + * If left off, type will be inferred, and if the topic doesn't exist then the command to subscribe will fail + */ topic: string; - type?: string; + /** + * The name of the topic to subscribe to + */ + type: string; + /** + * The minimum amount of time (in ms) that must elapse between messages being sent. Defaults to 0 + */ throttle_rate?: number; + /** + * the size of the queue to buffer messages. Messages are buffered as a result of the throttle_rate. Defaults to 0 (no queueing). + */ queue_length?: number; + /** + * The maximum size that a message can take before it is to be fragmented. + */ fragment_size?: number; - compression?: string; -} - -export function isRosbridgeSubscribeMessage( - message: RosbridgeMessage, -): message is RosbridgeSubscribeMessage { - return message.op === "subscribe"; + /** + * An optional string to specify the compression scheme to be used on messages. + * Valid values are "none", "png", "cbor", and "cbor-raw". + */ + compression?: BridgeCompressionType; } -export interface RosbridgeUnsubscribeMessage extends RosbridgeMessage { - op: "unsubscribe"; +export interface UnsubscribeOp extends BaseOp<"unsubscribe"> { + /** + * An id of the subscription to unsubscribe. + * If an id is provided, then only the corresponding subscription is unsubscribed. + * If no ID is provided, then all subscriptions are unsubscribed. + */ id?: string; + /** + * The name of the topic to unsubscribe from + */ topic: string; } -export function isRosbridgeUnsubscribeMessage( - message: RosbridgeMessage, -): message is RosbridgeUnsubscribeMessage { - return message.op === "unsubscribe"; -} - -export interface RosbridgeAdvertiseServiceMessage extends RosbridgeMessage { - op: "advertise_service"; - type: string; - service: string; -} +//endregion -export function isRosbridgeAdvertiseServiceMessage( - message: RosbridgeMessage, -): message is RosbridgeAdvertiseServiceMessage { - return message.op === "advertise_service"; -} +//region Service operations -export interface RosbridgeUnadvertiseServiceMessage extends RosbridgeMessage { - op: "unadvertise_service"; +/** + * Advertises an external ROS service server. Requests come to the client via Call Service. + */ +export interface AdvertiseServiceOp extends BaseOp<"advertise_service"> { + /** + * The name of the service to advertise + */ service: string; + /** + * The advertised service message type + */ + type: string; } -export function isRosbridgeUnadvertiseServiceMessage( - message: RosbridgeMessage, -): message is RosbridgeUnadvertiseServiceMessage { - return message.op === "unadvertise_service"; +/** + * Stops advertising an external ROS service server + */ +export interface UnadvertiseServiceOp extends BaseOp<"unadvertise_service"> { + /** + * The name of the service to unadvertise + */ + service: string; } -export interface RosbridgeCallServiceMessage - extends RosbridgeMessage { - op: "call_service"; +/** + * Calls a ROS service. + */ +export interface CallServiceOp + extends BaseOp<"call_service"> { + /** + * An optional id to distinguish this service call + */ id?: string; + /** + * The name of the service to call + */ service: string; /** - * @todo this should be deeply partial when *outgoing*, because rosbridge will "fill in the blanks", - * but it's not partial when *incoming* - need to figure out a way to represent this. + * if the service has no args, then args does not have to be provided, though an empty list is equally acceptable. + * Args should be a list of json objects representing the arguments to the service. + * + * If outgoing, the message can be partial; Rosbridge will "fill in the blanks". + * If incoming, the message will be complete + * + * @see IncomingCallServiceOp + */ + args: TRequest | DeepPartial; + /** + * The maximum size that the response message can take before it is fragmented */ - args: TArgs; fragment_size?: number; - compression?: string; + /** + * An optional string to specify the compression scheme to be used on messages. Valid values are "none" and "png" + */ + compression?: Extract; + /** + * The time, in seconds, to wait for a response from the server + */ timeout?: number; } -export function isRosbridgeCallServiceMessage( - message: RosbridgeMessage, -): message is RosbridgeCallServiceMessage { - return message.op === "call_service"; -} +/** + * Operation sent by the Bridge to RosLibJS to call a locally advertised service. + */ +export type IncomingCallServiceOp = Omit< + CallServiceOp, + "args" +> & { + /** + * Incoming message args will be "filled in" by Rosbridge. + */ + args: TRequest; +}; -interface BaseRosbridgeServiceResponseMessage extends RosbridgeMessage { - op: "service_response"; +export interface ServiceResponseOp + extends BaseOp<"service_response"> { + /** + * If an ID was provided to the service request, then the service response will contain the ID + */ id?: string; + /** + * The name of the service that was called + */ service: string; + /** + * The return values. If the service had no return values, then this field can be + * omitted (and will be by the rosbridge server) + */ + values?: TResponse | string; + /** + * Return value of service callback. true means success, false failure. + */ result: boolean; } -/** If the service call failed, `values` will be a string error message. */ -interface FailedRosbridgeServiceResponseMessage - extends BaseRosbridgeServiceResponseMessage { +export interface ServiceResponseSuccessOp + extends ServiceResponseOp { + /** + * The return values. If the service had no return values, then this field can be + * omitted (and will be by the rosbridge server) + */ + values: TResponse; + /** + * Return value of service callback. true means success, false failure. + */ + result: true; +} + +export interface ServiceResponseFailedOp extends ServiceResponseOp { + /** + * The return values. If the service had no return values, then this field can be + * omitted (and will be by the rosbridge server) + */ values?: string; + /** + * Return value of service callback. true means success, false failure. + */ result: false; } -interface SuccessfulRosbridgeServiceResponseMessage - extends BaseRosbridgeServiceResponseMessage { - values: TValues; - result: true; -} +export type AnyServiceResponseOp = + | ServiceResponseSuccessOp + | ServiceResponseFailedOp; -export type RosbridgeServiceResponseMessage = - | FailedRosbridgeServiceResponseMessage - | SuccessfulRosbridgeServiceResponseMessage; +export type AnyServiceOp< + TRequest extends object = object, + TResponse extends object = object, +> = + | AdvertiseServiceOp + | UnadvertiseServiceOp + | CallServiceOp + | IncomingCallServiceOp + | AnyServiceResponseOp; -export function isRosbridgeServiceResponseMessage( - message: RosbridgeMessage, -): message is RosbridgeServiceResponseMessage { - return message.op === "service_response"; +export function isServiceResponseSuccess( + obj: unknown, +): obj is ServiceResponseSuccessOp { + return ( + isBridgeProtoOp(obj) && + obj.op === "service_response" && + hasOwn(obj, "result") && + obj.result + ); } -export interface RosbridgeAdvertiseActionMessage extends RosbridgeMessage { - op: "advertise_action"; - type: string; - action: string; -} +//endregion -export function isRosbridgeAdvertiseActionMessage( - message: RosbridgeMessage, -): message is RosbridgeAdvertiseActionMessage { - return message.op === "advertise_action"; -} +//region Action operations -export interface RosbridgeUnadvertiseActionMessage extends RosbridgeMessage { - op: "unadvertise_action"; +/** + * Advertises an external ROS action server. + */ +export interface AdvertiseActionOp extends BaseOp<"advertise_action"> { + /** + * The name of the action to advertise + */ action: string; + /** + * The advertised action message type + */ + type: string; } -export function isRosbridgeUnadvertiseActionMessage( - message: RosbridgeMessage, -): message is RosbridgeUnadvertiseActionMessage { - return message.op === "unadvertise_action"; +/** + * Unadvertises an external ROS action server. + */ +export interface UnadvertiseActionOp extends BaseOp<"unadvertise_action"> { + /** + * The name of the action to unadvertise + */ + action: string; } -export interface RosbridgeSendActionGoalMessage - extends RosbridgeMessage { - op: "send_action_goal"; - id: string; +/** + * Sends a goal to a ROS action server. + */ +export interface SendActionGoalOp + extends BaseOp<"send_action_goal"> { + /** + * An optional id to distinguish this goal handle + */ + id?: string; + /** + * The name of the action to send a goal to + */ action: string; + /** + * The action message type + */ action_type: string; - args?: TArgs; + /** + * If the goal has no args, then args does not have to be provided, though an empty list is equally acceptable. + * Args should be a list of json objects representing the arguments to the service. + */ + args?: TGoal; + /** + * If true, sends feedback messages over rosbridge. Defaults to false. + */ feedback?: boolean; + /** + * The maximum size that the result and feedback messages can take before they are fragmented + */ fragment_size?: number; - compression?: string; -} - -export function isRosbridgeSendActionGoalMessage( - message: RosbridgeMessage, -): message is RosbridgeSendActionGoalMessage { - return message.op === "send_action_goal"; + /** + * An optional string to specify the compression scheme to be used on messages. Valid values are "none" and "png" + */ + compression?: Extract; } -export interface RosbridgeCancelActionGoalMessage extends RosbridgeMessage { - op: "cancel_action_goal"; +export interface CancelActionGoalOp extends BaseOp<"cancel_action_goal"> { + /** + * The id representing the goal handle to cancel. + * The id field must match an already in-progress goal. + */ id: string; + /** + * The name of the action to cancel + */ action: string; } -export function isRosbridgeCancelActionGoalMessage( - message: RosbridgeMessage, -): message is RosbridgeCancelActionGoalMessage { - return message.op === "cancel_action_goal"; -} - -export interface RosbridgeActionFeedbackMessage - extends RosbridgeMessage { - op: "action_feedback"; +/** + * Used to send action feedback for a specific goal handle. + */ +export interface ActionFeedbackOp + extends BaseOp<"action_feedback"> { + /** + * The id representing the goal handle. + * The id field must match an already in-progress goal. + */ id: string; + /** + * The name of the action to cancel + */ action: string; + /** + * The feedback values + */ values: TFeedback; } -export function isRosbridgeActionFeedbackMessage( - message: RosbridgeMessage, -): message is RosbridgeActionFeedbackMessage { - return message.op === "action_feedback"; -} - -interface RosbridgeActionResultMessageBase extends RosbridgeMessage { - op: "action_result"; +/** + * A result for a ROS action. + */ +export interface ActionResultSuccessOp + extends BaseOp<"action_result"> { + /** + * If an ID was provided to the action goal, then the action result will contain the ID + */ id: string; + /** + * The name of the action that was executed + */ action: string; - status: number; -} - -interface FailedRosbridgeActionResultMessage - extends RosbridgeActionResultMessageBase { - result: false; - values?: string; -} - -interface SuccessfulRosbridgeActionResultMessage - extends RosbridgeActionResultMessageBase { - values: TResultValues; + /** + * The result values. If the service had no return values, then this field can be omitted (and will be by the rosbridge server) + */ + values: TResult; + /** + * Return status of the action. This matches the enumeration in the action_msgs/msg/GoalStatus ROS message. + */ + status: GoalStatus; + /** + * Return value of action. True means success, false failure. + */ result: true; } -export type RosbridgeActionResultMessage = - | FailedRosbridgeActionResultMessage - | SuccessfulRosbridgeActionResultMessage; - -export function isRosbridgeActionResultMessage( - message: RosbridgeMessage, -): message is RosbridgeActionResultMessage { - return message.op === "action_result"; -} - -export interface RosbridgeActionStatusMessage extends RosbridgeMessage { - op: "action_status"; +/** + * A result for a ROS action. + */ +export interface ActionResultFailedOp extends BaseOp<"action_result"> { + /** + * If an ID was provided to the action goal, then the action result will contain the ID + */ id: string; + /** + * The name of the action that was executed + */ action: string; - status: number; + /** + * The result values. If the service had no return values, then this field can be omitted (and will be by the rosbridge server) + */ + values?: string; + /** + * Return status of the action. This matches the enumeration in the action_msgs/msg/GoalStatus ROS message. + */ + status: GoalStatus; + /** + * Return value of action. True means success, false failure. + */ + result: false; } -export function isRosbridgeActionStatusMessage( - message: RosbridgeMessage, -): message is RosbridgeActionStatusMessage { - return message.op === "action_status"; +export type AnyActionOp< + TGoal extends object = object, + TFeedback extends object = object, + TResult extends object = object, +> = + | AdvertiseActionOp + | UnadvertiseActionOp + | SendActionGoalOp + | CancelActionGoalOp + | ActionFeedbackOp + | ActionResultSuccessOp + | ActionResultFailedOp; + +//endregion + +// The discriminated union type for all RosBridge operations +export type BridgeProtoOp = + | AuthOp + | FragmentOp + | PngOp + | SetStatusLevelOp + | StatusOp + | AdvertiseOp + | UnadvertiseOp + | PublishOp + | SubscribeOp + | UnsubscribeOp + | AdvertiseServiceOp + | UnadvertiseServiceOp + | CallServiceOp + | ServiceResponseSuccessOp + | ServiceResponseFailedOp + | AdvertiseActionOp + | UnadvertiseActionOp + | SendActionGoalOp + | CancelActionGoalOp + | ActionFeedbackOp + | ActionResultSuccessOp + | ActionResultFailedOp; + +export type BridgeProtoOpKey = BridgeProtoOp["op"]; + +// Type guard to check if an unknown object is a valid BridgeProtoOp +export function isBridgeProtoOp(obj: unknown): obj is BridgeProtoOp { + return ( + typeof obj === "object" && + obj !== null && + hasOwn(obj, "op") && + typeof obj.op === "string" + ); } diff --git a/src/util/type-utils.ts b/src/util/type-utils.ts new file mode 100644 index 000000000..498edc8ee --- /dev/null +++ b/src/util/type-utils.ts @@ -0,0 +1,12 @@ +/** + * Type-safe wrapper for Object.hasOwn. + * Asserts on return that the object has the property, allowing it to be used without type complaints. + * @param obj The object to check. + * @param prop The property to check for. + */ +export function hasOwn( + obj: X, + prop: Y, +): obj is X & Record { + return Object.hasOwn(obj, prop); +} From 74d55b6d8dc1b1d7173d8dbe1ea5a45f0a7a341a Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 03:46:25 -0500 Subject: [PATCH 2/9] refactor(protocol): Build Ros client events based on OpType union Signed-off-by: Drew Hoener --- src/types/emitted_events.ts | 38 +++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 src/types/emitted_events.ts diff --git a/src/types/emitted_events.ts b/src/types/emitted_events.ts new file mode 100644 index 000000000..ebd3258a4 --- /dev/null +++ b/src/types/emitted_events.ts @@ -0,0 +1,38 @@ +import type { TransportEvent } from "../core/transport/Transport.js"; +import type { + BridgeProtoOpKey, + AnyActionOp, + AnyServiceOp, +} from "./protocol.js"; + +export type TransportEventString = "open" | "close" | "error"; +export type TransportEvents = Record< + TransportEventString, + [event: TransportEvent] +>; + +export type ActionIdString = `send_action_goal:${string}:${string}`; +export type ServiceCallIdString = `call_service:${string}:${string}`; +export type TopicKey = Exclude< + string, + TransportEventString | ActionIdString | ServiceCallIdString | BridgeProtoOpKey +>; + +// Event strings specific to actions +export type RosActionEvents = Record< + ActionIdString, + // Allow the action class to narrow the type itself. + [message: AnyActionOp] +>; + +export type RosServiceEvents = Record< + ServiceCallIdString, + // Allow the action class to narrow the type itself. + [message: AnyServiceOp] +>; + +export type RosEventTypes = TransportEvents & + RosActionEvents & // Action Events + RosServiceEvents & + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Record; // Service Calls & Topic messages, need to be `any` to be coerced by implementers From 7019f61699cb54cf70ebdba8bae72ef0c7f8d305 Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 03:46:51 -0500 Subject: [PATCH 3/9] refactor(protocol): Build Ros client events based on OpType union Signed-off-by: Drew Hoener --- src/core/Ros.ts | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/core/Ros.ts b/src/core/Ros.ts index 0b75188e1..7843d6b3e 100644 --- a/src/core/Ros.ts +++ b/src/core/Ros.ts @@ -32,6 +32,7 @@ import type { TransportEvent, } from "./transport/Transport.js"; import { WebSocketTransportFactory } from "./transport/WebSocketTransportFactory.ts"; +import type { RosEventTypes } from "../types/emitted_events.js"; interface TypeDefDict { [key: string]: string | string[] | TypeDefDict | TypeDefDict[]; @@ -47,14 +48,7 @@ interface TypeDefDict { * * <topicName> - A message came from rosbridge with the given topic name. * * <serviceID> - A service response came from rosbridge with the given ID. */ -export default class Ros extends EventEmitter< - { - open: [TransportEvent]; - close: [TransportEvent]; - error: [TransportEvent]; - // Any dynamically-named event should correspond to a rosbridge protocol message - } & Record -> { +export default class Ros extends EventEmitter { // private write, public read via getter method #isConnected: boolean; From 04322673927f14886d86ec5c7bc3be40b96575bc Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 04:05:35 -0500 Subject: [PATCH 4/9] refactor(protocol): Use discriminated union to do type narrowing based on "op" field instead of all the checker functions. Enforce typing of messages and callback types. Fix deviations from protocol spec Signed-off-by: Drew Hoener --- src/core/Action.ts | 67 ++++++++++--------- src/core/Ros.ts | 57 +++++++--------- src/core/Service.ts | 41 +++++++----- src/core/Topic.ts | 38 +++++------ .../transport/NativeWebSocketTransport.ts | 4 +- src/core/transport/Transport.ts | 48 ++++++------- src/core/transport/WsWebSocketTransport.ts | 4 +- 7 files changed, 125 insertions(+), 134 deletions(-) diff --git a/src/core/Action.ts b/src/core/Action.ts index 1e9a0e0b1..e3419ac98 100644 --- a/src/core/Action.ts +++ b/src/core/Action.ts @@ -4,15 +4,17 @@ */ import { GoalStatus } from "./GoalStatus.ts"; -import type { RosbridgeSendActionGoalMessage } from "../types/protocol.ts"; -import { - isRosbridgeActionFeedbackMessage, - isRosbridgeActionResultMessage, - isRosbridgeCancelActionGoalMessage, - isRosbridgeSendActionGoalMessage, -} from "../types/protocol.ts"; import type Ros from "./Ros.js"; import { v4 as uuidv4 } from "uuid"; +import type { ActionIdString } from "../types/emitted_events.js"; +import type { + AnyActionOp, + SendActionGoalOp, + CancelActionGoalOp, + ActionFeedbackOp, + ActionResultSuccessOp, + ActionResultFailedOp, +} from "../types/protocol.js"; /** * A ROS 2 action client. @@ -68,21 +70,24 @@ export default class Action< return; } - const actionGoalId = `send_action_goal:${this.name}:${uuidv4()}`; + const actionGoalId: ActionIdString = `send_action_goal:${this.name}:${uuidv4()}`; - this.ros.on(actionGoalId, function (message) { - if (isRosbridgeActionResultMessage(message)) { - if (!message.result) { - failedCallback(message.values ?? ""); - } else { - resultCallback(message.values); + this.ros.on( + actionGoalId, + (message: AnyActionOp) => { + if (message.op === "action_result") { + if (!message.result) { + failedCallback(message.values ?? ""); + } else { + resultCallback(message.values); + } + } else if (message.op === "action_feedback") { + feedbackCallback?.(message.values); } - } else if (isRosbridgeActionFeedbackMessage(message)) { - feedbackCallback?.(message.values); - } - }); + }, + ); - const call = { + const call: SendActionGoalOp = { op: "send_action_goal", id: actionGoalId, action: this.name, @@ -101,7 +106,7 @@ export default class Action< * @param id - The ID of the action goal to cancel. */ cancelGoal(id: string) { - const call = { + const call: CancelActionGoalOp = { op: "cancel_action_goal", id: id, action: this.name, @@ -166,16 +171,13 @@ export default class Action< * @param rosbridgeRequest.id - The ID of the action goal. * @param rosbridgeRequest.args - The arguments of the action goal. */ - #executeAction(rosbridgeRequest: RosbridgeSendActionGoalMessage) { + #executeAction(rosbridgeRequest: SendActionGoalOp) { const id = rosbridgeRequest.id; // If a cancellation callback exists, call it when a cancellation event is emitted. if (typeof id === "string") { - this.ros.on(id, (message) => { - if ( - isRosbridgeCancelActionGoalMessage(message) && - this.#cancelCallback - ) { + this.ros.on(id, (message: AnyActionOp) => { + if (message.op === "cancel_action_goal" && this.#cancelCallback) { this.#cancelCallback(id); } }); @@ -183,13 +185,12 @@ export default class Action< // Call the action goal execution function provided. if (this.#actionCallback) { - if (rosbridgeRequest.args) { - this.#actionCallback(rosbridgeRequest.args, id); - } else { + if (!rosbridgeRequest.args) { throw new Error( "Received Action goal with no arguments! This should never happen, because rosbridge should fill in blanks!", ); } + this.#actionCallback(rosbridgeRequest.args, id); } } @@ -200,7 +201,7 @@ export default class Action< * @param feedback - The feedback to send. */ sendFeedback(id: string, feedback: TFeedback) { - const call = { + const call: ActionFeedbackOp = { op: "action_feedback", id: id, action: this.name, @@ -216,7 +217,7 @@ export default class Action< * @param result - The result to set. */ setSucceeded(id: string, result: TResult) { - const call = { + const call: ActionResultSuccessOp = { op: "action_result", id: id, action: this.name, @@ -234,7 +235,7 @@ export default class Action< * @param result - The result to set. */ setCanceled(id: string, result: TResult) { - const call = { + const call: ActionResultSuccessOp = { op: "action_result", id: id, action: this.name, @@ -251,7 +252,7 @@ export default class Action< * @param id - The action goal ID. */ setFailed(id: string) { - const call = { + const call: ActionResultFailedOp = { op: "action_result", id: id, action: this.name, diff --git a/src/core/Ros.ts b/src/core/Ros.ts index 7843d6b3e..e2e580138 100644 --- a/src/core/Ros.ts +++ b/src/core/Ros.ts @@ -3,19 +3,12 @@ * @author Brandon Alexander - baalexander@gmail.com */ -import type { - RosbridgeMessage, - RosbridgeSetStatusLevelMessage, -} from "../types/protocol.js"; import { - isRosbridgeActionFeedbackMessage, - isRosbridgeActionResultMessage, - isRosbridgeCallServiceMessage, - isRosbridgeCancelActionGoalMessage, - isRosbridgePublishMessage, - isRosbridgeSendActionGoalMessage, - isRosbridgeServiceResponseMessage, - isRosbridgeStatusMessage, + type BridgeProtoOp, + isBridgeProtoOp, + type SetStatusLevelOp, + type BridgeStatusLevel, + type AuthOp, } from "../types/protocol.js"; import Topic from "./Topic.js"; @@ -106,7 +99,7 @@ export default class Ros extends EventEmitter { this.emit("error", event); }); - transport.on("message", (message: RosbridgeMessage) => { + transport.on("message", (message: BridgeProtoOp) => { this.handleMessage(message); }); } @@ -115,26 +108,31 @@ export default class Ros extends EventEmitter { this.transport?.close(); } - private handleMessage(message: RosbridgeMessage) { - if (isRosbridgePublishMessage(message)) { + private handleMessage(message: unknown) { + if (!isBridgeProtoOp(message)) { + console.error("Received non-BridgeProtoOp message:", message); + return; + } + + if (message.op === "publish") { this.emit(message.topic, message); - } else if (isRosbridgeServiceResponseMessage(message)) { + } else if (message.op === "service_response") { if (message.id) { this.emit(message.id, message); } else { console.error("Received service response without ID"); } - } else if (isRosbridgeCallServiceMessage(message)) { + } else if (message.op === "call_service") { this.emit(message.service, message); - } else if (isRosbridgeSendActionGoalMessage(message)) { + } else if (message.op === "send_action_goal") { this.emit(message.action, message); - } else if (isRosbridgeCancelActionGoalMessage(message)) { + } else if (message.op === "cancel_action_goal") { this.emit(message.id, message); - } else if (isRosbridgeActionFeedbackMessage(message)) { + } else if (message.op === "action_feedback") { this.emit(message.id, message); - } else if (isRosbridgeActionResultMessage(message)) { + } else if (message.op === "action_result") { this.emit(message.id, message); - } else if (isRosbridgeStatusMessage(message)) { + } else if (message.op === "status") { if (message.id) { this.emit(`status:${message.id}`, message); } else { @@ -159,12 +157,12 @@ export default class Ros extends EventEmitter { client: string, dest: string, rand: string, - t: object, + t: number, level: string, - end: object, + end: number, ) { // create the request - const auth = { + const auth: AuthOp = { op: "auth", mac: mac, client: client, @@ -182,10 +180,7 @@ export default class Ros extends EventEmitter { * Sends the message to the transport. * If not connected, queues the message to send once reconnected. */ - // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters -- to broaden argument type to any RosbridgeMessage variant - public callOnConnection( - message: T, - ) { + public callOnConnection(message: BridgeProtoOp) { if (this.isConnected()) { this.transport?.send(message); } else { @@ -201,8 +196,8 @@ export default class Ros extends EventEmitter { * @param level - Status level (none, error, warning, info). * @param [id] - Operation ID to change status level on. */ - public setStatusLevel(level: string, id?: string) { - const levelMsg: RosbridgeSetStatusLevelMessage = { + public setStatusLevel(level: BridgeStatusLevel, id?: string) { + const levelMsg: SetStatusLevelOp = { op: "set_level", level, id, diff --git a/src/core/Service.ts b/src/core/Service.ts index 5a681938e..f9d04e6eb 100644 --- a/src/core/Service.ts +++ b/src/core/Service.ts @@ -4,16 +4,16 @@ */ import { EventEmitter } from "eventemitter3"; -import type { - RosbridgeMessage, - RosbridgeServiceResponseMessage, -} from "../types/protocol.ts"; -import { - isRosbridgeCallServiceMessage, - isRosbridgeServiceResponseMessage, -} from "../types/protocol.ts"; import type Ros from "./Ros.js"; import { v4 as uuidv4 } from "uuid"; +import { + type AnyServiceOp, + isServiceResponseSuccess, + type CallServiceOp, + type IncomingCallServiceOp, +} from "../types/protocol.js"; +import type { ServiceCallIdString } from "../types/emitted_events.js"; + /** * A ROS service client. @@ -76,23 +76,28 @@ export default class Service extends EventEmitter { return; } - const serviceCallId = `call_service:${this.name}:${uuidv4()}`; + const serviceCallId: ServiceCallIdString = `call_service:${this.name}:${uuidv4()}`; - this.ros.once(serviceCallId, function (message) { - if (isRosbridgeServiceResponseMessage(message)) { - if (!message.result) { - failedCallback(message.values ?? ""); - } else { + this.ros.once( + serviceCallId, + (message: AnyServiceOp) => { + if (message.op !== "service_response") { + return; + } + + if (isServiceResponseSuccess(message)) { callback?.(message.values); + return; } - } - }); - const call = { + failedCallback(message.values ?? ""); + }, + ); + + const call: CallServiceOp = { op: "call_service", id: serviceCallId, service: this.name, - type: this.serviceType, args: request, timeout: timeout, }; diff --git a/src/core/Topic.ts b/src/core/Topic.ts index ab291c874..12867f132 100644 --- a/src/core/Topic.ts +++ b/src/core/Topic.ts @@ -6,14 +6,17 @@ import { EventEmitter } from "eventemitter3"; import Service from "./Service.js"; import type Ros from "./Ros.js"; -import { - isRosbridgePublishMessage, - type RosbridgeAdvertiseMessage, - type RosbridgeMessage, - type RosbridgeSubscribeMessage, -} from "../types/protocol.ts"; import type { rosapi } from "../types/rosapi.ts"; import { v4 as uuidv4 } from "uuid"; +import { + type BridgeCompressionType, + type SubscribeOp, + type AdvertiseOp, + type BridgeProtoOp, + isValidBridgeCompression, + type PublishOp, +} from "../types/protocol.js"; + /** * Publish and/or subscribe to a topic in ROS. @@ -34,7 +37,7 @@ export default class Topic extends EventEmitter<{ ros: Ros; name: string; messageType: string; - compression: string; + compression: BridgeCompressionType; throttle_rate: number; latch: boolean; queue_size: number; @@ -91,18 +94,10 @@ export default class Topic extends EventEmitter<{ this.reconnect_on_close = reconnect_on_close; // Check for valid compression types - if ( - this.compression && - this.compression !== "png" && - this.compression !== "cbor" && - this.compression !== "cbor-raw" && - this.compression !== "none" - ) { + if (!isValidBridgeCompression(this.compression)) { this.emit( "warning", - `${ - this.compression - } compression is not supported. No compression will be used.`, + `${compression} compression is not supported. No compression will be used.`, ); this.compression = "none"; } @@ -139,14 +134,13 @@ export default class Topic extends EventEmitter<{ } } - #messageCallback = (data: RosbridgeMessage) => { - if (isRosbridgePublishMessage(data)) { - this.emit("message", data.msg); - } else { + #messageCallback = (data: BridgeProtoOp) => { + if (data.op !== "publish") { throw new Error( `Unexpected message on topic channel: ${JSON.stringify(data)}`, ); } + this.emit("message", data.msg); }; /** * Every time a message is published for the given topic, the callback @@ -258,7 +252,7 @@ export default class Topic extends EventEmitter<{ this.advertise(); } - const call = { + const call: PublishOp = { op: "publish", id: `publish:${this.name}:${uuidv4()}`, topic: this.name, diff --git a/src/core/transport/NativeWebSocketTransport.ts b/src/core/transport/NativeWebSocketTransport.ts index 72d6a2507..0f3426ef4 100644 --- a/src/core/transport/NativeWebSocketTransport.ts +++ b/src/core/transport/NativeWebSocketTransport.ts @@ -1,5 +1,5 @@ -import type { RosbridgeMessage } from "../../types/protocol.js"; import { AbstractTransport } from "./Transport.js"; +import type { BridgeProtoOp } from "../../types/protocol.js"; /** * Uses the native `WebSocket` class to send and receive messages. @@ -15,7 +15,7 @@ export class NativeWebSocketTransport extends AbstractTransport { this.registerEventListeners(); } - public send(message: RosbridgeMessage): void { + public send(message: BridgeProtoOp): void { this.socket.send(JSON.stringify(message)); } diff --git a/src/core/transport/Transport.ts b/src/core/transport/Transport.ts index b3fe29fab..f0f90d577 100644 --- a/src/core/transport/Transport.ts +++ b/src/core/transport/Transport.ts @@ -1,18 +1,14 @@ import EventEmitter from "eventemitter3"; -import type { - RosbridgePngMessage, - RosbridgeMessage, - RosbridgeFragmentMessage, -} from "../../types/protocol.js"; -import { - isRosbridgeFragmentMessage, - isRosbridgeMessage, - isRosbridgePngMessage, -} from "../../types/protocol.js"; import { deserialize } from "bson"; import CBOR from "cbor-js"; import typedArrayTagger from "../../util/cborTypedArrayTags.js"; import decompressPng from "../../util/decompressPng.js"; +import { + type BridgeProtoOp, + type FragmentOp, + isBridgeProtoOp, + type PngOp, +} from "../../types/protocol.js"; /** * Because transport implementations may have different event types @@ -33,9 +29,9 @@ export interface ITransport { listener: (event: TransportEvent) => void, ): this; - on(event: "message", listener: (message: RosbridgeMessage) => void): this; + on(event: "message", listener: (message: BridgeProtoOp) => void): this; - send(message: RosbridgeMessage): void; + send(message: BridgeProtoOp): void; close(): void; @@ -60,7 +56,7 @@ export abstract class AbstractTransport open: [TransportEvent]; close: [TransportEvent]; error: [TransportEvent]; - message: [RosbridgeMessage]; + message: [BridgeProtoOp]; }> implements ITransport { @@ -68,7 +64,7 @@ export abstract class AbstractTransport * Buffer Map for incoming message fragments. */ #fragmentBuffer = new Map< - RosbridgeFragmentMessage["id"], + FragmentOp["id"], { fragments: string[]; received: number; @@ -76,7 +72,7 @@ export abstract class AbstractTransport } >(); - abstract send(message: RosbridgeMessage): void; + abstract send(message: BridgeProtoOp): void; abstract close(): void; abstract isConnecting(): boolean; abstract isOpen(): boolean; @@ -94,7 +90,7 @@ export abstract class AbstractTransport */ protected handleRawMessage(data: unknown): void { try { - if (isRosbridgeMessage(data)) { + if (isBridgeProtoOp(data)) { this.handleRosbridgeMessage(data); } else if (typeof Blob !== "undefined" && data instanceof Blob) { this.handleBsonMessage(data); @@ -114,10 +110,10 @@ export abstract class AbstractTransport * If the message is a PNG, it is decompressed and reprocessed. * Otherwise, the message is emitted. */ - private handleRosbridgeMessage(message: RosbridgeMessage) { - if (isRosbridgeFragmentMessage(message)) { + private handleRosbridgeMessage(message: BridgeProtoOp) { + if (message.op === "fragment") { this.handleRosbridgeFragmentMessage(message); - } else if (isRosbridgePngMessage(message)) { + } else if (message.op === "png") { this.handleRosbridgePngMessage(message); } else { this.emit("message", message); @@ -128,7 +124,7 @@ export abstract class AbstractTransport * Appends a fragment to the current fragment buffer for the message id. * If all fragments are received, the message is reconstructed and processed. */ - private handleRosbridgeFragmentMessage(fragment: RosbridgeFragmentMessage) { + private handleRosbridgeFragmentMessage(fragment: FragmentOp) { const { id, data, num, total } = fragment; if ( !id || @@ -174,7 +170,7 @@ export abstract class AbstractTransport } finally { this.#fragmentBuffer.delete(id); } - if (isRosbridgeMessage(message)) { + if (isBridgeProtoOp(message)) { this.handleRosbridgeMessage(message); } else { throw new Error("Received invalid rosbridge message!"); @@ -186,9 +182,9 @@ export abstract class AbstractTransport * Decompresses a PNG image expecting the result to be a RosbridgeMessage. * It is one technique for compressing JSON data. */ - private handleRosbridgePngMessage(message: RosbridgePngMessage) { + private handleRosbridgePngMessage(message: PngOp) { const decoded = decompressPng(message.data); - if (isRosbridgeMessage(decoded)) { + if (isBridgeProtoOp(decoded)) { this.handleRosbridgeMessage(decoded); } else { throw new Error("Decompressed PNG data was invalid!"); @@ -205,7 +201,7 @@ export abstract class AbstractTransport if (reader.result instanceof ArrayBuffer) { const uint8Array = new Uint8Array(reader.result); const data: unknown = deserialize(uint8Array); - if (isRosbridgeMessage(data)) { + if (isBridgeProtoOp(data)) { this.handleRosbridgeMessage(data); } else { this.emit("error", new Error("Decoded BSON data was invalid!")); @@ -221,7 +217,7 @@ export abstract class AbstractTransport */ private handleCborMessage(cbor: ArrayBuffer) { const data: unknown = CBOR.decode(cbor, typedArrayTagger); - if (isRosbridgeMessage(data)) { + if (isBridgeProtoOp(data)) { this.handleRosbridgeMessage(data); } else { throw new Error("Decoded CBOR data was invalid!"); @@ -233,7 +229,7 @@ export abstract class AbstractTransport */ private handleJsonMessage(json: string) { const message: unknown = JSON.parse(json); - if (isRosbridgeMessage(message)) { + if (isBridgeProtoOp(message)) { this.handleRosbridgeMessage(message); } else { throw new Error("Received invalid rosbridge message!"); diff --git a/src/core/transport/WsWebSocketTransport.ts b/src/core/transport/WsWebSocketTransport.ts index dc853f114..bcbe96550 100644 --- a/src/core/transport/WsWebSocketTransport.ts +++ b/src/core/transport/WsWebSocketTransport.ts @@ -1,5 +1,5 @@ import * as ws from "ws"; -import type { RosbridgeMessage } from "../../types/protocol.js"; +import type { BridgeProtoOp } from "../../types/protocol.js"; import { AbstractTransport } from "./Transport.js"; /** @@ -16,7 +16,7 @@ export class WsWebSocketTransport extends AbstractTransport { this.registerEventListeners(); } - public send(message: RosbridgeMessage): void { + public send(message: BridgeProtoOp): void { this.socket.send(JSON.stringify(message)); } From a641f00ff97eb81c26f4117fe149add4bfb3007f Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 04:07:56 -0500 Subject: [PATCH 5/9] refactor(protocol): Add missing type constraints to bound messages to objects. Add helper types to increase readability in constructors and function parameters. Signed-off-by: Drew Hoener --- src/actionlib/ActionClient.ts | 8 ++--- src/actionlib/ActionListener.ts | 2 +- src/actionlib/Goal.ts | 6 ++-- src/core/Action.ts | 52 +++++++++++++++++++-------------- src/core/Ros.ts | 10 +++++-- src/core/Service.ts | 12 ++++++-- src/core/Topic.ts | 29 +++++++++--------- 7 files changed, 68 insertions(+), 51 deletions(-) diff --git a/src/actionlib/ActionClient.ts b/src/actionlib/ActionClient.ts index b3104d173..adc9c319f 100644 --- a/src/actionlib/ActionClient.ts +++ b/src/actionlib/ActionClient.ts @@ -20,13 +20,13 @@ import type Goal from "./Goal.js"; * */ export default class ActionClient< - TGoal = unknown, - TFeedback = unknown, - TResult = unknown, + TGoal = never, + TFeedback = never, + TResult = never, > extends EventEmitter<{ timeout: undefined; }> { - goals: Partial>> = {}; + goals: Partial>> = {}; /** flag to check if a status has been received */ receivedStatus = false; ros: Ros; diff --git a/src/actionlib/ActionListener.ts b/src/actionlib/ActionListener.ts index e3890484d..7d494986f 100644 --- a/src/actionlib/ActionListener.ts +++ b/src/actionlib/ActionListener.ts @@ -20,7 +20,7 @@ import type { actionlib_msgs } from "../types/actionlib_msgs.js"; * */ export default class ActionListener< - TGoal, + TGoal extends object, TFeedback, TResult, > extends EventEmitter<{ diff --git a/src/actionlib/Goal.ts b/src/actionlib/Goal.ts index d4c1f6f89..f4c37d117 100644 --- a/src/actionlib/Goal.ts +++ b/src/actionlib/Goal.ts @@ -15,9 +15,9 @@ import { v4 as uuidv4 } from "uuid"; * * 'timeout' - If a timeout occurred while sending a goal. */ export default class Goal< - TGoal, - TFeedback = unknown, - TResult = unknown, + TGoal = never, + TFeedback = never, + TResult = never, > extends EventEmitter<{ timeout: undefined; status: [actionlib_msgs.GoalStatus]; diff --git a/src/core/Action.ts b/src/core/Action.ts index e3419ac98..79936fcbf 100644 --- a/src/core/Action.ts +++ b/src/core/Action.ts @@ -16,35 +16,43 @@ import type { ActionResultFailedOp, } from "../types/protocol.js"; +type ActionCallback = ( + goal: TGoal, + id: string | undefined, +) => void; +type ActionCancelCallback = (id: string) => void; + +interface ActionOptions { + /** + * The ROSLIB.Ros connection handle. + */ + ros: Ros; + /** + * The action name, like '/fibonacci'. + */ + name: string; + /** + * The action type, like 'example_interfaces/Fibonacci'. + */ + actionType: string; +} + /** * A ROS 2 action client. */ export default class Action< - TGoal = unknown, - TFeedback = unknown, - TResult = unknown, + TGoal extends object = object, + TFeedback extends object = object, + TResult extends object = object, > { isAdvertised = false; - #actionCallback: ((goal: TGoal, id: string) => void) | null = null; - #cancelCallback: ((id: string) => void) | null = null; + #actionCallback: ActionCallback | null = null; + #cancelCallback: ActionCancelCallback | null = null; ros: Ros; name: string; actionType: string; - /** - * @param options - * @param options.ros - The ROSLIB.Ros connection handle. - * @param options.name - The action name, like '/fibonacci'. - * @param options.actionType - The action type, like 'example_interfaces/Fibonacci'. - */ - constructor({ - ros, - name, - actionType, - }: { - ros: Ros; - name: string; - actionType: string; - }) { + + constructor({ ros, name, actionType }: ActionOptions) { this.ros = ros; this.name = name; this.actionType = actionType; @@ -122,8 +130,8 @@ export default class Action< * @param cancelCallback - A callback function to execute when the action is canceled. */ advertise( - actionCallback: (goal: TGoal, id: string) => void, - cancelCallback: (id: string) => void, + actionCallback: ActionCallback, + cancelCallback: ActionCancelCallback, ) { if (this.isAdvertised || typeof actionCallback !== "function") { return; diff --git a/src/core/Ros.ts b/src/core/Ros.ts index e2e580138..5c3ba90bc 100644 --- a/src/core/Ros.ts +++ b/src/core/Ros.ts @@ -724,7 +724,7 @@ export default class Ros extends EventEmitter { ); } - public Topic( + public Topic( options: Omit>[0], "ros">, ) { return new Topic({ ros: this, ...options }); @@ -736,7 +736,7 @@ export default class Ros extends EventEmitter { return new Param({ ros: this, ...options }); } - public Service( + public Service( options: Omit< ConstructorParameters>[0], "ros" @@ -751,7 +751,11 @@ export default class Ros extends EventEmitter { return new TFClient({ ros: this, ...options }); } - public ActionClient( + public ActionClient< + TGoal extends object, + TFeedback extends object, + TResult extends object, + >( options: Omit< ConstructorParameters>[0], "ros" diff --git a/src/core/Service.ts b/src/core/Service.ts index f9d04e6eb..9a4177253 100644 --- a/src/core/Service.ts +++ b/src/core/Service.ts @@ -14,16 +14,22 @@ import { } from "../types/protocol.js"; import type { ServiceCallIdString } from "../types/emitted_events.js"; +type ServiceMessageHandler< + TRequest extends object, + TResponse extends object, +> = (request: AnyServiceOp) => void; /** * A ROS service client. */ -export default class Service extends EventEmitter { +export default class Service< + TRequest extends object, + TResponse extends object, +> extends EventEmitter { /** * Stores a reference to the most recent service callback advertised so it can be removed from the EventEmitter during un-advertisement */ - #serviceCallback: ((rosbridgeRequest: RosbridgeMessage) => void) | null = - null; + #serviceCallback: ServiceMessageHandler | null = null; isAdvertised = false; /** * Queue for serializing advertise/unadvertise operations to prevent race conditions diff --git a/src/core/Topic.ts b/src/core/Topic.ts index 12867f132..fec6bf0b9 100644 --- a/src/core/Topic.ts +++ b/src/core/Topic.ts @@ -17,6 +17,17 @@ import { type PublishOp, } from "../types/protocol.js"; +interface TopicOptions { + ros: Ros; + name: string; + messageType: string; + compression?: BridgeCompressionType; + throttle_rate?: number; + queue_size?: number; + latch?: boolean; + queue_length?: number; + reconnect_on_close?: boolean; +} /** * Publish and/or subscribe to a topic in ROS. @@ -25,7 +36,7 @@ import { * * 'warning' - If there are any warning during the Topic creation. * * 'message' - The message data from rosbridge. */ -export default class Topic extends EventEmitter<{ +export default class Topic extends EventEmitter<{ message: [T]; warning: [string]; unsubscribe: undefined; @@ -43,9 +54,7 @@ export default class Topic extends EventEmitter<{ queue_size: number; queue_length: number; reconnect_on_close: boolean; - callForSubscribeAndAdvertise: ( - message: RosbridgeSubscribeMessage | RosbridgeAdvertiseMessage, - ) => void; + callForSubscribeAndAdvertise: (message: SubscribeOp | AdvertiseOp) => void; subscribeId: string | null = null; advertiseId?: string; /** @@ -70,17 +79,7 @@ export default class Topic extends EventEmitter<{ queue_size = 100, queue_length = 0, reconnect_on_close = true, - }: { - ros: Ros; - name: string; - messageType: string; - compression?: string; - throttle_rate?: number; - queue_size?: number; - latch?: boolean; - queue_length?: number; - reconnect_on_close?: boolean; - }) { + }: TopicOptions) { super(); this.ros = ros; From 8b5c4fb3f7bfda8bb834252212ea2ad0061ad442 Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 04:08:47 -0500 Subject: [PATCH 6/9] fix(action): Fix `executeAction` not being called when it probably should have. Fixes #1068 Signed-off-by: Drew Hoener --- src/core/Action.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/Action.ts b/src/core/Action.ts index 79936fcbf..1c2d33b7b 100644 --- a/src/core/Action.ts +++ b/src/core/Action.ts @@ -139,14 +139,14 @@ export default class Action< this.#actionCallback = actionCallback; this.#cancelCallback = cancelCallback; - this.ros.on(this.name, (msg) => { - if (isRosbridgeSendActionGoalMessage(msg)) { - this.#executeAction.bind(this); - } else { + this.ros.on(this.name, (msg: AnyActionOp) => { + if (msg.op !== "send_action_goal") { throw new Error( "Received unrelated message on Action server event stream!", ); } + + this.#executeAction(msg); }); this.ros.callOnConnection({ op: "advertise_action", From e4c8e90baa50a017373c2c78d3b567198ada0836 Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 04:09:14 -0500 Subject: [PATCH 7/9] refactor(TFClient): Fix suspicious typing of `false` when an undefined check works Signed-off-by: Drew Hoener --- src/tf/TFClient.ts | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/tf/TFClient.ts b/src/tf/TFClient.ts index 8ce6c06c1..1c6adef2d 100644 --- a/src/tf/TFClient.ts +++ b/src/tf/TFClient.ts @@ -16,13 +16,11 @@ import BaseTFClient from "./BaseTFClient.js"; * A TF Client that listens to TFs from tf2_web_republisher. */ export default class TFClient extends BaseTFClient { - currentGoal: - | Goal< - tf2_web_republisher.TFSubscriptionGoal, - tf2_web_republisher.TFSubscriptionFeedback - > - | false = false; - currentTopic: Topic | false = false; + currentGoal?: Goal< + tf2_web_republisher.TFSubscriptionGoal, + tf2_web_republisher.TFSubscriptionFeedback + >; + currentTopic?: Topic; actionClient: ActionClient< tf2_web_republisher.TFSubscriptionGoal, tf2_web_republisher.TFSubscriptionFeedback From 0073117cb7a2de36e9b83d526d3361541bb6bc4c Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 04:10:19 -0500 Subject: [PATCH 8/9] refactor(Service): Refactor Service advertise methods to decrease Cyclomatic Complexity and increase readability. Signed-off-by: Drew Hoener --- src/core/Service.ts | 169 +++++++++++++++++++++----------------------- 1 file changed, 80 insertions(+), 89 deletions(-) diff --git a/src/core/Service.ts b/src/core/Service.ts index 9a4177253..4fe278780 100644 --- a/src/core/Service.ts +++ b/src/core/Service.ts @@ -110,57 +110,20 @@ export default class Service< this.ros.callOnConnection(call); } + /** - * Advertise the service. This turns the Service object from a client - * into a server. The callback will be called with every request - * that's made on this service. - * - * @param callback This works similarly to the callback for a C++ service in that you should take care not to overwrite the response object. - * Instead, only modify the values within. + * Common logic for advertising a service */ - async advertise( - callback: (request: TRequest, response: Partial) => boolean, + #advertiseWithCallback( + callbackWrapper: (bridgeMessage: AnyServiceOp) => void, ): Promise { - // Queue this operation to prevent race conditions this.#operationQueue = this.#operationQueue .then(() => { - // If already advertised, unadvertise first if (this.isAdvertised) { this.#doUnadvertise(); } - // Store the new callback for removal during un-advertisement - this.#serviceCallback = (rosbridgeRequest) => { - if (!isRosbridgeCallServiceMessage(rosbridgeRequest)) { - throw new Error( - `Invalid message received on service channel: ${JSON.stringify(rosbridgeRequest)}`, - ); - } - const response = {}; - let success: boolean; - try { - success = callback(rosbridgeRequest.args, response); - } catch { - success = false; - } - - if (success) { - this.ros.callOnConnection({ - op: "service_response", - service: this.name, - values: response, - result: success, - id: rosbridgeRequest.id, - } satisfies RosbridgeServiceResponseMessage>); - } else { - this.ros.callOnConnection({ - op: "service_response", - service: this.name, - result: success, - id: rosbridgeRequest.id, - } satisfies RosbridgeServiceResponseMessage>); - } - }; + this.#serviceCallback = callbackWrapper; this.ros.on(this.name, this.#serviceCallback); this.ros.callOnConnection({ @@ -178,6 +141,53 @@ export default class Service< return this.#operationQueue; } + /** + * Advertise the service. This turns the Service object from a client + * into a server. The callback will be called with every request + * that's made on this service. + * + * @param callback This works similarly to the callback for a C++ service in that you should take care not to overwrite the response object. + * Instead, only modify the values within. + */ + advertise( + callback: (request: TRequest, response: Partial) => boolean, + ): Promise { + return this.#advertiseWithCallback((bridgeMessage) => { + if (bridgeMessage.op !== "call_service") { + throw new Error( + `Invalid message received on service channel: ${JSON.stringify(bridgeMessage)}`, + ); + } + + const request = bridgeMessage as IncomingCallServiceOp; + const response: Partial = {}; + + let success: boolean; + try { + success = callback(request.args, response); + } catch { + success = false; + } + + if (success) { + this.ros.callOnConnection({ + op: "service_response", + service: this.name, + values: response, + result: success, + id: request.id, + }); + } else { + this.ros.callOnConnection({ + op: "service_response", + service: this.name, + result: success, + id: request.id, + }); + } + }); + } + /** * Internal method to perform unadvertisement without queueing */ @@ -233,56 +243,37 @@ export default class Service< * An alternate form of Service advertisement that supports a modern Promise-based interface for use with async/await. * @param callback An asynchronous callback processing the request and returning a response. */ - async advertiseAsync( + advertiseAsync( callback: (request: TRequest) => Promise, ): Promise { - // Queue this operation to prevent race conditions - this.#operationQueue = this.#operationQueue - .then(() => { - // If already advertised, unadvertise first - if (this.isAdvertised) { - this.#doUnadvertise(); - } + return this.#advertiseWithCallback((bridgeMessage) => { + if (bridgeMessage.op !== "call_service") { + throw new Error( + `Invalid message received on service channel: ${JSON.stringify(bridgeMessage)}`, + ); + } - this.#serviceCallback = (rosbridgeRequest) => { - if (!isRosbridgeCallServiceMessage(rosbridgeRequest)) { - throw new Error( - `Invalid message received on service channel: ${JSON.stringify(rosbridgeRequest)}`, - ); - } - (async () => { - try { - this.ros.callOnConnection({ - op: "service_response", - service: this.name, - result: true, - values: await callback(rosbridgeRequest.args), - id: rosbridgeRequest.id, - } satisfies RosbridgeServiceResponseMessage); - } catch (err) { - this.ros.callOnConnection({ - op: "service_response", - service: this.name, - result: false, - values: String(err), - id: rosbridgeRequest.id, - } satisfies RosbridgeServiceResponseMessage); - } - })().catch(console.error); - }; - this.ros.on(this.name, this.#serviceCallback); - this.ros.callOnConnection({ - op: "advertise_service", - type: this.serviceType, - service: this.name, - }); - this.isAdvertised = true; - }) - .catch((err: unknown) => { - this.emit("error", err); - throw err; - }); + const request = bridgeMessage as IncomingCallServiceOp; - return this.#operationQueue; + (async () => { + try { + this.ros.callOnConnection({ + op: "service_response", + service: this.name, + result: true, + values: await callback(request.args), + id: request.id, + }); + } catch (err) { + this.ros.callOnConnection({ + op: "service_response", + service: this.name, + result: false, + values: String(err), + id: request.id, + }); + } + })().catch(console.error); + }); } } From 3caa750d76cf50c57e202ce63226722a7906c5a4 Mon Sep 17 00:00:00 2001 From: Drew Hoener Date: Fri, 14 Nov 2025 04:10:40 -0500 Subject: [PATCH 9/9] refactor(tests): Update tests to use proper typing, coerce where necessary. Signed-off-by: Drew Hoener --- test/examples/fibonacci.example.ts | 13 ++++++++++++- test/examples/topic-listener.example.ts | 8 ++++++-- test/ros.test.ts | 18 ++++++++++++------ test/service.test.ts | 20 ++++---------------- test/transport.test.ts | 25 +++++++++++-------------- 5 files changed, 45 insertions(+), 39 deletions(-) diff --git a/test/examples/fibonacci.example.ts b/test/examples/fibonacci.example.ts index c07514d5e..d4ea7562c 100644 --- a/test/examples/fibonacci.example.ts +++ b/test/examples/fibonacci.example.ts @@ -1,6 +1,13 @@ import { describe, it, expect } from "vitest"; import * as ROSLIB from "../../src/RosLib.js"; +interface FibonacciGoal { + order: number; +} +interface FibonacciResult { + sequence: number[]; +} + // Noetic is the only version of ROS 1 we support, so we skip based on distro name // instead of adding extra plumbing for ROS_VERSION. describe.skipIf(process.env["ROS_DISTRO"] !== "noetic")( @@ -18,7 +25,11 @@ describe.skipIf(process.env["ROS_DISTRO"] !== "noetic")( * ---------------- */ - const fibonacciClient = new ROSLIB.ActionClient({ + const fibonacciClient = new ROSLIB.ActionClient< + FibonacciGoal, + FibonacciResult, + FibonacciResult + >({ ros: ros, serverName: "/fibonacci", actionName: "actionlib_tutorials/FibonacciAction", diff --git a/test/examples/topic-listener.example.ts b/test/examples/topic-listener.example.ts index 7defe06e6..dfa261729 100644 --- a/test/examples/topic-listener.example.ts +++ b/test/examples/topic-listener.example.ts @@ -12,7 +12,7 @@ const messages = ["1", "2", "3", "4"].map(format); describe("Topics Example", function () { function createAndStreamTopic(topicName: string) { - const topic = ros.Topic({ + const topic = ros.Topic<{ data: string }>({ name: topicName, messageType: "std_msgs/String", }); @@ -20,7 +20,11 @@ describe("Topics Example", function () { function emit() { setTimeout(function () { - topic.publish(messages[idx++]); + const msg = messages[idx++]; + if (!msg) { + return; + } + topic.publish(msg); if (idx < messages.length) { emit(); } else { diff --git a/test/ros.test.ts b/test/ros.test.ts index 45e36e68b..bf5bad7af 100644 --- a/test/ros.test.ts +++ b/test/ros.test.ts @@ -8,7 +8,7 @@ import type { } from "../src/core/transport/Transport.js"; import { WebSocketTransportFactory } from "../src/core/transport/WebSocketTransportFactory.js"; import Ros from "../src/core/Ros.js"; -import type { RosbridgeMessage } from "../src/types/protocol.js"; +import type { BridgeProtoOp } from "../src/types/protocol.js"; describe("Ros", function () { let mockRosUrl: string; @@ -19,7 +19,7 @@ describe("Ros", function () { open: ((event: TransportEvent) => void)[]; close: ((event: TransportEvent) => void)[]; error: ((event: TransportEvent) => void)[]; - message: ((event: RosbridgeMessage) => void)[]; + message: ((event: BridgeProtoOp) => void)[]; }; const publishMockTransportEvent = (event: string, value: unknown) => { @@ -51,7 +51,7 @@ describe("Ros", function () { break; case "message": mockTransportListeners.message.forEach((listener) => { - listener(value as RosbridgeMessage); + listener(value as BridgeProtoOp); }); break; } @@ -64,7 +64,7 @@ describe("Ros", function () { open: new Array<(event: TransportEvent) => void>(), close: new Array<(event: TransportEvent) => void>(), error: new Array<(event: TransportEvent) => void>(), - message: new Array<(event: RosbridgeMessage) => void>(), + message: new Array<(event: BridgeProtoOp) => void>(), }; mockTransport = { @@ -229,7 +229,10 @@ describe("Ros", function () { publishMockTransportEvent("open", new Event("open")); const rosOnceSpy = vi.spyOn(ros, "once"); - ros.callOnConnection({ op: "test" }); + + // Coerce this message to be a valid bridge operation (it's not) + // Needs to be cast because type checking will catch it otherwise + ros.callOnConnection({ op: "test" } as unknown as BridgeProtoOp); expect(rosOnceSpy).not.toHaveBeenCalled(); expect(mockTransport.send).toHaveBeenCalledWith({ op: "test" }); @@ -242,7 +245,10 @@ describe("Ros", function () { // When disconnected, the message is queued to send const rosOnceSpy = vi.spyOn(ros, "once"); - ros.callOnConnection({ op: "test" }); + + // Coerce this message to be a valid bridge operation (it's not) + // Needs to be cast because type checking will catch it otherwise + ros.callOnConnection({ op: "test" } as unknown as BridgeProtoOp); expect(rosOnceSpy).toHaveBeenCalledWith("open", expect.any(Function)); expect(mockTransport.send).not.toHaveBeenCalled(); diff --git a/test/service.test.ts b/test/service.test.ts index 48bc98ba8..3d1e05657 100644 --- a/test/service.test.ts +++ b/test/service.test.ts @@ -37,10 +37,7 @@ describe("Service", () => { expect(ros.listenerCount(server.name)).toEqual(0); }); it("Successfully advertises a service with a synchronous return", async () => { - const server = new Service< - undefined, - { success: boolean; message: string } - >({ + const server = new Service({ ros, serviceType: "std_srvs/Trigger", name: "/test_service", @@ -71,10 +68,7 @@ describe("Service", () => { }); it("Handles re-advertisement gracefully without throwing errors", async () => { - const server = new Service< - undefined, - { success: boolean; message: string } - >({ + const server = new Service({ ros, serviceType: "std_srvs/Trigger", name: "/test_readvertise", @@ -106,10 +100,7 @@ describe("Service", () => { }); it("Handles multiple unadvertise calls gracefully", async () => { - const server = new Service< - undefined, - { success: boolean; message: string } - >({ + const server = new Service({ ros, serviceType: "std_srvs/Trigger", name: "/test_multiple_unadvertise", @@ -164,10 +155,7 @@ describe("Service", () => { }); it("Ensures operations are serialized through queue", async () => { - const server = new Service< - undefined, - { success: boolean; message: string } - >({ + const server = new Service({ ros, serviceType: "std_srvs/Trigger", name: "/test_queue", diff --git a/test/transport.test.ts b/test/transport.test.ts index cc73fdb29..9249d5d45 100644 --- a/test/transport.test.ts +++ b/test/transport.test.ts @@ -4,16 +4,13 @@ import type { MockedObject } from "vitest"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { AbstractTransport } from "../src/core/transport/Transport.js"; import { WebSocketTransportFactory } from "../src/core/transport/WebSocketTransportFactory.js"; -import type { - RosbridgeMessage, - RosbridgePngMessage, -} from "../src/types/protocol.js"; import CBOR from "cbor-js"; import * as fastpng from "fast-png"; import * as bson from "bson"; import * as ws from "ws"; import { NativeWebSocketTransport } from "../src/core/transport/NativeWebSocketTransport.js"; import { WsWebSocketTransport } from "../src/core/transport/WsWebSocketTransport.js"; +import type { BridgeProtoOp } from "../src/types/protocol.js"; vi.mock("fast-png"); @@ -55,9 +52,9 @@ describe("Transport", () => { }); it("should handle RosbridgeMessage", () => { - const message: RosbridgeMessage = { + const message = { op: "test", - }; + } as unknown as BridgeProtoOp; const messageEvent: Partial = { type: "message", @@ -202,12 +199,12 @@ describe("Transport", () => { // Obviously these are not real PNG encoded messages. // But they're good enough for mocking responses in our tests. - const successMessage: RosbridgePngMessage = { + const successMessage = { op: "png", data: Buffer.from("success").toString("base64"), }; - const failureMessage: RosbridgePngMessage = { + const failureMessage = { op: "png", data: Buffer.from("failure").toString("base64"), }; @@ -370,7 +367,7 @@ describe("Transport", () => { it("should send messages as JSON", () => { const transport = new NativeWebSocketTransport(mockSocket); - transport.send({ op: "test" }); + transport.send({ op: "test" } as unknown as BridgeProtoOp); expect(mockSocket.send).toHaveBeenCalledWith( JSON.stringify({ op: "test" }), @@ -482,9 +479,9 @@ describe("Transport", () => { transport.on("message", messageListener); - const message: RosbridgeMessage = { + const message = { op: "test", - }; + } as unknown as BridgeProtoOp; const messageEvent: Partial = { type: "message", @@ -515,7 +512,7 @@ describe("Transport", () => { it("should send messages as JSON", () => { const transport = new WsWebSocketTransport(mockSocket); - transport.send({ op: "test" }); + transport.send({ op: "test" } as unknown as BridgeProtoOp); expect(mockSocket.send).toHaveBeenCalledWith( JSON.stringify({ op: "test" }), @@ -627,9 +624,9 @@ describe("Transport", () => { transport.on("message", messageListener); - const message: RosbridgeMessage = { + const message = { op: "test", - }; + } as unknown as BridgeProtoOp; const messageEvent: ws.MessageEvent = { type: "message",