adding embObjFTsensor support to ETH robots using strain2

[davidetome]

This PR is intended to add support for the embObjFTsensor for all robots using strain2.
In particular :

• Changes embObjStraindevice to embObjFTsensor in all files under hardware/FT folders of the robots using strain2
• Adds a new line necessary for the temperature reading configuration
• Adds the new wrapper files inside wrappers/FT folders calling them *_multipleSens.xml
• Changes in all above new wrapper files the device type from analogServer to multipleanalogsensorsserver
• Changes in all above new wrapper files the yarp port name (i.e. from /icub//left_leg/analog:o to /icub//left_leg)

⚠️ In order to use the new functionality the icub_all.xml (or the file used by yarprobotinterface) has to be modified including the new wrappers

The script I made is the following (it excludes iCubGenova04 and iRonCub-Mk1 since the mods were already done)

#!/bin/bash

# modify the array content to exclude dirs from processing
declare -a exclude_dirs=("iCubGenova04" "iRonCub-Mk1")

find . -maxdepth 1 -type d > output.txt
filename='output.txt'
n=1
found=0
while read line; do
n=$((n+1))
    for i in "${exclude_dirs[@]}"
    do
    if [[ "$line" == *"$i"* ]] ; then
    found=1
    fi
    done
if [ $found == 1 ]; then
echo "skipping " $line
else
echo $line >> output1.txt
fi
found=0
done < $filename

filename='output1.txt'
while read line; do
# reading each line
echo "Line No. $n : $line"
n=$((n+1))
grep "type.*strain2" $line/hardware/FT/* >> output2.txt
done < $filename
sed -e "s/:.*param>//" output2.txt > output3.txt

filename='output3.txt'
n=1
while read line; do
# reading each line
#echo "Line No. $n : $line"
n=$((n+1))
sed -i "s/embObjStrain/embObjFTsensor/" $line
grep temperature-acquisitionRate $line
check=$?
if [ $check == 1 ]; then
sed -i '/enabledSensors/a <param name="temperature-acquisitionRate"> 1000                  </param> <!-- 1 seconds -->' $line
sed -i "s/<param name=\"temperature-acquisitionRate\">/\t\t\t\t <param name=\"temperature-acquisitionRate\">/" $line
fi
wrapper_name=$(sed -e "s/^.*FT\///" -e "s/\.xml//" <<< $line)
wrapper_dir=$(sed -e "s/hardware.*$//" <<< $line)
wrapper_file=$(grep $wrapper_name $wrapper_dir/wrappers/FT/*wrapper.xml)
wrapper_file=$(sed -e "s/:.*$//" <<< $wrapper_file)
wrapper_file_multi=$(sed -e "s/_wrapper.xml/_wrapper_multipleSens.xml/" <<< $wrapper_file)
echo $wrapper_file_multi
cp $wrapper_file $wrapper_file_multi
sed -i "s/analogServer/multipleanalogsensorsserver/" $wrapper_file_multi 
sed -i "s/\/analog:o//" $wrapper_file_multi 

done < $filename
rm output.txt output1.txt output2.txt output3.txt

cc @maggia80 @marcoaccame

[pattacini]

Thanks a lot @davidetome 👍🏻

I've removed the pointer to an internal issue.

Could you only better clarify the port name change below by making one example?

Changes in all above new wrapper files the yarp port name (i.e. from /icub//left_leg/analog:o to /icub//left_leg)

Finally, I would ask @traversaro for a review as well.

[pattacini]

Also, it's difficult to see from the change list if you have addressed XML template files like those in the folder experimentalSetups.

[davidetome]

Thanks a lot @davidetome 👍🏻

I've removed the pointer to an internal issue.

Could you only better clarify the port name change below by making one example?

Changes in all above new wrapper files the yarp port name (i.e. from /icub//left_leg/analog:o to /icub//left_leg)

Finally, I would ask @traversaro for a review as well.

I followed the yarp name format already done in iCubGenova04, w/ the new device the port created will be for example /icub/left_leg/measures:o instead of /icub/left_leg/analog:o/measures:o

[davidetome]

Also, it's difficult to see from the change list if you have addressed XML template files like those in the folder experimentalSetups

No, I modified only robots folders as I understood was required
Anyway I can run the script also under experimentalSetups

[pattacini]

Anyway I can run the script also under experimentalSetups

Yes please as I bet we will be generating wrong files again in the future if we won't fix them up.

[davidetome]

Anyway I can run the script also under experimentalSetups

Yes please as I bet we will be generating wrong files again in the future if we won't fix them up.

I modified the experimentalSetups (only one folder)

The iCubTemplates folder is up to date, anyway as far as I know (as I did in the past) all new robots come from the copy of the last robot delivered

[pattacini]

The changes proposed with this PR has been successfully tested on iCubGenova09 🤖

@S-Dafarra you may want to give a look at it too.

[pattacini]

@traversaro @S-Dafarra, any objections?
Otherwise, I'll merge the PR.

[traversaro]

@traversaro @S-Dafarra, any objections?
Otherwise, I'll merge the PR.

Unfortunately I can't review until tomorrow. However as long as the old analogServer wrappers are still there (as they are used by wbd) for me it is good to go, I can do further comments after the merge.

[pattacini]

Unfortunately I can't review until tomorrow.

No rush @traversaro ! We can wait.

[S-Dafarra]

My main concerns were also related to the back-compatibility. If we can switch from the two configurations easily after some time, then it's a go for me.
The other point was related to WBD, but this can be addressed at a later stage I guess.

[pattacini]

However as long as the old analogServer wrappers are still there (as they are used by wbd) for me it is good to go

I'm afraid this is not the case.
@davidetome any comments?
WBD won't work w/o the analog:o ports, actually. To get around this, we should provide both wrappers, probably.

However, @traversaro will give us more hints on this PR later on.

If we can switch from the two configurations easily after some time

@S-Dafarra we don't aim to maintain multiple configurations for all the robots.
You may want to have a specific branch in your fork for that. Alternatively, you may propose to arrange this configuration in your fork and then open up a PR for the robots you deal with.

[S-Dafarra]

If we can switch from the two configurations easily after some time

@S-Dafarra we don't aim to maintain multiple configurations for all the robots.
You may want to have a specific branch in your fork for that. Alternatively, you may propose to arrange this configuration in your fork and then open up a PR for the robots you deal with.

If this is the case, I don't feel comfortable in pushing these changes for all robots, to be honest. From what I understood, this is basically the situation:

1. These devices are sort of experimental, as we don't have any experience of prolonged usage on any robot.
2. It would not be possible to switch between one and the other to check the performances
3. WBD would be broken on all the robots affected by these changes

I guess the situation is not as bad, so please correct me 😁

[pattacini]

Hi @S-Dafarra

1. The request was raised by @traversaro who may have more in-depth insights.
2. We cannot maintain multiple configurations of this sort as we simply don't have the infrastructure for that. That's why I was implicitly asking for your help about how you would arrange the folder of your robots. Since you're working on your fork, the PR could be landing anyway. We can then integrate the specific modifications with subsequent PR's. Or, if you like, you could take a look at this exact PR and tell us how you would proceed in this respect for your robot's folders.
3. The WBD operations will be certainly fixed as per the comments above.

[pattacini]

Anyway, if we need to restore the old analog:o ports alongside the newest ones to address the WBD problem, this may somehow help in terms of back-compatibility too.

[S-Dafarra]

2. We cannot maintain multiple configurations of this sort as we simply don't have the infrastructure for that. That's why I was implicitly asking for your help about how you would arrange the folder of your robots. Since you're working on your fork, the PR could be landing anyway. We can then integrate the specific modifications with subsequent PR's. Or, if you like, you could take a look at this exact PR and tell us how you would proceed in this respect for your robot's folders.

Even though we are using our fork, I am still concerned about having potentially breaking changes upstream. In my opinion, any major change should be absorbed soon. Otherwise, this might be blocking for any future change and potentially be very problematic when we are close to a release.

In the previous comments, it was mentioned that these changes would have taken effect only after including them in the icub_all.xml file. So, I supposed that according to which files were called in the icub_all.xml file, we would have used different devices. IMHO, this would be a feasible solution. Both files are available, and one set is actually used according to the configuration of icub_all.xml. The new files would be there, silent, until we are in the condition to switch.

We could leave this duplication for some time, and then delete the old files in the next release, in a sort of "deprecation" pattern.

Does it make sense?

[pattacini]

Does it make sense?

This would be very reasonable, but I can see there are modifications to the *strain.xml files so, ideally, we should replicate at least those files as well.

Let me however call for @davidetome 's help on this.

Given #287 (comment), we'd very likely need to have replications for those files either way. The latter should bring us back to the icub_all.xml single-file entry point for switching the configuration.

[davidetome]

Not sure to clearly understand your doubts, but just to clarify, since the new *_multipleSens.xml are not included in the let's say icub_all.xml file the ports created still remain the */analog:o even if embObjFTsensor has been included in the hardware/FT/*-strain.xm files, I tested it on the iCubGenova09 and that's the reason I avoided to duplicate those files.
Am I wrong?

[pattacini]

Ok cool @davidetome
Your comment clears up most of the doubts 👍🏻

even if embObjFTsensor has been included in the hardware/FT/*-strain.xm files, I tested it on the iCubGenova09 and that's the reason I avoided to duplicate those files

Ports are correctly opened up but still the FT data are published through a new device driver. Thus, @S-Dafarra 's concern number 1 (These devices are sort of experimental, as we don't have any experience of prolonged usage on any robot.) may in theory still hold, though I think it's unlikely that there's a problem in this respect.

At any rate, let's wait until @traversaro chimes in.

[traversaro]

At any rate, let's wait until @traversaro chimes in.

I feel quite a bit of pressure of resolving the discussion at this point. :D

Ports are correctly opened up but still the FT data are published through a new device driver. Thus, @S-Dafarra 's concern number 1 (These devices are sort of experimental, as we don't have any experience of prolonged usage on any robot.) may in theory still hold, though I think it's unlikely that there's a problem in this respect.

Actually the embObjFTsensor has been used for the last ~three years on legs of the iCubGenova04, as it was first introduced in #68 (comment), so I think it has been fairly tested. The problem is that we never propagate the use of embObjFTsensor to all other robots. Note that embObjFTsensor in theory is perfectly compatible with STRAIN1 boards, so I think that in theory it should be possible to replace all instances of embObjStrain and deprecate the embObjStrain device, to reduce code duplication (embObjFTSensor was created by copying embObjStrain in robotology/icub-main@05c4f82 and then adding features like STRAIN2 compatibility and the use of MultipleAnalogSensors interfaces.

For this reason, I think that merging this PR is quite safe. I think it would be also safe to enable the new wrappers on the top of the old ones, a bit like @Nicogene did back when he migrated all the IMUs of iCub to use the new MAS interfaces. To be continued.

[pattacini]

Actually the embObjFTsensor has been used for the last ~three years on legs of the iCubGenova04, as it was first introduced in #68 (comment), so I think it has been fairly tested.

Cool 👍🏻

Note that embObjFTsensor in theory is perfectly compatible with STRAIN1 boards, so I think that in theory it should be possible to replace all instances of embObjStrain and deprecate the embObjStrain device, to reduce code duplication (embObjFTSensor was created by copying embObjStrain in robotology/icub-main@05c4f82 and then adding features like STRAIN2 compatibility and the use of MultipleAnalogSensors interfaces

We might aim to update all the robots at this point with a pair of subsequent PR's:

• in here to remove the use of embObjStrain for all robots.
• in icub-main to remove the device driver.

This needs to be tested anyway and can be done later.

For this reason, I think that merging this PR is quite safe. I think it would be also safe to enable the new wrappers on the top of the old ones, a bit like @Nicogene did back when he migrated all the IMUs of iCub to use the new MAS interfaces. To be continued.

Interesting! So we can publish everything through the current analog:o and new MultipleAnalogSensors ports at once without the need for tinkering with the icub_all.xml file.

@davidetome you may do that once back next week.

[traversaro]

For this reason, I think that merging this PR is quite safe. I think it would be also safe to enable the new wrappers on the top of the old ones, a bit like @Nicogene did back when he migrated all the IMUs of iCub to use the new MAS interfaces. To be continued.

Interesting! So we can publish everything through the current analog:o and new MultipleAnalogSensors ports at once without the need for tinkering with the icub_all.xml file.

Yes. However, I wonder if we could think a bit what is the best way of doing so. By its nature, the MultipleAnalogSensors wrappers have been designed to expose multiple sensors, so the user did not need to know before hand al the ports of each sensor that was available (that can be quicky becoming unfesible) but would just connect to a small number of devices (such as one for part, or one for part for type of sensor) and get all the available sensors. This would also simplify documetation, as we do not need to document explicitly the port of each sensor. From that point of view, we may want to have a fewer number of wrappers when using MAS infrastructure as opposed to the old single-sensor wrappers.

However, keeping multiple sensor measurments in the same wrapper has also downsides, so we may need to discuss what's the tradeoff we may want on this.

[pattacini]

The main downside I can see now is that if we don't publish analog:o we break WBD, I think, unless we aim to update it – although we said that at a certain point we should switch to the use of WBD-device-driver.

At any rate, lots of stuff to cook that may be conveniently addressed with upcoming work so that we can keep this PR light.

👉🏻 The thing we should decide here is whether we modify the PR in order to expose analog:o and MultipleAnalogSensors ports at the same time for the robots we already touched by enabling both the wrappers from within icub_all.xml.

I vote for this 👍🏻

Only then, we could deal with the outstanding points:

• replace embObjStrain in every robot
• decide what's next with the MultipleAnalogSensors

[traversaro]

👉🏻 The thing we should decide here is whether we modify the PR in order to expose analog:o and MultipleAnalogSensors ports at the same time for the robots we already touched by enabling both the wrappers from within icub_all.xml.

I vote for this 👍🏻

Ok, the only thing that I am a bit afraid is that we rush it trough and then we have two legacy behaviour to support, analog:o and the per-sensor MultipleAnalogSensors wrapper.

[pattacini]

Ok, the only thing that I am a bit afraid is that we rush it trough and then we have two legacy behaviour to support, analog:o and the per-sensor MultipleAnalogSensors wrapper.

I think we're forced to still publish analog:o ports in order not to break WBD and be able to merge the PR soon.

For what concerns MultipleAnalogSensors ports instead, my point was more to try to activate them anyway (as of now the PR provides only the infrastructure but it doesn't open them up yet) so that users can already start seeing new ports and plan for upgrading their codebase.

In this latter respect, we're free to choose the best strategy for us. Maybe, we could target with this strategy @S-Dafarra 's needs and help his group incrementally shift to the new paradigm. To this end, what would be your thoughts @traversaro @S-Dafarra ?

Perhaps, we could already do the big jump and go for the multi-sensors approach?

[traversaro]

Ok, the only thing that I am a bit afraid is that we rush it trough and then we have two legacy behaviour to support, analog:o and the per-sensor MultipleAnalogSensors wrapper.

I think we're forced to still publish analog:o ports in order not to break WBD and be able to merge the PR soon.

Yes, there is no doubt on this. I was simple afraid that we added single-sensor MultipleAnalogSensors wrappers, and then we understand that we like multi-sensor MultipleAnalogSensors wrappers, and then we need to support all three (analog:o, single-sensor and multi-sensor MultipleAnalogSensor wrappers). So I would prefer to wait for @S-Dafarra feedback and then just do the migration to the selected MultipleAnalogSensor wrappers strategy.

[S-Dafarra]

Just to understand, the blocking point right now would be that WBD would not work anymore, right? Even if we merge this, what would be the plan?

Also, what is the point of merging this knowing already that something will break? Can we try to avoid this to happen, eventually from the WBD side?

[Nicogene]

If you are referring to the icub-main wbd the changes needed should be similar to robotology/icub-main#596

[pattacini]

Just to understand, the blocking point right now would be that WBD would not work anymore, right? Even if we merge this, what would be the plan?

Also, what is the point of merging this knowing already that something will break? Can we try to avoid this to happen, eventually from the WBD side?

Hi @S-Dafarra
WBD will work perfectly as it is right now w/o any change since the analog:o ports are still operational.

The idea is now to add more ports since the beginning from within the icub_all.xml by enabling new wrappers and not disabling current wrappers.

[S-Dafarra]

Just to understand, the blocking point right now would be that WBD would not work anymore, right? Even if we merge this, what would be the plan?
Also, what is the point of merging this knowing already that something will break? Can we try to avoid this to happen, eventually from the WBD side?

Hi @S-Dafarra
WBD will work perfectly as it is right now w/o any change since the analog:o ports are still operational.

Sorry, I misunderstood. I thought we were going toward the choice of not supporting them. Regarding the single sensor vs multisensor strategy choice, at the moment I don't have a strong opinion.

What would be the pros and cons of the two approaches?

Let me drop in the discussion also @prashanthr05 given his expertise on these devices

[pattacini]

Well, the mono-sensor approach with the new wrappers is actually already a legacy; that's the reason why we're heading to a multi-sensors approach.

See discussion from #287 (comment) onward.

[prashanthr05]

Just to understand, the blocking point right now would be that WBD would not work anymore, right? Even if we merge this, what would be the plan?
Also, what is the point of merging this knowing already that something will break? Can we try to avoid this to happen, eventually from the WBD side?

Hi @S-Dafarra
WBD will work perfectly as it is right now w/o any change since the analog:o ports are still operational.

Sorry, I misunderstood. I thought we were going toward the choice of not supporting them. Regarding the single sensor vs multisensor strategy choice, at the moment I don't have a strong opinion.

What would be the pros and cons of the two approaches?

Let me drop in the discussion also @prashanthr05 given his expertise on these devices

I believe this PR stems from the conversation that we (@traversaro @fjandrad) had in whole-body-estimators in robotology/whole-body-estimators#45 (comment) which added the functionality to handle MAS FT sensors being back-compatible with handling also analog sensors.

I believe WBD will work fine.

What would be the pros and cons of the two approaches?

The mono-sensor approach will result in too many configurations files (for starters) but it's the most intuitve to understand the MAS architecture in the beginning, I suppose (atleast, in my case).

[pattacini]

Quick update.

The use of the multi-sensors approach needs to be configured, thus we decided to postpone these new ports with a different PR.

However, we could enable the new device embObjFTsensor also for the ETH robot mounting the STRAIN1 after a test.
@davidetome will be doing this.

[davidetome]

However, we could enable the new device embObjFTsensor also for the ETH robot mounting the STRAIN1 after a test.
@davidetome will be doing this.

Just tested on iCubGenova09, on the left arm but it seems that the new device is not compatible w/ the strain

[traversaro]

However, we could enable the new device embObjFTsensor also for the ETH robot mounting the STRAIN1 after a test.
@davidetome will be doing this.

Just tested on iCubGenova09, on the left arm but it seems that the new device is not compatible w/ the strain

Can you try to change the type from strain2 in strain in

robots-configuration/experimentalSetups/singleETHboard/hardware/FT/left_arm-eb1-j0_3-strain.xml

Line 19 in 47ac9ec

<param name="type"> strain2 </param>

?

[davidetome]

Can you try to change the type from strain2 in strain in https://github.com/robotology/robots-configuration/pull/287/files#diff-7344d02c77298f42cec50dec63b845846ec2248c457f96638124277e2224d7c8R19 ?

@traversaro , sorry, what do you mean?

[traversaro]

Can you try to change the type from strain2 in strain in https://github.com/robotology/robots-configuration/pull/287/files#diff-7344d02c77298f42cec50dec63b845846ec2248c457f96638124277e2224d7c8R19 ?

@traversaro , sorry, what do you mean?

I edited the original message with a new reference on the line that should be changed, see

robots-configuration/experimentalSetups/singleETHboard/hardware/FT/left_arm-eb1-j0_3-strain.xml

Line 19 in 47ac9ec

<param name="type"> strain2 </param>

.

[pattacini]

After a F2F update w/ @traversaro and @davidetome, we decided to merge the PR as it is and postpone the changes we discussed later.