Add iwear_logger wrapper device and AddInstallRPathSupport cmake module #113

prashanthr05 · 2021-05-11T13:23:59Z

This PR,

adds an AddInstallRPATHSupport cmake module that was required to load the shared objects privately linked by the YARP device plugin.
adds an iwear_logger wrapper device that can be attached to any wearable producer to log the data from all the sensors associated with the producer.

At the heart of its implementation, It replaces,

the iwear_wrapper's logic of encapsulating the wearable sensors data into a WearableData msg and passing through a YARP port

with

yarp-telemetry's logic of dumping the data into a mat file.

A template for configuring the iwear_logger wrapper device can be as follows,

<?xml version="1.0" encoding="UTF-8" ?>
<!DOCTYPE robot PUBLIC "-//YARP//DTD yarprobotinterface 3.0//EN" "http://www.yarp.it/DTD/yarprobotinterfaceV3.0.dtd">
<robot name="robot-name" build=0 portprefix="">
    <!--Producer Device -->
    <device type="producer_name" name="ProducerDevice">
        <!-- producer parameters go here -->
    </device>
    
    <device type="iwear_logger" name="ProducerLoggerDevice">
        <param name="period">0.01</param>
        
        <param name="logAllQuantities">false</param>
        <param name="logAccelerometers">false</param>
        <param name="logEMGSensors">false</param>
        <param name="logForce3DSensors">false</param>
        <param name="logForceTorque6DSensors">true</param>
        <param name="logFreeBodyAccelerationSensors">true</param>
        <param name="logGyroscopes">true</param>
        <param name="logMagnetometers">false</param>
        <param name="logOrientationSensors">true</param>
        <param name="logPoseSensors">false</param>
        <param name="logPositionSensors">false</param>
        <param name="logTemperatureSensors">false</param>
        <param name="logTorque3DSensors">false</param>
        <param name="logVirtualLinkKinSensors">false</param>
        <param name="logVirtualJointKinSensors">false</param>
        <param name="logVirtualSphericalJointKinSensors">false</param>                
        
        <param name="saveBufferManagerConfiguration">true</param>
        <param name="experimentName">test_iwear_logger</param>
        <param name="path">/path/to/some/folder/</param>
        <param name="n_samples">100000</param>
        <param name="save_periodically">true</param>
        <param name="save_period">120.0</param>
        <param name="data_threshold">300</param>
        <param name="auto_save">true</param>

        <action phase="startup" level="5" type="attach">
            <paramlist name="networks">
            	<!-- attach the source device here -->
                <elem name="ProducerDeviceWrapperLabel"> ProducerDevice </elem>
            </paramlist>
        </action>
        <action phase="shutdown" level="5" type="detach"/>
    </device>

</robot>

The data is saved as a <experimentName>_<timestamp>.mat in the specified <path> parameter.
This mat file contains a struct called <experimentName>

The sensors data are stored as a struct one for each wearable sensor. The name of this struct is a modified format of Wearable sensor name accounting for Matlab variable name handling. (Special characters like @/()# are converted to underscores and the wearable separator is converted to underscore.)

<producer-device-name>_<sensor-type>_<sensor-name>
   --- data
   --- dimensions
   --- name
   --- timestamps

TODO

Update CHANGELOG
Add example xml file to app/xml
Update CI for yarp-telemetry
Update robology-superbuild's wearable dependency for yarp-telemetry (Add YARP_telemetry dependency for wearables robotology-superbuild#730)

Closes #112

prashanthr05 · 2021-05-11T13:26:01Z

I need to update the CI for yarp-telemetry.

traversaro · 2021-05-11T13:33:15Z

FYI @Nicogene @AlexAntn

@prashanthr05 can you add the dependency of wearables on yarp-telemetry on the supernbuild? Thanks!

lrapetti · 2021-05-11T13:53:19Z

Thanks a lot @prashanthr05, this is indeed a very useful tool that has always been missing.

Just a comment on your description

It replaces,

the iwear_wrapper's logic of encapsulating the wearable sensors data into a WearableData msg and passing through a YARP port

the iwear_remapper is not replaced by itself, since it remains the device responsible for streaming iwear data on the network through yarp ports. It rather replaces the pipeline where the yarpdatadumper was used to save those data.

prashanthr05 · 2021-05-11T13:54:32Z

Thanks a lot @prashanthr05, this is indeed a very useful tool that has always been missing.

Just a comment on your description

It replaces,

the iwear_wrapper's logic of encapsulating the wearable sensors data into a WearableData msg and passing through a YARP port

the iwear_remapper is not replaced by itself, since it remains the device responsible for streaming iwear data on the network through yarp ports. It rather replaces the pipeline where the yarpdatadumper was used to save those data.

True, I meant those lines at an implementation level and not at an architecture level. I will update the comment for clarity.

lrapetti

Before going through the code, I have a couple of comments. Can you please update the CHANGELOG.md with the changes, and can you upload the example XML in https://github.com/robotology/wearables/tree/master/app/xml?

prashanthr05 · 2021-05-11T14:18:50Z

Before going through the code, I have a couple of comments. Can you please update the CHANGELOG.md with the changes, and can you upload the example XML in https://github.com/robotology/wearables/tree/master/app/xml?

This is done in 7016cbd and 1940f66

lrapetti

Together with the sensor data, we can dump also the status of each sensor that is exposed with getSensorStatus method.

prashanthr05 · 2021-05-11T15:51:37Z

Together with the sensor data, we can dump also the status of each sensor that is exposed with getSensorStatus method.

Do you think it would be a good idea to prefix the actual measurements with the sensor status?
Otherwise, if we would like it as a separate vector, we might have to propose a feature request in yarp-telemetry.

lrapetti · 2021-05-11T15:55:29Z

wrappers/IWearLogger/src/IWearLogger.cpp

+    while (pImpl->iWear->getStatus() == WearStatus::Calibrating
+           || pImpl->iWear->getStatus() == WearStatus::WaitingForFirstRead) {
+        if (pImpl->waitingFirstReadCounter++ % 1000 == 0) {
+            pImpl->waitingFirstReadCounter = 1;
+            yInfo() << logPrefix << "IWear interface waiting for first data. Waiting...";
+        }
+        return;
+    }
+
+    if (pImpl->iWear->getStatus() == WearStatus::Error
+            || pImpl->iWear->getStatus() == WearStatus::Unknown) {
+        yError() << logPrefix << "The status of the IWear interface is not Ok ("
+                 << static_cast<int>(pImpl->iWear->getStatus()) << ")";
+        askToStop();
+        return;
+    }
+
+    // case status is TIMEOUT or DATA_OVERFLOW
+    if (pImpl->iWear->getStatus() != WearStatus::Ok) {
+        yWarning() << logPrefix << "The status of the IWear interface is not Ok ("
+                 << static_cast<int>(pImpl->iWear->getStatus()) << ")";
+    }


Just a general observation, not strictly related to this PR.

The status that we are reading here is the status of the IWear interface, which should basically return the status of the device that is streaming the sensors data (e.g. getting the status of the driver), and in general is independent from the status of each sensor.
Indeed, it is possible that the status of the device is Ok, but the status of one of the sensor interface is different (and that's why it makes sense to dump it as I was pointing out).

Unfortunately, we are often using an IWearRemapper device in between the producer device, and the consumer device (in this case would be the iwear_logger), which merge data from multiple sources.

In this case, however, the state returned by getStatus() is the "worst status" that is found among all the associated sensors interface (see https://github.com/robotology/wearables/blob/master/devices/IWearRemapper/src/IWearRemapper.cpp#L802-L845). As a consequence, the specific status of the sensors interface is hidden.

I think we can eventually discuss the current choice in the IWearRemapper, and change it.

These lines were a copy-paste from iWearWrapper. Do you think this might not be needed here?

These lines were a copy-paste from iWearWrapper. Do you think this might not be needed here?

No, I think is fine as it is. I would rather change some logic on the Remapper side

lrapetti · 2021-05-11T15:57:45Z

Together with the sensor data, we can dump also the status of each sensor that is exposed with getSensorStatus method.

Do you think it would be a good idea to prefix the actual measurements with the sensor status?
Otherwise, if we would like it as a separate vector, we might have to propose a feature request in yarp-telemetry.

Maybe we can add it as a prefix, and meanwhile propose to add it in yarp-telemetry if it can be useful also for other applications.

prashanthr05 · 2021-05-12T10:32:12Z

Together with the sensor data, we can dump also the status of each sensor that is exposed with getSensorStatus method.

Do you think it would be a good idea to prefix the actual measurements with the sensor status?
Otherwise, if we would like it as a separate vector, we might have to propose a feature request in yarp-telemetry.

Maybe we can add it as a prefix, and meanwhile propose to add it in yarp-telemetry if it can be useful also for other applications.

Done in 1b87ac3

traversaro · 2021-05-12T11:07:51Z

I don't know what's going on with the Windows CI. For me we can disable it and substitute it in a following PR with a conda-based CI that gets YARP in binary form.

prashanthr05 · 2021-05-12T11:10:49Z

Apart from that, looks like CI Ubuntu checks on iwear_logger addition seems to be working now. We can wait for macOS for confirmation.

prashanthr05 · 2021-05-12T11:18:13Z

@prashanthr05 can you add the dependency of wearables on yarp-telemetry on the supernbuild? Thanks!

@traversaro , for this adding the line

find_or_build_package(YARP COMPONENTS telemetry QUIET)

to https://github.com/robotology/robotology-superbuild/blob/master/cmake/Buildwearables.cmake must suffice right?

Nicogene · 2021-05-12T11:57:42Z

@prashanthr05 the find_or_build_package should be like this

find_or_build_package(YARP_telemetry)

(See https://github.com/robotology/robotology-superbuild/blob/0a336d66d2d438243bdc12298767350460363cde/cmake/RobotologySuperbuildLogic.cmake#L51)

I am not sure if COMPONENTS works with that macro

prashanthr05 · 2021-05-12T12:01:28Z

@prashanthr05 the find_or_build_package should be like this

find_or_build_package(YARP_telemetry)

(See https://github.com/robotology/robotology-superbuild/blob/0a336d66d2d438243bdc12298767350460363cde/cmake/RobotologySuperbuildLogic.cmake#L51)

I am not sure if COMPONENTS works with that macro

Thanks for the heads up!! @Nicogene

lrapetti

I think we can proceed merging, and open a separate issue to move to conda-based Windows CI as @traversaro was suggesting

lrapetti · 2021-05-12T15:19:24Z

I think we can proceed merging, and open a separate issue to move to conda-based Windows CI as @traversaro was suggesting

@prashanthr05 let me know if you want to clean the commit history or we can squash them and merge

prashanthr05 · 2021-05-12T15:27:15Z

I think we can proceed merging, and open a separate issue to move to conda-based Windows CI as @traversaro was suggesting

@prashanthr05 let me know if you want to clean the commit history or we can squash them and merge

I have reorganized the commits.

lrapetti · 2021-05-13T08:25:39Z

Thanks @prashanthr05!

diegoferigo · 2021-05-13T09:05:33Z

Sorry for coming late to this PR, it looks good @prashanthr05! Though, it seems that your IDE didn't use clang-format. The next time someone is going to edit the file, a possibly big diff will get generated. If possible, I'd fix the style in a new PR. I'm not sure if recently the style has been enforced on all files @lrapetti @Yeshasvitvs.

lrapetti · 2021-05-13T09:20:07Z

If possible, I'd fix the style in a new PR. I'm not sure if recently the style has been enforced on all files @lrapetti @Yeshasvitvs.

On my side, not really. Do you think it would make sense to open a PR to fix the style in the whole project?

diegoferigo · 2021-05-13T09:42:11Z

Until I was the maintainer of this project, I was enforcing the style strictly. With low priority, it could be done. I always thought to start using github actions for that, but I still haven't found enough time to check what the community offers.

prashanthr05 · 2021-05-13T14:00:17Z

Though, it seems that your IDE didn't use clang-format.

I am using KDevelop. I tried adding the source-formatting settings to support the clang-format and reformatted the source. Unfortunately, there's no change from the reformatting. Looks like something is not working on my IDE.

diegoferigo · 2021-05-13T14:17:04Z

Mmh ok, qtcreator automatically detects the .clang-format in the root of the repo and uses it for all the files. No idea about KDevelop.

I checked again the sources, and for sure the style is not uniform. Just check the if brackets, sometimes they are inline and some other time not. Be aware, when you start using clang-format there's no way back 😆 I'm a bit nazi on it, for reasons :)

prashanthr05 requested review from diegoferigo, lrapetti and yeshasvitirupachuri as code owners May 11, 2021 13:23

lrapetti reviewed May 11, 2021

View reviewed changes

lrapetti approved these changes May 12, 2021

View reviewed changes

prashanthr05 added 3 commits May 12, 2021 17:25

Add IWearLogger wrapper device and AddInstallRPath cmake module

f95dea1

Update ci.yml

497953f

Update CHANGELOG.md

36c95fb

prashanthr05 force-pushed the feature/iwear-logger branch from 0baad03 to 36c95fb Compare May 12, 2021 15:26

lrapetti merged commit 33a049c into robotology:master May 13, 2021

lrapetti mentioned this pull request May 13, 2021

Replace Windows CI with conda-based CI #114

Closed

lrapetti assigned prashanthr05 May 13, 2021

prashanthr05 deleted the feature/iwear-logger branch May 13, 2021 08:29

lrapetti added this to the AnDy - Iteration 62 milestone May 13, 2021

prashanthr05 mentioned this pull request May 13, 2021

Add YARP_telemetry dependency for wearables robotology/robotology-superbuild#730

Merged

yeshasvitirupachuri mentioned this pull request May 21, 2021

Update Paexo wearable device to serialize wearable data/commands to access through yarp ports #109

Closed

lrapetti mentioned this pull request Jun 1, 2021

Fix Changelog #119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add iwear_logger wrapper device and AddInstallRPathSupport cmake module #113

Add iwear_logger wrapper device and AddInstallRPathSupport cmake module #113

prashanthr05 commented May 11, 2021 •

edited

Loading

prashanthr05 commented May 11, 2021

traversaro commented May 11, 2021

lrapetti commented May 11, 2021

prashanthr05 commented May 11, 2021

lrapetti left a comment

prashanthr05 commented May 11, 2021

lrapetti left a comment

prashanthr05 commented May 11, 2021

lrapetti May 11, 2021

lrapetti May 11, 2021

prashanthr05 May 12, 2021

lrapetti May 12, 2021

lrapetti commented May 11, 2021

prashanthr05 commented May 12, 2021

traversaro commented May 12, 2021

prashanthr05 commented May 12, 2021

prashanthr05 commented May 12, 2021 •

edited

Loading

Nicogene commented May 12, 2021

prashanthr05 commented May 12, 2021

lrapetti left a comment

lrapetti commented May 12, 2021

prashanthr05 commented May 12, 2021

lrapetti commented May 13, 2021

diegoferigo commented May 13, 2021

lrapetti commented May 13, 2021

diegoferigo commented May 13, 2021 •

edited

Loading

prashanthr05 commented May 13, 2021

diegoferigo commented May 13, 2021

Add iwear_logger wrapper device and AddInstallRPathSupport cmake module #113

Add iwear_logger wrapper device and AddInstallRPathSupport cmake module #113

Conversation

prashanthr05 commented May 11, 2021 • edited Loading

TODO

prashanthr05 commented May 11, 2021

traversaro commented May 11, 2021

lrapetti commented May 11, 2021

prashanthr05 commented May 11, 2021

lrapetti left a comment

Choose a reason for hiding this comment

prashanthr05 commented May 11, 2021

lrapetti left a comment

Choose a reason for hiding this comment

prashanthr05 commented May 11, 2021

lrapetti May 11, 2021

Choose a reason for hiding this comment

lrapetti May 11, 2021

Choose a reason for hiding this comment

prashanthr05 May 12, 2021

Choose a reason for hiding this comment

lrapetti May 12, 2021

Choose a reason for hiding this comment

lrapetti commented May 11, 2021

prashanthr05 commented May 12, 2021

traversaro commented May 12, 2021

prashanthr05 commented May 12, 2021

prashanthr05 commented May 12, 2021 • edited Loading

Nicogene commented May 12, 2021

prashanthr05 commented May 12, 2021

lrapetti left a comment

Choose a reason for hiding this comment

lrapetti commented May 12, 2021

prashanthr05 commented May 12, 2021

lrapetti commented May 13, 2021

diegoferigo commented May 13, 2021

lrapetti commented May 13, 2021

diegoferigo commented May 13, 2021 • edited Loading

prashanthr05 commented May 13, 2021

diegoferigo commented May 13, 2021

prashanthr05 commented May 11, 2021 •

edited

Loading

prashanthr05 commented May 12, 2021 •

edited

Loading

diegoferigo commented May 13, 2021 •

edited

Loading