Small refactoring of ParametersHandler

[S-Dafarra]

Initially, the refactoring started adding utility functions like make_unique, or the possibility to set the handler from another generic object. Then, @GiulioRomualdi realized (#20) there were issues with the usage of unique_ptr which caused other issues about ownership and how the groups were handled.

One possibility to fix these issues was to return a weak_ptr to the group. In this way, it is clear who owns the group and ideally, any parameter modified in the group would have appeared also in the original handler. But then the issue was that the original handler should have held a shared pointer to group. Then, I decided that the handler should also contain a map where I associate the group name to a shared pointer containing the group. But, how to populate this map? And here the complications started.

The definition of "group" is foggy in yarp. Indeed, any list can be treated as a group. In fact, given its internal representation, the following appeared to be the same:

• "joint_list" (torso neck)
• NEW_GROUP (frame torso).

The actual discriminant is the user himself, meaning that he only knows which key corresponds to a group and which one to a list. Hence, I thought of populating the group map only after a user request. The user call getGroup("gianni"), then I know that "gianni" is a group, create the shared pointer and move the group from the container to the map. Profit (?). Nope, getGroup is const and it absolutely makes sense for it to be so.

Another idea, populate the map together with the container as soon as the handler is created. Since we cannot know how to distinguish groups from lists, we treat every list as a group. Not so fast. Consider the case we have a list like

"joint_list" (torso neck knee)

This would have been treated like a group named "joint_list" with a parameter named "torso" whose value is "neck". "knee" is gone. This because internally we use a yarp::os::Property to store the parameters and it requires every value to have a key. Those elements which don't satisfy this pattern are simply lost. For this reason, I decided to use directly a yarp::os::Bottle as a container.

Finally, every list is stored into a new handler and by using a yarp::os::Bottle we avoid losing data. Hence, "joint_list" (torso neck knee) would be stored into a new handler pointed by a shared pointer from the original handler. Now, in order to make sure that getParameter("joint_list") actually returns our beloved list, we also need to search if any of the pointed lists has the name "joint_list". Since the utilities to get a list out of a Searchable (like a Bottle) were already in place, I wanted this line to work

lists.at(parameterName)->getParameter(parameterName, parameter);

This was possible by adding the name of the group into the Bottle of the group itself. Bonus point, this makes printing easier.

Tl;Dr
All the functionalities we had before are preserved plus:

• there is a specific method (not const) to set a new group
• a group is returned with a weak_ptr
• any modification in the group is reflected in the handler from which we retrieve it
• we can clear the handler
• we can set the handler also after its creation
• added new test cases, including one with an actual config file.

[GiulioRomualdi]

Suggested change

	if (m_lists.find(parameterName) != m_lists.end()){ // A list is called with the same name of the parameter we are searching
	if (m_lists.find(parameterName) != m_lists.end()) // A list is called with the same name of the parameter we are searching
	{

[GiulioRomualdi]

Why is this required?

[S-Dafarra]

In brief, this is to check if there exists a list with the name you are searching. I have updated the first comment of the PR with the long explanation.

Eventually, we can discuss if you prefer to search first in the lists on in the container.

[GiulioRomualdi]

Correct me if I'm mistaken. if the group does not exist. A new shared_ptr should be added to m_list

 m_lists.emplace(name, make_shared(*subSub));

Indeed if the new group is not added in the list the lock() method of the weak pointer should fail

int main()
{
    ....
    YarpImplementation::weak_ptr  weakPtr = handler->getGroup("foo"); // <---- foo is not a name of
                                                                      //       any group 

    if(auto ptr = weakPtr.lock())
        std::cout << ptr->toString() << std::endl;
     else                                      // <----- according to the implemetation  of the 
                                               //        YarpImplementation::getGroup the pointer
                                               //        should be expired 
        std::cout << "pointer expired" << std::endl;
    ....
}

[S-Dafarra]

if the group does not exist. A new shared_ptr should be added to m_list

I cannot, the method is const.

Indeed if the new group is not added in the list the lock() method of the weak pointer should fail

This is exactly what I want. Thus we have a way to understand if the group actually exists or not. To create a new group you can use the setGroup method.

[S-Dafarra]

I realized this was not clear from the documentation. I changed it with b2197cd

[GiulioRomualdi]

This one should not be required anymore

[S-Dafarra]

I did not remove the unique_ptr logic. You are still free to own the handler with a unique_ptr. The issue was only in the way the group was returned.

[GiulioRomualdi]

Fine! Let me know if I can merge it

[S-Dafarra]

Fine! Let me know if I can merge it

As soon as we have the green flag from CI, I think we can merge.

[GiulioRomualdi]

All checks have passed. Merging!