Remove gps_attempt_time and use broadcast interval instead

[thebentern]

No description provided.

[Mictronics]

I believe that this change will not finally fix the issue but move the problem to another variable.
The root cause is still that GPS::getWakeTime() can return a zero value. Default values were set also for gps_attempt_time in actual code but failed.

[thebentern]

The root cause is still that GPS::getWakeTime() can return a zero value.

If position_broadcast_secs is 0, wouldn't the problem be moot? That's the basis for sending positions in the module in the first place.

[Mictronics]

Tried your code change in my master branch. I see two different behavior now when config.position.position_broadcast_secs is set to zero (0):

1. On soft reboot via app the Echo stays with No Sats and No GPS Lock and there is no more position broadcast.
2. On hard reset via button the Echo shows the same error like in the bug report. No GPS and No GPS Module.

IMHO, the code needs some sanity check for all configuration values the are coming from user inputs or via OTA transfer. Keep in mind that a Protobuf value defaults to zero (0). While this is a valid number, it is not a valid value for certain configuration parameters. So sanity checks should fix any configuration parameter into its valid range on configuration load and change.
This is, as far as I am familiar with the code, not the case actually.

[thebentern]

IMHO, the code needs some sanity check for all configuration values the are coming from user inputs or via OTA transfer. Keep in mind that a Protobuf value defaults to zero (0). While this is a valid number, it is not a valid value for certain configuration parameters. So sanity checks should fix any configuration parameter into its valid range on configuration load and change. This is, as far as I am familiar with the code, not the case actually.

I just committed a change that aligns it with the coalesce to the default interval like we have in Position (and other modules). That seems like a sensible choice.

[Mictronics]

GPS::getWakeTime() returns milliseconds while the config has seconds. So we still need the *1000 multiplier.
return getConfiguredOrDefaultMs(t * 1000, default_broadcast_interval_secs * 1000);

[thebentern]

GPS::getWakeTime() returns milliseconds while the config has seconds. So we still need the *1000 multiplier. return getConfiguredOrDefaultMs(t * 1000, default_broadcast_interval_secs * 1000);

Take another look at the implementation of getConfiguredOrDefaultMs. We use it specifically because it converts our seconds based intervals to milliseconds

[Mictronics]

The code change works. Detects GPS module and gets position lock on

• Soft reboot via app
• Hard reset via button
• Button reset after shutdown

Position broadcast interval is set to zero in app during testing.

[geeksville]

This pull request has been mentioned on Meshtastic. There might be relevant details there:

https://meshtastic.discourse.group/t/no-gps-module-t-echo/8824/5