icub-tech-iit / study-icub-head

Main collector for the design of the iCub head
BSD 3-Clause "New" or "Revised" License
3 stars 2 forks source link

Update the PID gains for all the relevant robots #4

Closed pattacini closed 1 year ago

pattacini commented 2 years ago

Follow-up of #3.

While doing this, we should publish a nice writeup on the community for documentation purposes.

pattacini commented 1 year ago

@martinaxgloria could be doing this update.

@mfussi66 and/or @Nicogene can act as sidekicks.

pattacini commented 1 year ago

We could grab the opportunity to take @martinaxgloria to the physical robot and start the training for using it.

mfussi66 commented 1 year ago

We could grab the opportunity to take @martinaxgloria to the physical robot and start the training for using it.

To this end we can check the availability of iCubGenova11 or iCub3.

S-Dafarra commented 1 year ago

We could grab the opportunity to take @martinaxgloria to the physical robot and start the training for using it.

To this end we can check the availability of iCubGenova11 or iCub3.

iCub3 currently has the arms unmounted for iRonCub tests. You may want to double-check with @gabrielenava (not sure if he has access to this repo) to use it

pattacini commented 1 year ago

(not sure if he has access to this repo) to use it

Yep, this repo has been made public.

gabrielenava commented 1 year ago

iCub3 currently has the arms unmounted for iRonCub tests. You may want to double-check with @gabrielenava (not sure if he has access to this repo) to use it

we unmounted the arms for FT calibration tests, but the robot can be used. If you need to mount back the arms we can organize (cc @hosameldinmohamed @vpunithreddy)

mfussi66 commented 1 year ago

iCub3 currently has the arms unmounted for iRonCub tests. You may want to double-check with @gabrielenava (not sure if he has access to this repo) to use it

we unmounted the arms for FT calibration tests, but the robot can be used. If you need to mount back the arms we can organize (cc @HosameldinMohamed @vpunithreddy)

Actually we would just need to move the head, thanks!

mfussi66 commented 1 year ago

iCub3 booked for friday morning.

martinaxgloria commented 1 year ago

Yesterday I started modifying the PID values in the robots-configuration files. I started filtering the iCub with a head_version ≥ 2.0. Among those, I found some anomalies reported in the table above:

ROBOT NAME SERIAL No. DESCRIPTION
iCubBielefeld03 S/N: 019 No .xml files
iCubDarmstadt01
iCubNottingham01
iCubSheffield01
iCubTwente01
S/N: 025
S/N: 027
S/N: 028
S/N: 030
Device type: canmotioncontrol (no xml file concerning the head parameters)
iCubOsaka01
iCubGrenoble01
iCubParis02
S/N: 022
S/N: 023
S/N: 024
Device type canmotioncontrol (the icub_head.xml contains PID values strangely different wrt iCubGenova11, for example)

Just to be more precise, the configuration files relative to the robots listed above are not changed for at least 5 years.

cc @mfussi66 @pattacini @davidetome @Fabrizio69

pattacini commented 1 year ago

Hi @martinaxgloria

Good that you reported these anomalies, which need to be definitely discussed and analyzed 👍🏻

Just a comment regarding:

I started filtering the iCub with a head_version higher than 2.0.

⚠️ The condition shall be head_version $\ge$ 2.0.

martinaxgloria commented 1 year ago

⚠️ The condition shall be head_version2.0.

Yes, I considered this, I just reported it wrong, sorry. I edited the previous comment with the right condition.

martinaxgloria commented 1 year ago

Update: I talked with @davidetome, who gave me some information about the anomalies observed.

ROBOT NAME SERIAL No. DESCRIPTION
iCubDarmstadt01
iCubNottingham01
iCubSheffield01
iCubTwente01
S/N: 025
S/N: 027
S/N: 028
S/N: 030
Device type: canmotioncontrol (there is only the mechanical head configuration file and not the motor control one)

The robots above have a head_version ≥ 2.0 but of type canmotioncontrol, while the rest of the body is embObjMotionControl.

ROBOT NAME SERIAL No. DESCRIPTION
iCubOsaka01
iCubGrenoble01
iCubParis02
S/N: 022
S/N: 023
S/N: 024
Device type canmotioncontrol (the icub_head.xml contains PID values strangely different wrt iCubGenova11, for example)

Those, instead, are entirely canmotioncontrol robots.

The PID values are reported above and, in particular, the first two columns regard the neck_pitch and neck_roll:

<group name="POS_PIDS">      
        <param name="kp">           6000          -6000         3200          -400          100           100           </param>       
        <param name="kd">           500           -500          3200          -400          1000          1000          </param>       
        <param name="ki">           5             -5            10            -4            10            10            </param>        
</group>      

while the PID values of iCubGenova11 and of the other embObjMotionControl robots are:

<group name="POS_PID_DEFAULT">  
        <param name="kp">                       -300        +300        </param>
        <param name="kd">                       -10         +10         </param>
        <param name="ki">                       -100        +100        </param>
</group>
pattacini commented 1 year ago

If I can make a general comment, I would proceed here iteratively.

First off, attack all the robots that do not have anomalies. For this, a preliminary PR can be opened. Of course, it's important to copy out the PID gains from the correct robot (@mfussi66 can point you at it).

Then (or in the meantime), we can understand how to deal with the anomalies. We may even decide not to touch them. The CAN-based robots are pretty old. TBD w/ @Nicogene too.

martinaxgloria commented 1 year ago

PR opened here.

pattacini commented 1 year ago

Nice message in the PR body!

pattacini commented 1 year ago

Hi @martinaxgloria

I've seen that you updated 3 robots with the latest commits, so how did you ensure full coverage of the robots? Which method did you use? Any robot still missing?

martinaxgloria commented 1 year ago

Before the last commit in which I uploaded the 3 robots you mentioned I followed this list maintained by @Fabrizio69. Since you pointed out that some robots were forgotten, I double-checked this file with the iKinGazeCrtl files in robots-configuration: in doing this, I saw that three robots I missed were not present in that list, this is why I didn't upload them with the others.

Actually, there are some robots with incongruences between the head_version in the iKinGazeCtrl.ini and what it is reported in that list of robots. Until now, I have uploaded only the ones with a coherent head_version between the two files.

pattacini commented 1 year ago

in doing this, I saw that three robots I missed were not present in that list, this is why I didn't upload them with the others.

Yeah, indeed, this is the pitfall of relying on lists that are "far" from the SW. They may be not up-to-date.

Better off performing a thorough check then.

martinaxgloria commented 1 year ago

Today I can do a search in folder in robots-configuration to check here the correct head_version and see if I forgot some robots.

martinaxgloria commented 1 year ago

After a further check, I found out that iCubLisboa01 was the last robot to be updated and I did it.

pattacini commented 1 year ago

Cool! I'll merge the PR then.

pattacini commented 1 year ago

For the leftover, I don't have a strong opinion. We may leave them unchanged.

@Nicogene can say the last word.

Nicogene commented 1 year ago

For the leftover, I don't have a strong opinion. We may leave them unchanged.

@Nicogene can say the last word.

I don't hear news about those robots in a while, I think we can mark as done this activity, and eventually, we will tackle also the remaining ones in another stint if needed

pattacini commented 1 year ago

Super! Thanks again @martinaxgloria!