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

Modify resource manager logic to support multiple hardware components #1391

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

chama1176
Copy link

What does this implement/fix?

The current implementation requires that all components have the appropriate interface and will not work well if the hardware is split into multiple components.
It should only be necessary for one component to have an interface, so we will modify the logic accordingly.

Specifically, we are considering the following examples.
If you want to connect to follower_joint1/effort, it should be no problem if component 1 has the corresponding interface.

Hardware Component 1
        command interfaces
                follower_joint1/position [available] [unclaimed]
                follower_joint1/effort [available] [claimed]
Hardware Component 2
        command interfaces
                leader_joint1/position [available] [unclaimed]
                leader_joint1/effort [available] [claimed]

Does this close any currently open issues?

I don't have Franka Emika Panda so I am not able to reproduce the same situation completely,
but this PR may solve this issue.
#1177

@chama1176
Copy link
Author

@christophfroehlich
I had missed the test. I have corrected it, could you please check again.

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (786d5b5) 47.49% compared to head (eaed55d) 75.91%.
Report is 11 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1391       +/-   ##
===========================================
+ Coverage   47.49%   75.91%   +28.41%     
===========================================
  Files          41       41               
  Lines        3556     3363      -193     
  Branches     1938     1937        -1     
===========================================
+ Hits         1689     2553      +864     
+ Misses        459      417       -42     
+ Partials     1408      393     -1015     
Flag Coverage Δ
unittests 75.91% <69.23%> (+28.41%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
hardware_interface/src/resource_manager.cpp 76.99% <100.00%> (+28.16%) ⬆️
controller_manager/src/controller_manager.cpp 69.43% <20.00%> (+30.66%) ⬆️

... and 30 files with indirect coverage changes

@bmagyar
Copy link
Member

bmagyar commented Feb 16, 2024

I'm sorry but I don't get the explanation you've provided.

From the code I'm guessing you are trying to implement something like "BEST_EFFORT" as defined in this message?

@chama1176
Copy link
Author

@bmagyar
Thanks for checking the code.
I didn't know about that BEST_EFFORT setting, but it sounds a little different than what I want to do.

In the following example, two hardware components have different interfaces.

Hardware Component 1
        command interfaces
                follower_joint1/position [available] [unclaimed]
                follower_joint1/effort [available] [claimed]
Hardware Component 2
        command interfaces
                leader_joint1/position [available] [unclaimed]
                leader_joint1/effort [available] [claimed]

Suppose we have a controller with interfaces follower_joint1/effort and leader_joint1/effort, In the current implementation, each hardware component is checked to see if it has interfaces(follower_joint1/effort and leader_joint1/effort),
And if any component does not have an interface, it fails. Therefore, it cannot be used in this case.

@destogl
Copy link
Member

destogl commented Mar 4, 2024

I don't get the issue here. It seems to me that he Hardware Interface is not accepting this combination which is poor implementation of HW interface. So I see here nothing to fix.

We can discuss if we should give a component just joint that component has. This might be a good, but not so simple solution.

Can you write a test for this because I don't really understand the issue

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.

None yet

3 participants