Skip to content

Inconsistency between protocol doc and code #1012

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

Open
ahoenerBE opened this issue Feb 20, 2025 · 4 comments
Open

Inconsistency between protocol doc and code #1012

ahoenerBE opened this issue Feb 20, 2025 · 4 comments
Labels

Comments

@ahoenerBE
Copy link

ahoenerBE commented Feb 20, 2025

Description
I'm currently working on a PR to RosLibJS and noticed some inconsistency between the protocol doc and the actual code.

e.g. In Advertising, rosbridge / roslib add fields latch and queue_size to the packet and are pulling them from the message

# rosbridge_suite/rosbridge_library/src/rosbridge_library/capabilities/advertise.py
def advertise(self, message):
        # Pull out the ID
        aid = message.get("id", None)

        self.basic_type_check(message, self.advertise_msg_fields)
        topic = message["topic"]
        msg_type = message["type"]
        latch = message.get("latch", False)
        queue_size = message.get("queue_size", 100)
        # snip...

But the protocol document doesn't include these fields. Latch is mentioned briefly in its own section, and the phrase queue_size doesn't appear in the entire document.

#### 3.3.1 Advertise ( _advertise_ )

If you wish to advertise that you are or will be publishing a topic, then use the advertise command.


{ "op": "advertise",
  (optional) "id": <string>,
  "topic": <string>,
  "type": <string>
}


 * **topic** – the string name of the topic to advertise
 * **type** – the string type to advertise for the topic

   * 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.

It seems like publishers/advertisement use queue_size and subscriptions use queue_length?

Is this intentional? Are these fields no longer supported? Looking for some answers to guide my development.

@ahoenerBE ahoenerBE added the bug label Feb 20, 2025
@ahoenerBE
Copy link
Author

@bjsowa @sea-bass sorry to bother with the ping, but I'd like to try and knock some of this out before the weekend if you have any idea if these extra arguments that roslibjs is passing along are still supported.
Is it some sort of ROS1 compat?

@bjsowa
Copy link
Collaborator

bjsowa commented Feb 24, 2025

Sorry for a late reply. The queue_size and latch arguments for publishers are supported. In ROS1, these arguments are just passed to Publishers. In ROS2, they are translated to QoS policies (the names are left for backward compatibility):

  • All publishers use trancient local durability and a lifespan of 1 second by default.
  • queue_size is directly translated to QoS Depth policy.
  • If latch is set, the publisher will use infinite lifespan and depth value of 1 (regardless of the queue_size value).

The queue_length argument for subscribers works a little bit different. When set, it creates an internal queue of messages to set via the protocol. It doesn't affect any parameters passed to the RMW layer.

@ahoenerBE
Copy link
Author

Hey @bjsowa Thanks for the response.
I've gotten a good deal of work done if you want to check out my draft to RosLibJS.
Still has a bit of work to do but I'm getting there. I've ended up doing a lot of TS coercing in one of my libs that implements roslibjs, since there's not strong typing for things like message content so this would be a big help to get accepted.

A couple more questions:

  • call_service lists args as optional, but rosbridge seems to always send them when the direction is ROS -> Bridge -> Client. Is this true / guaranteed?
  • Similarly, service_response indicates that the rosbridge server will omit values, but in the ServiceCaller class, it always seems to return something, even if just an empty object. Is there some code I've missed where it turns to None if empty?

@bjsowa
Copy link
Collaborator

bjsowa commented Feb 27, 2025

Hey @bjsowa Thanks for the response. I've gotten a good deal of work done if you want to check out my draft to RosLibJS. Still has a bit of work to do but I'm getting there.

Impressive work! I think roslibjs would really benefit from a proper typescript support.

  • call_service lists args as optional, but rosbridge seems to always send them when the direction is ROS -> Bridge -> Client. Is this true / guaranteed?

Looking at the implementation it seems true but the type is not completely correct. The Server allows passing the args in 2 ways:

  1. As a JSON array literal, in which case the elements will be used to populate the fields of the request in the order they are defined.
  2. As a JSON object literal, in which case the fields will be populated based in the matching keys in the JSON object.

As an example, assuming the request type is example_interfaces/srv/AddTwoInts, passing [2, 3] or {a: 2, b: 3} will both be parsed to AddTwoInts(a=2, b=3)

When Server sends call_service messages to the Client, it always passes args as a JSON object literal.

  • Similarly, service_response indicates that the rosbridge server will omit values, but in the ServiceCaller class, it always seems to return something, even if just an empty object. Is there some code I've missed where it turns to None if empty?

It seems to always return something but the type in the protocol doc is wrong. It returns a JSON object literal with response message fields if the service call succeeds (result set to true) or a string with the error message if it fails.

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

2 participants