Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support TypeAdapters for Publisher and Subscription #846

Open
wayneparrott opened this issue Apr 26, 2022 · 0 comments
Open

Support TypeAdapters for Publisher and Subscription #846

wayneparrott opened this issue Apr 26, 2022 · 0 comments
Labels

Comments

@wayneparrott
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant