robotology / robometry

Telemetry suite for logging data from your robot 🤖
https://robotology.github.io/robometry
Other
14 stars 9 forks source link

Add the possibility to store the elements_names for each channel #161

Closed GiulioRomualdi closed 2 years ago

GiulioRomualdi commented 2 years ago

This PR adds a new entry for each stored variable called elements_names. The entry is a cell that will contain the name of each element of a variable.

BufferInfo is now a struct that contains the name, the dimension, and the elements_names.

cc @traversaro @S-Dafarra @Nicogene @AlexAntn

GiulioRomualdi commented 2 years ago

Hi @Nicogene I updated the documentation https://github.com/robotology/yarp-telemetry/pull/161/commits/31eba39d95d6259d72e54ad7f1ddf262dc6b689e and I added a test https://github.com/robotology/yarp-telemetry/pull/161/commits/9266fd2df3fe40f2ad2adaaf9be2b0bca9e3acd5

I've also noticed that the project version is 0.2.0, even if the current release is 0.4.0. Do you think it is possible to bump the CMake project version to 0.4.100 as soon as this PR is merged so the consumer application can ask for yarp_telementry 0.4.100 to use the new features implemented?

https://github.com/robotology/yarp-telemetry/blob/f291b7dc0d32f9d217a8a14e2eaf13ad3b3469d8/CMakeLists.txt#L9

GiulioRomualdi commented 2 years ago

I'm not able to figure out why the conda ci on windows is failing

traversaro commented 2 years ago

I'm not able to figure out why the conda ci on windows is failing

I was able to reproduce the failure locally.

traversaro commented 2 years ago

I'm not able to figure out why the conda ci on windows is failing

I was able to reproduce the failure locally.

On Windows yarp-telemetry manages the visibility of functions manually, different from most of our project were we rely on CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS. Now that ChannelInfo is a proper type, we need to mark it is as visible by declaring it as:

diff --git a/src/libYARP_telemetry/src/yarp/telemetry/experimental/BufferConfig.h b/src/libYARP_telemetry/src/yarp/telemetry/experimental/BufferConfig.h
index ae8bb0e..af19eea 100644
--- a/src/libYARP_telemetry/src/yarp/telemetry/experimental/BufferConfig.h
+++ b/src/libYARP_telemetry/src/yarp/telemetry/experimental/BufferConfig.h
@@ -23,7 +23,7 @@ using elements_names_t = std::vector<std::string>;
  * @brief Struct representing a channel(variable) in terms of
  * name and dimensions and names of the each element of a variable.
  */
-struct ChannelInfo {
+struct YARP_telemetry_API  ChannelInfo {
     std::string name; /**< Name of the channel */
     dimensions_t dimensions; /**< Dimension of the channel */
     elements_names_t elements_names; /**< Vector containing the names of each element of the channel */

fyi @GiulioRomualdi

Nicogene commented 2 years ago

I've also noticed that the project version is 0.2.0, even if the current release is 0.4.0. Do you think it is possible to bump the CMake project version to 0.4.100 as soon as this PR is merged so the consumer application can ask for yarp_telementry 0.4.100 to use the new features implemented?

Ah good catch! Yes I forgot to bump it, if you can change it otherwise I can do it later thanks!

GiulioRomualdi commented 2 years ago

I've also noticed that the project version is 0.2.0, even if the current release is 0.4.0. Do you think it is possible to bump the CMake project version to 0.4.100 as soon as this PR is merged so the consumer application can ask for yarp_telementry 0.4.100 to use the new features implemented?

Ah good catch! Yes I forgot to bump it, if you can change it otherwise I can do it later thanks!

I can change it 😄

GiulioRomualdi commented 2 years ago

I rebased the PR on master and I updated the CMakeLists.txt file.

GiulioRomualdi commented 2 years ago

The conda ci is failing on unix systems. However I don't think it is related to this PR