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

constants support in messages #550

Closed
koonpeng opened this issue Jan 18, 2020 · 9 comments
Closed

constants support in messages #550

koonpeng opened this issue Jan 18, 2020 · 9 comments

Comments

@koonpeng
Copy link
Collaborator

koonpeng commented Jan 18, 2020

is there a way to reference the constant value like in rclpy? Something like

const marker = {...
  type: ImageMarker.CIRCLE,
...}

I can see that constants are generated in the js messages like so

// Define constants (7 in total)
Object.defineProperty(ImageMarkerWrapper, "CIRCLE", {value: 0, writable: false, enumerable: true, configurable: true});
Object.defineProperty(ImageMarkerWrapper, "LINE_STRIP", {value: 1, writable: false, enumerable: true, configurable: true});
Object.defineProperty(ImageMarkerWrapper, "LINE_LIST", {value: 2, writable: false, enumerable: true, configurable: true});
Object.defineProperty(ImageMarkerWrapper, "POLYGON", {value: 3, writable: false, enumerable: true, configurable: true});
Object.defineProperty(ImageMarkerWrapper, "POINTS", {value: 4, writable: false, enumerable: true, configurable: true});
Object.defineProperty(ImageMarkerWrapper, "ADD", {value: 0, writable: false, enumerable: true, configurable: true});
Object.defineProperty(ImageMarkerWrapper, "REMOVE", {value: 1, writable: false, enumerable: true, configurable: true});

But it needs to be imported like so

const ImageMarker = require('rclnodejs/generated/visualization_msgs/visualization_msgs__msg__ImageMarker');

which doesn't seem like the intended usage.

Also, the typescript interfaces are generated like this

      export type ImageMarker = {
        CIRCLE: 0,
        LINE_STRIP: 1,
        LINE_LIST: 2,
        POLYGON: 3,
        POINTS: 4,
        ADD: 0,
        REMOVE: 1,
        header: std_msgs.msg.Header,
        ns: string,
        id: number,
        type: number,
        action: number,
        position: geometry_msgs.msg.Point,
        scale: number,
        outline_color: std_msgs.msg.ColorRGBA,
        filled: undefined,
        fill_color: std_msgs.msg.ColorRGBA,
        lifetime: builtin_interfaces.msg.Duration,
        points: geometry_msgs.msg.Point[],
        outline_colors: std_msgs.msg.ColorRGBA[]
      };

The constants have to be defined in objects that implements the type.

const marler: ImageMarker = {
  // All the constants have to be defined
  CIRCLE: 0,
  LINE_STRIP: 1,
...
};

Is there an easier way to reference the constants in rclnodejs?

@minggangw
Copy link
Member

Thanks for your question, I checked the usage of const variable defined in .msg files, and I think the code probably looks like:

const ImageMarker = rclnodejs.require('visualization_msgs/msg/ImageMarker');
const LINE_LIST = ImageMarker.LINE_LIST;  // you get 2 here.

Not sure it's what you want or you could suggest a better usage, thanks!

@minggangw
Copy link
Member

Maybe I should introduce a little bit of the message objects used in rclnodejs to make it clear:

  • You can use so called plain objects to pass a message to publisher/client, e.g.
 const node = rclnodejs.createNode('publisher_message_example_node');
 const publisher = node.createPublisher('sensor_msgs/msg/JointState', 'JointState');
 publisher.publish({
      header: {
        stamp: {
          sec: 123456,
          nanosec: 789,
        },
        frame_id: 'main frame',
      },
      name: ['Tom', 'Jerry'],
      position: [1, 2],
      velocity: [2, 3],
      effort: [4, 5, 6],
    });

see https://github.com/RobotWebTools/rclnodejs/blob/develop/example/publisher-message-example.js

  • You can also require a JS object which is defined by the generated classes (usually under /generated), e.g.
let JointState = rclnodejs.require('sensor_msgs/msg/JointState');  // rclnodejs.require('sensor_msgs').msg.JointState;
let state = new JointState();
state.position = [1, 2];
state.velocity = [2, 3];
state.effort = [4, 5, 6];

Currently, most the usage is shown by the first way, because it's easier and more obvious and you can use console.log(state) to get the detailed values of it directly.

@koonpeng
Copy link
Collaborator Author

koonpeng commented Jan 19, 2020

Thanks! I wasn't aware of the require function. For the typescript types, would it be good to make the constants optionals? So there is no need to define them in an object, I am also looking at the possibility of generating the require type declaration so it returns a strongly typed interface instead of a generic object.

@minggangw
Copy link
Member

@wayneparrott added the TS support recently, I know he is busy implementing another important feature of rclnodejs, so Wayne would you please have a look if you have time? or @koonpeng you could help to submit a PR based on your requirement, thanks a lot!

@koonpeng
Copy link
Collaborator Author

I have been using a tool I made to generate typescript interfaces, now that rclnodejs has support for typescript, it would be better to use the native support. I will try to see if I can make a PR for this, thanks!

@wayneparrott
Copy link
Collaborator

wayneparrott commented Jan 21, 2020

@koonpeng thanks for identifying my goof up on the type-alias msg constants. Definitely not what I envisioned. After reviewing, I'm not sure what I was thinking at the time I impl'ed this. I was experimented with a couple of approaches. Please share suggestions/PR for improvement. In the meantime I plan to carve out some time to look into resolving this asap as well.

@wayneparrott
Copy link
Collaborator

wayneparrott commented Jan 21, 2020

A quick thought is to represent the IDL constants for a type_class/msg in interfaces.d.ts as an object literal similar to the snippet below. In this case a dev would need to know that in addition to importing ImageMarker he would also need to import ImageMarkerConstants.

export const ImageMarkerConstants = {
        CIRCLE: 0,
        LINE_STRIP: 1,
        LINE_LIST: 2,
        POLYGON: 3,
        POINTS: 4,
        ADD: 0,
        REMOVE: 1
}
export type ImageMarker = {
        header: std_msgs.msg.Header,
        ns: string,
        id: number,
        type: number,
        action: number,
...

Thoughts?

@koonpeng
Copy link
Collaborator Author

normally this wouldnt work because ImageMarkerConstants doesn't exist in the javascript code. But I just found out there is a special case in 'const enum' where typescript will inline the value in the generated js code. I will see if I can add it to my PR.

@koonpeng
Copy link
Collaborator Author

#551 This PR improve typing information and generate constants as const enum

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

No branches or pull requests

3 participants