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

unsigned cast for listener_serialized_message.cpp #279

Closed
jeising opened this issue Oct 23, 2018 · 8 comments
Closed

unsigned cast for listener_serialized_message.cpp #279

jeising opened this issue Oct 23, 2018 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@jeising
Copy link

jeising commented Oct 23, 2018

Bug report

Required Info:

  • Operating System:
    • Ubuntu 16.04
  • Installation type:
    • From source (but should not matter)
  • Version or commit hash:
  • DDS implementation:
    • proprietary
  • Client library (if applicable):
    • rclcpp

Steps to reproduce issue

  • Have binary char data with leading 1 (not 0)
    -> Interpreted as negative number
    -> Print fails

Expected behavior

Print data correctly aligned

Actual behavior

Print failed

Additional information

To fix this issue add a unsigned cast to:

demos/demo_nodes_cpp/src/topics/listener_serialized_message.cpp:55

@dirk-thomas
Copy link
Member

Please create a PR for your proposed patch.

@Karsten1987
Copy link
Contributor

Karsten1987 commented Oct 23, 2018

Can you elaborate a little bit on the char data with leading 1? How did you get this data?
The demo creates the serialized data out of the rmw_serialize_message function and thus gets correctly displayed on the listener side as well. I am having trouble understanding on how to reproduce the behavior.

Could you come up with an example which will fail to transport correctly? I feel like - if that example doesn't work - having the cast is only hiding the symptoms. Maybe then the underlying char array of the raw message should indeed be a unsigned char array.

@jeising
Copy link
Author

jeising commented Oct 24, 2018

This issue seems to explain this well: stackoverflow - printf adds extra ffffff to hex print from a char array

The main idea is that for large char values e.g.: b1000'0000 to b1111'1111 they are interpreted as negative value and displayed incorrectly.

To reproduce this you could try to send chars in that range. This can be done via:

  1. ros2 topic pub /chatter std_msgs/string "data: string"
  2. instead of string add a ascii char via crtl + shift + u (ubuntu) e.g. 00ff. This results in a y with dots.
  3. ros2 run demo_nodes_cpp listener_serialized_message

Btw: A serialized print of middle ware data could be a nice feature for ros topic echo e.g.: ros2 topic echo --serialized

To fix this it would be sufficient to add a c-style cast (unsigned char) to listener_serialized_message.cpp. This makes sure that printf displays the data always like a hex viewer. Newer the less changing the buffer to be unsigned char* and potentially const on receiver side seem to be valid points.

@Karsten1987
Copy link
Contributor

The above referenced PRs should fix this problem. It uses the rcutils_uint8_array_t and casts to char * when fetching the data from the middleware.

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987
Copy link
Contributor

typo in the rmw package. New round of CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987
Copy link
Contributor

buildfarm hiccup, one more round:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987
Copy link
Contributor

Aaaaaaaaand one more time, fixing the windows warning:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@Karsten1987 Karsten1987 added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Nov 24, 2018
@Karsten1987
Copy link
Contributor

with the related changes, this should be fixed.

@Karsten1987 Karsten1987 removed the in review Waiting for review (Kanban column) label Nov 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants