-
Notifications
You must be signed in to change notification settings - Fork 299
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
Update test_resource_manager.cpp - Fixed Typo #1609
Conversation
Fixed typo causing errors during build, test case was written as initilizable and not initializable as was expected by the tests.
Apologies, I also missed correcting the typo on line 118 of the same file, and on line 258 in ros2_control/install/ros2_control_test_assets/include/ros2_control_test_assets/descriptions.hpp at line 458. Not quite used to using the web editor for pull requests. I am uncertain the appropriate way to handle this? |
You are right about a typo, but it is still not correct: it should be
You can checkout the PR locally, or just open the patch-1 branch of your fork in the github UI, edit file, and commit directly to the branch. |
While using colcon build, I got a blocking error stating the following, among other unrelated errors (I replaced my local path with ~~)
|
also I am laughing at the fact I didn't catch the fact that it's in and not un. Good catch on your part |
Fixed ininitializable to uninitializable (Even for a native English speaker that's a rough one)
The changes look legit but we fixed those typos months ago and had many releases since. I'm wondering how a few creeped back into the codebase especially w the CI being happy |
I'm wondering the same, maybe @Parker-Drouillard has a very old release installed as binary, and colcon uses now the test assets from the old release instead of the new one from the src folder? |
I doubt that, I only started with ROS a week ago, and the hardware I'm on was a fresh install as of two days ago. I have seen the typo in both the master and humble branches. This is my first ever open source contribution though so it's possible I'm doing something wrong! |
From what I remember earlier, the changes had been pushed back in something like 5 days ago by someone. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1609 +/- ##
=======================================
Coverage 87.30% 87.30%
=======================================
Files 108 108
Lines 9866 9866
Branches 890 890
=======================================
Hits 8614 8614
Misses 929 929
Partials 323 323
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The typo fix of the current commit is fine, but should not change anything regarding your build failure. And our CI on the master as well as humble branch is happy.. |
I believe yes. I faced the same as your said @christophfroehlich |
Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
Is this possible considering I'm on a fresh install of Ubuntu 22.04 as of a few days ago? I also only started getting into ros2 about a week ago, am I somehow pulling old binaries? |
Yes, because since a couple of months there is no rolling release to 22.04 any more -> there was a switch to noble 24.04. I'm not sure what we can do to avoid such a confusion. @saikishor a compile-time error if the wrong version of hardware_interface_testing is to be built? |
I've tried using jammy and compiling it from source and it works. I have been working on Jammy for last 2 months |
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
* [ResourceManager] Make destructor virtual for use in derived classes (#1607) * Fix typos in test_resource_manager.cpp (#1609) * [CM] Remove support for the description parameter and use only `robot_description` topic (#1358) --------- Co-authored-by: Felix Exner <exner@fzi.de> Co-authored-by: Christoph Froehlich <christoph.froehlich@ait.ac.at> * move critical variables to the private context (#1623) * Fix controller starting with later load of robot description test (#1624) * Fix the duplicated restart of the controller_manager services initialization * Scope the ControllerManagerRunner to avoid malloc and other test issues * reorder the tests for consistent log at the end * Remove noqa (#1626) * Unused header cleanup (#1627) * Create debugging.rst (#1625) --------- Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com> Co-authored-by: Christoph Froehlich <christoph.froehlich@ait.ac.at> * Update changelogs * 4.14.0 * add missing rclcpp logging include for Humble compatibility build (#1635) * [CM] Remove deprecated spawner args (#1639) * Add a pytest launch file to test ros2_control_node (#1636) * Fix rst markup (#1642) * Fix rqt_cm paragraph * Fix indent * CM: Add missing includes (#1641) * [RM] Add `get_hardware_info` method to the Hardware Components (#1643) * Fix the namespaced controller_manager spawner + added tests (#1640) * Bump version of pre-commit hooks (#1649) Co-authored-by: christophfroehlich <3367244+christophfroehlich@users.noreply.github.com> * Add missing include for executors (#1653) * Update changelogs * 4.15.0 * refactor SwitchParams to fix the incosistencies in the spawner tests (#1638) * Modify test with missing CM to have a timeout * Catch exception when CM services are not found And print the error and exit in the application * Exit with code 1 on unreached CM --------- Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com> Co-authored-by: Parker Drouillard <parker@pepcorp.ca> Co-authored-by: Dr. Denis <denis@stoglrobotics.de> Co-authored-by: Christoph Froehlich <christoph.froehlich@ait.ac.at> Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com> Co-authored-by: Henry Moore <henry.moore@picknik.ai> Co-authored-by: Lennart Nachtigall <firesurfer127@gmail.com> Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com> Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: christophfroehlich <3367244+christophfroehlich@users.noreply.github.com>
Fixed typo causing errors during build, test case was written as initilizable and not initializable as was expected by the tests.