-
Notifications
You must be signed in to change notification settings - Fork 402
Activate hardware components by group sequentially #2984
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
base: master
Are you sure you want to change the base?
Activate hardware components by group sequentially #2984
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2984 +/- ##
==========================================
- Coverage 89.42% 89.28% -0.15%
==========================================
Files 157 157
Lines 18747 18788 +41
Branches 1503 1513 +10
==========================================
+ Hits 16765 16774 +9
- Misses 1363 1394 +31
- Partials 619 620 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Hey @saikishor great that you created this PR :) I currently do not have hardware access to test it. Perhaps I managed to try out the PR next week. Few remarks from my side:
I would suggest adding the possibility to configure flags for a group. E.g. |
IMO, no, it doesn't make sense
Why? Can you explain why this type of setup is needed? |
Could you elaborate why? (Just for my understanding)
|
Right now, the initialization happens when the loading of the robot description happens, so we have less control over there. Atleast as per my understanding
Do you know of a cases that would affect by this change?. This is only at startup, for the people who don't have a singleton approach, do you think it will make a difference? |
|
Just pointing out potential pitfalls. I'll try to test this PR the coming week. |
Fixes: #2811
When a hardware component group is defined, from now on. It will first configure all the hardware components and then activate them all together. Instead of activating one by one, as requested by 2811.