It would be nice if the polling interval could be variable, based on accelerometer values (for example 3-axis RAK1904 or 9-axis RAK12034).

If there's no accelerometer, default to 5 mins, and if there's an accelerometer and acceleration is detected then adjust the GPS update interval based on the acceleration rate.

@4np ya I considered adding some more sophistication to it, we could wait for a minimum accuracy, take an average over 3 readings, waiting for accelerometer values, I didn't want to try add too much too fast when some of this can be layered on. (plus I don't have an accelerometer to test with)

Yeah makes sense, it probably deviates too far from the scope of the PR. Still something that would be nice to have on the roadmap at some point.

As I've played a little with gps lately I think I'll handle this one ;) I think idea and implementation are good

@awolden have you considered implementing pwm in the LocationProvider instead of sensor manager ? I think it might make sense ...

For debugging, the new ui page I propose in #820 will be good, it shows sat count, fix status and coordinates as they come from nmea ...

I'll wait for #820 before going further, as it removes unnecessary duplicate code between sensor manager and location provider ;)

I can move it, will have time to do that soon

@ripplebiz you were saying a rewrite of the gps part would probably be necessary ...

We should probably take the opportunity of this PR to try rationalizing things a bit around GPS ... because, here we have gps -> locationProvider -> gpsmanager -> SensorManager ... and everyone has his own loop ;)

Ya something can probably be improved with that. LocationProvider no longer has a loop in this case though. It was replaced with a sync method. In this usage the location providers only handle impl specific IO. GpsManager has the only loop.

This is getting really interesting ;)

I wanted to try on T1000 (my techo is at home right now), I can't get it compiled without rewriting some code because T1000 does not use EnvironmentSensorManager (t1000SensorManager was there before and t1000 does not have any sensor from ESM but has its own sensors hooked to adcs, so there is no reason to use ESM ...)

I would prefer sticking with LocationProvider handling all the gps logic. I agree there is a fundamental issue with behaviour inheritance (don't know if thats the term in english) ... at least, your proposal does a very good cleaning both in ESM and LocationProvider, which is good ...

In the semantics, if we were to use composition, I would rather have LocationProvider delegating to a GpsDriver than GpsManager delegating to a LocationProvider. Or we could have GpsManager and GpsDriver (but LocationProvider sounds a little too high level in my ears).

I'll try to make your implementation work with T1000, it will help me understand it better ... (T1000 was the first to get gps, LocationManager has been made for it in the old custom fw)

@fdlamotte The inheritance route gets a little wonky with the detection logic and hard to parse with the way the individual providers inherit from the super class. Would have to put the detection logic back into the SensorManager. The around keeping LocationProvider and considering the implementation specific classes 'drivers' is a great one. I refactored it around that idea.

LocationProvider now handles the GPS management logic
GpsDriver now handles the specific IO logic

Closing out due to lack of activity and merge conflicts. I believe several PRs have been merged semi recently regarding gps interval config via cli.