Skip to content

Conversation

@jsantoso91
Copy link
Contributor

Summary:
I am updating the tests to use smart pointers for the gps broadcaster and gpio controller. This is similar to other tests e.g. test_pose_broadcaster.cpp
This to remove warning LifecycleNode is not shut down during test as shown in test log https://github.com/ros-controls/ros2_controllers/actions/runs/14608595520/job/40982298949. Issue is captured in #1656

Qualification
$ pre-commit install
$ colcon build --packages-select gpio_controllers gps_sensor_broadcaster
$ colcon test --packages-select gpio_controllers gps_sensor_broadcaster --event-handlers console_direct+
See that the warnings are no longer present

I am relatively new to ROS/2 and ros_control ecosystems, let me know if I am missing anything, thanks!

===

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.

@codecov
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

Attention: Patch coverage is 98.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 84.79%. Comparing base (8c649a3) to head (5ab8b2e).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
..._controllers/test/test_gpio_command_controller.cpp 98.18% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1658      +/-   ##
==========================================
+ Coverage   84.77%   84.79%   +0.02%     
==========================================
  Files         127      127              
  Lines       12096    12114      +18     
  Branches     1036     1036              
==========================================
+ Hits        10254    10272      +18     
  Misses       1503     1503              
  Partials      339      339              
Flag Coverage Δ
unittests 84.79% <98.66%> (+0.02%) ⬆️

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

Files with missing lines Coverage Δ
...r_broadcaster/test/test_gps_sensor_broadcaster.cpp 100.00% <100.00%> (ø)
..._controllers/test/test_gpio_command_controller.cpp 99.00% <98.18%> (+0.03%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich linked an issue Apr 26, 2025 that may be closed by this pull request
8 tasks
@christophfroehlich christophfroehlich changed the title Fix for issue #1656 - Update test_gps_sensor_broadcaster.cpp and test_gpio_command_controller.cpp to use smart pointers Use smart pointer of ctrl in GpsSensor and GpioCommandController tests Apr 26, 2025
Copy link
Contributor

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

Thanks, this LGTM

@christophfroehlich christophfroehlich merged commit e2e3df0 into ros-controls:master Apr 26, 2025
24 of 25 checks passed
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.

Fix tests of GPSSensorBroadcasterTest and GPIO controller

2 participants