-
Notifications
You must be signed in to change notification settings - Fork 986
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
extras: add covariance parsing to vision_speed_estimate #996
Conversation
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.
Mostly Ok. Only few minor things.
@@ -46,6 +48,7 @@ class VisionSpeedEstimatePlugin : public plugin::PluginBase { | |||
|
|||
if (listen_twist) | |||
vision_vel_sub = sp_nh.subscribe("speed_twist", 10, &VisionSpeedEstimatePlugin::vel_twist_cb, this); | |||
|
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.
Should not be there.
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.
actually I did this PR quickly and didn't realize I forgot to add the speed_twist_cov
option.
@@ -62,12 +65,11 @@ class VisionSpeedEstimatePlugin : public plugin::PluginBase { | |||
|
|||
/* -*- low-level send -*- */ | |||
|
|||
void vision_speed_estimate(uint64_t usec, Eigen::Vector3d &v) | |||
void vision_speed_estimate(uint64_t usec, const Eigen::Vector3d &v, const mavros::ftf::Covariance3d &cov) |
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.
mavros
namespace shold be already used. So just ftf::Covariance3d
.
@@ -76,38 +78,53 @@ class VisionSpeedEstimatePlugin : public plugin::PluginBase { | |||
vs.y = v.y(); | |||
vs.z = v.z(); | |||
// [[[end]]] (checksum: aee3cc9a73a2e736b7bc6c83ea93abdb) | |||
ftf::covariance_to_mavlink(cov, vs.covariance); |
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.
Leave blank lines before and after cog block.
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.
Any good reason for it or is just for aesthetics?
void vel_twist_cb(const geometry_msgs::TwistStamped::ConstPtr &req) { | ||
send_vision_speed(req->twist.linear, req->header.stamp); | ||
void vel_twist_with_cov_cb(const geometry_msgs::TwistStamped::ConstPtr &req) { | ||
ftf::Covariance3d cov {}; // zero initialized |
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.
Does mavlink propose some value for incorrect cov? Like [-1, 0, 0...] in ROS msgs.
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.
It doesn't. But should be a good idea to bring yes.
|
||
// only the linear velocity will be sent | ||
cov6d = req->twist.covariance; | ||
cov3d_map = cov6d_map.block<3, 3>(0, 0); |
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.
Unnecessary copy.
ftf::EigenMapConstCovariance6d cov6d_map(req->twist.covariance);
cov3d_map = cov6d_map.block<3, 3>(0, 0);
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.
Makes sense. Thanks
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.
ftf::EigenMapConstCovariance6d cov6d_map(req->twist.covariance.data());
btw.
a83c251
to
32fe279
Compare
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.
What about covariance unused/unknown special value? All 0?
{ | ||
mavlink::common::msg::VISION_SPEED_ESTIMATE vs{}; | ||
|
||
vs.usec = usec; | ||
|
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.
You forgot to remove that change.
//Transform from ENU to NED frame | ||
auto vel = ftf::transform_frame_enu_ned(vel_); | ||
Eigen::Vector3d vel; | ||
tf::vectorMsgToEigen(vel_enu, vel); |
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.
Use ftf::to_eigen()
, lets remove most of tf:vectorMsgToEigen()
.
32fe279
to
d67711b
Compare
Still no special value. I will do a RFC on Mavlink to that respect. |
Jenkins test this please. |
Jenkins already test it. Builds ok, just mavconn test segfaults again. Also i'm not sure that ros jenkins watch on comments. When you have |
No description provided.