RobotWebTools / rclnodejs

Node.js version of ROS 2.0 client
https://docs.ros.org/en/humble/Concepts/Basic/About-Client-Libraries.html?highlight=rclnodejs#community-maintained
Apache License 2.0
311 stars 70 forks source link

Support TypeAdapters for Publisher and Subscription #846

Open wayneparrott opened 2 years ago

wayneparrott commented 2 years ago

Proposal: Supporting type adaptation for Publishers and Subscriptions similar to the rclcpp TypeAdapter that will be included in the Humble release.

From the rclcpp relnotes:

After defining a type adapter, custom data structures can be used directly by publishers and subscribers, which helps to avoid additional work for the programmer and potential sources of errors. This is especially useful when working with complex data types, such as when converting OpenCV’s cv::Mat to ROS’s sensor_msgs/msg/Image type.

I've implementing this convenience on a couple of projects that required converting messages to/from an application objects such as https://github.com/ros2jsguy/three.math/tree/master/src.

Following is a discussion for a couple of approaches that we might consider for supporting type adaption. I'm limiting the discussion to Publishers as the concepts can be applied to other entities such as Subscriptions, Services, Clients, Actions.

Type adaptation for Ros Messages:

interface MessageTypeAdapter<T extends Message, AppClass> {
    convertToRos(customObj: AppClass): Message;
    convertToCustom(rosObj: Message): AppClass;
}

Option-1: extend Node#createPublisher() with an additional optional TypeAdapter parameter

Node#createPublisher<T extends TypeClass<MessageTypeClassName>, AppClass=object>(
      typeClass: T,
      topic: string,
      options?: Options,
      typeAdapter?: MessageTypeAdapter<T,AppClass >
 ): Publisher<T,AppClass>;

interface Publisher <T extends TypeClass<MessageTypeClassName>, AppClass=object> extends Entity {

    readonly topic: string;

    publish(message: MessageType<T> | AppClass | Buffer): void;
}

Option1a: Incorporate the typeAdapter into the optional Options parameter that is passed to Node#createPublisher()

In theory I like this idea but it seems like a TypeScript generic signature for Options could grow into something really awkward in the future if the adapter pattern is extended to services, clients and actions. So I'm only mentioning it for completeness.

Option-2: Only implement TypeAdapter and invoke it manually

This simpler approach requires manually invocation of the typeAdapter before calling publish(), e.g.,

publish(adapter.convertToRos(myObj))

No modification is required to Publisher for the TypeAdapter.

Wrapping up: this feature request falls in the nice-to-have category for me. I wanted to post this for additional comment and interest level.