add a broadcaster for range sensor

[flochre]

Hello!

I have been working on a broadcaster for Range sensors.
Here is a Pull Request to integrate it!

In the file ros2_controllers/range_sensor_broadcaster/include/range_sensor_broadcaster/range_sensor_broadcaster.hpp I have not put my name since I basically just change IMU with Range.. Is this ok or should I put my name on it?

I have prepared the following file ros2_control/controller_interface/include/semantic_components/range_sensor.hpp
on my fork https://github.com/flochre/ros2_control/blob/feature/add-range-sensor-broadcast/controller_interface/include/semantic_components/range_sensor.hpp - Should I already make a pull request by ros2_control?

Thanks in advance,

Cheers!

Contributions via pull requests are much appreciated. Before sending us a pull request, please ensure that:

1. Limited scope. Your PR should do one thing or one set of things. Avoid adding “random fixes” to PRs. Put those on separate PRs.
2. Give your PR a descriptive title. Add a short summary, if required.
3. Make sure the pipeline is green.
4. Don’t be afraid to request reviews from maintainers.
5. New code = new tests. If you are adding new functionality, always make sure to add some tests exercising the code and serving as live documentation of your original intention.

To send us a pull request, please:

• Fork the repository.
• Modify the source; please focus on the specific change you are contributing. If you also reformat all the code, it will be hard for us to focus on your change.
• Ensure local tests pass. (colcon test and pre-commit run (requires you to install pre-commit by pip3 install pre-commit)
• Commit to your fork using clear commit messages.
• Send a pull request, answering any default questions in the pull request interface.
• Pay attention to any automated CI failures reported in the pull request, and stay involved in the conversation.

[bmagyar]

Please check the results from the CI. The format stage is failing, you need to run pre-commit run --all to let it fix things for you. Also check the other stages. The code currently doesn't compile and you also have a lot of conversion warnings (purple text)

[flochre]

The compilation is also not working because of the requirement to ros2_control

repositories:
    ros2_control:
        type: git
        url: https://github.com/flochre/ros2_control.git
        version: feature/add-range-sensor-broadcast

[flochre]

About the conversion warning, I don't get them for the range_sensor_broadcaster
Those I have are coming from other packages.. Should I still correct them?

[bmagyar]

Well let's start at the compilation first.
Could you please open a PR against ros2_control then? We can discuss here all we want if we are missing the other side ;)

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.88%. Comparing base (e7f9962) to head (f795157).
Report is 588 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #725      +/-   ##
==========================================
- Coverage   35.78%   30.88%   -4.90%     
==========================================
  Files         189        7     -182     
  Lines       17570      832   -16738     
  Branches    11592      505   -11087     
==========================================
- Hits         6287      257    -6030     
+ Misses        994      133     -861     
+ Partials    10289      442    -9847     

Flag	Coverage Δ
unittests	30.88% <ø> (-4.90%)	⬇️

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

see 194 files with indirect coverage changes

[flochre]

I think everything should be set now ;)

[flochre]

Nice, compiling and testing worked!
Should I update the branch or you do it? Because if I do it the git actions will need to run again?
What do you prefere?
Thanks!

[bmagyar]

I didn't notice the load controller test here. Would you mind adding that too? Just copy it from any controller

[flochre]

Load test as been added and all test are passing in my workspace

[flochre]

Is there anything more I should do?

[bmagyar]

Thank you for sticking through the review, will wait for the CI to turn green and merge it!
I'll also backport this PR to humble

[christophfroehlich]

And I already got a notification of a warning in the docs ;)