Merge lp:~ssweeny/location-service/fusion-provider.15.04 into lp:location-service/15.04

[Scott Sweeny]

Use a fusion engine provider to generate higher quality position updates

Use a fusion engine provider to generate higher quality position updates

[Thomas Voß (thomas-voss)]

Thanks for the changes. Some design feedback first: I think you only need a FusioningProvider and you can get rid of BagOfProviders by defining a helper interface:

class UpdateSelector
{
public:
  virtual void Update<Position> select(const Update<Position>& old, const Update<Position>& new) = 0
};

and passing an implementation of UpdateSelector to the FusioningProvider ctor. The BagOfProviders behavior can then be achieved by having:

class AlwaysSelectNewUpdate : public UpdateSelector
{
public:
  void Update<Position> select(const Update<Position>&, const Update<Position>& new) override
  {
    return new;
  }
};

The is_better_than predicate would then end up being an implementation of UpdateSelector, along the lines of:

class SignificantlyNewerOrMoreAccurateSelector : public UpdateSelector
{
public:
   ...
};

Whenever a new session is constructed, we could select between the current or the new behavior by just passing a different implementation.

One more comment inline.

review: Needs Fixing

[lp:~ssweeny/location-service/fusion-provider.15.04]

220. By Scott Sweeny on 2016-03-03

FusionProvider: make last_position optional for a more robust test of its validity

221. By Scott Sweeny on 2016-03-07

Per review comments, eliminate BagOfProviders and use a helper to select update strategies for FusionProvider

[Konrad Zapałowicz (kzapalowicz)]

Good stuff just two places where reference could be considered.

review: Needs Fixing

[lp:~ssweeny/location-service/fusion-provider.15.04]

222. By Scott Sweeny on 2016-03-07

Pass UpdateSelectors and return Updates by reference

223. By Scott Sweeny on 2016-03-08

Remove a couple of debug statements

[Thomas Voß (thomas-voss)]

Minor remaining niggles inline.

review: Needs Fixing

[lp:~ssweeny/location-service/fusion-provider.15.04]

224. By Scott Sweeny on 2016-03-10

Small changes addressing review comments

[Scott Sweeny (ssweeny)]

One of the changes doesn't seem to work, but the others are done.

[Scott Sweeny (ssweeny)]

> One of the changes doesn't seem to work, but the others are done.

The way LP displays the comments is confusing since the code is updated so I'm going to repeat myself here:

For the changes in fusion_provider.cpp:

These two changes don't work for me. The tests freeze up with a "this->is_initialized()" assertion failure when I use '*' notation with last_position, but the compile fails when I don't use it.

[lp:~ssweeny/location-service/fusion-provider.15.04]

225. By Scott Sweeny on 2016-03-10

Prettify the last_position update code

[Thomas Voß (thomas-voss)]

LGTM.

review: Approve

Unmerged revisions

225. By Scott Sweeny on 2016-03-10

Prettify the last_position update code

224. By Scott Sweeny on 2016-03-10

Small changes addressing review comments

223. By Scott Sweeny on 2016-03-08

Remove a couple of debug statements

222. By Scott Sweeny on 2016-03-07

Pass UpdateSelectors and return Updates by reference

221. By Scott Sweeny on 2016-03-07

Per review comments, eliminate BagOfProviders and use a helper to select update strategies for FusionProvider

220. By Scott Sweeny on 2016-03-03

FusionProvider: make last_position optional for a more robust test of its validity

219. By Scott Sweeny on 2016-02-25

Add selection policy for fusion provider and use it by default

218. By Scott Sweeny on 2016-02-25

Add test for FusionProvider

217. By Scott Sweeny on 2016-02-25

Add fusion provider based on BagOfProviders

216. By Scott Sweeny on 2016-02-08

Factor BagOfProviders out into its own class so it can be reused

=== added file 'include/location_service/com/ubuntu/location/fusion_provider.h'
--- include/location_service/com/ubuntu/location/fusion_provider.h	1970-01-01 00:00:00 +0000
+++ include/location_service/com/ubuntu/location/fusion_provider.h	2016-03-10 19:30:01 +0000
@@ -0,0 +1,59 @@
+/*
+ * Copyright © 2016 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: Scott Sweeny <[e-mail]
+ */
+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_FUSION_PROVIDER_H
+#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_FUSION_PROVIDER_H
+
+#include <com/ubuntu/location/provider.h>
+#include <com/ubuntu/location/provider_selection_policy.h>
+#include <com/ubuntu/location/update_selector.h>
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+class FusionProvider : public Provider
+{
+public:
+    typedef std::shared_ptr<FusionProvider> Ptr;
+
+    FusionProvider(const std::set<Provider::Ptr>& providers, const UpdateSelector::Ptr& update_selector);
+
+    bool matches_criteria(const Criteria &criteria) override;
+    void on_wifi_and_cell_reporting_state_changed(location::WifiAndCellIdReportingState state) override;
+    void on_reference_location_updated(const Update<Position>& position) override;
+    void on_reference_velocity_updated(const Update<Velocity>& velocity) override;
+    void on_reference_heading_updated(const Update<Heading>& heading) override;
+    void start_position_updates() override;
+    void stop_position_updates() override;
+    void start_heading_updates() override;
+    void stop_heading_updates() override;
+    void start_velocity_updates() override;
+    void stop_velocity_updates() override;
+
+private:
+    Optional<Update<Position>> last_position;
+    std::set<Provider::Ptr> providers;
+    std::vector<core::ScopedConnection> connections;
+};
+}
+}
+}
+
+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_FUSION_PROVIDER_H

=== added file 'include/location_service/com/ubuntu/location/fusion_provider_selection_policy.h'
--- include/location_service/com/ubuntu/location/fusion_provider_selection_policy.h	1970-01-01 00:00:00 +0000
+++ include/location_service/com/ubuntu/location/fusion_provider_selection_policy.h	2016-03-10 19:30:01 +0000
@@ -0,0 +1,39 @@
+/*
+ * Copyright © 2016 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: Scott Sweeny <[e-mail]
+ */
+#ifndef COM_UBUNTU_LOCATION_FUSION_PROVIDER_SELECTION_POLICY_H
+#define COM_UBUNTU_LOCATION_FUSION_PROVIDER_SELECTION_POILCY_H
+
+#include <com/ubuntu/location/provider_selection_policy.h>
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+// This policy selects all providers, supplying them in the form of a fusion
+// provider for higher quality position updates
+struct FusionProviderSelectionPolicy : public ProviderSelectionPolicy
+{
+    ProviderSelection determine_provider_selection_for_criteria(const Criteria &criteria, const ProviderEnumerator &enumerator);
+};
+}
+}
+}
+
+#endif // COM_UBUNTU_LOCATION_FUSION_PROVIDER_SELECTION_POILCY_H

=== added file 'include/location_service/com/ubuntu/location/newer_or_more_accurate_update_selector.h'
--- include/location_service/com/ubuntu/location/newer_or_more_accurate_update_selector.h	1970-01-01 00:00:00 +0000
+++ include/location_service/com/ubuntu/location/newer_or_more_accurate_update_selector.h	2016-03-10 19:30:01 +0000
@@ -0,0 +1,60 @@
+/*
+ * Copyright © 2016 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: Scott Sweeny <[e-mail]
+ */
+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATIONNEWER_OR_MORE_ACCURATE_UPDATE_SELECTOR_H
+#define LOCATION_SERVICE_COM_UBUNTU_LOCATIONNEWER_OR_MORE_ACCURATE_UPDATE_SELECTOR_H
+
+#include <com/ubuntu/location/update_selector.h>
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+class NewerOrMoreAccurateUpdateSelector : public UpdateSelector
+{
+public:
+    typedef std::shared_ptr<NewerOrMoreAccurateUpdateSelector> Ptr;
+
+    Update<Position> select(const Update<Position>& older,
+                            const Update<Position>& newer) override
+    {
+        // Basically copied this value from the Android fusion provider
+        static const std::chrono::seconds cutoff(11);
+
+        // If the new position is newer by a significant margin then just use it
+        if (newer.when > older.when + cutoff) {
+            return newer;
+        }
+
+        // Choose the position with the smaller accuracy circle if both have them
+        if (!older.value.accuracy.horizontal)
+            return newer;
+        if (!newer.value.accuracy.horizontal)
+            return older;
+        if (newer.value.accuracy.horizontal < older.value.accuracy.horizontal)
+            return newer;
+        else
+            return older;
+    }
+};
+}
+}
+}
+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATIONNEWER_OR_MORE_ACCURATE_UPDATE_SELECTOR_H
+

=== added file 'include/location_service/com/ubuntu/location/newer_update_selector.h'
--- include/location_service/com/ubuntu/location/newer_update_selector.h	1970-01-01 00:00:00 +0000
+++ include/location_service/com/ubuntu/location/newer_update_selector.h	2016-03-10 19:30:01 +0000
@@ -0,0 +1,45 @@
+/*
+ * Copyright © 2016 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: Scott Sweeny <[e-mail]
+ */
+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_ALWAYS_SELECT_NEW_UPDATE_H
+#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_ALWAYS_SELECT_NEW_UPDATE_H
+
+#include <com/ubuntu/location/update_selector.h>
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+class NewerUpdateSelector : public UpdateSelector
+{
+public:
+    typedef std::shared_ptr<NewerUpdateSelector> Ptr;
+
+    Update<Position> select (const Update<Position>& older,
+                             const Update<Position>& newer) override
+    {
+        return newer;
+    }
+};
+}
+}
+}
+
+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_ALWAYS_SELECT_NEW_UPDATE_H
+

=== added file 'include/location_service/com/ubuntu/location/update_selector.h'
--- include/location_service/com/ubuntu/location/update_selector.h	1970-01-01 00:00:00 +0000
+++ include/location_service/com/ubuntu/location/update_selector.h	2016-03-10 19:30:01 +0000
@@ -0,0 +1,49 @@
+/*
+ * Copyright © 2016 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: Scott Sweeny <[e-mail]
+ */
+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_UPDATE_SELECTOR_H
+#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_UPDATE_SELECTOR_H
+
+#include <com/ubuntu/location/update.h>
+#include <com/ubuntu/location/position.h>
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+class UpdateSelector
+{
+public:
+    typedef std::shared_ptr<UpdateSelector> Ptr;
+
+    UpdateSelector(const UpdateSelector&) = delete;
+    UpdateSelector& operator=(const UpdateSelector&) = delete;
+    virtual ~UpdateSelector() = default;
+
+    virtual Update<Position> select(const Update<Position>& older,
+                                    const Update<Position>& newer) = 0;
+
+protected:
+    UpdateSelector() = default;
+};
+}
+}
+}
+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_UPDATE_SELECTOR_H
+

=== modified file 'src/location_service/com/ubuntu/location/CMakeLists.txt'
--- src/location_service/com/ubuntu/location/CMakeLists.txt	2015-11-26 08:28:38 +0000
+++ src/location_service/com/ubuntu/location/CMakeLists.txt	2016-03-10 19:30:01 +0000
@@ -16,10 +16,12 @@
   ${UBUNTU_LOCATION_SERVICE_PUBLIC_HEADERS}
 
   default_provider_selection_policy.cpp
+  fusion_provider_selection_policy.cpp
   non_selecting_provider_selection_policy.cpp
 
   criteria.cpp
   engine.cpp
+  fusion_provider.cpp
   init_and_shutdown.cpp
   position.cpp
   provider.cpp

=== added file 'src/location_service/com/ubuntu/location/fusion_provider.cpp'
--- src/location_service/com/ubuntu/location/fusion_provider.cpp	1970-01-01 00:00:00 +0000
+++ src/location_service/com/ubuntu/location/fusion_provider.cpp	2016-03-10 19:30:01 +0000
@@ -0,0 +1,141 @@
+/*
+ * Copyright © 2016 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: Scott Sweeny <[e-mail]
+ */
+ #include <com/ubuntu/location/fusion_provider.h>
+ #include <com/ubuntu/location/logging.h>
+ #include <com/ubuntu/location/update.h>
+
+namespace cu = com::ubuntu;
+namespace cul = com::ubuntu::location;
+
+cul::Provider::Features all_features()
+{
+    return cul::Provider::Features::position |
+           cul::Provider::Features::heading |
+           cul::Provider::Features::velocity;
+}
+
+cul::Provider::Requirements all_requirements()
+{
+    return cul::Provider::Requirements::cell_network |
+           cul::Provider::Requirements::data_network |
+           cul::Provider::Requirements::monetary_spending |
+           cul::Provider::Requirements::satellites;
+}
+
+
+cul::FusionProvider::FusionProvider(const std::set<location::Provider::Ptr>& providers, const UpdateSelector::Ptr& update_selector)
+    : Provider{all_features(), all_requirements()},
+      providers{providers}
+{
+
+    for (auto provider : providers)
+    {
+        connections.push_back(provider->updates().position.connect(
+              [this, update_selector](const cul::Update<cul::Position>& u)
+              {
+                  // if this is the first update, use it
+                  if (!last_position) {
+                      mutable_updates().position(*(last_position = u));
+                  } else {
+                      try {
+                          mutable_updates().position(*(last_position = update_selector->select(*last_position, u)));
+                      } catch (const std::exception& e) {
+                          LOG(WARNING) << "Error while updating position";
+                      }
+                  }
+              }));
+        connections.push_back(provider->updates().heading.connect(
+              [this](const cul::Update<cul::Heading>& u)
+              {
+                  mutable_updates().heading(u);
+              }));
+        connections.push_back(provider->updates().velocity.connect(
+              [this](const cul::Update<cul::Velocity>& u)
+              {
+                  mutable_updates().velocity(u);
+              }));
+    }
+
+}
+
+// We always match :)
+bool cul::FusionProvider::matches_criteria(const cul::Criteria&)
+{
+    return true;
+}
+
+// We forward all events to the other providers.
+void cul::FusionProvider::on_wifi_and_cell_reporting_state_changed(cul::WifiAndCellIdReportingState state)
+{
+    for (auto provider : providers)
+        provider->on_wifi_and_cell_reporting_state_changed(state);
+}
+
+void cul::FusionProvider::on_reference_location_updated(const cul::Update<cul::Position>& position)
+{
+    for (auto provider : providers)
+        provider->on_reference_location_updated(position);
+}
+
+void cul::FusionProvider::on_reference_velocity_updated(const cul::Update<cul::Velocity>& velocity)
+{
+    for (auto provider : providers)
+        provider->on_reference_velocity_updated(velocity);
+}
+
+void cul::FusionProvider::on_reference_heading_updated(const cul::Update<cul::Heading>& heading)
+{
+    for (auto provider : providers)
+        provider->on_reference_heading_updated(heading);
+}
+
+void cul::FusionProvider::start_position_updates()
+{
+    for (auto provider : providers)
+        provider->state_controller()->start_position_updates();
+}
+
+void cul::FusionProvider::stop_position_updates()
+{
+    for (auto provider : providers)
+        provider->state_controller()->stop_position_updates();
+}
+
+void cul::FusionProvider::start_heading_updates()
+{
+    for (auto provider : providers)
+        provider->state_controller()->start_heading_updates();
+}
+
+void cul::FusionProvider::stop_heading_updates()
+{
+    for (auto provider : providers)
+        provider->state_controller()->stop_heading_updates();
+}
+
+void cul::FusionProvider::start_velocity_updates()
+{
+    for (auto provider : providers)
+        provider->state_controller()->start_velocity_updates();
+}
+
+void cul::FusionProvider::stop_velocity_updates()
+{
+    for (auto provider : providers)
+        provider->state_controller()->stop_velocity_updates();
+}

=== added file 'src/location_service/com/ubuntu/location/fusion_provider_selection_policy.cpp'
--- src/location_service/com/ubuntu/location/fusion_provider_selection_policy.cpp	1970-01-01 00:00:00 +0000
+++ src/location_service/com/ubuntu/location/fusion_provider_selection_policy.cpp	2016-03-10 19:30:01 +0000
@@ -0,0 +1,26 @@
+#include <com/ubuntu/location/fusion_provider.h>
+#include <com/ubuntu/location/fusion_provider_selection_policy.h>
+#include <com/ubuntu/location/newer_or_more_accurate_update_selector.h>
+
+namespace location = com::ubuntu::location;
+
+location::ProviderSelection location::FusionProviderSelectionPolicy::determine_provider_selection_for_criteria(
+        const location::Criteria&,
+        const location::ProviderEnumerator& enumerator)
+{
+    // We put all providers in a set.
+    std::set<location::Provider::Ptr> bag;
+    enumerator.for_each_provider([&bag](const location::Provider::Ptr& provider)
+    {
+        bag.insert(provider);
+    });
+
+    auto fusion_providers = std::make_shared<location::FusionProvider>(bag, std::make_shared<location::NewerOrMoreAccurateUpdateSelector>());
+
+    return location::ProviderSelection
+    {
+        fusion_providers, // position
+        fusion_providers, // heading
+        fusion_providers, // velocity
+    };
+}

=== modified file 'src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.cpp'
--- src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.cpp	2015-11-18 09:40:10 +0000
+++ src/location_service/com/ubuntu/location/non_selecting_provider_selection_policy.cpp	2016-03-10 19:30:01 +0000
@@ -17,130 +17,13 @@
  */
 
 #include <com/ubuntu/location/non_selecting_provider_selection_policy.h>
+#include <com/ubuntu/location/fusion_provider.h>
+#include <com/ubuntu/location/newer_update_selector.h>
 
 #include <set>
 
 namespace location = com::ubuntu::location;
 
-namespace
-{
-
-location::Provider::Features all_features()
-{
-    return location::Provider::Features::position |
-           location::Provider::Features::heading |
-           location::Provider::Features::velocity;
-}
-
-location::Provider::Requirements all_requirements()
-{
-    return location::Provider::Requirements::cell_network |
-           location::Provider::Requirements::data_network |
-           location::Provider::Requirements::monetary_spending |
-           location::Provider::Requirements::satellites;
-}
-
-// A simple helper that takes a set of existing providers
-// and maps calls to each of them.
-struct BagOfProviders : public location::Provider
-{
-    BagOfProviders(const std::set<location::Provider::Ptr>& providers)
-        : location::Provider{all_features(), all_requirements()},
-          providers{providers}
-    {
-        // We connect to all updates of each provider.
-        for (auto provider : BagOfProviders::providers)
-        {
-            connections.push_back(provider->updates().position.connect([this](const location::Update<location::Position>& update)
-            {
-                mutable_updates().position(update);
-            }));
-
-            connections.push_back(provider->updates().heading.connect([this](const location::Update<location::Heading>& update)
-            {
-                mutable_updates().heading(update);
-            }));
-
-            connections.push_back(provider->updates().velocity.connect([this](const location::Update<location::Velocity>& update)
-            {
-                mutable_updates().velocity(update);
-            }));
-        }
-
-    }
-
-    // We always match :)
-    bool matches_criteria(const location::Criteria&) override
-    {
-        return true;
-    }
-
-    // We forward all events to the other providers.
-    void on_wifi_and_cell_reporting_state_changed(location::WifiAndCellIdReportingState state) override
-    {
-        for (auto provider : providers)
-            provider->on_wifi_and_cell_reporting_state_changed(state);
-    }
-
-    void on_reference_location_updated(const location::Update<location::Position>& position) override
-    {
-        for (auto provider : providers)
-            provider->on_reference_location_updated(position);
-    }
-
-    void on_reference_velocity_updated(const location::Update<location::Velocity>& velocity) override
-    {
-        for (auto provider : providers)
-            provider->on_reference_velocity_updated(velocity);
-    }
-
-    void on_reference_heading_updated(const location::Update<location::Heading>& heading) override
-    {
-        for (auto provider : providers)
-            provider->on_reference_heading_updated(heading);
-    }
-
-    void start_position_updates() override
-    {
-        for (auto provider : providers)
-            provider->state_controller()->start_position_updates();
-    }
-
-    void stop_position_updates() override
-    {
-        for (auto provider : providers)
-            provider->state_controller()->stop_position_updates();
-    }
-
-    void start_heading_updates() override
-    {
-        for (auto provider : providers)
-            provider->state_controller()->start_heading_updates();
-    }
-
-    void stop_heading_updates() override
-    {
-        for (auto provider : providers)
-            provider->state_controller()->stop_heading_updates();
-    }
-
-    void start_velocity_updates() override
-    {
-        for (auto provider : providers)
-            provider->state_controller()->start_velocity_updates();
-    }
-
-    void stop_velocity_updates() override
-    {
-        for (auto provider : providers)
-            provider->state_controller()->stop_velocity_updates();
-    }
-
-    std::set<location::Provider::Ptr> providers;
-    std::vector<core::ScopedConnection> connections;
-};
-}
-
 location::ProviderSelection location::NonSelectingProviderSelectionPolicy::determine_provider_selection_for_criteria(
         const location::Criteria&,
         const location::ProviderEnumerator& enumerator)
@@ -152,7 +35,7 @@
         bag.insert(provider);
     });
 
-    auto bag_of_providers = std::make_shared<BagOfProviders>(bag);
+    auto bag_of_providers = std::make_shared<FusionProvider>(bag, std::make_shared<NewerUpdateSelector>());
 
     // Our bag of providers is responsible for delivering position/heading/velocity updates.
     return location::ProviderSelection

=== modified file 'src/location_service/com/ubuntu/location/service/default_configuration.cpp'
--- src/location_service/com/ubuntu/location/service/default_configuration.cpp	2014-11-14 11:33:05 +0000
+++ src/location_service/com/ubuntu/location/service/default_configuration.cpp	2016-03-10 19:30:01 +0000
@@ -20,7 +20,7 @@
 #include <com/ubuntu/location/service/trust_store_permission_manager.h>
 
 #include <com/ubuntu/location/engine.h>
-#include <com/ubuntu/location/non_selecting_provider_selection_policy.h>
+#include <com/ubuntu/location/fusion_provider_selection_policy.h>
 
 namespace cul = com::ubuntu::location;
 namespace culs = com::ubuntu::location::service;
@@ -39,7 +39,7 @@
 
 cul::ProviderSelectionPolicy::Ptr culs::DefaultConfiguration::the_provider_selection_policy()
 {
-    return std::make_shared<cul::NonSelectingProviderSelectionPolicy>();
+    return std::make_shared<cul::FusionProviderSelectionPolicy>();
 }
 
 std::set<cul::Provider::Ptr> culs::DefaultConfiguration::the_provider_set(

=== modified file 'tests/provider_test.cpp'
--- tests/provider_test.cpp	2015-01-21 20:04:56 +0000
+++ tests/provider_test.cpp	2016-03-10 19:30:01 +0000
@@ -284,3 +284,128 @@
     mp2.inject_update(cul::Update<cul::Heading>());
     mp3.inject_update(cul::Update<cul::Velocity>());
 }
+
+#include <com/ubuntu/location/fusion_provider.h>
+#include <com/ubuntu/location/newer_or_more_accurate_update_selector.h>
+
+TEST(FusionProvider, start_and_stop_of_updates_propagates_to_correct_providers)
+{
+    using namespace ::testing;
+
+    NiceMock<MockProvider> mp1, mp2, mp3;
+    EXPECT_CALL(mp1, start_position_updates()).Times(Exactly(1));
+    EXPECT_CALL(mp1, stop_position_updates()).Times(Exactly(1));
+    EXPECT_CALL(mp2, start_heading_updates()).Times(Exactly(1));
+    EXPECT_CALL(mp2, stop_heading_updates()).Times(Exactly(1));
+    EXPECT_CALL(mp3, start_velocity_updates()).Times(Exactly(1));
+    EXPECT_CALL(mp3, stop_velocity_updates()).Times(Exactly(1));
+
+    cul::Provider::Ptr p1{std::addressof(mp1), [](cul::Provider*){}};
+    cul::Provider::Ptr p2{std::addressof(mp2), [](cul::Provider*){}};
+    cul::Provider::Ptr p3{std::addressof(mp3), [](cul::Provider*){}};
+
+    cul::ProviderSelection selection{p1, p2, p3};
+    std::set<cul::Provider::Ptr> providers{p1, p2, p3};
+
+    //cul::FusionProvider pp{selection};
+    cul::FusionProvider fp{providers, std::make_shared<cul::NewerOrMoreAccurateUpdateSelector>()};
+
+    fp.start_position_updates();
+    fp.stop_position_updates();
+
+    fp.start_heading_updates();
+    fp.stop_heading_updates();
+
+    fp.start_velocity_updates();
+    fp.stop_velocity_updates();
+}
+
+TEST(FusionProvider, update_signals_are_routed_from_correct_providers)
+{
+    using namespace ::testing;
+
+    NiceMock<MockProvider> mp1, mp2, mp3;
+
+    cul::Provider::Ptr p1{std::addressof(mp1), [](cul::Provider*){}};
+    cul::Provider::Ptr p2{std::addressof(mp2), [](cul::Provider*){}};
+    cul::Provider::Ptr p3{std::addressof(mp3), [](cul::Provider*){}};
+
+    std::set<cul::Provider::Ptr> providers{p1, p2, p3};
+
+    cul::FusionProvider fp{providers, std::make_shared<cul::NewerOrMoreAccurateUpdateSelector>()};
+
+    NiceMock<MockEventConsumer> mec;
+    EXPECT_CALL(mec, on_new_position(_)).Times(1);
+    EXPECT_CALL(mec, on_new_velocity(_)).Times(1);
+    EXPECT_CALL(mec, on_new_heading(_)).Times(1);
+
+    fp.updates().position.connect([&mec](const cul::Update<cul::Position>& p){mec.on_new_position(p);});
+    fp.updates().heading.connect([&mec](const cul::Update<cul::Heading>& h){mec.on_new_heading(h);});
+    fp.updates().velocity.connect([&mec](const cul::Update<cul::Velocity>& v){mec.on_new_velocity(v);});
+
+    mp1.inject_update(cul::Update<cul::Position>());
+    mp2.inject_update(cul::Update<cul::Heading>());
+    mp3.inject_update(cul::Update<cul::Velocity>());
+}
+
+#include <com/ubuntu/location/clock.h>
+
+TEST(FusionProvider, more_timely_update_is_chosen)
+{
+    using namespace ::testing;
+
+    NiceMock<MockProvider> mp1, mp2;
+
+    cul::Provider::Ptr p1{std::addressof(mp1), [](cul::Provider*){}};
+    cul::Provider::Ptr p2{std::addressof(mp2), [](cul::Provider*){}};
+
+    std::set<cul::Provider::Ptr> providers{p1, p2};
+
+    cul::FusionProvider fp{providers, std::make_shared<cul::NewerOrMoreAccurateUpdateSelector>()};
+
+    cul::Update<cul::Position> before, after;
+    before.when = cul::Clock::now() - std::chrono::seconds(12);
+    before.value = cul::Position(cul::wgs84::Latitude(), cul::wgs84::Longitude(), cul::wgs84::Altitude(), cul::Position::Accuracy::Horizontal{50*cul::units::Meters});
+    after.when = cul::Clock::now();
+    after.value = cul::Position(cul::wgs84::Latitude(), cul::wgs84::Longitude(), cul::wgs84::Altitude(), cul::Position::Accuracy::Horizontal{500*cul::units::Meters});
+
+    NiceMock<MockEventConsumer> mec;
+    EXPECT_CALL(mec, on_new_position(before)).Times(1);
+    EXPECT_CALL(mec, on_new_position(after)).Times(1);
+
+    fp.updates().position.connect([&mec](const cul::Update<cul::Position>& p){mec.on_new_position(p);});
+
+    mp1.inject_update(before);
+    mp2.inject_update(after);
+
+}
+
+TEST(FusionProvider, more_accurate_update_is_chosen)
+{
+    using namespace ::testing;
+
+    NiceMock<MockProvider> mp1, mp2;
+
+    cul::Provider::Ptr p1{std::addressof(mp1), [](cul::Provider*){}};
+    cul::Provider::Ptr p2{std::addressof(mp2), [](cul::Provider*){}};
+
+    std::set<cul::Provider::Ptr> providers{p1, p2};
+
+    cul::FusionProvider fp{providers, std::make_shared<cul::NewerOrMoreAccurateUpdateSelector>()};
+
+    cul::Update<cul::Position> before, after;
+    before.when = cul::Clock::now() - std::chrono::seconds(5);
+    before.value = cul::Position(cul::wgs84::Latitude(), cul::wgs84::Longitude(), cul::wgs84::Altitude(), cul::Position::Accuracy::Horizontal{50*cul::units::Meters});
+    after.when = cul::Clock::now();
+    after.value = cul::Position(cul::wgs84::Latitude(), cul::wgs84::Longitude(), cul::wgs84::Altitude(), cul::Position::Accuracy::Horizontal{500*cul::units::Meters});
+
+    NiceMock<MockEventConsumer> mec;
+    // We should see the "older" position in two events
+    EXPECT_CALL(mec, on_new_position(before)).Times(2);
+
+    fp.updates().position.connect([&mec](const cul::Update<cul::Position>& p){mec.on_new_position(p);});
+
+    mp1.inject_update(before);
+    mp2.inject_update(after);
+
+}