Added Vector Collection Server for publishing information for real-time users in the YARPRobotLoggerDevice #796

nicktrem · 2024-01-18T14:45:17Z

Edited the functionality of the real-time logger to use a vector collection client/server

… json format

…-visualizer

S-Dafarra

Well done!

GiulioRomualdi · 2024-01-26T16:07:41Z

bindings/python/YarpUtilities/src/VectorsCollection.cpp

+        .def("getMetadata",
+        [](VectorsCollectionClient& impl) -> std::map<std::string, std::vector<std::string>>
+        {
+            BipedalLocomotion::YarpUtilities::VectorsCollectionMetadata metadata;
+            impl.getMetadata(metadata);
+            return metadata.vectors;


Two things:

in Python we tend to use snake case for methods and variables for example get_metadata

Can you directly return a metadata and not the vector? In this case you need to write the bindings for metadata

Added more pyBind methods for the metadata in e5d0cdb. Renamed functions to snake case in 568addd

GiulioRomualdi · 2024-01-26T16:10:26Z

...ces/YarpRobotLoggerDevice/include/BipedalLocomotion/YarpRobotLoggerDeviceVariableTreeNames.h

@@ -0,0 +1,57 @@
+#ifndef BIPEDAL_LOCOMOTION_fRAMEWORK_YARP_ROBOT_LOGGER_DEVICE_VTN_H


I'm not a big fan of these kinds of define. Can you add these names directly in the main class as attributes?

Fixed in 568addd

GiulioRomualdi · 2024-02-02T17:18:32Z

devices/YarpRobotLoggerDevice/src/YarpRobotLoggerDevice.cpp

+}
+
+void YarpRobotLoggerDevice::storeAndSendLoggingData(const std::string& name,
+                                                    const Eigen::VectorXd& data,


You can use eigen ref here

The use of Eigen::Ref to the data parameter causes the data to not be saved correctly, see the image below regarding the mesh:

This is actually a problem. we should understand why

…m/bipedal-locomotion-framework into vectorCollectionServerRT

GiulioRomualdi

Hi @nicktrem, sorry for the delayed response. I've been occupied with various lab activities and personal deadlines, which delayed the review process. Upon revisiting the PR, there are some points that need to be addressed before merging it. Once this is resolved, I'll move on to ami-iit/robot-log-visualizer#77

GiulioRomualdi · 2024-02-28T16:58:08Z

bindings/python/YarpUtilities/src/VectorsCollection.cpp

+             [](const VectorsCollectionMetadata& impl, yarp::os::ConnectionWriter& connection) -> bool {
+                 return impl.write(connection);
+             })
+        .def("toString", &VectorsCollectionMetadata::toString)


Suggested change

.def("toString", &VectorsCollectionMetadata::toString)

.def("to_string", &VectorsCollectionMetadata::toString)

Fixed in 107860c

GiulioRomualdi · 2024-02-28T17:02:22Z

bindings/python/YarpUtilities/src/VectorsCollection.cpp

+        .def("readWireReader",
+             [](VectorsCollectionMetadata& impl, yarp::os::idl::WireReader& reader) -> bool {
+                 return impl.read(reader);
+             })
+        .def("readConnectionReader",
+             [](VectorsCollectionMetadata& impl, yarp::os::ConnectionReader& connection) -> bool {
+                 return impl.read(connection);
+             })
+        .def("writeWireWriter",
+             [](const VectorsCollectionMetadata& impl, const yarp::os::idl::WireWriter& writer) -> bool {
+                 return impl.write(writer);
+             })
+        .def("writeConnectionWriter",
+             [](const VectorsCollectionMetadata& impl, yarp::os::ConnectionWriter& connection) -> bool {
+                 return impl.write(connection);
+             })


This is not clear to me, in theory, there are no bindings of yarp::os::ConnectionWriter and yarp::os::idl::WireReader so you cannot call this method from python. How did you test them? moreover you can check this for function overloading in pybind11 https://pybind11.readthedocs.io/en/stable/classes.html#overloaded-methods

Better to discuss face-to-face

GiulioRomualdi · 2024-02-28T17:04:14Z

src/YarpUtilities/src/VectorsCollectionServer.cpp

+    // if (m_pimpl->isMetadataFinalized)
+    // {
+    //     log()->error("[VectorsCollectionServer::populateMetadata] The metadata has been already "
+    //                  "populated.");
+    //     return false;
+    // }


This needs to be re-added before merging this PR 🤔. Why was it removed in the first place?

This would be better to discuss face-to-face. This has to do with the issue regarding adding new data to the vectors Collection on the fly

GiulioRomualdi · 2024-02-28T17:04:26Z

src/YarpUtilities/src/VectorsCollectionServer.cpp


-    // check if the key has been already used
+    // // check if the key has been already used


Suggested change

// // check if the key has been already used

// check if the key has been already used

Fixed in cf0370b

GiulioRomualdi · 2024-02-28T17:04:37Z

src/YarpUtilities/src/VectorsCollectionServer.cpp

+    // if (!m_pimpl->isMetadataFinalized)
+    // {
+    //     log()->error("{} The metadata has not been finalized.", logPrefix);
+    //     return false;
+    // }


Same comment as before

Better to discuss face-to-face

GiulioRomualdi · 2024-02-28T17:27:06Z

devices/YarpRobotLoggerDevice/src/YarpRobotLoggerDevice.cpp

+        }
+        else if (externalSignalCollection == nullptr && signal.connected)
+        {
+            signal.numMissedPackets++;


This statement is inaccurate. The logger does not consider the period of the exogenous signal. In fact, the signal may arrive at a frequency different from that of the logger, which is acceptable. I will completely remove this logic as it would introduce misleading information. For example, if the exogenous signal arrives from a controller running at 100 Hz while the logger operates at 1 kHz, there are no missing packets.

Better to discuss face-to-face. I understand the issue here. This logic was initially put in to disconnect exogenous signals which were no longer running and to reset the signal.connected flag. If an exogenous signal is not disconnected when a YARP device is restarted, it was noticed that it will not be able to receive new data from that device

GiulioRomualdi · 2024-02-28T17:27:52Z

devices/YarpRobotLoggerDevice/src/YarpRobotLoggerDevice.cpp

+    if(sendDataRT)
+        m_vectorCollectionRTDataServer.sendData();


Suggested change

if(sendDataRT)

m_vectorCollectionRTDataServer.sendData();

if(sendDataRT)

{

m_vectorCollectionRTDataServer.sendData();

}

Fixed in cf0370b

GiulioRomualdi · 2024-02-28T17:29:02Z

devices/YarpRobotLoggerDevice/include/BipedalLocomotion/YarpRobotLoggerDevice.h

@@ -62,6 +65,9 @@ class YarpRobotLoggerDevice : public yarp::dev::DeviceDriver,
    std::unique_ptr<BipedalLocomotion::RobotInterface::YarpSensorBridge> m_robotSensorBridge;
    std::unique_ptr<BipedalLocomotion::RobotInterface::YarpCameraBridge> m_cameraBridge;

+    bool sendDataRT;


Suggested change

bool sendDataRT;

bool m_sendDataRT;

Fixed in 70e0b6f

GiulioRomualdi · 2024-02-28T17:29:25Z

devices/YarpRobotLoggerDevice/include/BipedalLocomotion/YarpRobotLoggerDevice.h

@@ -93,16 +99,17 @@ class YarpRobotLoggerDevice : public yarp::dev::DeviceDriver,
        BipedalLocomotion::YarpUtilities::VectorsCollectionClient client;
        BipedalLocomotion::YarpUtilities::VectorsCollectionMetadata metadata;
        std::string signalName;
+        unsigned int numMissedPackets;


As explained below I would remove this information

Better to discuss face-to-face

GiulioRomualdi · 2024-02-28T17:30:50Z

devices/YarpRobotLoggerDevice/src/YarpRobotLoggerDevice.cpp

+    Eigen::Matrix<double, 1, 1> newMetadataVector;
+    if(newMetadata)
+        m_updatedMetadataSignalVal = (m_updatedMetadataSignalVal + 1) % 2;
+    newMetadataVector << m_updatedMetadataSignalVal;
+
+    m_vectorCollectionRTDataServer.populateData(ROBOT_RT_ROOT_NAME + TREE_DELIM + "newMetadata", newMetadataVector);


It's not clear to me the what is the purpose of this code

Better to discuss face-to-face. This code informs the robot-log-visualizer when new metadata has been added to the vectors collection thus allowing the visualizer to append the data to the variable tree so the user can visualize it. It is a flag

nicktrem · 2024-03-04T14:38:54Z

After face-to-face discussions with @S-Dafarra, @GiulioRomualdi, and @gabrielenava. This PR will be broken up into smaller PRs so it is easier to manage

See the list of smaller PRs below:
Part 1) pull request #817
Part 2) to be created soon
Part 3) to be created soon

nicktrem added 27 commits December 18, 2023 15:54

Edited to test sending of logging data to YARP port

c8134c6

Added a test for streaming data, it was successful

76c01a2

Added dummy value to send for testing

4267db2

temporarily removed sending all of the data for testing purposes

45bade1

Sending FT data to log visualizer

c213ed7

Moved the code for sending data to a more centralized function

c4da8b5

Added more logging data to send to the visualizer

4667433

Added Cartesian Wrench valus to be streamed in real-time

98b0563

Added external data, moved current system of sending data to a better…

d498c21

… json format

Fixed bug related to json key field

cf20448

Cleaned up the code a little bit

296cd2c

Cleaned up the code, removed hard-coded values

a735dd4

Added back motor state logging

f43d2d7

Changed rt logging port name

325d378

Fixed merge conflict

04c14cd

Added Vector Collection Client

5bd0122

Initial test sucessful of sending vector collection data to robot-log…

ca1fffb

…-visualizer

Added get metadata method for pybind11

6ca1fe9

Added metadata values

6284950

Got joint data sending over the vector Collection Network

5f4c3fb

Added more values to send over via the vector collection server

edf8b45

Cleaned up the code

c25f25d

got plot scaling updating correctly

5409fab

Flight data streaming

b83a7f6

Got metadata naming correct

5eabf65

Added ability to stream motor states

554fa1e

Cleaned up the code a bit

f45bfb5

nicktrem requested review from GiulioRomualdi and S-Dafarra as code owners January 18, 2024 14:45

github-actions bot added the 🐍 bindings label Jan 18, 2024

nicktrem added 3 commits January 25, 2024 10:10

Fixed issue with header definition

2ccff2d

Moved to lock guard instead of mutex lock

5ef8bb4

Removed whitespace

d24e886

S-Dafarra approved these changes Jan 25, 2024

View reviewed changes

nicktrem added 2 commits February 1, 2024 17:53

Fixed data prepare bug with vector server

482e195

Merge branch 'master' into vectorCollectionServerRT

3be2c8a

GiulioRomualdi reviewed Feb 2, 2024

View reviewed changes

DanielePucci mentioned this pull request Feb 4, 2024

Adding visualizers ami-iit/comodo#10

Draft

nicktrem added 8 commits February 5, 2024 16:08

Cleaned up the code regarding constants

841138c

Added pybind functions for metadata

e5d0cdb

cleaned up variable locations

568addd

Merge branch 'vectorCollectionServerRT' of https://github.com/nicktre…

c16b958

…m/bipedal-locomotion-framework into vectorCollectionServerRT

Added an option to open the RT port from XML

52499b5

Cleaned up the code a bit more with the function pointers

7244bca

Fixed issue with vector collection server

51f9a8a

Cleaned up the code

cc97f40

github-actions bot added the 🔨 component: YarpUtilities label Feb 13, 2024

nicktrem added 4 commits February 13, 2024 16:33

Removed whitespace

609e3c2

Removed whitespace

3cfaf3d

Fixed but regarding closing and opening logger devices

6c1fdd1

Removed bug regarding metadata

0e166b4

GiulioRomualdi requested changes Feb 28, 2024

View reviewed changes

nicktrem added 5 commits February 29, 2024 10:38

changed function name to snake case

107860c

Updated the git ignore

26f7a3a

Replaced yInfo with log()->info()

0392a04

Fixed formatting

cf0370b

Fixed variable names

70e0b6f

nicktrem closed this Mar 4, 2024

nicktrem mentioned this pull request Mar 4, 2024

RT Capabilities Part 1 - YARP Logger Device #817

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Vector Collection Server for publishing information for real-time users in the YARPRobotLoggerDevice #796

Added Vector Collection Server for publishing information for real-time users in the YARPRobotLoggerDevice #796

nicktrem commented Jan 18, 2024

S-Dafarra left a comment

GiulioRomualdi Jan 26, 2024

nicktrem Feb 6, 2024

GiulioRomualdi Jan 26, 2024

nicktrem Feb 6, 2024

GiulioRomualdi Feb 2, 2024

nicktrem Feb 5, 2024

GiulioRomualdi Feb 28, 2024

GiulioRomualdi left a comment

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024 •

edited

Loading

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024 •

edited

Loading

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

GiulioRomualdi Feb 28, 2024

nicktrem Feb 29, 2024

nicktrem commented Mar 4, 2024 •

edited

Loading

		@@ -0,0 +1,57 @@
		#ifndef BIPEDAL_LOCOMOTION_fRAMEWORK_YARP_ROBOT_LOGGER_DEVICE_VTN_H

	.def("toString", &VectorsCollectionMetadata::toString)
	.def("to_string", &VectorsCollectionMetadata::toString)


		// check if the key has been already used
		// // check if the key has been already used

Added Vector Collection Server for publishing information for real-time users in the YARPRobotLoggerDevice #796

Added Vector Collection Server for publishing information for real-time users in the YARPRobotLoggerDevice #796

Conversation

nicktrem commented Jan 18, 2024

S-Dafarra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GiulioRomualdi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicktrem Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicktrem Feb 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicktrem commented Mar 4, 2024 • edited Loading

nicktrem Feb 29, 2024 •

edited

Loading

nicktrem Feb 29, 2024 •

edited

Loading

nicktrem commented Mar 4, 2024 •

edited

Loading