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

Prevent duplicate reporting of GPS heading #10346

Merged
merged 2 commits into from
Aug 28, 2018

Conversation

priseborough
Copy link
Contributor

@priseborough priseborough commented Aug 27, 2018

During replay log testing of changes for issue #10284 is was noticed that the GPS driver was not setting the .heading data field to NAN when the data was not updated by the receiver.

The receiver used to generate the replay logs was updating heading at a 1Hz rate resulting in the last measurement being held for subsequent frames instead of setting the value to NAN. This resulted in repeated fusion of the same data and large innovation transients when the vehicle yawed:

screen shot 2018-08-24 at 7 46 15 am

The 1Hz update and hold behaviour for the yaw data can be seen here:

screen shot 2018-08-24 at 7 48 58 am

Test data / coverage
I do not have access to physical hardware to gather logs with this change.

@@ -882,6 +882,11 @@ GPS::publish()
if (_instance == Instance::Main || _is_gps_main_advertised) {
orb_publish_auto(ORB_ID(vehicle_gps_position), &_report_gps_pos_pub, &_report_gps_pos, &_gps_orb_instance,
ORB_PRIO_DEFAULT);
if (_mode == GPS_DRIVER_MODE_ASHTECH) {
Copy link
Member

Choose a reason for hiding this comment

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

I would remove the check for the driver mode, and always set it to NAN.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK

@bkueng bkueng merged commit ad1c2b3 into PX4:master Aug 28, 2018
@priseborough priseborough deleted the pr-gpsHeadingFix branch August 28, 2018 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants