search

robotology / walking-teleoperation

Software related to walking and teleoperation.
BSD 3-Clause "New" or "Revised" License
30 stars 14 forks source link

Add gaze tracking to SRanipalModule #82

Closed S-Dafarra closed 2 years ago

S-Dafarra commented 2 years ago

I moved most of the implementation of the SRanipalModule to separate files, but actually, the main new thing is the gaze retargeting.

The gaze retargeting also works thanks to https://github.com/ami-iit/yarp-device-openxrheadset/pull/10

S-Dafarra commented 2 years ago

Friendly ping @kouroshD

S-Dafarra commented 2 years ago

I have just rebase on top of devel. I will start working on the comments soon

S-Dafarra commented 2 years ago
  • try to be explicit if some variable or function is related to the human or robot, specially if the can make confusion

Thanks, please let me know if now is ok.

  • I have seen temporary variables are made in function that are called at run time, which may reduce the efficiency (since the module is running at 100 Hz, at least as set in config file!)

Can you point me in the code where this is happening? Note that there is a difference between dynamically allocated memory, and statically allocated memory.

  • If there is an image showing the frames could be great, to associate easier/better between the approach you considered, the frame transformations, and the implemented code here

I think this is technical and does not fit well in the readme. Let me know if there are passages in the code where you want me to add some detail. Nonetheless, I am planning to write a paper about this, so in the future, there should be more documentation.

In any case, this has to be tested again.

kouroshD commented 2 years ago

Thanks, please let me know if now is ok.

Sounds good to me.

Can you point me in the code where this is happening? Note that there is a difference between dynamically allocated memory, and statically allocated memory.

You are right, those local variables are statically allocated, so no overhead.

I think this is technical and does not fit well in the readme. Let me know if there are passages in the code where you want me to add some detail. Nonetheless, I am planning to write a paper about this, so in the future, there should be more documentation.

Agree on that. As you asked, the part related to the image frames and human gaze pointing to the image is a bit hard to understand theoretically without knowing where the frames are defined, although some explanations are already provided.

kouroshD commented 2 years ago

let me know, when you are done @S-Dafarra so I merge the PR. Thanks

S-Dafarra commented 2 years ago

let me know, when you are done @S-Dafarra so I merge the PR. Thanks

Yes, I am done. I will do some enhancements in other PRs

kouroshD commented 2 years ago

@S-Dafarra , would you mind if rebase the branch again on devel because of #90 ? we will wait for the CI, and then I will merge.

S-Dafarra commented 2 years ago

No problem, I will do it!

S-Dafarra commented 2 years ago

I just realized that I have introduced a problem with https://github.com/ami-iit/yarp-device-openxrheadset/commit/2ffc4404fc74e420e9d03db7389fd1826bc173b0. I am going to commit the fix

kouroshD commented 2 years ago

can I merge this now @S-Dafarra ?

kouroshD commented 2 years ago

I asked him in person, done!