Semantic components cleanup

[Wiktor-99]

I was going to introduce a new semantic component (GPSSensor), but I noticed that the current implementation of semantic components has a few flaws. Therefore, I decided to prepare a small PR with minor fixes. All UTs are green.

Some of the changes I made:

• getters used to return by value and store to member, now getters return only by value (RVO will work)
• modernize constructors
• remov defaulted destructors, in this case default destructors will be generated
• add virtual destructors in base class

[codecov]

Codecov Report

Attention: Patch coverage is 97.22222% with 3 lines in your changes missing coverage. Please review.

Project coverage is 88.07%. Comparing base (038a05f) to head (108348e).
Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
...nclude/semantic_components/force_torque_sensor.hpp	96.87%	1 Missing ⚠️
...terface/include/semantic_components/imu_sensor.hpp	97.61%	1 Missing ⚠️
...erface/include/semantic_components/pose_sensor.hpp	96.42%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1940      +/-   ##
==========================================
+ Coverage   88.03%   88.07%   +0.04%     
==========================================
  Files         121      121              
  Lines       13235    13225      -10     
  Branches     1193     1193              
==========================================
- Hits        11651    11648       -3     
+ Misses       1141     1139       -2     
+ Partials      443      438       -5     

Flag	Coverage Δ
unittests	88.07% <97.22%> (+0.04%)	⬆️

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

Files with missing lines	Coverage Δ
...mantic_components/semantic_component_interface.hpp	90.90% <100.00%> (+0.43%)	⬆️
...nclude/semantic_components/force_torque_sensor.hpp	97.95% <96.87%> (-2.05%)	⬇️
...terface/include/semantic_components/imu_sensor.hpp	97.67% <97.61%> (-2.33%)	⬇️
...erface/include/semantic_components/pose_sensor.hpp	96.55% <96.42%> (-3.45%)	⬇️

... and 7 files with indirect coverage changes

[bmagyar]

Generally looks pretty good. I'm not a huge fan of the {} value initializers but they have their uses.

[saikishor]

In general LGTM.

I'm not a huge fan of the {} value initializers as @bmagyar. At some places it is fine, I added some suggestions to my taste.

Apart from the {} initializers, there are other changes

[saikishor]

@Wiktor-99 all the open review comments aren't addressed yet

[Wiktor-99]

@saikishor Sorry, I must have overlooked those unresolved comments. Now should be okay.

[saikishor]

@saikishor Sorry, I must have overlooked those unresolved comments. Now should be okay.

No worries.
Thank you.

[saikishor]

@christophfroehlich do you know why the clang jobs are not starting properly here?

[christophfroehlich]

FastCDR regression

/usr/bin/ld: /opt/ros/rolling/lib/libtest_msgs__rosidl_typesupport_fastrtps_c.so: undefined reference to `eprosima::fastcdr::Cdr::serialize(char*)'
/usr/bin/ld: /opt/ros/rolling/lib/libtest_msgs__rosidl_typesupport_fastrtps_c.so: undefined reference to `eprosima::fastcdr::Cdr::serialize(signed char)'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

don't ask me why clang complains while linking, and gcc doesn't

[saikishor]

FastCDR regression

/usr/bin/ld: /opt/ros/rolling/lib/libtest_msgs__rosidl_typesupport_fastrtps_c.so: undefined reference to `eprosima::fastcdr::Cdr::serialize(char*)'
/usr/bin/ld: /opt/ros/rolling/lib/libtest_msgs__rosidl_typesupport_fastrtps_c.so: undefined reference to `eprosima::fastcdr::Cdr::serialize(signed char)'
clang++: error: linker command failed with exit code 1 (use -v to see invocation)

don't ask me why clang complains while linking, and gcc doesn't

Ahhh gotcha!

[saikishor]

@Wiktor-99 Thank you for fixing things around!

[saikishor]

LGTM

[christophfroehlich]

Thx!