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

sensors/vehicle_gps_position: untangle and remove unnecessary state #15827

Merged
merged 1 commit into from
Jan 14, 2021

Conversation

dagar
Copy link
Member

@dagar dagar commented Sep 27, 2020

@JacobCrabill @bresch this is the refactor/cleanup of GPS blending originally from #14447.

NOT TESTED.

This would be a good excuse to add proper testing in SITL for GPS blending and perhaps even restructure the blending logic to make it unit testable.

@dagar
Copy link
Member Author

dagar commented Sep 27, 2020

Relatedly, now that GPS blending has moved to the sensors hub we can think about expanding it slightly for Multi-EKF (#14650).

  • SENS_GPS_MODE
    • publish all GPS, publish a specific instance, publish blended, publish highest priority, etc

We also need to add per instance metadata at the sensors level instead of single set of position offsets in EKF2.

  • EKF2_GPS_POS_{X,Y,Z} -> SENS_GPS_n_POS_{X,Y,Z} (FYI @CarlOlsson)
  • to keep things simple we can pass the position in vehicle_gps_position since the message is low rate (5-20 Hz)
  • GPS will need device ids

float min_alpha = constrain(omega_lpf * 1e-6f * (float)(_gps_state[i].timestamp - _time_prev_us[i]),
0.0f, 1.0f);

// scale the filter coefficient so that time constant is inversely proprtional to weighting
Copy link
Member

Choose a reason for hiding this comment

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

isn't that useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like it was removed back in ekf2, but somehow got lost in the drawn out PR sitting around for 6 months.

8de1f52#diff-72bb106f8fdfef0de0e8174f5b63ad2987bd47a8f07ff445bde40a812c87651e

@dagar dagar force-pushed the pr-sensors_vehicle_gps_position_cleanup branch from c0751e5 to 129dd31 Compare December 11, 2020 15:27
@dagar dagar changed the title [WIP] sensors/vehicle_gps_position: untangle and remove unnecessary state sensors/vehicle_gps_position: untangle and remove unnecessary state Dec 11, 2020
@dagar dagar marked this pull request as ready for review December 11, 2020 15:28
@dagar
Copy link
Member Author

dagar commented Dec 11, 2020

A lot of the state has been removed, but this is still hard to follow. Let's layout a simple test plan here.

@JacobCrabill
Copy link
Member

Just took a brief look through, and seems ok - I'll give myself a TODO to try it out with a couple GPSs on a real board, and capture logs.

@JacobCrabill
Copy link
Member

Blending looks good (using dual Here3 GPS units over UAVCAN) (rebased just before testing to ensure the UAVCAN bridge would work):
image
image

@JacobCrabill JacobCrabill force-pushed the pr-sensors_vehicle_gps_position_cleanup branch from 129dd31 to 78c61a1 Compare January 13, 2021 23:50
@JacobCrabill
Copy link
Member

@dagar to be clear, all I did for testing was ensure that two sensor_gps publications were combined in a convex combination into the vehicle_gps_position topic; not exactly thorough, but more of a sanity check.

@dagar
Copy link
Member Author

dagar commented Jan 14, 2021

Thanks @JacobCrabill

@dagar dagar merged commit a0d8d5a into master Jan 14, 2021
@dagar dagar deleted the pr-sensors_vehicle_gps_position_cleanup branch January 14, 2021 01:17
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.

None yet

3 participants