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

Remodel component interfaces #203

Merged
merged 6 commits into from Nov 2, 2020
Merged

Conversation

Karsten1987
Copy link
Contributor

Second part of #164

Introduces StateHandle and CommandHandle.
Adapts the hardware components interfaces to instantiate these handles given the configuration info passed to the components on a prior call to configure.

Sits on top of #201

Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

Looks good in general. Happy to merge once we settled the std::move point.

@bmagyar bmagyar changed the base branch from remove_logical_components to master October 26, 2020 17:08
Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

I added a few discussion points.

Most importantly: I am not sure we need templates with Handles/Interfaces. Or we could use templates to set the data type of Handle/Interface (in general it has not be double, it could be also bool for valves or some status string)

Karsten1987 and others added 6 commits October 30, 2020 15:13
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Signed-off-by: Karsten Knese <Karsten1987@users.noreply.github.com>
Co-authored-by: Denis Štogl <destogl@users.noreply.github.com>
@bmagyar bmagyar force-pushed the remodel_component_interfaces branch from 9f82e64 to 608ad83 Compare October 30, 2020 15:14
@bmagyar
Copy link
Member

bmagyar commented Oct 30, 2020

I've rebased this PR after merging #209

@codecov-io
Copy link

Codecov Report

Merging #203 into master will increase coverage by 1.31%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##           master     #203      +/-   ##
==========================================
+ Coverage   31.41%   32.73%   +1.31%     
==========================================
  Files          38       44       +6     
  Lines        2489     2688     +199     
  Branches     1638     1734      +96     
==========================================
+ Hits          782      880      +98     
- Misses        251      258       +7     
- Partials     1456     1550      +94     
Flag Coverage Δ
#unittests 32.73% <40.00%> (+1.31%) ⬆️

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

Impacted Files Coverage Δ
...include/hardware_interface/components/actuator.hpp 100.00% <ø> (ø)
...rdware_interface/components/actuator_interface.hpp 100.00% <ø> (ø)
...e/include/hardware_interface/components/sensor.hpp 100.00% <ø> (ø)
...hardware_interface/components/sensor_interface.hpp 100.00% <ø> (ø)
...e/include/hardware_interface/components/system.hpp 100.00% <ø> (ø)
...hardware_interface/components/system_interface.hpp 100.00% <ø> (ø)
...dware_interface/test/test_component_interfaces.cpp 28.18% <28.18%> (ø)
hardware_interface/test/test_joint_handle.cpp 33.33% <40.00%> (+3.33%) ⬆️
hardware_interface/src/components/sensor.cpp 46.66% <50.00%> (+46.66%) ⬆️
...re_interface/include/hardware_interface/handle.hpp 82.14% <87.50%> (+8.80%) ⬆️
... and 12 more

@bmagyar bmagyar merged commit 3466439 into master Nov 2, 2020
@Karsten1987 Karsten1987 deleted the remodel_component_interfaces branch November 2, 2020 20:01
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

4 participants