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

revert msp method for gps sat info messages #13245

Conversation

ctzsnooze
Copy link
Member

@ctzsnooze ctzsnooze commented Dec 27, 2023

This restores the reliable sending of SatInfo data from the GPS module on power up, which was lost with PR #13240.

Any version of Configurator should reliably see satellite info on connecting a firmware 4.5 quad to betaflight. There is no requirement to have the latest nightly version of Configurator. Sat info data will be sent by the GPS once it has established a serial connection and once the early configuration is complete, typically about 2s after powering up. Then, on arming, a message is sent to the module to tell it to stop sending the Satellite Info data.

This achieves the same effective outcome as the code before PR #13240, which it largely reverts, but does not require Configurator to be connected within a certain time at the start. The module will just get told to send satellite info until it is armed. After that, no more Satellite info until a power cycle.

it is a simpler alternative to #13244, which introduces a CLI value to change the time for the check for whether Configurator is connected.

I've tested it and it works. Sat info is reliably displayed by all Configurator versions back to 10.8, and stops on arming.

There may be a possible user interest in re-enabling the Sat Info on disarming, but I don't see it being an issue at this point in time. And previously it was not possible to re-enable the data.

Nor should there be a problem if we send SatInfo data from the time the module turns on.

Copy link

Do you want to test this code? You can flash it directly from Betaflight Configurator:

  • Simply put #13245 (this pull request number) in the Select commit field of the Configurator firmware flasher tab (you need to Enable expert mode, Show release candidates and Development).

WARNING: It may be unstable. Use only for testing!

@haslinghuis
Copy link
Member

First : #13244

@ctzsnooze
Copy link
Member Author

#13244 is not required before this PR.

@@ -29,7 +29,7 @@

#include "gps.h"

PG_REGISTER_WITH_RESET_TEMPLATE(gpsConfig_t, gpsConfig, PG_GPS_CONFIG, 4);
PG_REGISTER_WITH_RESET_TEMPLATE(gpsConfig_t, gpsConfig, PG_GPS_CONFIG, 3);
Copy link
Member Author

Choose a reason for hiding this comment

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

The variables for the GPS PG were not changed in PR #13240, so the PG did not increasing from 3 to 4 in that PR.
This PR is basically a reversion of 13240, and it seems correct to keep the PG at 3, especially since nothing changed.
However, if it is best to keep it at 4 please let me know.

@blckmn
Copy link
Member

blckmn commented Dec 27, 2023

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> FAIL
  • cooling off period lapsed -> FAIL
  • commit count less or equal to three -> PASS
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@ctzsnooze
Copy link
Member Author

closing since required changes are now all in PR #13244

@ctzsnooze ctzsnooze closed this Dec 27, 2023
@ctzsnooze ctzsnooze deleted the revert-MSP-initiated-GPS-Sat-Info-messages branch April 3, 2024 06:07
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