-
Notifications
You must be signed in to change notification settings - Fork 303
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
add parse_state_interface_descriptions and parse_command_interface_description and reuse components #1768
base: master
Are you sure you want to change the base?
Conversation
…scriptions to import the components
b185a02
to
b3ea566
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1768 +/- ##
==========================================
+ Coverage 86.90% 86.92% +0.01%
==========================================
Files 118 118
Lines 11438 11423 -15
Branches 1051 1047 -4
==========================================
- Hits 9940 9929 -11
+ Misses 1131 1129 -2
+ Partials 367 365 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
f819ef2
to
62e98b9
Compare
void parse_state_interface_descriptions( | ||
const std::vector<ComponentInfo> & component_info, | ||
std::unordered_map<std::string, InterfaceDescription> & state_interfaces_map) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to help with future-proofing this a little bit I am wondering if we could use a template for this function with some traits or concepts (or none if none works yet w this C++ version) to capture that we really only expect this to be an STL container, we aren't using any API specific to an unordered map
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean here std::unordered_map<std::string, InterfaceDescription> & state_interfaces_map
?
I'm not sure what traits can be used here. As both methods use the same set of arguments. If you have some ideas, please let me know :)
As we discussed in yesterday's WG meeting, as the #1688 is merged, we would like to modify the import_state_interface_descriptions and import_command_interface_descriptions instead of parsing the hardware info to them, we parse the hardware_info_.joints and hardware_info_.sensors and it imports accordingly etc
Fixes: #1766