Add to TelemetryDeviceDumper streaming of rawValues from low level

[MSECode]

Add to the telemetryDeviceDumper a device based on the nwc rawValuesPublisherClient thus to save in the MATLAB file generated by this plugin the raw values (and whatever one desired for debugs based on the Interface iCub::debugLibrary::IRawValuesPublisher) coming from the boards at the fw level.

Side Note: I'll open this PR since I've tested the update done on my bench setup. However, the work, even though at a good point, cannot be considered fully completed. The PR will be kept in draft for now and used for discussion on the updates done and to revise them together

cc: @valegagge @Nicogene @traversaro

[S-Dafarra]

To be honest, I believe that the dependency on iCub is not the best here, as it would complexify its usage with other robots. An alternative could be to instead create a similar device in icub-main instead

[valegagge]

To be honest, I believe that the dependency on iCub is not the best here, as it would complexify its usage with other robots. An alternative could be to instead create a similar device in icub-main instead

Hi @S-Dafarra ,
I don't understand your suggestion, sorry! :( The new device belongs to icub-main, so here we add the dependence from it.

[valegagge]

Hi @MSECode ,
for me is ok.
I found a small bug related to the use of logControlBoardQuantities with false value, but it seems not dependent on your changes.
I fixed here. Asap I'll put the code (I need the permission.)

[S-Dafarra]

The new device belongs to icub-main, so here we add the dependence from it.

That's the problem, the new dependency from icub-main, while it should be the other way around. In other words, it should be icub-main to depend from robometry, and this can be done by moving or copying the TelemetryDeviceDumper device into icub-main.

[Nicogene]

The new device belongs to icub-main, so here we add the dependence from it.

That's the problem, the new dependency from icub-main, while it should be the other way around. In other words, it should be icub-main to depend from robometry, and this can be done by moving or copying the TelemetryDeviceDumper device into icub-main.

Note that the dependency from icub-main would be added only if telemetryDeviceDumper is compiled, and it is disabled by default.

In any case, I am more comfortable to move it eventually in icub-main, rather than have yet another logger device around

[S-Dafarra]

In any case, I am more comfortable to move it eventually in icub-main, rather than have yet another logger device around

I agree! Your call anyway 👍

[Nicogene]

Another possibility is to add a find_package(iCub QUIET) and then use a ifdef in the telemetryDeviceDumper code, this would keep it compilable also without icub-main

[valegagge]

Hi @Nicogene and @S-Dafarra ,
I'm sorry for not getting back to you sooner.

@S-Dafarra : when we decided to add this functionality we knew we were adding a dependency to icub-main, but agreed with @Nicogene we decided to go ahead.
As you point out maybe it is too strong to make this dependency permanent.
If I can post my few cents, I think we can keep the dependency from Telemetrydumper optional as @Nicogene suggested in the previous comment.

What do you think?

[S-Dafarra]

If I can post my few cents, I think we can keep the dependency from Telemetrydumper optional as @Nicogene suggested in the previous comment.

What do you think?

I believe it is not ideal. If in a near future we decide to enable it by default, this can increase the compilation time of the supebuild for example, as everything that depends on robometry will need to wait for icub-main to be compiled, that in turn depends on yarp. The other way around would make it smoother

[valegagge]

Hi @MSECode ,
this is the commit of the fis I mentioned here.

[MSECode]

Hi @MSECode , this is the commit of the fis I mentioned here.

Yes, I saw it and squashed to 1 single commit for the PR.
Let's now decide how to managed the CMakeLists.txt file for what said in the thread and eventually add the fixes and proceed with the PR. Is it OK?

[valegagge]

As discussed with @MSECode and @Nicogene , we'll organize a meeting with all the involved people to define if moving the device to icub-main or not or, in any case, point out the best solution.

[S-Dafarra]

Let me add that the device is already dependent on yarp, as it should be, so in any case, we already need for yarp to be compiled before. In general, I keep not liking to have icub-main as a dependency here because it hinders any usage of robometry within icub-main. Still, I don't want to make a big fuzz out of this, so feel free to proceed with the solution you feel more suitable.

[valegagge]

I created a meeting to address this discussion.

[pattacini]

I didn't follow this discussion in the first place, but probably a better place for the I/F's would have been YARP itself. The implementation of the devices can stay in icub-main, but we usually put the I/F's in YARP right to address this kind of situation.

At any rate, we can discuss this topic with the ad-hoc meeting you scheduled.

[traversaro]

I didn't follow this discussion in the first place, but probably a better place for the I/F's would have been YARP itself. The implementation of the devices can stay in icub-main, but we usually put the I/F's in YARP right to address this kind of situation.

At any rate, we can discuss this topic with the ad-hoc meeting you scheduled.

We can discuss in person, but the idea was to have the interface somewhere maintained by iCub Tech to be able to quicky iterate if necessary.

[Nicogene]

Have you bumped the patch version of icub-main? I would put here the required version that includes the new interface

[MSECode]

Yes, actually as suggested by @pattacini we bumped the minor version, current version is 2.7.0

https://github.com/robotology/icub-main/blob/devel/CMakeLists.txt

[Nicogene]

Great! It should be then find_package(iCubDev 2.7.0 REQUIRED), and I would also add it in the readme documentation of the device that that version of icub-main is required

[MSECode]

done

[Nicogene]

Some small changes requested