Merge lp:~mandel/location-service/better-position-selection into lp:location-service/trunk

[Manuel de la Peña]

Improve the selection of the bag of providers to ensure that the locations used are within a reasonable time margin.

Improve the selection of the bag of providers to ensure that the locations used are within a reasonable time margin.

[Thomas Voß (thomas-voss)]

Thanks for the changes, a high-level remark first: The ProviderSelectionStrategy implementation is not the right place to add this functionality. Instead, the functionality should be added here:

  http://bazaar.launchpad.net/~phablet-team/location-service/trunk/view/head:/src/location_service/com/ubuntu/location/engine.cpp#L205

location::Engine is the central hub receiving and dispatching updates and it is meant to carry out such tasks as proposed in this MP.

Also make sure that an automated test case is added in http://bazaar.launchpad.net/~phablet-team/location-service/trunk/view/head:/tests/engine_test.cpp. I would think that introducing a location::PositionUpdateFilter interface together with a default implementation should help a lot here. You can pass the location::PositionUpdateFilter to the location::Engine constructor as a functional dependency and subsequently test if the Engine calls into that object for position updates.

Rest of my comments inline.

review: Needs Fixing

[Thomas Voß (thomas-voss)]

One other bit: The predominant style in the codebase is snake_style. Please change the camelCase instances accordingly.

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:193
http://jenkins.qa.ubuntu.com/job/location-service-ci/357/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-amd64-ci/29
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/29
        deb: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/29/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-i386-ci/29

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/location-service-ci/357/rebuild

review: Approve (continuous-integration)

[lp:~mandel/location-service/better-position-selection]

194. By Manuel de la Peña on 2015-04-23

Merged remove-pimpl into better-position-selection.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:194
http://jenkins.qa.ubuntu.com/job/location-service-ci/360/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-amd64-ci/32
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/32
        deb: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/32/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/location-service-vivid-i386-ci/32/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/location-service-ci/360/rebuild

review: Needs Fixing (continuous-integration)

[lp:~mandel/location-service/better-position-selection]

195. By Manuel de la Peña on 2015-04-23

Move the heuristics to its own class to make it easier to have diff options. Move the position selection to the engine.

196. By Manuel de la Peña on 2015-04-23

Add missing files.

[Thomas Voß (thomas-voss)]

Looks better, but some comments inline.

review: Needs Fixing

[lp:~mandel/location-service/better-position-selection]

197. By Manuel de la Peña on 2015-04-23

Made changes following the comments from the reviews.

198. By Manuel de la Peña on 2015-04-23

Add missing space.

199. By Manuel de la Peña on 2015-04-23

Merged remove-pimpl into better-position-selection.

200. By Manuel de la Peña on 2015-04-23

Remove unused var.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:196
http://jenkins.qa.ubuntu.com/job/location-service-ci/364/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/location-service-vivid-amd64-ci/36/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/36
        deb: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/36/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-i386-ci/36

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/location-service-ci/364/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:200
http://jenkins.qa.ubuntu.com/job/location-service-ci/366/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-amd64-ci/38
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/38
        deb: http://jenkins.qa.ubuntu.com/job/location-service-vivid-armhf-ci/38/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/location-service-vivid-i386-ci/38

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/location-service-ci/366/rebuild

review: Approve (continuous-integration)

[Jim Hodapp (jhodapp)]

Some suggestions inline below.

review: Needs Fixing

[lp:~mandel/location-service/better-position-selection]

201. By Manuel de la Peña on 2015-04-23

Made changes following the code reviews.

202. By Manuel de la Peña on 2015-04-23

Set var to const.

[Jim Hodapp (jhodapp)]

Looks good

review: Approve (code)

=== modified file 'src/location_service/com/ubuntu/location/CMakeLists.txt'
--- src/location_service/com/ubuntu/location/CMakeLists.txt	2015-01-25 12:45:30 +0000
+++ src/location_service/com/ubuntu/location/CMakeLists.txt	2015-04-23 21:30:38 +0000
@@ -27,6 +27,7 @@
   proxy_provider.cpp
   satellite_based_positioning_state.cpp
   settings.cpp
+  time_based_update_policy.cpp
   set_name_for_thread.cpp
   wifi_and_cell_reporting_state.cpp
 

=== modified file 'src/location_service/com/ubuntu/location/engine.cpp'
--- src/location_service/com/ubuntu/location/engine.cpp	2015-01-25 12:44:20 +0000
+++ src/location_service/com/ubuntu/location/engine.cpp	2015-04-23 21:30:38 +0000
@@ -24,6 +24,8 @@
 #include <stdexcept>
 #include <unordered_map>
 
+#include "time_based_update_policy.h"
+
 namespace cul = com::ubuntu::location;
 
 const cul::SatelliteBasedPositioningState cul::Engine::Configuration::Defaults::satellite_based_positioning_state;
@@ -33,7 +35,8 @@
 cul::Engine::Engine(const cul::ProviderSelectionPolicy::Ptr& provider_selection_policy,
                     const cul::Settings::Ptr& settings)
           : provider_selection_policy(provider_selection_policy),
-            settings(settings)
+            settings(settings),
+            update_policy(std::make_shared<cul::TimeBasedUpdatePolicy>())
 {
     if (!provider_selection_policy) throw std::runtime_error
     {
@@ -204,7 +207,7 @@
     // We should come up with a better heuristic here.
     auto cpr = provider->updates().position.connect([this](const cul::Update<cul::Position>& src)
     {
-        updates.reference_location = src;
+        updates.reference_location = update_policy->verify_update(src);
     });
 
     std::lock_guard<std::mutex> lg(guard);

=== modified file 'src/location_service/com/ubuntu/location/engine.h'
--- src/location_service/com/ubuntu/location/engine.h	2015-01-14 13:41:54 +0000
+++ src/location_service/com/ubuntu/location/engine.h	2015-04-23 21:30:38 +0000
@@ -33,6 +33,8 @@
 #include <mutex>
 #include <set>
 
+#include "update_policy.h"
+
 namespace com
 {
 namespace ubuntu
@@ -193,6 +195,7 @@
     std::map<Provider::Ptr, ProviderConnections> providers;
     ProviderSelectionPolicy::Ptr provider_selection_policy;
     Settings::Ptr settings;
+    UpdatePolicy::Ptr update_policy;
 };
 
 /** @brief Pretty prints the given status to the given stream. */

=== added file 'src/location_service/com/ubuntu/location/time_based_update_policy.cpp'
--- src/location_service/com/ubuntu/location/time_based_update_policy.cpp	1970-01-01 00:00:00 +0000
+++ src/location_service/com/ubuntu/location/time_based_update_policy.cpp	2015-04-23 21:30:38 +0000
@@ -0,0 +1,121 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Manuel de la Pena <[e-mail]>
+ */
+
+#include "time_based_update_policy.h"
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+
+std::chrono::minutes TimeBasedUpdatePolicy::default_timeout()
+{
+    static const std::chrono::minutes default_limit(2);
+    return default_limit;
+}
+
+TimeBasedUpdatePolicy::TimeBasedUpdatePolicy(std::chrono::minutes mins)
+    : limit(mins)
+{
+
+}
+
+const location::Update<location::Position>& TimeBasedUpdatePolicy::verify_update(const location::Update<location::Position>& update)
+{
+    std::lock_guard<std::mutex> guard(position_update_mutex);
+    bool use_new_update;
+    if (is_significantly_newer(last_position_update, update, limit))
+    {
+        use_new_update = true;
+    }
+    else if (is_significantly_older(last_position_update, update, limit))
+    {
+        use_new_update = false;
+    }
+    else
+    {
+        // if the update has happened within a reasonable amount of time we will just use it if it is more accurate
+        // that the previous one.
+        use_new_update = last_position_update.value.accuracy.horizontal && update.value.accuracy.horizontal
+            && *last_position_update.value.accuracy.horizontal >= *update.value.accuracy.horizontal;
+    }
+
+    if (use_new_update)
+    {
+        last_position_update = update;
+        return update;
+    }
+    else
+    {
+        return last_position_update;
+    }
+}
+
+
+const location::Update<location::Heading>& TimeBasedUpdatePolicy::verify_update(const location::Update<location::Heading>& update)
+{
+    std::lock_guard<std::mutex> guard(heading_update_mutex);
+    bool use_new_update;
+    if (is_significantly_newer(last_heading_update, update, limit))
+    {
+        use_new_update = true;
+    }
+    else if (is_significantly_older(last_heading_update, update, limit))
+    {
+        use_new_update = false;
+    }
+    if (use_new_update)
+    {
+        last_heading_update = update;
+        return update;
+    }
+    else
+    {
+        return last_heading_update;
+    }
+}
+
+const location::Update<location::Velocity>& TimeBasedUpdatePolicy::verify_update(const location::Update<location::Velocity>& update)
+{
+    std::lock_guard<std::mutex> guard(velocity_update_mutex);
+    bool use_new_update;
+    if (is_significantly_newer(last_velocity_update, update, limit))
+    {
+        use_new_update = true;
+    }
+    else if (is_significantly_older(last_velocity_update, update, limit))
+    {
+        use_new_update = false;
+    }
+
+    if (use_new_update)
+    {
+        last_velocity_update = update;
+        return update;
+    }
+    else
+    {
+        return last_velocity_update;
+    }
+}
+
+}
+}
+}
\ No newline at end of file

=== added file 'src/location_service/com/ubuntu/location/time_based_update_policy.h'
--- src/location_service/com/ubuntu/location/time_based_update_policy.h	1970-01-01 00:00:00 +0000
+++ src/location_service/com/ubuntu/location/time_based_update_policy.h	2015-04-23 21:30:38 +0000
@@ -0,0 +1,70 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Manuel de la Pena <[e-mail]>
+ */
+#ifndef LOCATION_SERVICE_UBUNTU_LOCATION_SERVICE_TIMES_BASED_UPDATE_POLICY_H_
+#define LOCATION_SERVICE_UBUNTU_LOCATION_SERVICE_TIMES_BASED_UPDATE_POLICY_H_
+
+#include <chrono>
+#include <mutex>
+
+#include "update_policy.h"
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+
+// An interface that can be implemented to add heuristics on how heading, position or velocity updates will be chosen.
+// This class ensures that the best update possible is chosen within a reasonable time bracket.
+class TimeBasedUpdatePolicy : public UpdatePolicy {
+
+ public:
+    TimeBasedUpdatePolicy(std::chrono::minutes mins=default_timeout());
+    TimeBasedUpdatePolicy(const TimeBasedUpdatePolicy&) = delete;
+    ~TimeBasedUpdatePolicy() = default;
+
+    // Return if the given position update will be verified as the new position in the engine.
+    const location::Update<location::Position>& verify_update(const location::Update<location::Position>& update) override;
+    // Return if the given heading update will be verified as the new heading in the engine.
+    const location::Update<location::Heading>& verify_update(const location::Update<location::Heading>& update) override;
+    // Return if the given velocity update will be verified as the new velocity in the engine.
+    const location::Update<location::Velocity>& verify_update(const location::Update<location::Velocity>& update) override;
+
+    static std::chrono::minutes default_timeout();
+
+ protected:
+    // not private to simplify the testing but should be private
+    location::Update<location::Position> last_position_update;
+    location::Update<location::Heading> last_heading_update;
+    location::Update<location::Velocity> last_velocity_update;
+
+ private:
+    // callbacks can happen in diff threads, make sure multi-threading will work in this class
+    std::mutex position_update_mutex;
+    std::mutex heading_update_mutex;
+    std::mutex velocity_update_mutex;
+    // used to calculate the time accepted bracket
+    std::chrono::minutes limit;
+};
+
+}
+}
+}
+
+#endif //LOCATION_SERVICE_UBUNTU_LOCATION_SERVICE_TIMES_BASED_UPDATE_POLICY_H_

=== added file 'src/location_service/com/ubuntu/location/update_policy.h'
--- src/location_service/com/ubuntu/location/update_policy.h	1970-01-01 00:00:00 +0000
+++ src/location_service/com/ubuntu/location/update_policy.h	2015-04-23 21:30:38 +0000
@@ -0,0 +1,75 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Manuel de la Pena <[e-mail]>
+ */
+#ifndef LOCATION_SERVICE_UBUNTU_LOCATION_SERVICE_UPDATE_POLICY_H_
+#define LOCATION_SERVICE_UBUNTU_LOCATION_SERVICE_UPDATE_POLICY_H_
+
+#include <memory>
+
+#include <com/ubuntu/location/heading.h>
+#include <com/ubuntu/location/position.h>
+#include <com/ubuntu/location/update.h>
+#include <com/ubuntu/location/velocity.h>
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+
+// An interface that can be implemented to add heuristics on how heading, position or velocity updateswill be chosen.
+// This class allows developers to inject different heuristics in the engine to perform the update selection
+// so that the app developers can take advantage of it.
+class UpdatePolicy {
+ public:
+    typedef std::shared_ptr<UpdatePolicy> Ptr;
+
+    UpdatePolicy(const UpdatePolicy&) = delete;
+    UpdatePolicy(UpdatePolicy&&) = delete;
+    UpdatePolicy& operator=(const UpdatePolicy&) = delete;
+    virtual ~UpdatePolicy() = default;
+
+    // Return if the given position update will be verified as the new position in the engine.
+    virtual const location::Update<location::Position>& verify_update(const location::Update<location::Position>& update) = 0;
+    // Return if the given heading update will be verified as the new heading in the engine.
+    virtual const location::Update<location::Heading>& verify_update(const location::Update<location::Heading>& update) = 0;
+    // Return if the given velocity update will be verified as the new velocity in the engine.
+    virtual const location::Update<location::Velocity>& verify_update(const location::Update<location::Velocity>& update) = 0;
+ protected:
+    UpdatePolicy() = default;
+
+    template <class T> bool is_significantly_newer(const location::Update<T> last_update, const location::Update<T> update, std::chrono::minutes limit) const
+    {
+       auto delta = update.when - last_update.when;
+       return delta > limit;
+    }
+
+    template <class T> bool is_significantly_older(const location::Update<T> last_update, const location::Update<T> update, std::chrono::minutes limit) const
+    {
+       auto delta = update.when - last_update.when;
+       return delta < (-1 * limit);
+    }
+
+
+};
+}
+}
+}
+
+#endif // LOCATION_SERVICE_UBUNTU_LOCATION_SERVICE_UPDATE_POLICY_H_
+

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2014-11-14 11:26:45 +0000
+++ tests/CMakeLists.txt	2015-04-23 21:30:38 +0000
@@ -80,6 +80,7 @@
 LOCATION_SERVICE_ADD_TEST(engine_test engine_test.cpp)
 LOCATION_SERVICE_ADD_TEST(harvester_test harvester_test.cpp)
 LOCATION_SERVICE_ADD_TEST(demultiplexing_reporter_test demultiplexing_reporter_test.cpp)
+LOCATION_SERVICE_ADD_TEST(time_based_update_policy_test time_based_update_policy_test.cpp)
 
 if (NET_CPP_FOUND)
   LOCATION_SERVICE_ADD_TEST(ichnaea_reporter_test ichnaea_reporter_test.cpp)

=== added file 'tests/time_based_update_policy_test.cpp'
--- tests/time_based_update_policy_test.cpp	1970-01-01 00:00:00 +0000
+++ tests/time_based_update_policy_test.cpp	2015-04-23 21:30:38 +0000
@@ -0,0 +1,131 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Manuel de la Pena <[e-mail]>
+ */
+#include <com/ubuntu/location/time_based_update_policy.h>
+
+#include <gtest/gtest.h>
+
+using namespace ::testing;
+namespace cul = com::ubuntu::location;
+
+namespace
+{
+    auto timestamp = com::ubuntu::location::Clock::now();
+
+    com::ubuntu::location::Update<com::ubuntu::location::Position> reference_position_update
+            {
+                    {
+                            com::ubuntu::location::wgs84::Latitude{9. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Longitude{53. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Altitude{-2. * com::ubuntu::location::units::Meters},
+                    },
+                    timestamp
+            };
+}
+
+// make certain internal details public so that we can set the last update
+class PublicTimeBasedUpdatePolicy : public cul::TimeBasedUpdatePolicy {
+ public:
+    PublicTimeBasedUpdatePolicy(std::chrono::minutes mins) :  cul::TimeBasedUpdatePolicy(mins) {}
+    using cul::TimeBasedUpdatePolicy::last_position_update;
+    using cul::TimeBasedUpdatePolicy::last_heading_update;
+    using cul::TimeBasedUpdatePolicy::last_velocity_update;
+};
+
+TEST(TimeBasedUpdatePolicy, policy_ignores_updates_that_are_too_old)
+{
+    auto policy = std::make_shared<PublicTimeBasedUpdatePolicy>(std::chrono::minutes(2));
+    policy->last_position_update = reference_position_update;
+
+    com::ubuntu::location::Update<com::ubuntu::location::Position> old_update
+            {
+                    {
+                            com::ubuntu::location::wgs84::Latitude{10. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Longitude{60. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Altitude{10. * com::ubuntu::location::units::Meters}
+                    },
+                    timestamp - std::chrono::minutes(5)
+            };
+    policy->verify_update(old_update);
+
+    ASSERT_NE(policy->last_position_update.value.latitude, old_update.value.latitude);
+    ASSERT_EQ(policy->last_position_update.value.latitude, reference_position_update.value.latitude);
+
+    ASSERT_NE(policy->last_position_update.value.longitude, old_update.value.longitude);
+    ASSERT_EQ(policy->last_position_update.value.longitude, reference_position_update.value.longitude);
+
+    ASSERT_NE(policy->last_position_update.value.altitude, old_update.value.altitude);
+    ASSERT_EQ(policy->last_position_update.value.altitude, reference_position_update.value.altitude);
+}
+
+TEST(TimeBasedUpdatePolicy, policy_uses_very_recent_updates)
+{
+    auto policy = std::make_shared<PublicTimeBasedUpdatePolicy>(std::chrono::minutes(2));
+
+    policy->last_position_update = reference_position_update;
+
+    com::ubuntu::location::Update<com::ubuntu::location::Position> new_update
+            {
+                    {
+                            com::ubuntu::location::wgs84::Latitude{10. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Longitude{60. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Altitude{10. * com::ubuntu::location::units::Meters}
+                    },
+                    timestamp + std::chrono::minutes(3)
+            };
+
+    policy->verify_update(new_update);
+
+    ASSERT_EQ(policy->last_position_update.value.latitude, new_update.value.latitude);
+    ASSERT_NE(policy->last_position_update.value.latitude, reference_position_update.value.latitude);
+
+    ASSERT_EQ(policy->last_position_update.value.longitude, new_update.value.longitude);
+    ASSERT_NE(policy->last_position_update.value.longitude, reference_position_update.value.longitude);
+
+    ASSERT_EQ(policy->last_position_update.value.altitude, new_update.value.altitude);
+    ASSERT_NE(policy->last_position_update.value.altitude, reference_position_update.value.altitude);
+}
+
+TEST(TimeBasedUpdatePolicy, policy_ignores_inaccurate_updates)
+{
+    auto policy = std::make_shared<PublicTimeBasedUpdatePolicy>(std::chrono::minutes(2));
+    reference_position_update.value.accuracy.horizontal = 1. * com::ubuntu::location::units::Meters;
+    policy->last_position_update = reference_position_update;
+
+    com::ubuntu::location::Update<com::ubuntu::location::Position> new_update
+            {
+                    {
+                            com::ubuntu::location::wgs84::Latitude{10. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Longitude{60. * com::ubuntu::location::units::Degrees},
+                            com::ubuntu::location::wgs84::Altitude{10. * com::ubuntu::location::units::Meters},
+                    },
+                    timestamp + std::chrono::minutes(1)
+            };
+    new_update.value.accuracy.horizontal = 8. * com::ubuntu::location::units::Meters;
+
+    policy->verify_update(new_update);
+    ASSERT_TRUE(*new_update.value.accuracy.horizontal > *reference_position_update.value.accuracy.horizontal);
+
+    ASSERT_NE(policy->last_position_update.value.latitude, new_update.value.latitude);
+    ASSERT_EQ(policy->last_position_update.value.latitude, reference_position_update.value.latitude);
+
+    ASSERT_NE(policy->last_position_update.value.longitude, new_update.value.longitude);
+    ASSERT_EQ(policy->last_position_update.value.longitude, reference_position_update.value.longitude);
+
+    ASSERT_NE(policy->last_position_update.value.altitude, new_update.value.altitude);
+    ASSERT_EQ(policy->last_position_update.value.altitude, reference_position_update.value.altitude);
+}