v-kiniv / rws

WebSocket gateway for ROS2 topics and services
Apache License 2.0
10 stars 7 forks source link

Messages containing arrays don't get sent from roslibjs #37

Closed MoffKalast closed 1 month ago

MoffKalast commented 1 month ago

As mentioned in https://github.com/v-kiniv/rws/issues/30#issuecomment-2360085645, I've gotten around to setting up a small test case for this odd issue. The process to reproduce is as follows:

git clone https://github.com/MoffKalast/vizanti.git -b ros2-rws-arraytest

Source, compile and all that. Then open three terminals with the following subscribers which each have a message type that contains some sort of array somewhere:

ros2 topic echo /path nav_msgs/msg/Path
ros2 topic echo /floatarray std_msgs/msg/Float32MultiArray
ros2 topic echo /posearray geometry_msgs/msg/PoseArray

Then run:

ros2 launch vizanti_server vizanti_server.launch.py

Which will launch vizanti with rosbridge, and upon opening http://localhost:5000, three test messages will be automatically sent to those topics, which should arrive fine in the terminals.

Then clear the subscriber terminals and run:

ros2 launch vizanti_server vizanti_rws.launch.py

The subscribers don't receive any messages, and RWS prints the following:

[rws_server-1] [WARN] [1728306011.007179379] [vizanti_rws_server]: Failed to parse JSON: [json.exception.type_error.305] cannot use operator[] with a string argument with array

Interestingly this problem is only one way, roslibjs can receive arrays just fine. I'm wondering if I'm doing something wrong on the publishing side tbh, since there are some cases (like getting header sec/secs wrong) that rosbridge handles that aren't in spec, but I'm not sure what else to try.

I've tested the above on both humble and jazzy with respective branches of RWS, with Cyclone. The /path topic is set up as latched in roslibjs so it should be received again any time a subscriber connects, but alas that detail doesn't seem to work with rosbridge either.

v-kiniv commented 1 month ago

Thanks for the prepared repo, saved me a lot of time. When I found the bug, I had doubts if I had made such a blunt mistake or if I forgotten why I needed to prepend array index with the field name, and removing it would cause other problems:

index 2671a2c..9d2dc7f 100644
--- a/src/translate.cpp
+++ b/src/translate.cpp
@@ -263,7 +263,7 @@ static void json_to_serialized_message(cycser & ser, const MessageMembers * memb
           }

           for (size_t index = 0; index < array_size; ++index) {
-            json_to_serialized_message(ser, sub_members, field[member->name_][index]);
+            json_to_serialized_message(ser, sub_members, field[index]);
           }
         }
         break;

I tested the changes with my old project and nothing seems to be broken. But as always, please test and let me know if anything is broken on your end before we merge PR https://github.com/v-kiniv/rws/pull/38.

MoffKalast commented 1 month ago

Well that was an easy fix đź‘Ť

I can confirm it working on jazzy with the test subscribers, I've also ran my set of test nodes for the rest and it all checks out: image

I'll test on humble as well when it's backported.