Skip to content
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

AP_NavEKF3/AP_VisualOdom/SITL: add support for vision-speed-estimates #14404

Merged
merged 12 commits into from
Jun 1, 2020

Conversation

rmackay9
Copy link
Contributor

This is built on top of PR #14368 from @chobitsfan (this original PR should be merged first).

This PR has the following enhancements (on top of the older PR):

  • EKF3 is enhanced to consume the VISION_SPEED_ESTIMATE mavlink messages via the AP_VisualOdom library
    • Vision-position-estimate messages must also be received for it to function properly (this keeps the code similar to the GPS which can also provide position and velocity but not velocity alone)
    • ResetVelocity enhanced to reset to the external nav velocity
    • CorrectExtNavVelForSensorOffset() corrects the external navigation's velocity based on the sensor position
  • AP_Logger's VISV message is used to log all the new data
  • SITL's SIM_Vicon class is enhanced so it can send the vision-speed-estimate message and the following params are added:
    • SIM_VICON_TMASK (for "Type Mask") allows real-time control of which of the three supported messages (vision-position-estimate, vision-speed-estimate and vicon-position-estimate) are sent
    • SIM_VICON_VGLIT_X/Y/Z allows adding an earth-frame glitch (in m/s) to the simulated velocity
  • AP_Math::get_vel_correction_for_sensor_offset is added so we can reduce some duplicate code in EKF2, EKF3 and the simulator that adjusts the velocity based on the sensor's offsets. In this PR the new function is only used for external nav velocities so the scope is kept small

Some non-blocking issues:

  • if vision-speed-estimates are sent without also sending vision-position-estimates the EKF will not complete initialisation. Theoretically we should be able to provide only velocities but this does not work yet from some reason and is left for a future PR
  • the vision-speed-estimate messages's reset_counter are logged but not used. In the short-term this is OK because Thien's T265 scripts will send the reset using the vision-position-estimate message but eventually we will want to support this especially if we resolve the issue directly above
  • we should extend the AP_Math::get_vel_correction_for_sensor_offset to be used for the existing GPS code. This was not done in order to limit the scope of this PR's changes

Testing done so far:

  • The EKF2 changes have been tested in SITL and on a real vehicle
  • The EKF3 changes have been tested in SITL including
    • checking that the velocity corrections for sensor position are correct
    • vision sensor failure and glitches seem to be handled correctly

Some more SITL and actual flight tests (using EKF3) will be completed before merging

@rmackay9
Copy link
Contributor Author

This has been successfully test flown and below are some shots of the position and velocity innovations which appear quite low.
pos-innovs
vel-innovs

@rmackay9
Copy link
Contributor Author

Here's a flight video test of this PR
https://www.youtube.com/watch?v=jCY9-fWtA7c

@@ -251,6 +251,12 @@ const AP_Param::GroupInfo SITL::var_info3[] = {
// vicon yaw error in degrees (added to reported yaw sent to vehicle)
AP_GROUPINFO("VICON_YAWERR", 19, SITL, vicon_yaw_error, 0),

// vicon message type mask
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

be nice for vicon params to be in a child table

Copy link
Contributor

@priseborough priseborough left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main issue is the lack of accuracy information being passed via this interface. If unavailable from the sensor, then it should be set to a default or parameter defined value in the sensor driver.

Use of specified minimum GPS velocity error to set observation noise for this sensor type should be avoided.

A recommended improvement is to apply the position offset correction to the delayed data immediately after it is retrieved from the buffer rather than apply it separately in the fusion and reset operations.

@chobitsfan
Copy link
Contributor

chobitsfan commented May 28, 2020

Hi @rmackay9 I do some changes address @priseborough 's comments in #14368. Could you be able to take a look? Thank you very much

I do some flight test and it looks well.
22 2020-5-28 PM 02-34-22.zip

@rmackay9
Copy link
Contributor Author

rmackay9 commented Jun 1, 2020

This has been rebased on @chobitsfan's PR #14368

@rmackay9
Copy link
Contributor Author

rmackay9 commented Jun 1, 2020

Rebased on master and two commits added to address requests from @priseborough.

@priseborough priseborough self-requested a review June 1, 2020 21:09
@rmackay9 rmackay9 merged commit 93d8458 into ArduPilot:master Jun 1, 2020
@rmackay9 rmackay9 deleted the t265-sitl-vicon-vel3 branch June 2, 2020 00:35
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants