No consistent type checking for string before publish

[qiuzhong]

Suppose we create a publish with std_msg/msg/String message.

var node = rclnodejs.createNode('any_node_name');
var publisher = node.createPublisher('std_msgs/msg/String', 'chatter');

// This doesn't throw a TypeError!
publisher.publish({a: 1});

Seems publisher with string type can work with object type.
But if I publish another inconsistent primitive type like publisher.publish(255); then a TypeError can be thrown:

TypeError: writeUInt64: Number/String 64-bit value required

[qiuzhong]

@kenny-y , @minggangw , I was doing some fuzzing API testing with random parameters. And I was using the inconsistent message type on purpose.

[qiuzhong]

Not limited to std_msgs/msg/String, seems any standard message types can accept object as the publishing data.

[kenny-y]

Yes, .publish() should throw an error when the argument is ill-formed.

I remember this was planned but delayed... I had talked to @minggangw on this topic when I was doing plain message translation (#189), it was tracked in #190

[qiuzhong]

Well, publisher.publish() can accept JavaScript primitive types or object. The issue is not whether it can accept object while the message is known, but any object data no matter if it's legal or not can be published.

[kenny-y]

From my understanding we want to do this:

For 'std_msgs/std/String', we want to accept {data: 'xxx'} OR 'xxx' as an argument but reject {what: 129}
To generalize that we can conclude that the unexpected object structure/value-type should be rejected.

Anything missing here?

[qiuzhong]

For 'std_msgs/std/String', we want to accept {data: 'xxx'} OR 'xxx' as an argument but reject {what: 129}

I agree with you.

And the rule should be applied to all standard message types.

[minggangw]

dup with #243