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

Zenoh-only examples communicating with ROS Nodes via the bridge #70

Open
JEnoch opened this issue Jan 29, 2024 · 8 comments · May be fixed by #80
Open

Zenoh-only examples communicating with ROS Nodes via the bridge #70

JEnoch opened this issue Jan 29, 2024 · 8 comments · May be fixed by #80
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@JEnoch
Copy link
Member

JEnoch commented Jan 29, 2024

Describe the feature

The goal is to provide examples of applications using the Zenoh APIs to communicate with ROS 2 Nodes via zenoh-bridge-ros2dds. At minimum, those examples shall include some which are compatible with the ROS 2 demos.

@JEnoch JEnoch added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 29, 2024
@JEnoch JEnoch self-assigned this Jan 29, 2024
@JEnoch JEnoch linked a pull request Feb 6, 2024 that will close this issue
28 tasks
@JEnoch
Copy link
Member Author

JEnoch commented Feb 6, 2024

Draft PR in progress in examples branch.

ping @fsteff !

@fsteff
Copy link

fsteff commented Feb 8, 2024

thanks @JEnoch !
Great, straightforward implementation. Somehow I expected that services and actions are mapped to Zenoh Queryables means more ROS-specific functionality built-in... I guess that simply confused my overcomplicated thinking brain.

@JEnoch
Copy link
Member Author

JEnoch commented Feb 8, 2024

Which built-in ROS-specific functionalities did you expect for services and actions ?

@fsteff
Copy link

fsteff commented Feb 8, 2024

Somehow I expected that mandatory stuff, e.g. goal_id is part of the query parameters / zenoh selector. But it makes much more sense to just put it into the value anyway.

@JEnoch
Copy link
Member Author

JEnoch commented Feb 8, 2024

The action messages are implemented as such (with goal_id in payload) in the ROS 2 RCL layer, not in RMW.
Meaning that whatever RMW is used, a ROS 2 Node expects to get the goal_id within the action messages.

Indeed the bridge could accept queries with goal_id as parameter, but it would have to reconstruct the payload to insert goal_id within before re-publication on DDS. That would make it use more resources (buffer copies, CPU, latency...)

Moreover, as we plan to support RMW_ZENOH (i.e. the ros2dds plugin being able to bridge between rmw_zenoh Nodes and rmw_cyclonedds_cpp Nodes), it has to support goal_id in payload of queries anyway.

@uupks
Copy link

uupks commented Apr 2, 2024

I implemented a zenoh-pico-example based on cyclonedds cdrstream, but now there are several problems:

  1. add_two_ints_client

client send reqeust with a = 123456(0x01_E240), b = 654321(0x09_FBF1)

while server received request with a = 530239482494992(0x01_E240_0000_0010), b = 2810287296086016(0x09_FBF1_0000_0000)

client received response with sum = 3340526778581008(0x0B_DE31_0000_0010), actual sum = 777777(0x0B_DE31)

Sending Query 'add_two_ints'... with a = 123456, b = 654321
0 0X1 0 0 0X10 0 0 0 0X40 0XE2 0X1 0 0 0 0 0 0XF1 0XFB 0X9 0 0 0 0 0 
>> Received ('add_two_ints': 3340526778581008)
[INFO] [1712021043.259895789] [add_two_ints_server]: Incoming request
a: 530239482494992 b: 2810287296086016
  1. listener
dds_is_get1: Assertion `is->m_index < is->m_size' failed.

I don't know if it's a problem with my use of cdrstream or a problem with bridge...

@JEnoch
Copy link
Member Author

JEnoch commented Apr 2, 2024

@uupks The issue is in your usage of cdrstream:

You should call dds_stream_write_sampleLE() instead.

Similarly, when deserializing the reply, you should check the 4 bytes CDR header to check if the encoding is little or big endian, and then call dds_stream_read_sampleLE() or dds_stream_read_sample() accordingly.

@uupks
Copy link

uupks commented Apr 2, 2024

I tested both dds_stream_write_sampleLE and dds_stream_write_sampleBE, but not working.

  • dds_stream_write_sampleLE
Sending Query 'add_two_ints'... with a = 123456, b = 654321
Send : 0 0X1 0 0 0X10 0 0 0 0X40 0XE2 0X1 0 0 0 0 0 0XF1 0XFB 0X9 0 0 0 0 0 
Received : 0 0X1 0 0 0X10 0 0 0 0X31 0XDE 0XB 0 
>> Received ('add_two_ints': 3340526778581008)
>> Received query final notification
  • dds_stream_write_sampleBE
Sending Query 'add_two_ints'... with a = 123456, b = 654321
Send : 0 0X1 0 0 0 0 0 0X10 0 0 0 0 0 0X1 0XE2 0X40 0 0 0 0 0 0X9 0XFB 0XF1
Received : 0 0X1 0 0 0 0X1 0XE2 0X50 0 0 0 0 
>> Received ('add_two_ints': 1356988672)
>> Received query final notification

and there is no dds_stream_read_sampleLE() with the latest cdrstream, only the dds_stream_read_sample()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Status: Backlog
Development

Successfully merging a pull request may close this issue.

3 participants