robotology / superquadric-model

Framework for modeling and visualizing objects through superquadrics
GNU General Public License v2.0
6 stars 4 forks source link

How to deal with RPC communication #34

Open pattacini opened 6 years ago

pattacini commented 6 years ago

As already anticipated in #33, RPC comm doesn't seem to be working too, in the conditions detailed in the tutorial as well as in the PR.

@giuliavezzani could you please take a look and instruct us on how to proceed in this respect?

cc @fbottarel

giuliavezzani commented 6 years ago

If I understood well, you managed to solve the issues you had with streaming mode in standalone with #33, while you are not able to use the RPC, am I right? In this case, could you please detail your use case?

I had a look of the code I used in the superquadric-grasp-demo code and this is slighty different from what I wrote in the tutorial. I don't know if this might help.

For sure, as soon as you'll tell me more details about your use case, I'm gonna solve the issue as long as I'll be able reproduce the behavior locally on my computer (i.e. reading points from file) or, hopefully, just to update the documentation.

pattacini commented 6 years ago

am I right?

Yep, correct.

You're sending in the code you referred a 6-dimensional point cloud, comprising - I imagine - colors. However, in the documentation it's said 3-dimensional point clouds are expected.

Also, getPoints3D used in streaming does expect 3-dimenional data: https://github.com/robotology/superquadric-model/blob/aa709917be6a493a80368aebdf4b8958752e76b7/src/superqComputation.cpp#L594-L603

I believe it should be fairly simple to run this experiment. You would need only to compile VTK 8.1.0:

git clone git://vtk.org/VTK.git
cd VTK
git checkout v8.1.0
mkdir build && cd build
cmake ..
make

Finally, export from .bashrc the env variable VTK_DIR pointing to the build. Be careful since in Xenial there's a VTK old version pre-installed.

The experiment makes use of streaming as said, but can be easily tailored to use RPC.

giuliavezzani commented 6 years ago

@pattacini I performed the tests you asked for.

Here is the correct way to communicate with the /superquadric-model/rpc port:

// Connect to the rpc port
superqRpc.open("/test-superquadric/rpc:i");
if (!Network::connect(superqRpc.getName(),"/superquadric-model/rpc"))
{
      yError()<<"Unable to connect to superquadric-model rpc ";
      close();
      return false;
}
// Send the object point cloud
Bottle cmd, superq_b;
cmd.addString("send_point_clouds");
Bottle &in1=cmd.addList();
 for (size_t i=0; i<dwn_points.size(); i++)
{
      Bottle &in=in1.addList();
      in.addDouble(dwn_points[i][0]);
      in.addDouble(dwn_points[i][1]);
      in.addDouble(dwn_points[i][2]);
      in.addDouble(dwn_points[i][3]);
      in.addDouble(dwn_points[i][4]);
      in.addDouble(dwn_points[i][5]);
}
superqRpc.write(cmd, superq_b);

// Ask for the estimated superquadric
cmd.clear();
cmd.addString("get_superq"); // Return the standard superquadric
/*cmd.addString("get_superq_filtered");  Return the filtered superquadric
                                       (if multiple point clouds are sent) */
superqRpc.write(cmd, superq_b);
yInfo()<<"Received superquadric: "<<superq_b.toString();

// Then you have to extract the superquadric parameters from Bottle superq_b

I found also two bugs in the superquadric-model.

1) The get_superquadric rpc command was not updated and didn't work. Fixed now. 2) I have again synchronization problems. I though I fixed this problem before leaving IIT, but apparently the changes I made were not robust enough and they might be inconsistent with the new yarp version (is it?).

In the branch fix-rpc of the superquadric-model I fixed 1) corretly, by updating the rpc command. Instead, I just used a patch for 2) : I add a Time::delay(0.5) in the get_superq and get_superq_filtered commands. Without the delay they are not waiting until the superquadric is computed and they might return a null superquadric.

As soon as I'll have more time, I'll fix properly 2) and I'll update the documentation both in the source codeand in the tutorial page (as soon as it will be merged in master).

@pattacini, if you can test the code, it'd be helpful. I hope I did things properly.

pattacini commented 6 years ago

Thank you @giuliavezzani for looking at this.

Let me raise the following point though. RPC communication shall be self-contained and self-consistent, meaning that we forward the request awaiting the corresponding reply in one single command. This allows guaranteeing synchronization between the request and the reply intrinsically.

Here, instead, two consecutive requests are necessary (plus the tweak of the synch you had to apply), ending up with no difference at all with the streaming mode. Therefore, I tend to believe we should correct this behavior having only one RPC service which receives as input the point cloud and gives back the superquadric parameters.

Another point. Why are we required to send colors as well? They're not used in the optimization. Perhaps, it's for the sake of saving the point cloud along with the resulting superquadric in a file. However, this way we have clearly an asymmetry with the streaming mode, which instead does not expect color information. To this end, one possibility would be to make color optional for RPC (and maybe also for streaming), that is the service automatically detects whether the color information is present and thus it'll take it into consideration.

giuliavezzani commented 6 years ago

Therefore, I tend to believe we should correct this behavior having only one RPC service which receives as input the point cloud and gives back the superquadric parameters.

I'll fix it designing new rpc commands following your (correct) policy. Honestly, I don't remember why I switched to this new weird use of rpc.

Why are we required to send colors as well? They're not used in the optimization. Perhaps, it's for the sake of saving the point cloud along with the resulting superquadric in a file.

Correct.

To this end, one possibility would be to make color optional for RPC (and maybe also for streaming), that is the service automatically detects whether the color information is present and thus it'll take it into consideration.

I'll do something like that. Sending the colours will be optional and available also in streaming mode.

If you don't mind, I'll open new issues #35 and #36 for dealing with these fix and as a brief reminder to me and I'll leave this open for discussion.