As communicated with @Austin Hendrix, Brice Rebsamen, Our team at Persistent System is extending our help in porting diagnostic-aggregator and diagnostic-updater packages to ROS2. Please let us know if anyone has any concern or any important information that we will help us in this porting effort.
Hi,
You can find a first port of the diagnostic-updater package for ROS2 Ardent here : https://github.com/bponsler/diagnostics/tree/ros2-devel.
I don’t know if it’s a full port of the diagnostic-updater or not though.
I did a minor update to make it compile for ROS2 Bouncy here : https://github.com/MarcTestier/diagnostics/tree/ros2-devel. Just had to change from c++11 to c++14.
Hope it can help you.
@ MarcTestier, this is really helpful information and great pointer to start with. We also have additional communication with bponsler. We will check the state of the first port of diagnostic-updater and take necessary actions as required.
Is this planned to be a direct port or is there a design document for review somewhere?
We had some issues with FrequencyStatus diagnostics not matching rostopic hz, but much of that seemed intrinsic to ROS1. The window_size_ being measured in units of diagnostic_period events instead of time was somewhat disintuitive. At one point we had a similar issue with topics that update at less than 1Hz.
While ROS2 could still expose more metrics, ros2 topic hz was just implemented, so that should provide a starting point for FrequencyStatus diagnostics. However, I’m personally in favor of integrating metrics into the Publisher.
On a previous project, we could have used better status filtering for some devices. In particular we had a FLIR camera that would do it’s own self-test / running recalibration if, for example, the ambient temperature changed by 0.1C it would drop 1-3 frames while closing a shutter to recalibrate. Ideally we would have a way to escalate the status level when the topic frequency drops below min_freq over a given period of time. Sonar sensors may be a similar aperiodic use case.
It was never clear to me if hardware_id should be a UUID, serial number, VID/PID, mac address, etc.
Does information such as the vendor, model number, hardware/firmware version and/or ip address belong in diagnostics if it does not produce a status level change? I’m not sure it was the right choice but we ended up making separate topics to log introspected information to rosbags to track hardware changes. Much of the data really just needed to be stored once per bag file but the lack of metadata in ROS1 bag files prevented this from being a viable option given development resource constraints.
Does it make sense to implement a metadata/hardware_introspection/diagnostic group in the parameter server and have rosbag2 write everything from that parameter group into a chunk at the beginning of each bag? or a /metadata service that rosbag calls on a logged topic when the log rotates?
Our team also had questions about handling self_test when some of the tests should only be run when the system is not moving. Perhaps, diagnostic_msgs/DiagnosticStatus could use a SKIP status level.
Some of the tests we wanted to perform required external interaction / feedback but self_test_.checkTest() appears to block spin(). One way of looking at the problem is that much of what is needed for interactive testing is similar to a generic sensor calibration manager interface. Use cases include controlling rotating platform for IMU testing, dynamometer feedback, external pose estimation, calibration maneuvers (driving in circles, moving objects), etc.
We have always used the diagnostics not only for monitoring hardware, but also software behavior.
You can monitor the tf topic frequency in the same way you monitor a camera topic.
In that sense hardware_id has been something inconvenient, I don’t know if it makes sense for ROS2 to rename it to something more generic.
1 Like
@dirk-thomas : we have created new ros2_devel branch and merged changes from bponsler:ros2-devel branch. Additionally we have updated code and port test cases as per ROS2 guidelines to support ROS2 Bouncy.
Now we want to share this development but there is no branch for ros2 development on https://github.com/ros/diagnostics , so please suggest how to move forward in this case .
Create a ticket in the upstream repo asking for a `ros2 branch to be created so that you can create your PR against it.
@dirk-thomas, Thanks for your feedback. @trainman419 had created a ros2-devel branch under ros/diagnostics to support ROS2. we have raised a PR request https://github.com/ros/diagnostics/pull/91 against it but CI build check has failed (as below) that seems some build env issue as it is triggering ros-indigo-catkin. We have tested it locally and build/test is successful in Bouncy. so please suggest in this case.
$ sudo apt-get install python-catkin-pkg python-rosdep ros-indigo-catkin $ rosdep install --default-yes --from-paths ./ --ignore-src --rosdistro $CI_ROS_DISTRO ERROR: the following packages/stacks could not have their rosdep keys resolved to system dependencies: diagnostic_updater: Cannot locate rosdep definition for [ament_cppcheck] The command "rosdep install --default-yes --from-paths ./ --ignore-src --rosdistro $CI_ROS_DISTRO" failed and exited with 1 during
The .travis.yml file on your branch still uses ROS 1 logic to try to build and test the code. You either need to update it to actually test ROS 2 or remote it for now.
Hi dirk
thanks for your response and support .
@dirk-thomas @trainman419 Now we have successfully migrate diagnostic package to ros2 . we are looking forward for community response and suggestion for this package improvement.
pull request link is https://github.com/ros/diagnostics/pull/94
Sorry; I’ve been out of the loop here for a while (my discourse.ros.org subscription does not work very well).
The PR at https://github.com/ros/diagnostics/pull/94 seems like it has gotten out of hand; it’s over 160 files changed and over 65k lines of diff, and the CI has been removed. It’s unclear if it’s an update of the existing code, or a complete rewrite at this point.
As has been my feedback since the first PR, more than anything else, I’d like to see CI set up for this project, so that we can have some confidence in what works, and what still needs work.
I’ve haven’t seen any attempts to do that yet. I did some searching and found https://github.com/ros2/ci , https://ci.ros2.org/ and http://design.ros2.org/articles/changes.html . All of these mention CI, but none of them describe how to register the new ros2-devel branch with the ROS build farm or Travis.CI. I also found https://github.com/erlerobot/ros2_travis , but it seems very out of date and doesn’t seem to support testing on Windows (Since Windows is a first-class citizen in ROS2, I’d like to have some CI run on it).
@dirk-thomas: can you provide any references or tutorials on how to set up CI for ROS2 packages?
@vaibhavbhadade thank you for all of your hard work on this so far. I think to move forward, we need to break the current PR down into smaller pieces that can be reviewed incrementally. The obvious way to do that is:
- Renames and code changes in separate pull requests.
- Separate pull requests for each package within the diagnostics repository.
If you have functional or structural changes that you’re making, let’s discuss those before we discuss the details of the code changes.
@LucidOne suggests a common system for reporting sensor information (serial number, part number, etc). I do not know much about ROS2, but if the logger supports capturing parameters like that, then that would be an excellent feature to add, and would probably be good for it to replace the old hardware_id .
If ROS2 has support for embedding a generic message within another message type, it would be worth looking into ways to use that to replace the existing KeyValue message type with more structured data (trying to extract data from the ROS1 diagnostic messages is very painful).
There is no difference between ROS 1 and ROS 2 in that regards. You can do any of the following:
- Register the repository in the rosdistro database and the buildfarm will optionally build each commit on the target branch and/or every pull request against that target branch. This approach requires dependencies to be released and available as Debian packages.
- Use Travis (or any other service) to invoke a build of the repository using the
ros_buildfarmscripts. This can e.g. do a prerelease build which can span multiple not yet released repositories. - Manage your own custom build logic.
Ok; it seems like I’d like to register the repository with the rosdistro database so that the buildfarm builds every pull request against the ros2-devel branch. The bloom documentation at http://wiki.ros.org/bloom/Tutorials/FirstTimeRelease describes registration with rosdistro, but doesn’t describe how to enable builds for pull requests; is there some documentation on how to do that?
If I recall correctly bloom will interactively ask you if you want to enable the option.
This is correct. During the first release into a ROS distro, bloom will ask you interactively at the very end if you want to enable pull request testing. Also, you have to give ros-pull-request-builder access to your repo (or set up a webhook), as described here: http://wiki.ros.org/buildfarm/Pull%20request%20testing .
If you have already released a repo into a ROS distro, bloom won’t ask you again (even if you run bloom --edit). Then you have to make a manual pull request to rosdistro where you add the test_pull_requests flag).
@trainman419 I am working to break this PR into smaller but while porting from ros1 to ros2 multiple changes required in all files in single package so it will be difficult to insure code compilation at each commit but i will insure code compilation and test cases execution at each PR will it be fine ?
Also if we make one PR for sub packages like diagnostic_aggregator then that PR itself contain more than 30 files changes . will it be fine for you to review ?