addition to sensor_msgs/PointCloud2

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

addition to sensor_msgs/PointCloud2

Radu B. Rusu
Administrator
We are currently faced with a tiny problem in the way PointCloud2 structures are being used in certain parts of ROS.
Since we might be able to change that for the upcoming D-Turtle distribution, now would be a good time to see if we can
improve it.

So let's see what everyone thinks... This is more of a PCL_ROS/ROS/sensor_msgs issue than PCL, but pcl::PointCloud<T>
might need this too.


Step by step, itemized thoughts that lead to the problem:

* there are two ways of synchronizing message data in ROS:

   1) over topics: you simply specify that topic /A and topic /B contain data that your application needs (/A + /B)
   2) via time (header.stamp): in addition to the above, you can synchronize the messages on the two topics in time such
that the events that you care about occur simultaneously


* certain algorithms in PCL (but please remember, this is not a PCL problem - PCL might have been struck by it first,
that's all), will take a chunk of data... say a PointCloud2, and produce N smaller chunks as output. Example:

   - ExtractEuclideanClusters: takes 1 PointCloud2 as input, and produces N PointCloud2 clusters as output

[if you want an example with camera images, think Prosilica (5 MegaPixels). Some method that sends N regions of interest
that can be processed afterwards by other nodes]


* in ROS/PCL_ROS, the problem is: how do you send this data back to the user, especially when using nodes/nodelets. You
might want to treat each individual point cluster as an independent PointCloud2, such that you can apply further
operations on it. Current ways of sending data are:

   1) statically: provide N output topics ("/cluster_1 ... /cluster_N") -- _VERY BAD_ because you don't know the number
of clusters that your algorithm will find, so this doesn't work.

   2) dynamically: send the number of output topics via another topic ("/cluster_size") which the clients first read,
then output N topics ("/cluster_1 ... /cluster_N") -- _VERY BAD_, high costs for subscription, almost impossible to get
to work, as "/cluster_size" will vary while the processing node is up, so the subscriptions to "/cluster_X" might fail, etc

   3) create a new message type which contains an array/vector of point clouds, i.e. "PointCloud2[] clouds" -- _BAD_,
because that means that the entire ROS infrastructure that consumes PointCloud2 data needs to be re-written to accept
both PointCloud2 and PointCloud2[] as input (visualizers, etc, etc). This might not be trivial, as we also have no way
to accept two different message types on a single topic on ROS so now you have to listen to two sets of topics, etc.

   4) send all the N clusters over the same topic ("/output") -- _BAD/SEMI-BAD_. To get this to work, you need to
guarantee that: a) no data will be dropped so that you receive all messages being sent; b) the queue size of the
receiver has to be large enough to accommodate all clusters.




Out of the 4 solutions, ranging from very-bad to semi-bad, you would think that the last "kind of works". However,
solution number 4) has a huge problem too. This occurs in the following scenario:

* 1 PointCloud2 that represents a scene with 4 objects is given as input ("/input") to an algorithm that extracts the
four objects as 4 individual PointCloud2 clusters. The clusters are sent back on the network on the same topic ("/output")

* Another node(let) reads in the 4 clusters on its input topic (redirected from the previous output "/input2:=/output"),
computes some 3D features, normals, etc, doesn't matter what, and needs to republish the data back on the network. Due
to the way we currently defined these sort of operations right now (Garratt will take a swing at us here :) ), the
original data is not being republished (see
http://point-cloud-library-pcl-mailing-list.967500.n3.nabble.com/About-the-Normals-Point-type-td1651437.html#a1651437 
for more information why we decided to go on that route). This means that "/output2" now contains 4 PointCloud2 messages
with the same header, same timestamp, sent on the network.

* Finally, a third node(let) needs both the XYZ data and the features estimated so "/input3a:=/output" and
"/input3b:=/output2". Because the messages are coming in "bursts" on the two input topics with equal timestamps, the
data association problem (whose features correspond to who) becomes impossible to solve. Our TimeSynchronizer routines
will fail.





So we could start discussing a solution that involves concatenating the data together, but I propose we use the other
thread for that. Even so, this problem will soon occur in the 2D Image domain as well, as we strive to build blocks that
can be reused, rather than always having complicated complex modules that almost never reuse/share code. If you get an
input image with some RGB data, and you compute gradients on it, you don't (and even cannot in ROS!) want to re-send
both RGB + gradients over the network (btw, when I say "network" I mean just ros::Publisher -- if you're in a nodelet,
there's really no copy, no performance loss, etc)....



The solution that we adopted right now for an experimental setup is a horrible hack, and I would like to fix it after
discussing alternatives. We slightly modified the timestamp in the header by adding a negligible quantity to it, such
that TimeSynchronizer works.


Another solution that might be possible involves modifying the PointCloud2 message by adding an ID field. For the
clustering example, this would be perfect, as now we can build a TimeIDSynchronizer that takes stamp + ID into account.
However, I'm not sure this is the best solution.

An argument that we heard against ID, is that now we move away from strongly typed messages. I wonder how much of that
is true though, because in the case of messages that represent binary blobs (Image, PointCloud, PointCloud2), but also
other ROS messages (where people defined a bunch of consts that indicate how the message data should be used), you
almost always have to do some checking in order to interpret the data.



Any comments are appreciated.
--
Cheers,
Radu.

_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Marius Muja-2
In rein (recognition infrastructure) package, the solution I used was number 3 below, a detector nodelet publishes a DetectionArray message instead of several Detections on the same topic. I don't know of a good way to set the proper queue sizes before you run the algorithm since you don't know how many clusters/detections will be produced (you would end up using huge queue sizes for "good measure") and there is always an issues with synchronizing messages on two topics when there's a delay between the two topics being synchronized and the messages on one topic are coming at a higher frequency than on the other.

Another issue is that you might want later to synchronize the array of messages with the original message (for example I needed to synchronized the Image with the DetectionArray). This is not likely to be needed in the case of the PointCloud clusters and the original point cloud, but if we are looking for a general solution is something to be considered.

Here's another possibility of sending data (number 5). This is probably still _BAD_ as it requires a lot of changes in pcl and it's not particularly elegant, but I write it here for completeness: make all the pcl algorithms take a PointCloud[] instead of a PointCloud. Most algorithms would still work with a single point cloud so they would have an assert(cloud_array.size()==1) at the beginning. There will also need to be a converter nodelet that takes a single point cloud from the sensors and publishes a point cloud array of size 1 for the rest of pcl.

Cheers,
Marius



On Mon, Oct 11, 2010 at 1:13 PM, Radu Bogdan Rusu <[hidden email]> wrote:
We are currently faced with a tiny problem in the way PointCloud2 structures are being used in certain parts of ROS.
Since we might be able to change that for the upcoming D-Turtle distribution, now would be a good time to see if we can
improve it.

So let's see what everyone thinks... This is more of a PCL_ROS/ROS/sensor_msgs issue than PCL, but pcl::PointCloud<T>
might need this too.


Step by step, itemized thoughts that lead to the problem:

* there are two ways of synchronizing message data in ROS:

  1) over topics: you simply specify that topic /A and topic /B contain data that your application needs (/A + /B)
  2) via time (header.stamp): in addition to the above, you can synchronize the messages on the two topics in time such
that the events that you care about occur simultaneously


* certain algorithms in PCL (but please remember, this is not a PCL problem - PCL might have been struck by it first,
that's all), will take a chunk of data... say a PointCloud2, and produce N smaller chunks as output. Example:

  - ExtractEuclideanClusters: takes 1 PointCloud2 as input, and produces N PointCloud2 clusters as output

[if you want an example with camera images, think Prosilica (5 MegaPixels). Some method that sends N regions of interest
that can be processed afterwards by other nodes]


* in ROS/PCL_ROS, the problem is: how do you send this data back to the user, especially when using nodes/nodelets. You
might want to treat each individual point cluster as an independent PointCloud2, such that you can apply further
operations on it. Current ways of sending data are:

  1) statically: provide N output topics ("/cluster_1 ... /cluster_N") -- _VERY BAD_ because you don't know the number
of clusters that your algorithm will find, so this doesn't work.

  2) dynamically: send the number of output topics via another topic ("/cluster_size") which the clients first read,
then output N topics ("/cluster_1 ... /cluster_N") -- _VERY BAD_, high costs for subscription, almost impossible to get
to work, as "/cluster_size" will vary while the processing node is up, so the subscriptions to "/cluster_X" might fail, etc

  3) create a new message type which contains an array/vector of point clouds, i.e. "PointCloud2[] clouds" -- _BAD_,
because that means that the entire ROS infrastructure that consumes PointCloud2 data needs to be re-written to accept
both PointCloud2 and PointCloud2[] as input (visualizers, etc, etc). This might not be trivial, as we also have no way
to accept two different message types on a single topic on ROS so now you have to listen to two sets of topics, etc.

  4) send all the N clusters over the same topic ("/output") -- _BAD/SEMI-BAD_. To get this to work, you need to
guarantee that: a) no data will be dropped so that you receive all messages being sent; b) the queue size of the
receiver has to be large enough to accommodate all clusters.




Out of the 4 solutions, ranging from very-bad to semi-bad, you would think that the last "kind of works". However,
solution number 4) has a huge problem too. This occurs in the following scenario:

* 1 PointCloud2 that represents a scene with 4 objects is given as input ("/input") to an algorithm that extracts the
four objects as 4 individual PointCloud2 clusters. The clusters are sent back on the network on the same topic ("/output")

* Another node(let) reads in the 4 clusters on its input topic (redirected from the previous output "/input2:=/output"),
computes some 3D features, normals, etc, doesn't matter what, and needs to republish the data back on the network. Due
to the way we currently defined these sort of operations right now (Garratt will take a swing at us here :) ), the
original data is not being republished (see
http://point-cloud-library-pcl-mailing-list.967500.n3.nabble.com/About-the-Normals-Point-type-td1651437.html#a1651437
for more information why we decided to go on that route). This means that "/output2" now contains 4 PointCloud2 messages
with the same header, same timestamp, sent on the network.

* Finally, a third node(let) needs both the XYZ data and the features estimated so "/input3a:=/output" and
"/input3b:=/output2". Because the messages are coming in "bursts" on the two input topics with equal timestamps, the
data association problem (whose features correspond to who) becomes impossible to solve. Our TimeSynchronizer routines
will fail.





So we could start discussing a solution that involves concatenating the data together, but I propose we use the other
thread for that. Even so, this problem will soon occur in the 2D Image domain as well, as we strive to build blocks that
can be reused, rather than always having complicated complex modules that almost never reuse/share code. If you get an
input image with some RGB data, and you compute gradients on it, you don't (and even cannot in ROS!) want to re-send
both RGB + gradients over the network (btw, when I say "network" I mean just ros::Publisher -- if you're in a nodelet,
there's really no copy, no performance loss, etc)....



The solution that we adopted right now for an experimental setup is a horrible hack, and I would like to fix it after
discussing alternatives. We slightly modified the timestamp in the header by adding a negligible quantity to it, such
that TimeSynchronizer works.


Another solution that might be possible involves modifying the PointCloud2 message by adding an ID field. For the
clustering example, this would be perfect, as now we can build a TimeIDSynchronizer that takes stamp + ID into account.
However, I'm not sure this is the best solution.

An argument that we heard against ID, is that now we move away from strongly typed messages. I wonder how much of that
is true though, because in the case of messages that represent binary blobs (Image, PointCloud, PointCloud2), but also
other ROS messages (where people defined a bunch of consts that indicate how the message data should be used), you
almost always have to do some checking in order to interpret the data.



Any comments are appreciated.
--
Cheers,
Radu.

_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users


_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Geoffrey Biggs
In reply to this post by Radu B. Rusu
I think that a modified #3 is actually the best option (basically what
Marius said). If an algorithm is capable of working with multiple point
clouds, it should be receiving an array of them, even if that array
might only carry one. Most nodes should probably work this way. Nodes
that produce point clouds could be modified to do one of the following:

1) Produce only an array of clouds, with one member.
2) Have two topics, one for the singular cloud and one for the array
with one member.
3) No modification, and create a node that transforms a singular cloud
into an array with one member.

Geoff

On 12/10/10 05:13, Radu Bogdan Rusu wrote:

> We are currently faced with a tiny problem in the way PointCloud2 structures are being used in certain parts of ROS.
> Since we might be able to change that for the upcoming D-Turtle distribution, now would be a good time to see if we can
> improve it.
>
> So let's see what everyone thinks... This is more of a PCL_ROS/ROS/sensor_msgs issue than PCL, but pcl::PointCloud<T>
> might need this too.
>
>
> Step by step, itemized thoughts that lead to the problem:
>
> * there are two ways of synchronizing message data in ROS:
>
>    1) over topics: you simply specify that topic /A and topic /B contain data that your application needs (/A + /B)
>    2) via time (header.stamp): in addition to the above, you can synchronize the messages on the two topics in time such
> that the events that you care about occur simultaneously
>
>
> * certain algorithms in PCL (but please remember, this is not a PCL problem - PCL might have been struck by it first,
> that's all), will take a chunk of data... say a PointCloud2, and produce N smaller chunks as output. Example:
>
>    - ExtractEuclideanClusters: takes 1 PointCloud2 as input, and produces N PointCloud2 clusters as output
>
> [if you want an example with camera images, think Prosilica (5 MegaPixels). Some method that sends N regions of interest
> that can be processed afterwards by other nodes]
>
>
> * in ROS/PCL_ROS, the problem is: how do you send this data back to the user, especially when using nodes/nodelets. You
> might want to treat each individual point cluster as an independent PointCloud2, such that you can apply further
> operations on it. Current ways of sending data are:
>
>    1) statically: provide N output topics ("/cluster_1 ... /cluster_N") -- _VERY BAD_ because you don't know the number
> of clusters that your algorithm will find, so this doesn't work.
>
>    2) dynamically: send the number of output topics via another topic ("/cluster_size") which the clients first read,
> then output N topics ("/cluster_1 ... /cluster_N") -- _VERY BAD_, high costs for subscription, almost impossible to get
> to work, as "/cluster_size" will vary while the processing node is up, so the subscriptions to "/cluster_X" might fail, etc
>
>    3) create a new message type which contains an array/vector of point clouds, i.e. "PointCloud2[] clouds" -- _BAD_,
> because that means that the entire ROS infrastructure that consumes PointCloud2 data needs to be re-written to accept
> both PointCloud2 and PointCloud2[] as input (visualizers, etc, etc). This might not be trivial, as we also have no way
> to accept two different message types on a single topic on ROS so now you have to listen to two sets of topics, etc.
>
>    4) send all the N clusters over the same topic ("/output") -- _BAD/SEMI-BAD_. To get this to work, you need to
> guarantee that: a) no data will be dropped so that you receive all messages being sent; b) the queue size of the
> receiver has to be large enough to accommodate all clusters.
>
>
>
>
> Out of the 4 solutions, ranging from very-bad to semi-bad, you would think that the last "kind of works". However,
> solution number 4) has a huge problem too. This occurs in the following scenario:
>
> * 1 PointCloud2 that represents a scene with 4 objects is given as input ("/input") to an algorithm that extracts the
> four objects as 4 individual PointCloud2 clusters. The clusters are sent back on the network on the same topic ("/output")
>
> * Another node(let) reads in the 4 clusters on its input topic (redirected from the previous output "/input2:=/output"),
> computes some 3D features, normals, etc, doesn't matter what, and needs to republish the data back on the network. Due
> to the way we currently defined these sort of operations right now (Garratt will take a swing at us here :) ), the
> original data is not being republished (see
> http://point-cloud-library-pcl-mailing-list.967500.n3.nabble.com/About-the-Normals-Point-type-td1651437.html#a1651437 
> for more information why we decided to go on that route). This means that "/output2" now contains 4 PointCloud2 messages
> with the same header, same timestamp, sent on the network.
>
> * Finally, a third node(let) needs both the XYZ data and the features estimated so "/input3a:=/output" and
> "/input3b:=/output2". Because the messages are coming in "bursts" on the two input topics with equal timestamps, the
> data association problem (whose features correspond to who) becomes impossible to solve. Our TimeSynchronizer routines
> will fail.
>
>
>
>
>
> So we could start discussing a solution that involves concatenating the data together, but I propose we use the other
> thread for that. Even so, this problem will soon occur in the 2D Image domain as well, as we strive to build blocks that
> can be reused, rather than always having complicated complex modules that almost never reuse/share code. If you get an
> input image with some RGB data, and you compute gradients on it, you don't (and even cannot in ROS!) want to re-send
> both RGB + gradients over the network (btw, when I say "network" I mean just ros::Publisher -- if you're in a nodelet,
> there's really no copy, no performance loss, etc)....
>
>
>
> The solution that we adopted right now for an experimental setup is a horrible hack, and I would like to fix it after
> discussing alternatives. We slightly modified the timestamp in the header by adding a negligible quantity to it, such
> that TimeSynchronizer works.
>
>
> Another solution that might be possible involves modifying the PointCloud2 message by adding an ID field. For the
> clustering example, this would be perfect, as now we can build a TimeIDSynchronizer that takes stamp + ID into account.
> However, I'm not sure this is the best solution.
>
> An argument that we heard against ID, is that now we move away from strongly typed messages. I wonder how much of that
> is true though, because in the case of messages that represent binary blobs (Image, PointCloud, PointCloud2), but also
> other ROS messages (where people defined a bunch of consts that indicate how the message data should be used), you
> almost always have to do some checking in order to interpret the data.
>
>
>
> Any comments are appreciated.
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Jack O'Quin
On Mon, Oct 11, 2010 at 8:38 PM, Geoffrey Biggs
<[hidden email]> wrote:

> I think that a modified #3 is actually the best option (basically what
> Marius said). If an algorithm is capable of working with multiple point
> clouds, it should be receiving an array of them, even if that array
> might only carry one. Most nodes should probably work this way. Nodes
> that produce point clouds could be modified to do one of the following:
>
> 1) Produce only an array of clouds, with one member.
> 2) Have two topics, one for the singular cloud and one for the array
> with one member.
> 3) No modification, and create a node that transforms a singular cloud
> into an array with one member.

+1

Seems like many nodes that handle a cloud as input could easily be
changed to handle a vector of clouds by just adding a for loop around
the main message handler.
--
 joq
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Radu B. Rusu
Administrator
Ok, so the desired solution would be to have the nodelets accept data on two separate topics? One PointCloud2, and one
PointCloud2[] ? Solution number 2)

It doesn't seem "right" to modify everything to accept only PointCloud2[] (solution number 1). Marius, I think the
example that you gave for DetectionArray is ok, but the detections themselves are small in size, so having an array of
them does not introduces big penalties. It might be that something like PointCloud2[] might get misused though. Every
PointCloud2 has a header, and I guess the new message type will need to have a header too, etc.

Not sure if solution number 3 works by itself -- it feels like it needs either 1) or it should be reversed, as we might
need to transform from array to cloud to allow the current methods to work without modification.

Overall, 2) might not be insanely bad, though it will introduce a bunch of changes.... :) Argh, and we need to do this
for every PCL nodelet.

The problem is that while we can modify PCL, it will be hard to modify everything else in ROS. Any existing code that
simply accepts one PointCloud2 as input, will need something like an PointCloud2[]->PointCloud conversion, but it's not
exactly clear what that will do.

Any other thoughts before we start changing the code? :)

Cheers,
Radu.


On 10/11/2010 07:39 PM, Jack O'Quin wrote:

> On Mon, Oct 11, 2010 at 8:38 PM, Geoffrey Biggs
> <[hidden email]>  wrote:
>> I think that a modified #3 is actually the best option (basically what
>> Marius said). If an algorithm is capable of working with multiple point
>> clouds, it should be receiving an array of them, even if that array
>> might only carry one. Most nodes should probably work this way. Nodes
>> that produce point clouds could be modified to do one of the following:
>>
>> 1) Produce only an array of clouds, with one member.
>> 2) Have two topics, one for the singular cloud and one for the array
>> with one member.
>> 3) No modification, and create a node that transforms a singular cloud
>> into an array with one member.
>
> +1
>
> Seems like many nodes that handle a cloud as input could easily be
> changed to handle a vector of clouds by just adding a for loop around
> the main message handler.
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Geoffrey Biggs
On 12/10/10 14:39, Radu Bogdan Rusu wrote:

> Ok, so the desired solution would be to have the nodelets accept data on two separate topics? One PointCloud2, and one
> PointCloud2[] ? Solution number 2)
>
> It doesn't seem "right" to modify everything to accept only PointCloud2[] (solution number 1). Marius, I think the
> example that you gave for DetectionArray is ok, but the detections themselves are small in size, so having an array of
> them does not introduces big penalties. It might be that something like PointCloud2[] might get misused though. Every
> PointCloud2 has a header, and I guess the new message type will need to have a header too, etc.
>
> Not sure if solution number 3 works by itself -- it feels like it needs either 1) or it should be reversed, as we might
> need to transform from array to cloud to allow the current methods to work without modification.
>
> Overall, 2) might not be insanely bad, though it will introduce a bunch of changes.... :) Argh, and we need to do this
> for every PCL nodelet.
>
> The problem is that while we can modify PCL, it will be hard to modify everything else in ROS. Any existing code that
> simply accepts one PointCloud2 as input, will need something like an PointCloud2[]->PointCloud conversion, but it's not
> exactly clear what that will do.
>
> Any other thoughts before we start changing the code? :)

Actually, I vote for #1. :) If a node only wants to use one cloud then
it can either do a check for the length of the array, or just assume
only one member (and anyone who sends more should have read the
documentation). This gives more universality (is that a word?) to the
transmitted data types. Making nodes *accept* on two topics would be a
maintenance nightmare, I think, plus what happens if someone sends point
clouds to both topics at once? It's ok for publishing, but not for
receiving.

Having to modify other stuff in ROS is simply a fact of other stuff in
ROS using software that is technically still under development. You can
easily create in an interim solution using a node that takes the array
and spits out the first member only, and make it widely known that the
array version is what will be used from now on.

Geoff
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Radu B. Rusu
Administrator


On 10/11/2010 11:01 PM, Geoffrey Biggs wrote:

> On 12/10/10 14:39, Radu Bogdan Rusu wrote:
>> Ok, so the desired solution would be to have the nodelets accept data on two separate topics? One PointCloud2, and one
>> PointCloud2[] ? Solution number 2)
>>
>> It doesn't seem "right" to modify everything to accept only PointCloud2[] (solution number 1). Marius, I think the
>> example that you gave for DetectionArray is ok, but the detections themselves are small in size, so having an array of
>> them does not introduces big penalties. It might be that something like PointCloud2[] might get misused though. Every
>> PointCloud2 has a header, and I guess the new message type will need to have a header too, etc.
>>
>> Not sure if solution number 3 works by itself -- it feels like it needs either 1) or it should be reversed, as we might
>> need to transform from array to cloud to allow the current methods to work without modification.
>>
>> Overall, 2) might not be insanely bad, though it will introduce a bunch of changes.... :) Argh, and we need to do this
>> for every PCL nodelet.
>>
>> The problem is that while we can modify PCL, it will be hard to modify everything else in ROS. Any existing code that
>> simply accepts one PointCloud2 as input, will need something like an PointCloud2[]->PointCloud conversion, but it's not
>> exactly clear what that will do.
>>
>> Any other thoughts before we start changing the code? :)
>
> Actually, I vote for #1. :) If a node only wants to use one cloud then
> it can either do a check for the length of the array, or just assume
> only one member (and anyone who sends more should have read the
> documentation). This gives more universality (is that a word?) to the
> transmitted data types. Making nodes *accept* on two topics would be a
> maintenance nightmare, I think, plus what happens if someone sends point
> clouds to both topics at once? It's ok for publishing, but not for
> receiving.
>
> Having to modify other stuff in ROS is simply a fact of other stuff in
> ROS using software that is technically still under development. You can
> easily create in an interim solution using a node that takes the array
> and spits out the first member only, and make it widely known that the
> array version is what will be used from now on.
>
> Geoff

Hmm, I'm still not 100% convinced... I know that from a technical perspective, we can implement it without blinking
twice :) but isn't it just plain wrong in terms of "semantics"?

This means that we need to drop sending/receiving single PointCloud2 messages, and always use vectors of them. Is that
really ok? Looking at RViz (not that we are similar in any way with PCL, but just as an example), it seems that the
solution there was to add "Marker.msg" and "MarkerArray.msg" as two separate messages.

Cheers,
Radu.
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Geoffrey Biggs
On 12/10/10 15:08, Radu Bogdan Rusu wrote:
> Hmm, I'm still not 100% convinced... I know that from a technical
> perspective, we can implement it without blinking twice :) but isn't it
> just plain wrong in terms of "semantics"?
>
> This means that we need to drop sending/receiving single PointCloud2
> messages, and always use vectors of them. Is that really ok? Looking at
> RViz (not that we are similar in any way with PCL, but just as an
> example), it seems that the solution there was to add "Marker.msg" and
> "MarkerArray.msg" as two separate messages.

The way I see it, there are three potential events:

- A node uses only a single point cloud internally.
- A node may use zero, one, or more point clouds (or any combination of
these).
- A node requires two or more point clouds.

The only data type that covers all three is an array of point clouds.
Having a single-point cloud message is certainly technically feasible,
and best in many cases, but it means that nodes that may use one or more
cannot be directly connected to nodes that produce just one. So the
choice is to have all produce an array, even if it only contains one, or
to require some kind of translator node for those that require an array.
Having a node accept from two topic types is, I think, hacky and
potentially confusing ("Which one do I use when I only have one cloud?"
"Is there a difference in behaviour between the single cloud topic and
the array topic in the case of just one cloud?" etc).

Geoff
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

garratt
Why can't all of this be rolled into the pointcloud2 message?

PointCloud2 is already abstracted in such a way as to prevent the user
from accessing its information directly, and it even has a height and
width, which don't make any sense for normal point clouds.  

I suppose what this boils down to is the 'ID' field, which would
identify that there was clustering afoot, and if you so desired, you
could take advantage of said clustering.  What I don't understand is why
this couldn't just be another element in PointCloud2/fields: if we can
be expected to know that vp_x means the x component of the viewpoint, we
might as well assume that cl_1 indicates the cluster that the point
belongs to in the first clustering operation.

Which brings me to the next point:  what if we do multiple clustering
operations?  (say we cluster floors, walls and other, then cluster all
'unseen' objects.  The points would reamin the same, all we are doing is
applying more labels.  so why don't we just add a label to express this?

as an added bonus, we could make helper functions like
pcl::fromRosMsg(PointCloud2 input, vector<pcl::PointCloud > clusters,
string clusterfieldname="cl_1")

that would make the fact that we are passing a bunch of clusters as one
point cloud transparent to end pcl users.

In general, I'm against adding a PointCloud2[] message, (3) since we
_always_ have to decode PointCloud2 anyway (it might make sense for
PointCloud, where you could read the elements directly)

I'm _very_ against sending many messages with the same timestamp (4)
(forcing the end user to try to figure out when all the messages from a
single cloud have been recieved, etc)

creating multiple topics (1) and (2) are also out, for reasons radu
gave.  

As a response to the message below:
when are you using 3 point clouds?  are they from different times,
different sensors?  Internally, I use vectors of pcl::PointCloud to
represent clusters, and once I transform, downsample and find normals of
point clouds from different times/sensors, I just merge them together.
I haven't needed to publish the output of my clusters at the moment, but
if I did, it would be nice to have the format described above.

cheers
Garratt


On Tue, 2010-10-12 at 17:01 +0900, Geoffrey Biggs wrote:

> On 12/10/10 15:08, Radu Bogdan Rusu wrote:
> > Hmm, I'm still not 100% convinced... I know that from a technical
> > perspective, we can implement it without blinking twice :) but isn't it
> > just plain wrong in terms of "semantics"?
> >
> > This means that we need to drop sending/receiving single PointCloud2
> > messages, and always use vectors of them. Is that really ok? Looking at
> > RViz (not that we are similar in any way with PCL, but just as an
> > example), it seems that the solution there was to add "Marker.msg" and
> > "MarkerArray.msg" as two separate messages.
>
> The way I see it, there are three potential events:
>
> - A node uses only a single point cloud internally.
> - A node may use zero, one, or more point clouds (or any combination of
> these).
> - A node requires two or more point clouds.
>
> The only data type that covers all three is an array of point clouds.
> Having a single-point cloud message is certainly technically feasible,
> and best in many cases, but it means that nodes that may use one or more
> cannot be directly connected to nodes that produce just one. So the
> choice is to have all produce an array, even if it only contains one, or
> to require some kind of translator node for those that require an array.
> Having a node accept from two topic types is, I think, hacky and
> potentially confusing ("Which one do I use when I only have one cloud?"
> "Is there a difference in behaviour between the single cloud topic and
> the array topic in the case of just one cloud?" etc).
>
> Geoff
> _______________________________________________
> [hidden email] / http://pcl.ros.org
> https://code.ros.org/mailman/listinfo/pcl-users


_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Radu B. Rusu
Administrator
Garratt,

Great suggestion! Adding more labels as additional fields makes a lot of sense to me.

There's still one problem though:

* you get a XYZ PointCloud that you split into N clusters

* you assign a new label/field to the output PointCloud that now contains XYZ+Label

* you want to process one cluster out of the resultant PointCloud in another module (say you want to estimate surface
normals or something)

* your module now needs to know that field "Label" exists, and how to interpret it, in order to extract the cluster that
needs to be processed


Cheers,
Radu.


On 10/12/2010 09:35 AM, garratt wrote:

> Why can't all of this be rolled into the pointcloud2 message?
>
> PointCloud2 is already abstracted in such a way as to prevent the user
> from accessing its information directly, and it even has a height and
> width, which don't make any sense for normal point clouds.
>
> I suppose what this boils down to is the 'ID' field, which would
> identify that there was clustering afoot, and if you so desired, you
> could take advantage of said clustering.  What I don't understand is why
> this couldn't just be another element in PointCloud2/fields: if we can
> be expected to know that vp_x means the x component of the viewpoint, we
> might as well assume that cl_1 indicates the cluster that the point
> belongs to in the first clustering operation.
>
> Which brings me to the next point:  what if we do multiple clustering
> operations?  (say we cluster floors, walls and other, then cluster all
> 'unseen' objects.  The points would reamin the same, all we are doing is
> applying more labels.  so why don't we just add a label to express this?
>
> as an added bonus, we could make helper functions like
> pcl::fromRosMsg(PointCloud2 input, vector<pcl::PointCloud>  clusters,
> string clusterfieldname="cl_1")
>
> that would make the fact that we are passing a bunch of clusters as one
> point cloud transparent to end pcl users.
>
> In general, I'm against adding a PointCloud2[] message, (3) since we
> _always_ have to decode PointCloud2 anyway (it might make sense for
> PointCloud, where you could read the elements directly)
>
> I'm _very_ against sending many messages with the same timestamp (4)
> (forcing the end user to try to figure out when all the messages from a
> single cloud have been recieved, etc)
>
> creating multiple topics (1) and (2) are also out, for reasons radu
> gave.
>
> As a response to the message below:
> when are you using 3 point clouds?  are they from different times,
> different sensors?  Internally, I use vectors of pcl::PointCloud to
> represent clusters, and once I transform, downsample and find normals of
> point clouds from different times/sensors, I just merge them together.
> I haven't needed to publish the output of my clusters at the moment, but
> if I did, it would be nice to have the format described above.
>
> cheers
> Garratt
>
>
> On Tue, 2010-10-12 at 17:01 +0900, Geoffrey Biggs wrote:
>> On 12/10/10 15:08, Radu Bogdan Rusu wrote:
>>> Hmm, I'm still not 100% convinced... I know that from a technical
>>> perspective, we can implement it without blinking twice :) but isn't it
>>> just plain wrong in terms of "semantics"?
>>>
>>> This means that we need to drop sending/receiving single PointCloud2
>>> messages, and always use vectors of them. Is that really ok? Looking at
>>> RViz (not that we are similar in any way with PCL, but just as an
>>> example), it seems that the solution there was to add "Marker.msg" and
>>> "MarkerArray.msg" as two separate messages.
>>
>> The way I see it, there are three potential events:
>>
>> - A node uses only a single point cloud internally.
>> - A node may use zero, one, or more point clouds (or any combination of
>> these).
>> - A node requires two or more point clouds.
>>
>> The only data type that covers all three is an array of point clouds.
>> Having a single-point cloud message is certainly technically feasible,
>> and best in many cases, but it means that nodes that may use one or more
>> cannot be directly connected to nodes that produce just one. So the
>> choice is to have all produce an array, even if it only contains one, or
>> to require some kind of translator node for those that require an array.
>> Having a node accept from two topic types is, I think, hacky and
>> potentially confusing ("Which one do I use when I only have one cloud?"
>> "Is there a difference in behaviour between the single cloud topic and
>> the array topic in the case of just one cloud?" etc).
>>
>> Geoff
>> _______________________________________________
>> [hidden email] / http://pcl.ros.org
>> https://code.ros.org/mailman/listinfo/pcl-users
>
>
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

garratt
This breaks down into two problems:

for naming ease:
node a applies label 'c1' to cloud
node b finds the normals of the cloud points with a specific label in c1
node c then looks at the result

the problems are:
1) how does node b know what to do?
The solution here is conventions.  there can be a label of
'do_operation' for filter-type nodes, indicating that they should act on
certain data, or just 'cluster' for parsing a clustering operation.  

2)how does node c identify the data?
This is a bit trickier - now you have fields that indicate normal data,
when not all the points have useful data in that field.   You can solve
this by adding a 'namespace':  instead of labeling the new fields
normal_x, normal_y, etc, you can label them 'cluster::01::normal_x' or
'cluster::1,2,4-9::normal_y'.  if the convention is enforced, you
wouldn't even have to take ginormous amounts of memory to store the
whole thing! (since you know that certain fields won't be populated)

of course, decoding the cloud as a vector of clouds then becomes a
problem, if you wanted to preserve all type information...

pcl::fromRosMsg(PointCloud2 input, vector<pcl::PointCloud<what type goes
here?>  clusters, string clusterfieldname="clusters")


cheers
Garratt

On Tue, 2010-10-12 at 13:20 -0700, Radu Bogdan Rusu wrote:

> Garratt,
>
> Great suggestion! Adding more labels as additional fields makes a lot of sense to me.
>
> There's still one problem though:
>
> * you get a XYZ PointCloud that you split into N clusters
>
> * you assign a new label/field to the output PointCloud that now contains XYZ+Label
>
> * you want to process one cluster out of the resultant PointCloud in another module (say you want to estimate surface
> normals or something)
>
> * your module now needs to know that field "Label" exists, and how to interpret it, in order to extract the cluster that
> needs to be processed
>
>
> Cheers,
> Radu.
>
>
> On 10/12/2010 09:35 AM, garratt wrote:
> > Why can't all of this be rolled into the pointcloud2 message?
> >
> > PointCloud2 is already abstracted in such a way as to prevent the user
> > from accessing its information directly, and it even has a height and
> > width, which don't make any sense for normal point clouds.
> >
> > I suppose what this boils down to is the 'ID' field, which would
> > identify that there was clustering afoot, and if you so desired, you
> > could take advantage of said clustering.  What I don't understand is why
> > this couldn't just be another element in PointCloud2/fields: if we can
> > be expected to know that vp_x means the x component of the viewpoint, we
> > might as well assume that cl_1 indicates the cluster that the point
> > belongs to in the first clustering operation.
> >
> > Which brings me to the next point:  what if we do multiple clustering
> > operations?  (say we cluster floors, walls and other, then cluster all
> > 'unseen' objects.  The points would reamin the same, all we are doing is
> > applying more labels.  so why don't we just add a label to express this?
> >
> > as an added bonus, we could make helper functions like
> > pcl::fromRosMsg(PointCloud2 input, vector<pcl::PointCloud>  clusters,
> > string clusterfieldname="cl_1")
> >
> > that would make the fact that we are passing a bunch of clusters as one
> > point cloud transparent to end pcl users.
> >
> > In general, I'm against adding a PointCloud2[] message, (3) since we
> > _always_ have to decode PointCloud2 anyway (it might make sense for
> > PointCloud, where you could read the elements directly)
> >
> > I'm _very_ against sending many messages with the same timestamp (4)
> > (forcing the end user to try to figure out when all the messages from a
> > single cloud have been recieved, etc)
> >
> > creating multiple topics (1) and (2) are also out, for reasons radu
> > gave.
> >
> > As a response to the message below:
> > when are you using 3 point clouds?  are they from different times,
> > different sensors?  Internally, I use vectors of pcl::PointCloud to
> > represent clusters, and once I transform, downsample and find normals of
> > point clouds from different times/sensors, I just merge them together.
> > I haven't needed to publish the output of my clusters at the moment, but
> > if I did, it would be nice to have the format described above.
> >
> > cheers
> > Garratt
> >
> >
> > On Tue, 2010-10-12 at 17:01 +0900, Geoffrey Biggs wrote:
> >> On 12/10/10 15:08, Radu Bogdan Rusu wrote:
> >>> Hmm, I'm still not 100% convinced... I know that from a technical
> >>> perspective, we can implement it without blinking twice :) but isn't it
> >>> just plain wrong in terms of "semantics"?
> >>>
> >>> This means that we need to drop sending/receiving single PointCloud2
> >>> messages, and always use vectors of them. Is that really ok? Looking at
> >>> RViz (not that we are similar in any way with PCL, but just as an
> >>> example), it seems that the solution there was to add "Marker.msg" and
> >>> "MarkerArray.msg" as two separate messages.
> >>
> >> The way I see it, there are three potential events:
> >>
> >> - A node uses only a single point cloud internally.
> >> - A node may use zero, one, or more point clouds (or any combination of
> >> these).
> >> - A node requires two or more point clouds.
> >>
> >> The only data type that covers all three is an array of point clouds.
> >> Having a single-point cloud message is certainly technically feasible,
> >> and best in many cases, but it means that nodes that may use one or more
> >> cannot be directly connected to nodes that produce just one. So the
> >> choice is to have all produce an array, even if it only contains one, or
> >> to require some kind of translator node for those that require an array.
> >> Having a node accept from two topic types is, I think, hacky and
> >> potentially confusing ("Which one do I use when I only have one cloud?"
> >> "Is there a difference in behaviour between the single cloud topic and
> >> the array topic in the case of just one cloud?" etc).
> >>
> >> Geoff
> >> _______________________________________________
> >> [hidden email] / http://pcl.ros.org
> >> https://code.ros.org/mailman/listinfo/pcl-users
> >
> >


_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

Geoffrey Biggs
I agree with Garratt's idea. This neatly solves the problems, and
establishing conventions will easily prevent any problems with naming.
People who don't follow conventions are asking for trouble in any
situation. ;)

One of the benefits of Garratt's idea is its flexibility. It would be
possible for a chain of nodes to gradually annotate a cloud with more
and more information identifying features, without dramatically
increasing the size of the data. As an example implementation method, I
could add a "cluster" to a cloud which just means storing the indices of
those points in the cluster, and the original points provide the actual
data for the cluster. It's tidy and flexible. +1.

Geoff

On 13/10/10 06:16, garratt wrote:

> This breaks down into two problems:
>
> for naming ease:
> node a applies label 'c1' to cloud
> node b finds the normals of the cloud points with a specific label in c1
> node c then looks at the result
>
> the problems are:
> 1) how does node b know what to do?
> The solution here is conventions.  there can be a label of
> 'do_operation' for filter-type nodes, indicating that they should act on
> certain data, or just 'cluster' for parsing a clustering operation.  
>
> 2)how does node c identify the data?
> This is a bit trickier - now you have fields that indicate normal data,
> when not all the points have useful data in that field.   You can solve
> this by adding a 'namespace':  instead of labeling the new fields
> normal_x, normal_y, etc, you can label them 'cluster::01::normal_x' or
> 'cluster::1,2,4-9::normal_y'.  if the convention is enforced, you
> wouldn't even have to take ginormous amounts of memory to store the
> whole thing! (since you know that certain fields won't be populated)
>
> of course, decoding the cloud as a vector of clouds then becomes a
> problem, if you wanted to preserve all type information...
>
> pcl::fromRosMsg(PointCloud2 input, vector<pcl::PointCloud<what type goes
> here?>  clusters, string clusterfieldname="clusters")
>
>
> cheers
> Garratt
>
> On Tue, 2010-10-12 at 13:20 -0700, Radu Bogdan Rusu wrote:
>> Garratt,
>>
>> Great suggestion! Adding more labels as additional fields makes a lot of sense to me.
>>
>> There's still one problem though:
>>
>> * you get a XYZ PointCloud that you split into N clusters
>>
>> * you assign a new label/field to the output PointCloud that now contains XYZ+Label
>>
>> * you want to process one cluster out of the resultant PointCloud in another module (say you want to estimate surface
>> normals or something)
>>
>> * your module now needs to know that field "Label" exists, and how to interpret it, in order to extract the cluster that
>> needs to be processed
>>
>>
>> Cheers,
>> Radu.
>>
>>
>> On 10/12/2010 09:35 AM, garratt wrote:
>>> Why can't all of this be rolled into the pointcloud2 message?
>>>
>>> PointCloud2 is already abstracted in such a way as to prevent the user
>>> from accessing its information directly, and it even has a height and
>>> width, which don't make any sense for normal point clouds.
>>>
>>> I suppose what this boils down to is the 'ID' field, which would
>>> identify that there was clustering afoot, and if you so desired, you
>>> could take advantage of said clustering.  What I don't understand is why
>>> this couldn't just be another element in PointCloud2/fields: if we can
>>> be expected to know that vp_x means the x component of the viewpoint, we
>>> might as well assume that cl_1 indicates the cluster that the point
>>> belongs to in the first clustering operation.
>>>
>>> Which brings me to the next point:  what if we do multiple clustering
>>> operations?  (say we cluster floors, walls and other, then cluster all
>>> 'unseen' objects.  The points would reamin the same, all we are doing is
>>> applying more labels.  so why don't we just add a label to express this?
>>>
>>> as an added bonus, we could make helper functions like
>>> pcl::fromRosMsg(PointCloud2 input, vector<pcl::PointCloud>  clusters,
>>> string clusterfieldname="cl_1")
>>>
>>> that would make the fact that we are passing a bunch of clusters as one
>>> point cloud transparent to end pcl users.
>>>
>>> In general, I'm against adding a PointCloud2[] message, (3) since we
>>> _always_ have to decode PointCloud2 anyway (it might make sense for
>>> PointCloud, where you could read the elements directly)
>>>
>>> I'm _very_ against sending many messages with the same timestamp (4)
>>> (forcing the end user to try to figure out when all the messages from a
>>> single cloud have been recieved, etc)
>>>
>>> creating multiple topics (1) and (2) are also out, for reasons radu
>>> gave.
>>>
>>> As a response to the message below:
>>> when are you using 3 point clouds?  are they from different times,
>>> different sensors?  Internally, I use vectors of pcl::PointCloud to
>>> represent clusters, and once I transform, downsample and find normals of
>>> point clouds from different times/sensors, I just merge them together.
>>> I haven't needed to publish the output of my clusters at the moment, but
>>> if I did, it would be nice to have the format described above.
>>>
>>> cheers
>>> Garratt
>>>
>>>
>>> On Tue, 2010-10-12 at 17:01 +0900, Geoffrey Biggs wrote:
>>>> On 12/10/10 15:08, Radu Bogdan Rusu wrote:
>>>>> Hmm, I'm still not 100% convinced... I know that from a technical
>>>>> perspective, we can implement it without blinking twice :) but isn't it
>>>>> just plain wrong in terms of "semantics"?
>>>>>
>>>>> This means that we need to drop sending/receiving single PointCloud2
>>>>> messages, and always use vectors of them. Is that really ok? Looking at
>>>>> RViz (not that we are similar in any way with PCL, but just as an
>>>>> example), it seems that the solution there was to add "Marker.msg" and
>>>>> "MarkerArray.msg" as two separate messages.
>>>>
>>>> The way I see it, there are three potential events:
>>>>
>>>> - A node uses only a single point cloud internally.
>>>> - A node may use zero, one, or more point clouds (or any combination of
>>>> these).
>>>> - A node requires two or more point clouds.
>>>>
>>>> The only data type that covers all three is an array of point clouds.
>>>> Having a single-point cloud message is certainly technically feasible,
>>>> and best in many cases, but it means that nodes that may use one or more
>>>> cannot be directly connected to nodes that produce just one. So the
>>>> choice is to have all produce an array, even if it only contains one, or
>>>> to require some kind of translator node for those that require an array.
>>>> Having a node accept from two topic types is, I think, hacky and
>>>> potentially confusing ("Which one do I use when I only have one cloud?"
>>>> "Is there a difference in behaviour between the single cloud topic and
>>>> the array topic in the case of just one cloud?" etc).
>>>>
>>>> Geoff
>>>> _______________________________________________
>>>> [hidden email] / http://pcl.ros.org
>>>> https://code.ros.org/mailman/listinfo/pcl-users
>>>
>>>
>
>
> _______________________________________________
> [hidden email] / http://pcl.ros.org
> https://code.ros.org/mailman/listinfo/pcl-users
_______________________________________________
[hidden email] / http://pcl.ros.org
https://code.ros.org/mailman/listinfo/pcl-users
Reply | Threaded
Open this post in threaded view
|

Re: addition to sensor_msgs/PointCloud2

gre721
This post has NOT been accepted by the mailing list yet.
Hello,

could you solve this issue.
I am using ROS Kinetic with perception_pcl stack and I still don't know how to extract multiple point clouds.

Thanks!