-
Notifications
You must be signed in to change notification settings - Fork 993
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
[WIP] Set framework to define offset between global origin and current local position #691
Conversation
Somehow latest https://github.com/ros-geographic-info/geographic_info available on release package doesn't include http://docs.ros.org/api/geographic_msgs/html/msg/GeoPointStamped.html. Will need to wait for ros-geographic-info/geographic_info@b15dc33 to be included - at the same time, also mavlink/mavlink#696 so to be able to use the time-stamp. |
Travis failing because of undated |
Where translation of int32 encoded lat/long to double and back? |
Fixed in c4941a9 |
g_origin->position.altitude = glob_orig.altitude; | ||
g_origin->position.latitude = (double)glob_orig.latitude; // @warning TODO: #529 | ||
g_origin->position.longitude = (double)glob_orig.longitude; | ||
g_origin->position.altitude = (double)glob_orig.altitude; |
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.
Read message documentation! That is not how you should convert int32 encoded coord's to floating point.
We still need to consider #529 |
@mhkabir from your perspective, the |
Still waiting fo rosbuild farm update. I was expecting that |
Seems there's still no new release for indigo so travis is still failing when looking for new msg. In the other hand, the releases for jade and kinetic are already out. |
Great. So now we have this ready for indigo also. Will push the handler for |
gps_cov(2, 0) , gps_cov(2, 1) , gps_cov(2, 2) , 0.0 , 0.0 , 0.0 , | ||
0.0 , 0.0 , 0.0 , rot_cov , 0.0 , 0.0 , | ||
0.0 , 0.0 , 0.0 , 0.0 , rot_cov , 0.0 , | ||
0.0 , 0.0 , 0.0 , 0.0 , 0.0 , rot_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.
Hmm, we probably may do that:
pos_cov_out.setZero();
pos_cov_out.block<3, 3>(0, 0) = gps_cov;
pos_cov_out.block<3, 3>(3, 3).diagonal() = rot_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.
yeah. more reasonable
This seems reasonable to me now. At mavlink side we can probably add:
|
e5c6ef2
to
555c3f1
Compare
Ok according to REP 105, if we have a case where we may have different This same coordinate frame should be called This same coordinate system should be represented in the ECEF ("earth-centered, earth-fixed") geographic coordinate system, so this involves a conversion from AMSL to ECEF, that is supported by geographiclib. It can be added after #693 is merged. @jgoppert thanks for pointing this out. We can expect also that |
…s84, frame_id!=global_frame_id)
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.
Did you tested?
@@ -70,14 +71,24 @@ class GlobalPositionPlugin : public plugin::PluginBase { | |||
gp_odom_pub = gp_nh.advertise<nav_msgs::Odometry>("local", 10); | |||
gp_rel_alt_pub = gp_nh.advertise<std_msgs::Float64>("rel_alt", 10); | |||
gp_hdg_pub = gp_nh.advertise<std_msgs::Float64>("compass_hdg", 10); | |||
|
|||
// global origin | |||
gp_global_origin_pub = gp_nh.advertise<geographic_msgs::GeoPointStamped>("gp_origin", 10); |
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.
Why not latching type?
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 do you mean with latching type?
make_handler(&GlobalPositionPlugin::handle_global_position_int) | ||
make_handler(&GlobalPositionPlugin::handle_global_position_int), | ||
make_handler(&GlobalPositionPlugin::handle_gps_global_origin), | ||
make_handler(&GlobalPositionPlugin::handle_lpned_system_global_offset) |
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.
Wtf here with tab/space?
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.
can't tell. uncrustify made this
msg.latitude = point->position.latitude * 1E7; // deg | ||
msg.longitude = point->position.longitude * 1E7; // deg | ||
msg.altitude = point->position.altitude * 1E3; // m | ||
} |
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.
We need more mess with fill_xyz()
!
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.
cleaned
@@ -127,7 +160,7 @@ class GlobalPositionPlugin : public plugin::PluginBase { | |||
fix->status.status = sensor_msgs::NavSatStatus::STATUS_NO_FIX; | |||
} | |||
|
|||
fill_lla(raw_gps, fix); | |||
fill_lla_wgs84(raw_gps, fix); |
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.
And altitude is still AMSL.
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.
yes cause we are missing #693. Or we first merge this or the other
|
||
g_origin->header.frame_id = tf_global_frame_id; | ||
g_origin->header.stamp = ros::Time::now(); | ||
fill_lla_ecef(glob_orig, g_origin); // @warning TODO: #693 |
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.
Used once.
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.
ok I can push it to the body. I just then have to sync with #693 then.
0.0 , 0.0 , 0.0 , 0.0 , 0.0 , rot_cov ; | ||
pos_cov_out.setZero(); | ||
pos_cov_out.block<3, 3>(0, 0) << gps_cov; | ||
pos_cov_out.block<3, 3>(3, 3).diagonal() << rot_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.
I'm not sure, but odom
should be zeroed at make_shared()
. Also use operator=
, and check that scalar rot_cov
actually assigned to whole vector.
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.
ok sure
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.
covariance: [0.656100003862381, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.656100003862381, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 2.624400015449524, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 99999.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 99999.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 99999.0]
for:
pos_cov_out.setZero();
pos_cov_out.block<3, 3>(0, 0) = gps_cov;
pos_cov_out.block<3, 3>(3, 3).diagonal() <<
rot_cov,
rot_cov,
rot_cov;
|
||
gpo.target_system = m_uas->get_tgt_system(); | ||
// gpo.time_boot_ms = stamp.toNSec() / 1000; #TODO: requires Mavlink msg update | ||
fill_lla_amsl(req, gpo);// @warning TODO: #693 |
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.
Also used once.
…t local position (mavlink#691) * add handlers for GPS_GLOBAL_ORIGIN and SET_GPS_GLOBAL_ORIGIN * fix cast of encoding types * refactor gps coord conversions * uncrustify * global_position: add LOCAL_POSITION_NED_SYSTEM_GLOBAL_OFFSET handler * global_position: add trasform sender for offset * global_origin: refactor covariance matrix * global_position: update copyright * global_position: add initial support to REP 105 * px4_config: global_position: update frame description * global_position: correct identation * global position: be consistent with frame and methods names (ecef!=wgs84, frame_id!=global_frame_id) * global_position: updates to code structure * global_position: fix identation
This is WIP and pretends to implement a framework to set the global origin of the system so it can be used to compute the offset between the current
local_position_ned
of the drone and this same global origin. Useful for flexible nav between GPS denied environments and GPS permitted environments.GPS_GLOBAL_ORIGIN
andSET_GPS_GLOBAL_ORIGIN
LOCAL_POSITION_NED_SYSTEM_GLOBAL_OFFSET