Bridge DVL messages #789
Draft
Bridge DVL messages #789
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Carlos Agüero <caguero@osrfoundation.org>
ahcorde
requested changes
Jul 28, 2025
…gs.hpp Co-authored-by: Alejandro Hernández Cordero <ahcorde@gmail.com> Signed-off-by: Carlos Agüero <cen.aguero@gmail.com>
hidmic
reviewed
Jul 31, 2025
|
|
||
| // Unsupported. | ||
| for (auto j = 0; j < 9; ++j) { | ||
| beam->mutable_velocity()->add_covariance(0); |
There was a problem hiding this comment.
@caguero gz-sim accepts a null covariance matrix?
| ros_msg.range[numGoodBeams] = gz_msg.beams()[i].range().mean(); | ||
| ros_msg.range_covar[numGoodBeams] = gz_msg.beams()[i].range().variance(); | ||
| ros_msg.beam_quality[numGoodBeams] = gz_msg.beams()[i].rssi(); | ||
| gz::math::Vector3d v = gz::msgs::Convert(gz_msg.beams()[i].velocity().mean()); |
Comment on lines
+149
to
+151
| ros_msg.beam_unit_vec[numGoodBeams].x = -1; | ||
| ros_msg.beam_unit_vec[numGoodBeams].y = -1; | ||
| ros_msg.beam_unit_vec[numGoodBeams].z = -1; |
There was a problem hiding this comment.
@caguero why not use the normalized mean velocity vector? I don't know if the frames match but we are already assuming they do for the ros to gz conversion.
| compareTestMsg(std::make_shared<gz::msgs::Header>(_msg->header())); | ||
| EXPECT_EQ(expected_msg.type(), _msg->type()); | ||
| // compareTestMsg(std::make_shared<gz::msgs::DVLTrackingTarget>(_msg->target())); | ||
| // compareTestMsg(std::make_shared<gz::msgs::DVLKinematicEstimate>(_msg->velocity())); |
Comment on lines
+126
to
+128
| ros_msg.course_gnd = std::atan2(ros_msg.velocity.y, ros_msg.velocity.x); | ||
| ros_msg.speed_gnd = std::sqrt(ros_msg.velocity.x * ros_msg.velocity.x + ros_msg.velocity.y * | ||
| ros_msg.velocity.y); |
There was a problem hiding this comment.
@caguero meta: I would double check frames are correct. It's not clear to me what the ROS message definition means by ground speed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🎉 New feature
Closes #741
Summary
This patch adds support to bridge DVL messages.
I'm using this DVL message definition in the ROS side and this one on the Gazebo side.
I'm opening this PR as a draft because we still need to figure out some discrepancies between messages. I'll start with the Gazebo -> ROS conversion. I invoke @rolker and @hidmic . I'd love you two to join the discussion if have some spare cycles. cc/ @bsb808.
Gazebo -> ROS Discussion
altituderequires to know the location of the DVL beams with respect to the vehicle. I don't think we have the pose of the sensor in the Gazebo message. We could potentially include aposein theDVLBeamStatemessage. At the same time this will be a redundant message sent over and over all the time.num_good_beams. How that field should be used? If this field tells me only two beams are good, which of the four beams are the two good ones? Should their elements be rearranged within the array in a way that the two first ones are the good ones? Or this is just purely informative but it shouldn't be used as an index in the arrays. Gazebo has thelockedfield in each beam which captures the same spirit. Should the "bad beams" be completely ignored? That's what I'm trying to do when copying the data into Gazebo.sound_speedisn't supported in Gazebo.-1for now? Add a field inDVLVelocityTracking? Is1500a good default value?beam_unit_vec: See (1).beam_quality. I'm associating it torssibut we could also set it to-1if it's too specific.velocity_covaris afloat32. In Gazebo this is a 3x3 matrix. I guess we have three variance values in the diagonal in Gazebo, one for each velocity component. Do you have a suggestion about how to populate thevelocity_covarfor each beam here?Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-byandGenerated-bymessages.