Merge lp:~osomon/qtubuntu-sensors/shield-against-nans into lp:qtubuntu-sensors

[Olivier Tilloy]

Shield position updates against NaN values for horizontal and vertical accuracy.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:75
http://jenkins.qa.ubuntu.com/job/qtubuntu-sensors-ci/59/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-sensors-utopic-amd64-ci/25
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-sensors-utopic-armhf-ci/21
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-sensors-utopic-armhf-ci/21/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-sensors-utopic-i386-ci/21

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtubuntu-sensors-ci/59/rebuild

review: Approve (continuous-integration)

[Loïc Minier (lool)]

LGTM

Typo horizontal/vertical was obviously a mistake, and the nan fix is about making sure we only set the attributes which indeed are good values, so makes sense.

While at it, is it correct to set the altitude to 0 and to copy over the timestamp?

review: Approve

[Olivier Tilloy (osomon)]

> While at it, is it correct to set the altitude to 0 and to copy over the
> timestamp?

Setting the altitude to 0 if no value is set seems fine to me. I just checked and the constructor of QGeoCoordinate sets altitude to NaN by default, so we’d better be covered and initialize it to 0 if unknown. In any case I also updated my oxide MR (https://code.launchpad.net/~osomon/oxide/geolocation-doesnt-like-nans/+merge/235667) to shield it against invalid altitudes too (better safe than sorry).

As for the timestamp, I’m not really sure it makes sense to check for NaN for the value returned by ua_location_position_update_get_timestamp(…). I would expect the timestamp to be a mandatory value set by the location plugin. Maybe Thomas can comment on this?

=== modified file 'plugins/position/core_geo_position_info_source.cpp'
--- plugins/position/core_geo_position_info_source.cpp	2014-09-17 14:20:27 +0000
+++ plugins/position/core_geo_position_info_source.cpp	2014-09-23 16:19:20 +0000
@@ -18,6 +18,8 @@
 
 #include "core_geo_position_info_source.h"
 
+#include <cmath>
+
 #include <QtCore>
 
 #include <ubuntu/application/location/service.h>
@@ -273,14 +275,18 @@
     lastKnownPosition.setCoordinate(coord);
 
     if (ua_location_position_update_has_horizontal_accuracy(position))
-        lastKnownPosition.setAttribute(
-                    QGeoPositionInfo::HorizontalAccuracy,
-                    ua_location_position_update_get_horizontal_accuracy_in_meter(position));
+    {
+        double accuracy = ua_location_position_update_get_horizontal_accuracy_in_meter(position);
+        if (!std::isnan(accuracy))
+            lastKnownPosition.setAttribute(QGeoPositionInfo::HorizontalAccuracy, accuracy);
+    }
 
     if (ua_location_position_update_has_vertical_accuracy(position))
-        lastKnownPosition.setAttribute(
-                    QGeoPositionInfo::HorizontalAccuracy,
-                    ua_location_position_update_get_vertical_accuracy_in_meter(position));
+    {
+        double accuracy = ua_location_position_update_get_vertical_accuracy_in_meter(position);
+        if (!std::isnan(accuracy))
+            lastKnownPosition.setAttribute(QGeoPositionInfo::VerticalAccuracy, accuracy);
+    }
 
     lastKnownPosition.setTimestamp(
         QDateTime::fromMSecsSinceEpoch(