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

Move emit of primary-GPS-changed into AP_GPS #18163

Merged

Conversation

peterbarker
Copy link
Contributor

This wasn't really a sensor-health thing, either....

New addition to the blending test ensures we see the message come out in the log

This change will mean all vehicles can expect to see this event emitted.

Copy link
Contributor

@amilcarlucas amilcarlucas left a comment

Choose a reason for hiding this comment

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

Nice change

@peterbarker peterbarker changed the title Move emit of primary-compass-changed into AP_GPS Move emit of primary-GPS-changed into AP_GPS Aug 3, 2021
@rmackay9
Copy link
Contributor

rmackay9 commented Aug 3, 2021

As discussed, let's initialise the primary value so we don't get a message in the log every time soon after startup.

@peterbarker
Copy link
Contributor Author

Incidentally, I'm kind of the opinion it would be OK to retain the current behaviour of logging a primary change if you're set up to not-auto-switch and not auto-switching. It's an atypical setup and we could save flash and complexity.

@peterbarker
Copy link
Contributor Author

I've added the requested change - so now we don't emit this message for any change on the first loop, which is what will happen if you both set GPS_PRIMARY to (e.g.) 1 and change GPS_AUTO_SWITCH to zero.

Initialising the primary_instance state to be equal to the GPS_PRIMARY variable is insufficient without also checking GPS_AUTO_SWITCH. While we probably should honour GPS_PRIMARY even at initialisation regardless of GPS_AUTO_SWITCH being on or off, this patch should still be the same shape IMO - leave the logic in update_primary, don't duplicate it in initialisation.

@peterbarker
Copy link
Contributor Author

Oh. Tested in SITL by setting LOG_DISARMED, GPS_PRIMARY and GPS_AUTO_SWITCH and checking what EV messages got emitted.

@peterbarker peterbarker force-pushed the pr/compass-primary-changed branch 2 times, most recently from 5f16901 to 4c7ed37 Compare August 4, 2021 07:26
@tridge tridge removed the DevCallEU label Aug 4, 2021
@peterbarker peterbarker merged commit 44d5885 into ArduPilot:master Aug 6, 2021
@peterbarker peterbarker deleted the pr/compass-primary-changed branch August 6, 2021 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants