Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improved SkinGUI for fingertip #930

Merged
merged 3 commits into from
Jan 5, 2024
Merged

Conversation

simeonedussoni
Copy link
Contributor

implemented:

  • skin type fingertip 4L/R for mk1.1 PCB testing
  • skin type fingertipMID for 3DMID visualization
  • palm workaround (added a fakePalm with just one virtual taxel with zero gain and size to avoid crashing of the GUI)

@simeonedussoni simeonedussoni self-assigned this Jan 5, 2024
@simeonedussoni simeonedussoni marked this pull request as draft January 5, 2024 09:54
@simeonedussoni
Copy link
Contributor Author

Need to update also the CMakeList file.

@simeonedussoni simeonedussoni marked this pull request as ready for review January 5, 2024 10:02
Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @simeonedussoni

A couple of inline comments to address.

A few more comments:

  • Do we need also configuration files?
  • If you did some tests like showing the results on the GUI, would you be willing to post them here?

src/tools/iCubSkinGui/plugin/CMakeLists.txt Show resolved Hide resolved
src/tools/iCubSkinGui/plugin/include/SkinMeshThreadCan.h Outdated Show resolved Hide resolved
@simeonedussoni
Copy link
Contributor Author

Hi @simeonedussoni

A couple of inline comments to address.

A few more comments:

* Do we need also configuration files?

* If you did some tests like showing the results on the GUI, would you be willing to post them here?

here we have an example of configuration file showing all 5 types of layout, one of which activated by touchin the PCB

immagine

concerning the configuration files:

  • for the (soon) mounted icub hands they are already in the repo under /app/skinGui/conf/skinGui/left_hand_V2_2.ini
  • for ergocub: they are still to be produced, I can put there in the same location with "speaking names" and add to the PR
  • for using the unwrapped version of the layout will be deployed in the test-skin-patches repo to keep them separate from the actual robot patches

@pattacini
Copy link
Member

pattacini commented Jan 5, 2024

for ergocub: they are still to be produced, I can put there in the same location with "speaking names" and add to the PR

This repo contains icub-specific SW. A better place for the ergoCub conf files is definitely https://github.com/icub-tech-iit/ergocub-software. I would wait for @Nicogene to show up before proceeding in this respect though.

@pattacini
Copy link
Member

for using the unwrapped version of the layout will be deployed in the test-skin-patches repo to keep them separate from the actual robot patches

Please, get aligned with @davidetome for that.

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awaiting the CI before merging.

Copy link

sonarcloud bot commented Jan 5, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@pattacini pattacini merged commit d393231 into robotology:devel Jan 5, 2024
5 checks passed
@maggia80
Copy link
Contributor

maggia80 commented Jan 8, 2024

@simeonedussoni a few comments:

please check the author and the copyright of the committed files.

add somewhere an explanation about which gui corresponds to which fingertip and in which configuration (wrapped or unwrapped).

@Nicogene
Copy link
Member

Nicogene commented Jan 8, 2024

for ergocub: they are still to be produced, I can put there in the same location with "speaking names" and add to the PR

This repo contains icub-specific SW. A better place for the ergoCub conf files is definitely icub-tech-iit/ergocub-software. I would wait for @Nicogene to show up before proceeding in this respect though.

Yes but I have no idea how can the SkinGui load fingertip compiled externally, if they are loaded run-time I am ok to move in ergocub-software

@pattacini
Copy link
Member

Yes but I have no idea how can the SkinGui load fingertip compiled externally, if they are loaded run-time I am ok to move in ergocub-software

I meant only the .ini files.

@Nicogene
Copy link
Member

Nicogene commented Jan 8, 2024

Yes but I have no idea how can the SkinGui load fingertip compiled externally, if they are loaded run-time I am ok to move in ergocub-software

I meant only the .ini files.

Ok for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants