Added some suggestion for the PR

src/libraries/icubmod/fakeRawValuesPublisher/FakeRawValuesPublisher.cpp

@@ -41,14 +41,21 @@ FakeRawValuesPublisher::FakeRawValuesPublisher()
     m_sawthootTestVal = 0;
     m_rawValuesVectorTag = "rawJomoEncoderValues";
     m_rawDataAuxVector = {};
+    //VALE missing the inizializaion of the map m_rawValuesMetadataMap= {};
 }
 
 bool FakeRawValuesPublisher::getRawDataMap(std::map<std::string, std::vector<std::int32_t>> &map)
 {
     for (auto it = m_rawValuesMetadataMap.begin(); it != m_rawValuesMetadataMap.end(); it++)
     {
-        m_rawDataAuxVector.clear();
-        if(!getRawData(it->first, m_rawDataAuxVector))
+        // VALE: here it should be better to avoid to call the getrawData API because it performes a serach on the map.
+        // this gets down the performance
+        //I suggest you to create a private function that is the core of the getRawData nad call it both here and in getRawData.
+        // This is a suggestion. If you find a better one, it is wellcome!!
+
+        //m_rawDataAuxVector.clear(); moved in getRawData_core
+
+        if(!getRawData_core(it->first, m_rawDataAuxVector)) 
         {
             yCError(FAKERAWVALUESPUBLISHER) << "getRawData() failed. Cannot retrieve all data from local memory";
             return false;
@@ -58,15 +65,23 @@ bool FakeRawValuesPublisher::getRawDataMap(std::map<std::string, std::vector<std
     return true;
 }
 
+bool FakeRawValuesPublisher::getRawData_core(std::string key, std::vector<std::int32_t> &data)
+{
+    //HERe I need to be sure 100% the key exists!!!
+    data.clear();
+    for (uint8_t i = 0; i < m_rawValuesMetadataMap[key].size; i++)
+    {
+        data.push_back(m_sawthootTestVal);
+    }
+
+}
+
 bool FakeRawValuesPublisher::getRawData(std::string key, std::vector<std::int32_t> &data)
 {   
     if (m_rawValuesMetadataMap.find(key) != m_rawValuesMetadataMap.end())
     {
         m_sawthootTestVal = (m_sawthootTestVal < m_sawtoothThreshold) ? (++m_sawthootTestVal) : 0;
-        for (uint8_t i = 0; i < m_rawValuesMetadataMap[key].size; i++)
-        {
-            data.push_back(m_sawthootTestVal);
-        }
+        getRawData_core(key, data); //VALE I changed see comment in getRawDataMap
     }
     else
     {
@@ -79,6 +94,7 @@ bool FakeRawValuesPublisher::getRawData(std::string key, std::vector<std::int32_
 
 bool FakeRawValuesPublisher::getKeys(std::vector<std::string> &keys)
 {
+    //VALE: add keys.clear();
     for (const auto &p : m_rawValuesMetadataMap)
     {
         keys.push_back(p.first);
@@ -95,9 +111,10 @@ int FakeRawValuesPublisher::getNumberOfKeys()
 
 bool FakeRawValuesPublisher::getMetadataMAP(rawValuesKeyMetadataMap &metamap)
 {
-    for (auto [k, v] : m_rawValuesMetadataMap)
+
+    for (auto [k, v] : m_rawValuesMetadataMap) //VALE: add this debug code under debug macro
     {
-        yCDebug(FAKERAWVALUESPUBLISHER) << "size of elements name at key:" << k << "is:" << v.rawValueNames.size();
+        yCDebug(FAKERAWVALUESPUBLISHER) << "size of elements name at key:" << k << "is:" << v.rawValueNames.size(); 
         for (size_t e = 0; e < v.rawValueNames.size(); e++)
         {
             yCDebug(FAKERAWVALUESPUBLISHER) << "GOT to rawValueNames:" << v.rawValueNames[e];
@@ -146,6 +163,8 @@ bool FakeRawValuesPublisher::open(yarp::os::Searchable& config)
     {
         m_rawValuesMetadataMap[m_rawValuesVectorTag].rawValueNames.push_back("fake_jomo_"+std::to_string(i));
     }
+
+    //VALE: init here m_rawDataAuxVector with the correct size(m_numberOfJomos)
     return true;
 }
 

src/libraries/icubmod/fakeRawValuesPublisher/FakeRawValuesPublisher_params.md

@@ -1,3 +1,3 @@
 * |         |  name          | string  | -              |   -           | Yes        | Name of the device | |
-* |         |  njomos        | int     | -              |   -           | Yes        | Numbe of joint or motors to be instantiated | |
+* |         |  njomos        | int     | -              |   -           | Yes        | Number of joint or motors to be instantiated | |
 * |         |  threshold     | int     | -              |   -           | Yes        | Threshold used for defining the amplitude of the sawthooth curve that simulates periodic raw encoder data | |

src/libraries/icubmod/rawValuesPublisherClient/RawValuesPublisherClient.cpp

@@ -116,6 +116,7 @@ bool RawValuesPublisherClient::open(yarp::os::Searchable& config)
             return false;
         }
 
+        //VALE: old commented code. remove this please
         // m_rawValuesMetadata = m_RPCInterface.getMetadata();
 
         // yCDebug(RAWVALUESPUBLISHERCLIENT) << "Got Metadata from m_RPCInterface with size:" << m_rawValuesMetadata.metadataMap.size(); 
@@ -129,7 +130,7 @@ bool RawValuesPublisherClient::open(yarp::os::Searchable& config)
     }
     m_streamingPort.receivedRawDataMap = {};
 
-    yCDebug(RAWVALUESPUBLISHERCLIENT) << "Open complete";
+    yCDebug(RAWVALUESPUBLISHERCLIENT) << "Open complete"; // VALE: use YINFO
     return true;
 }
 
@@ -138,7 +139,7 @@ bool RawValuesPublisherClient::close()
     m_streamingPort.close();
     m_rpcPort.close();
 
-    yCDebug(RAWVALUESPUBLISHERCLIENT) << "Close complete";
+    yCDebug(RAWVALUESPUBLISHERCLIENT) << "Close complete"; // VALE: use YINFO
 
     return true;
 }
@@ -169,6 +170,7 @@ bool RawValuesPublisherClient::getRawData(std::string key, std::vector<std::int3
 
 bool RawValuesPublisherClient::getKeys(std::vector<std::string> &keys)
 {
+    //VALE: I think  this function should rely on the RPC service. So for the moment I sugget to put Not_supported info
     for (const auto &p : m_streamingPort.receivedRawDataMap)
     {
         keys.push_back(p.first);
@@ -179,7 +181,7 @@ bool RawValuesPublisherClient::getKeys(std::vector<std::string> &keys)
 
 int RawValuesPublisherClient::getNumberOfKeys()
 {
-    return 0;
+    return 0; //VALE: I think  this function should rely on the RPC service. So for the moment I sugget to put Not_supported info
 }
 
 bool RawValuesPublisherClient::getMetadataMAP(rawValuesKeyMetadataMap &metamap)
@@ -191,5 +193,5 @@ bool RawValuesPublisherClient::getMetadataMAP(rawValuesKeyMetadataMap &metamap)
 }
 bool RawValuesPublisherClient::getKeyMetadata(std::string key, rawValuesKeyMetadata &meta)
 {
-    return NOT_YET_IMPLEMENTED("getKeyMetadata");
+    return NOT_YET_IMPLEMENTED("getKeyMetadata"); //VALE: Why not implemented?
 }

src/libraries/icubmod/rawValuesPublisherClient/RawValuesPublisherClient.h

@@ -49,6 +49,7 @@ class RawValuesPublisherClient :
         bool open(yarp::os::Searchable& config) override;
         bool close() override;
 
+        /* IRawValuesPublisher methods */
         virtual bool getRawDataMap(std::map<std::string, std::vector<std::int32_t>> &map) override;
         virtual bool getRawData(std::string key, std::vector<std::int32_t> &data) override;
         virtual bool getKeys(std::vector<std::string> &keys) override;

src/libraries/icubmod/rawValuesPublisherServer/RawValuesPublisherServer.cpp

@@ -49,7 +49,7 @@ bool RawValuesPublisherServer::open(yarp::os::Searchable& config)
         return false;
     }
 
-    // TODO: do same for rpc port - Do we need the RPC port, don't we?
+    // TODO: do same for rpc port - Do we need the RPC port, don't we? //VALE: remove the comment
     m_rpcPortName = m_name + "/rpc:o";
 
     // Attach to port
@@ -179,10 +179,10 @@ bool RawValuesPublisherServer::populateMetadata(rawValuesKeyMetadataMap &metamap
 {
     if(!m_iRawValuesPublisher->getMetadataMAP(metamap))
     {
-        yCError(RAWVALUESPUBLISHERSERVER, "Failure in populateMetadata()");
+        yCError(RAWVALUESPUBLISHERSERVER, "Failure in populateMetadata()"); //VALE: you print the error double. here and where you call the function. Please remove one.
         return false;
     }
-
+    //VALE: Put this debug print under debug option. 
     for (auto [k,v] : metamap.metadataMap)
     {
         yCDebug(RAWVALUESPUBLISHERSERVER) << "Metadata at key:" << k << "with size:" << v.size;