Yarp: Broken handling of timestamp of robot motor ports

[pattacini]

I've verified that 6ecffb7163fa380a98066bf7d58fa0d42878c9e9 (i.e. devel at the time of writing) show a critical bug 🐞 in the handling of the timestamp attached to the robot motor ports.

The timestamp on the robot does not increment, remaining frozen, while on iCub_Sim the timestamp deltas are only of 1s each - strangely.

The test can be run easily via:

$ yarp read ... /[icub|icubSim]/head/state:o envelope

Alternatively, you can connect the dumper:

$ yarpdatadumper --connect /[icub|icubSim]/head/state:o

and then explore the content of the file containing the logged data.

cc @vvasco @traversaro @francesco-romano @claudiofantacci @drdanz @aerydna

[traversaro]

I am unable to replicate the same bug in Gazebo on macOS Sierra 10.12.5 (16F73) with YARP version 6ecffb7163fa380a98066bf7d58fa0d42878c9e9 , i.e. :
~
$ yarp read ... /icubSim/head/state:o envelope
~
works fine.

[barbalberto]

From what you guys tell, since the behavior is different from platform to platform, it may be related to how each device implements the getEncoderTimed.

I'll take a look

[barbalberto]

Ok, I found out that actually the setEnvelope / getEnvelope functions are not working properly.
Here is a small example of data sent vs received:

| sent | received |
|:------:|:-------:|
| 0.0001200000 |0.000120 |
| 0.0012300000 |0.001230 |
| 0.0123400000 |0.012340 |
| 0.1234500000 |0.123450 |
| 1.2345600000 |1.234560 |
| 12.3456700000 |12.345700 |
| 123.4567800000 |123.457000 |
| 1234.5678900000 |1234.570000 |
| 12345.6789000000 |12345.700000 |
| 123456.7890100000 |123457.000000 |
| 1234567.8901199999 | 1234570.000000 |
| 12345678.9012299981 | 12345700.000000 |
| 123456789.0123399794 | 123457000.000000 |
| 1234567890.1234498024 | 1234570000.000000 |
| 12345678901.2345561981 | 12.345700 |
| 123456789012.3456420898 | 123457000000.000000 |
| 1234567890123.4565429688 | 1234570000000.000000 |
| 12345678901234.5664062500 | 12345700000000.000000 |
| 123456789012345.6562500000 | 123457000000000.000000 |
| 1234567890123456.5000000000 | 1234570000000000.000000 |
| 12345678901234564.0000000000 | 12345700000000000.000000 |
| 123456789012345632.0000000000 | 123457000000000000.000000 |
| 1234567890123456256.0000000000 | 1234570000000000000.000000 |
| 12345678901234563072.0000000000 | 12345700000000000000.000000 |
| 123456789012345634816.0000000000 | 123.457000 |
| 1234567890123456249856.0000000000 | 1234569999999999934464.000000 |
| 12345678901234563547136.0000000000 | 12345699999999999344640.000000 |
| 123456789012345635471360.0000000000 | 123456999999999993446400.000000 |

So it looks like the double used for timestamp is handled with a fixed small number of meaningful digits.
Gazebo works only because the timing starts from zero.

[Nicogene]

My bad, in #1356 I added this commit 704c600c01213f7b9f78d835da1da729bf2c173e for fixing the double truncation issue in the Property created fromString.
@barbalberto fixed it in #1400 :pray:
As soon as passes all tests(Travis and appveyor) we can merge it.
Sorry guys for the inconvenience, fixing a bug ( #1057 ) I created another one :see_no_evil:

[pattacini]

Thank you @Nicogene and @barbalberto for fast reacting! 👍

Even though I think we can improve the way we cover the code with our testing, this time the problem is really upstream, that is how we send the timestamp (as string rather than double).

Bad designs create connections among components so distant that nobody can think of.

[Nicogene]

I totally agree with you @pattacini , I think that the "real" bug is that the envelope is sent ALWAYS as string. In my opinion we have to give high priority priority to fix this issue, probably we can reduce the overhead in the communications.

[barbalberto]

Yes, I'm already on it :muscle:

[francesco-romano]

I agree that this should be high priority.
Just a comment: serialise a double to byte is not trivial.
IEEE 754 for example

[pattacini]

@francesco-romano
isn't there a way in yarp to deal with data encapsulation/serialization of the envelope without fiddling with such a low-level details?

[francesco-romano]

I don't know. Probably things work as we use similar architectures, and nobody tested YARP communication between an x86 and a ppc machine.

[barbalberto]

This is not about serialization to bytes, but printing as string.

[pattacini]

yep @barbalberto I think @francesco-romano was referring to the near future when we'll be transmitting a double in place of a string.

[barbalberto]

That's never been a problem. I mean, this issue happens only on timestamp, because somehow a toString is called when it should not.
All the double ever sent through yarp are actually sent in binary mode, therefore here we will do the same.
Please do not think that all doubles are always sent as string, only timestamps.

[pattacini]

Doubles are certainly handled in the comm correctly otherwise we could have called it a day far before this point 😉

Out of sheer curiosity, is the packet counter attached to the envelope an integer, or is it a string as well?

Perhaps, originally, the envelope was meant to be a sort of general purpose - yet naive - container for any kind of information, ahead of the need for transmitting timestamps.

[claudiofantacci]

Perhaps, originally, the envelope was meant to be a sort of general purpose - yet naive - container for any kind of information, ahead of the need for transmitting timestamps.

This is a valid point. I'm curious as well.

[barbalberto]

I just found this comment from Paul Fitzpatrick in PortCore.cpp

    // It looks like envelopes are constrained to be printable ASCII?
    // I'm not sure why this would be.  TODO check.

If _he_ doesn't know, it's probably gonna be tougher than expected.

[claudiofantacci]

@barbalberto I reckon that you are capable of handling this.
Should you need any help I'm also sure that you will find valuable people ready for support.

[drdanz]

Just a comment: serialise a double to byte is not trivial.

@francesco-romano In yarp we have NetFloat32 and NetFloat64 that should handle endianness