Bug 101975 - gps location updates are only every 30s
Summary: gps location updates are only every 30s
Status: RESOLVED FIXED
Alias: None
Product: GeoClue
Classification: Unclassified
Component: Modem source (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Geoclue Bugs
QA Contact: Geoclue Bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-07-30 23:04 UTC by Valentin Blot
Modified: 2018-04-19 07:21 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
instant gps location updates (2.23 KB, patch)
2017-08-11 15:41 UTC, Valentin Blot
Details | Splinter Review
Instant GPS location update (2.23 KB, patch)
2017-08-11 16:22 UTC, Valentin Blot
Details | Splinter Review

Description Valentin Blot 2017-07-30 23:04:31 UTC
ModemManager sends gps location updates every 30s by default, which is too much for real-time routing in gnome-maps for example. The following commit:

https://cgit.freedesktop.org/ModemManager/ModemManager/commit/?id=6c35878f12ab37604d85cb3a864e3859973bd195

fixing the following bug:

https://bugs.freedesktop.org/show_bug.cgi?id=89924

introduced the option of specifying a different refresh rate with a method named mm_modem_location_set_gps_refresh_rate. Geoclue should either call this method when a gps source is detected and specify a higher rate, or provide a function to the client application so it can use a higher rate if needed. I would lean towards the first approach because the applications using geoclue should not have to deal with the particular sources providing location and make a special case for gps. Is there any downside in putting a refresh rate of 0 (send updates as soon as the gps transmits data)?
Comment 1 Zeeshan Ali 2017-08-09 15:21:31 UTC
Hmm. I didnt know of this API in ModemManager. Please feel free to provide a patch that sets this property to 0.
Comment 2 Valentin Blot 2017-08-11 15:41:25 UTC
Created attachment 133440 [details] [review]
instant gps location updates

Here it is.
Comment 3 Zeeshan Ali 2017-08-11 16:04:38 UTC
Comment on attachment 133440 [details] [review]
instant gps location updates

Review of attachment 133440 [details] [review]:
-----------------------------------------------------------------

Looks good. Just some minor things and then I can merge. I hope you tested it.

* gps -> GPS, it's an abbreviation.
* Missing '.' after the last sentence in the description part.

::: src/gclue-modem-manager.c
@@ +550,5 @@
> +                         gpointer      user_data)
> +{
> +        GError *error = NULL;
> +
> +        gboolean finish = mm_modem_location_set_gps_refresh_rate_finish

For consistency:

* Declarations at the top, before calling methods.
* 'finish' -> 'ret'.

@@ +553,5 @@
> +
> +        gboolean finish = mm_modem_location_set_gps_refresh_rate_finish
> +                (MM_MODEM_LOCATION (source_object), res, &error);
> +
> +        if (finish == FALSE) {

if (!finish) :)

@@ +554,5 @@
> +        gboolean finish = mm_modem_location_set_gps_refresh_rate_finish
> +                (MM_MODEM_LOCATION (source_object), res, &error);
> +
> +        if (finish == FALSE) {
> +                g_warning ("Failed to set gps refresh rate: %s",

nitpick: gps -> GPS.
Comment 4 Valentin Blot 2017-08-11 16:22:05 UTC
Created attachment 133442 [details] [review]
Instant GPS location update

Thanks for the comments. I think I did all the suggested improvements. And yes, I did test it.
Comment 5 Zeeshan Ali 2017-08-11 16:30:28 UTC
(In reply to Valentin Blot from comment #4)
> Created attachment 133442 [details] [review] [review]
> Instant GPS location update
> 
> Thanks for the comments. I think I did all the suggested improvements. And
> yes, I did test it.

Thanks, pushed with some minor changes.
Comment 6 Zeeshan Ali 2018-03-15 17:18:35 UTC
Actually, I think we should not be setting the value to '0' but based on:

https://www.freedesktop.org/software/geoclue/docs/gdbus-org.freedesktop.GeoClue2.Client.html#gdbus-property-org-freedesktop-GeoClue2-Client.TimeThreshold
Comment 7 Zeeshan Ali 2018-04-11 21:25:23 UTC
(In reply to Zeeshan Ali from comment #6)
> Actually, I think we should not be setting the value to '0' but based on:
> 
> https://www.freedesktop.org/software/geoclue/docs/gdbus-org.freedesktop.
> GeoClue2.Client.html#gdbus-property-org-freedesktop-GeoClue2-Client.
> TimeThreshold

That turned out to be not so simple. Since the patch here simply sets up back to the old behaviour, I will make a release shortly with ModemManager's GPS rate hardcoded to 0 and then look at solving the issue the right way, for which I filed bug#105993.
Comment 8 Zeeshan Ali 2018-04-18 22:36:49 UTC
Just FYI, a more thorough fix for this has been pushed to git master as part of bug#105993.
Comment 9 Valentin Blot 2018-04-19 07:21:32 UTC
Sorry I've been busy lately. Thanks for the fix, I'll test it as soon as I've got more time.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.