data_length should be made into an uint32_t or higher

[ibaranov-cp]

data_length is currently usually set as a uint8_t. This breaks very quick once you want to send more than 255 entries.

Can we increase the size of data_length for things like pointcloud2 (the one I am currently working with). I just don't see the case when you would ever find 256 bytes of point cloud data useful for anything...

Or perhaps I am using it wrong?
My own attempts to hack up PointCloud2.h in the windows rosserial library cause lots of memory access faults.

[mikepurvis]

The protocol itself has length-of-message as a 16-bit value— I believe you're referring to the generated header for an Array. I'd happily review a PR for indigo which changes this to a uint16.

As for going to uint32, that would be a breaking change for the protocol. We'd either need to wait for J-turtle, or make the implementation in rosserial_server and rosserial_python able to detect and support both versions (similar to how rosserial_server supports both Groovy and Hydro clients).

[ibaranov-cp]

So the most reasonable steps in the meantime are:

1. change generated header for Array
2. split my message into a multiple of arrays (mulitarray perhaps?) then reassemble on the other end?

Unless you can see a more reasonable method to get point cloud data through rosserial?

[mikeferguson]

A couple of notes:

@mikepurvis -- the "length-of-message" you note in the protocol is for the overall message. I concur that changing that would be a fairly big break (and so the sync flag value should again be updated, to 0xfc this time). In theory, both types of messages could be supported side by side (although it increases maintainence overhead).

As for the uint8_t in the array length, it's clearly a hack and not fully implemented. This length should be a uint32_t (a uint32_t is already used in the ROS message protocol, so that four bytes of data is there, just isn't handled by our code). See also lines 234-237 and line 250 and 253 for where the length is serialized/deserialized. This fix would allow significantly larger arrays.

@ibaranov-cp could you tell us a bit more about your use case? How big are the point clouds -- and why use have you chosen to use rosserial? (PointClouds were not envisioned as a use case really, since most embedded controllers lack significant RAM).

[ibaranov-cp]

We are trying to get Kinect 2.0 data (Windows 8 only :( ) into ROS.
Rosserial is nice way to do so, and we recently made the rosserial windows
release so it is fairly easy to hack into Kinect V2 code.

Edit:
Point clouds are 512x424 with 16bits/depth pixel. This runs at 30FPS.
This means 3392kb per frame, and a data rate of ~100Mb/s.

On 10 Aug 2014 00:11, "Michael Ferguson" notifications@github.com wrote:

A couple of notes:

@mikepurvis https://github.com/mikepurvis -- the "length-of-message"
you note in the protocol is for the overall message. I concur that changing
that would be a fairly big break (and so the sync flag value should again
be updated, to 0xfc this time). In theory, both types of messages could be
supported side by side (although it increases maintainence overhead).

As for the uint8_t in the array length, it's clearly a hack and not fully
implemented. This length should be a uint32_t (a uint32_t is already used
in the ROS message protocol, so that data is there, just isn't handled by
our code). See also lines 234-237 and line 250 and 253 for where the length
is serialized/deserialized. This fix would allow significantly larger
arrays.

@ibaranov-cp https://github.com/ibaranov-cp could you tell us a bit
more about your use case? How big are the point clouds -- and why use have
you chosen to use rosserial? (PointClouds were not envisioned as a use case
really, since most embedded controllers lack significant RAM).

—
Reply to this email directly or view it on GitHub
#130 (comment)
.

[mikepurvis]

@ibaranov-cp Splitting the message sounds like a huge pain in the butt. I think this effort would be best invested in revving the protocol. I'd propose:

• rosserial_client gets the new protocol on jade-devel (to become the default for Jade)
• rosserial_python gets the new protocol on jade-devel
• rosserial_server is made to detect and support 0xfc firmwares, in indigo-devel, with the changes merged forward to jade-devel when complete.

This scheme means your system can still deploy rosserial_server from debs, and you can generate your rosserial_windows-based client library for Kinect 2 using a source checkout. Does this seem reasonable?

Alternatively, given this limitation, it might make a lot more sense to go native and just use roscpp/winros. I tried the pre-assembled SDK a few months ago with Hydro and found it very usable.

[ibaranov-cp]

I like this solution.

The end goal for me has always been a package that is easy for people to simply download on windows, open up in VS, and execute.

[cjwills]

Also interested in this, since I can't send sensor_msgs/Image from windows. Is this planned to be fixed in J?

[mikeferguson]

This should definitely be targeted in J, along with a bump in the protocol header byte (as we've been doing when breaking the wire protocol).

[mikepurvis]

Going to defer this to K— can't afford to wait longer on a Jade release.

[sevenbitbyte]

Should be fixed by PR #222