Merge lp:~thomas-voss/location-service/expose-engine-state-information into lp:location-service/trunk

[Thomas Voß]

Expose service::State to the bus.

Expose service::State to the bus.

[Scott Sweeny (ssweeny)]

LGTM

review: Approve

[Marcus Tomlinson (marcustomlinson)]

The silo is currently failing due to a changeling mismatch in Yakkety (...-0ubuntu4 vs ...-0ubuntu1)

Failing that, I've built this branch here: https://launchpad.net/~unity-api-team/+archive/ubuntu/dev-build-1. However, once installed on the phone, apps that use location begin crashing.

To test, before installing this, make sure only the Apps scope is favourited (Otherwise, you'll end up with the dash stuck in a restart loop).

As soon as a scope that uses location is opened (such as Nearby or News), the dash crashes. Also, as soon as you hit the locate button in uNav, uNav crashes.

Oddly though, I've got nothing in /var/crash.

review: Needs Fixing

[Marcus Tomlinson (marcustomlinson)]

Hmmm, I seem to only get a PropertiesChanged signal for "State" when I disable location, no signal on enable, and no signal on active (afaict).

I'm running this on the phone:

dbus-monitor --system "interface='org.freedesktop.DBus.Properties',path=/com/ubuntu/location/Service"

I toggle the indicator switch on/off/on:

signal sender=:1.110 -> dest=(null destination) serial=559 path=/com/ubuntu/location/Service; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
   string "com.ubuntu.location.Service"
   array [
      dict entry(
         string "IsOnline"
         variant boolean true
      )
   ]
   array [
   ]
signal sender=:1.110 -> dest=(null destination) serial=561 path=/com/ubuntu/location/Service; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
   string "com.ubuntu.location.Service"
   array [
      dict entry(
         string "IsOnline"
         variant boolean false
      )
   ]
   array [
   ]
signal sender=:1.110 -> dest=(null destination) serial=562 path=/com/ubuntu/location/Service; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
   string "com.ubuntu.location.Service"
   array [
      dict entry(
         string "State"
         variant string "disabled"
      )
   ]
   array [
   ]
signal sender=:1.110 -> dest=(null destination) serial=564 path=/com/ubuntu/location/Service; interface=org.freedesktop.DBus.Properties; member=PropertiesChanged
   string "com.ubuntu.location.Service"
   array [
      dict entry(
         string "IsOnline"
         variant boolean true
      )
   ]
   array [
   ]

review: Needs Fixing

[lp:~thomas-voss/location-service/expose-engine-state-information]

248. By Thomas Voß on 2016-06-01

Fix copy'n'paste leftover.

[Marcus Tomlinson (marcustomlinson)]

Just to recap where we are now: Your last change indeed fixed the "enabled" / "disabled" properties changes, thanks! We can at least rest assured that these signals will propagate through to the indicator. However, we now face an issue where we don't ever get an "active" status (even when it is clear that our position is being tracked).

review: Needs Fixing

[lp:~thomas-voss/location-service/expose-engine-state-information]

249. By Thomas Voß on 2016-06-11

Introduce a StateTrackingProvider enabling the engine to track enabled/active state.

[Marcus Tomlinson (marcustomlinson)]

This really is looking good now, thanks!

So one last concern is the silo's current build status: "Failed to build (location-service/vivid, location-service/yakkety)."

A few builds are failing, not sure which are expected: https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/landing-049/+packages

review: Needs Fixing

[Marcus Tomlinson (marcustomlinson)]

Awesome, Good to go here. Thanks Thomas!

review: Approve

=== modified file 'include/location_service/com/ubuntu/location/codec.h'
--- include/location_service/com/ubuntu/location/codec.h	2014-09-12 15:04:21 +0000
+++ include/location_service/com/ubuntu/location/codec.h	2016-06-11 19:59:38 +0000
@@ -25,6 +25,7 @@
 #include <com/ubuntu/location/space_vehicle.h>
 #include <com/ubuntu/location/update.h>
 #include <com/ubuntu/location/velocity.h>
+#include <com/ubuntu/location/service/state.h>
 #include <com/ubuntu/location/units/units.h>
 #include <com/ubuntu/location/wgs84/altitude.h>
 #include <com/ubuntu/location/wgs84/latitude.h>
@@ -32,12 +33,56 @@
 
 #include <core/dbus/codec.h>
 
+#include <sstream>
+
 namespace core
 {
 namespace dbus
 {
 namespace helper
 {
+template<>
+struct TypeMapper<com::ubuntu::location::service::State>
+{
+    constexpr static ArgumentType type_value()
+    {
+        return ArgumentType::string;
+    }
+
+    constexpr static bool is_basic_type()
+    {
+        return true;
+    }
+    constexpr static bool requires_signature()
+    {
+        return false;
+    }
+
+    static std::string signature()
+    {
+        static const std::string s = TypeMapper<std::string>::signature();
+        return s;
+    }
+};
+}
+
+template<>
+struct Codec<com::ubuntu::location::service::State>
+{
+    static void encode_argument(Message::Writer& writer, const com::ubuntu::location::service::State& in)
+    {
+        std::stringstream ss; ss << in; auto s = ss.str();
+        writer.push_stringn(s.c_str(), s.size());
+    }
+
+    static void decode_argument(Message::Reader& reader, com::ubuntu::location::service::State& in)
+    {
+        auto s = reader.pop_string();
+        std::stringstream ss{s}; ss >> in;
+    }
+};
+namespace helper
+{
 template<typename T>
 struct TypeMapper<com::ubuntu::location::units::Quantity<T>>
 {

=== modified file 'include/location_service/com/ubuntu/location/service/interface.h'
--- include/location_service/com/ubuntu/location/service/interface.h	2014-08-01 15:59:25 +0000
+++ include/location_service/com/ubuntu/location/service/interface.h	2016-06-11 19:59:38 +0000
@@ -18,6 +18,7 @@
 #ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_INTERFACE_H_
 #define LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_INTERFACE_H_
 
+#include <com/ubuntu/location/service/state.h>
 #include <com/ubuntu/location/service/session/interface.h>
 
 #include <com/ubuntu/location/space_vehicle.h>
@@ -87,6 +88,23 @@
 
     struct Properties
     {
+        struct State
+        {
+            inline static const std::string& name()
+            {
+                static const std::string s
+                {
+                    "State"
+                };
+                return s;
+            }
+
+            typedef com::ubuntu::location::service::Interface Interface;
+            typedef com::ubuntu::location::service::State ValueType;
+            static const bool readable = true;
+            static const bool writable = true;
+        };
+
         struct DoesSatelliteBasedPositioning
         {
             inline static const std::string& name()
@@ -176,6 +194,12 @@
     virtual ~Interface() = default;
 
     /**
+      * @brief The overall state of the service.
+      * @return a getable/observable property.
+      */
+    virtual const core::Property<State>& state() const = 0;
+
+    /**
      * @brief Whether the service uses satellite-based positioning.
      * @return  A setable/getable/observable property.
      */

=== modified file 'include/location_service/com/ubuntu/location/service/skeleton.h'
--- include/location_service/com/ubuntu/location/service/skeleton.h	2015-02-13 13:19:18 +0000
+++ include/location_service/com/ubuntu/location/service/skeleton.h	2016-06-11 19:59:38 +0000
@@ -111,11 +111,15 @@
     ~Skeleton() noexcept;
 
     // From com::ubuntu::location::service::Interface
+    const core::Property<State>& state() const;
     core::Property<bool>& does_satellite_based_positioning();
     core::Property<bool>& does_report_cell_and_wifi_ids();
     core::Property<bool>& is_online();
     core::Property<std::map<SpaceVehicle::Key, SpaceVehicle>>& visible_space_vehicles();
 
+protected:
+    // Enable subclasses to alter the state.
+    core::Property<State>& mutable_state();
 private:
     // Handles incoming message calls for create_session_for_criteria.
     // Dispatches to the actual implementation, and manages object lifetimes.
@@ -131,6 +135,8 @@
     // Removes the session with the given path from the session store.
     void remove_from_session_store_for_path(const core::dbus::types::ObjectPath& path);
 
+    // Called whenever the overall state of the service changes.
+    void on_state_changed(State state);
     // Called whenever the value of the respective property changes.
     void on_does_satellite_based_positioning_changed(bool value);
     // Called whenever the value of the respective property changes.
@@ -154,6 +160,7 @@
     // DBus properties as exposed on the bus for com.ubuntu.location.service.Interface
     struct
     {
+        std::shared_ptr< core::dbus::Property<Interface::Properties::State> > state;
         std::shared_ptr< core::dbus::Property<Interface::Properties::DoesSatelliteBasedPositioning> > does_satellite_based_positioning;
         std::shared_ptr< core::dbus::Property<Interface::Properties::DoesReportCellAndWifiIds> > does_report_cell_and_wifi_ids;
         std::shared_ptr< core::dbus::Property<Interface::Properties::IsOnline> > is_online;
@@ -162,6 +169,7 @@
     // We sign up to property changes here, to be able to report them to the bus
     struct
     {
+        core::ScopedConnection state;
         core::ScopedConnection does_satellite_based_positioning;
         core::ScopedConnection does_report_cell_and_wifi_ids;
         core::ScopedConnection is_online;

=== added file 'include/location_service/com/ubuntu/location/service/state.h'
--- include/location_service/com/ubuntu/location/service/state.h	1970-01-01 00:00:00 +0000
+++ include/location_service/com/ubuntu/location/service/state.h	2016-06-11 19:59:38 +0000
@@ -0,0 +1,43 @@
+/*
+ * 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: Thomas Voß <[e-mail]>
+ */
+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_STATE_H_
+#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_STATE_H_
+
+#include <iosfwd>
+
+namespace com { namespace ubuntu { namespace location { namespace service
+{
+/// @brief State enumerates the known states of the service.
+enum class State
+{
+    disabled, ///< The service has been disabled by the user.
+    enabled,  ///< The service is enabled but not actively carrying out positioning.
+    active    ///< The service is actively trying to determine the position of the device.
+};
+
+/// @brief operator== returns true if lhs == rhs.
+bool operator==(State lhs, State rhs);
+/// @brief operator != returns true if !(lhs == rhs).
+bool operator!=(State lhs, State rhs);
+
+/// @brief operator<< inserts state into the out.
+std::ostream& operator<<(std::ostream& out, State state);
+/// @brief operator>> extracts state from in.
+std::istream& operator>>(std::istream& in, State& state);
+}}}}
+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_STATE_H_

=== modified file 'include/location_service/com/ubuntu/location/service/stub.h'
--- include/location_service/com/ubuntu/location/service/stub.h	2014-02-03 13:52:21 +0000
+++ include/location_service/com/ubuntu/location/service/stub.h	2016-06-11 19:59:38 +0000
@@ -40,6 +40,8 @@
     ~Stub() noexcept;
 
     session::Interface::Ptr create_session_for_criteria(const Criteria& criteria);
+
+    const core::Property<State>& state() const;
     core::Property<bool>& does_satellite_based_positioning();
     core::Property<bool>& does_report_cell_and_wifi_ids();
     core::Property<bool>& is_online();    

=== modified file 'src/location_service/com/ubuntu/location/CMakeLists.txt'
--- src/location_service/com/ubuntu/location/CMakeLists.txt	2016-04-05 21:26:06 +0000
+++ src/location_service/com/ubuntu/location/CMakeLists.txt	2016-06-11 19:59:38 +0000
@@ -35,6 +35,7 @@
   proxy_provider.cpp
   satellite_based_positioning_state.cpp
   settings.cpp
+  state_tracking_provider.h
   time_based_update_policy.cpp
   set_name_for_thread.cpp
   time_since_boot.cpp
@@ -50,6 +51,7 @@
   service/runtime.cpp
   service/runtime_tests.h
   service/runtime_tests.cpp
+  service/state.cpp
   service/trust_store_permission_manager.cpp
 
   service/implementation.cpp

=== modified file 'src/location_service/com/ubuntu/location/engine.cpp'
--- src/location_service/com/ubuntu/location/engine.cpp	2016-03-02 14:23:41 +0000
+++ src/location_service/com/ubuntu/location/engine.cpp	2016-06-11 19:59:38 +0000
@@ -19,6 +19,7 @@
 
 #include <com/ubuntu/location/logging.h>
 #include <com/ubuntu/location/provider_selection_policy.h>
+#include <com/ubuntu/location/state_tracking_provider.h>
 
 #include <iostream>
 #include <stdexcept>
@@ -138,16 +139,13 @@
     return provider_selection_policy->determine_provider_selection_for_criteria(criteria, *this);
 }
 
-bool cul::Engine::has_provider(const cul::Provider::Ptr& provider) noexcept
-{
-    return providers.count(provider) > 0;
-}
-
-void cul::Engine::add_provider(const cul::Provider::Ptr& provider)
-{
-    if (!provider)
+void cul::Engine::add_provider(const cul::Provider::Ptr& impl)
+{
+    if (!impl)
         throw std::runtime_error("Cannot add null provider");
 
+    auto provider = std::make_shared<StateTrackingProvider>(impl);
+
     // We synchronize to the engine state.
     if (provider->requires(Provider::Requirements::satellites) && configuration.satellite_based_positioning_state == SatelliteBasedPositioningState::off)
         provider->state_controller()->disable();
@@ -207,24 +205,24 @@
         updates.last_known_location = update_policy->verify_update(src);
     });
 
-    std::lock_guard<std::mutex> lg(guard);
-    providers.emplace(provider, std::move(ProviderConnections{cp, ch, cv, cr, cs, cpr}));
-}
-
-void cul::Engine::remove_provider(const cul::Provider::Ptr& provider) noexcept
-{
-    std::lock_guard<std::mutex> lg(guard);
-
-    auto it = providers.find(provider);
-    if (it != providers.end())
+    auto cps = provider->state().changed().connect([this](const StateTrackingProvider::State&)
     {
-        providers.erase(it);
-    }
+        bool is_any_active = false;
+
+        std::lock_guard<std::recursive_mutex> lg(guard);
+        for (const auto& pair : providers)
+            is_any_active = pair.first->state() == StateTrackingProvider::State::active;
+
+        configuration.engine_state = is_any_active ? Engine::Status::active : Engine::Status::on;
+    });
+
+    std::lock_guard<std::recursive_mutex> lg(guard);
+    providers.emplace(provider, std::move(ProviderConnections{cp, ch, cv, cr, cs, cpr, cps}));
 }
 
 void cul::Engine::for_each_provider(const std::function<void(const Provider::Ptr&)>& enumerator) const noexcept
 {
-    std::lock_guard<std::mutex> lg(guard);
+    std::lock_guard<std::recursive_mutex> lg(guard);
     for (const auto& provider : providers)
     {
         try

=== modified file 'src/location_service/com/ubuntu/location/engine.h'
--- src/location_service/com/ubuntu/location/engine.h	2015-11-19 09:55:37 +0000
+++ src/location_service/com/ubuntu/location/engine.h	2016-06-11 19:59:38 +0000
@@ -24,6 +24,7 @@
 #include <com/ubuntu/location/provider_selection_policy.h>
 #include <com/ubuntu/location/satellite_based_positioning_state.h>
 #include <com/ubuntu/location/space_vehicle.h>
+#include <com/ubuntu/location/state_tracking_provider.h>
 #include <com/ubuntu/location/wifi_and_cell_reporting_state.h>
 
 #include <com/ubuntu/location/settings.h>
@@ -151,24 +152,12 @@
     virtual ProviderSelection determine_provider_selection_for_criteria(const Criteria& criteria);
 
     /**
-     * @brief Checks if the engine knows about a specific provider.
-     * @return True iff the engine knows about the provider.
-     */
-    virtual bool has_provider(const Provider::Ptr& provider) noexcept;
-
-    /**
      * @brief Makes a provider known to the engine.
      * @param provider The new provider.
      */
     virtual void add_provider(const Provider::Ptr& provider);
 
     /**
-     * @brief Removes a provider from the engine.
-     * @param provider The provider to be removed.
-     */
-    virtual void remove_provider(const Provider::Ptr& provider) noexcept;
-
-    /**
      * @brief Iterates all known providers and invokes the provided enumerator for each of them.
      * @param enumerator The functor to be invoked for each provider.
      */
@@ -189,10 +178,11 @@
         core::ScopedConnection wifi_and_cell_id_reporting_state_updates;
         core::ScopedConnection space_vehicle_visibility_updates;
         core::ScopedConnection provider_position_updates;
+        core::ScopedConnection provider_state_updates;
     };
 
-    mutable std::mutex guard;
-    std::map<Provider::Ptr, ProviderConnections> providers;
+    mutable std::recursive_mutex guard;
+    std::map<StateTrackingProvider::Ptr, ProviderConnections> providers;
     ProviderSelectionPolicy::Ptr provider_selection_policy;
     Settings::Ptr settings;
     UpdatePolicy::Ptr update_policy;

=== modified file 'src/location_service/com/ubuntu/location/service/daemon.cpp'
--- src/location_service/com/ubuntu/location/service/daemon.cpp	2016-03-31 20:31:05 +0000
+++ src/location_service/com/ubuntu/location/service/daemon.cpp	2016-06-11 19:59:38 +0000
@@ -265,6 +265,7 @@
     options.add("help", "Produces this help message");
     options.add("property",
                 "Property to set/get from a running service, known properties are:\n"
+                "   state [get]\n"
                 "   is_online [get/set]\n"
                 "   does_satellite_based_positioning [get/set]\n"
                 "   does_report_wifi_and_cell_ids [get/set]\n"
@@ -346,6 +347,15 @@
 
     switch (config.property)
     {
+    case Property::state:
+        switch (config.command)
+        {
+        case Command::get:
+            std::cout << "Location service is " << location_service->state() << std::endl;
+            break;
+        default:
+            break;
+        }
     case Property::is_online:
         switch (config.command)
         {
@@ -468,6 +478,7 @@
 {
     static const std::map<std::string, location::service::Daemon::Cli::Property> lut =
     {
+        {"state", location::service::Daemon::Cli::Property::state},
         {"is_online", location::service::Daemon::Cli::Property::is_online},
         {"does_satellite_based_positioning", location::service::Daemon::Cli::Property::does_satellite_based_positioning},
         {"does_report_wifi_and_cell_ids", location::service::Daemon::Cli::Property::does_report_wifi_and_cell_ids},
@@ -492,6 +503,8 @@
 {
     switch (property)
     {
+    case location::service::Daemon::Cli::Property::state:
+        out << "state"; break;
     case location::service::Daemon::Cli::Property::is_online:
         out << "is_online"; break;
     case location::service::Daemon::Cli::Property::does_satellite_based_positioning:

=== modified file 'src/location_service/com/ubuntu/location/service/daemon.h'
--- src/location_service/com/ubuntu/location/service/daemon.h	2016-01-11 15:20:19 +0000
+++ src/location_service/com/ubuntu/location/service/daemon.h	2016-06-11 19:59:38 +0000
@@ -63,6 +63,8 @@
         {
             /** @brief Marks the unknown property. */
             unknown,
+            /** @brief State describes the overall state of the service. */
+            state,
             /** @brief Indicates whether the positioning engine is online. */
             is_online,
             /** @brief Indicates whether the positioning engine uses satellite-based positioning. */
@@ -82,6 +84,7 @@
              * --help                    Produces this help message
              * --property arg (=unknown) Property to set/get from a running service, known
                                          properties are:
+                                            state [get]
                                             is_online [get/set]
                                             does_satellite_based_positioning [get/set]
                                             does_report_wifi_and_cell_ids [get/set]

=== modified file 'src/location_service/com/ubuntu/location/service/implementation.cpp'
--- src/location_service/com/ubuntu/location/service/implementation.cpp	2015-12-09 15:18:14 +0000
+++ src/location_service/com/ubuntu/location/service/implementation.cpp	2016-06-11 19:59:38 +0000
@@ -98,9 +98,21 @@
           configuration.engine->configuration.engine_state.changed().connect(
               [this](Engine::Status status)
               {
-                  is_online() =
-                          status == Engine::Status::on ||
-                          status == Engine::Status::active;
+                  switch (status)
+                  {
+                      case Engine::Status::off:
+                          is_online() = false;
+                          mutable_state() = State::disabled;
+                          break;
+                      case Engine::Status::on:
+                          is_online() = true;
+                          mutable_state() = State::enabled;
+                          break;
+                      case Engine::Status::active:
+                          is_online() = true;
+                          mutable_state() = State::active;
+                          break;
+                  }
               }),
           configuration.engine->configuration.satellite_based_positioning_state.changed().connect(
               [this](cul::SatelliteBasedPositioningState state)
@@ -132,9 +144,22 @@
     if (!configuration.permission_manager)
         throw std::runtime_error("Cannot create service for null permission manager.");
 
-    is_online() =
-            configuration.engine->configuration.engine_state == Engine::Status::on ||
-            configuration.engine->configuration.engine_state == Engine::Status::active;
+    switch (configuration.engine->configuration.engine_state)
+    {
+        case Engine::Status::off:
+            is_online() = false;
+            mutable_state() = State::disabled;
+            break;
+        case Engine::Status::on:
+            is_online() = true;
+            mutable_state() = State::enabled;
+            break;
+          case Engine::Status::active:
+            is_online() = true;
+            mutable_state() = State::active;
+            break;
+    };
+
     does_report_cell_and_wifi_ids() =
             configuration.engine->configuration.wifi_and_cell_id_reporting_state ==
             cul::WifiAndCellIdReportingState::on;

=== modified file 'src/location_service/com/ubuntu/location/service/skeleton.cpp'
--- src/location_service/com/ubuntu/location/service/skeleton.cpp	2015-02-13 13:19:18 +0000
+++ src/location_service/com/ubuntu/location/service/skeleton.cpp	2016-06-11 19:59:38 +0000
@@ -73,6 +73,7 @@
       properties_changed(object->get_signal<core::dbus::interfaces::Properties::Signals::PropertiesChanged>()),
       properties
       {
+          object->get_property<culs::Interface::Properties::State>(),
           object->get_property<culs::Interface::Properties::DoesSatelliteBasedPositioning>(),
           object->get_property<culs::Interface::Properties::DoesReportCellAndWifiIds>(),
           object->get_property<culs::Interface::Properties::IsOnline>(),
@@ -80,6 +81,10 @@
       },
       connections
       {
+          properties.state->changed().connect([this](State state)
+          {
+              on_state_changed(state);
+          }),
           properties.does_satellite_based_positioning->changed().connect([this](bool value)
           {
               on_does_satellite_based_positioning_changed(value);
@@ -105,6 +110,11 @@
     object->uninstall_method_handler<culs::Interface::CreateSessionForCriteria>();
 }
 
+core::Property<culs::State>& culs::Skeleton::mutable_state()
+{
+    return *properties.state;
+}
+
 void culs::Skeleton::handle_create_session_for_criteria(const dbus::Message::Ptr& in)
 {
     VLOG(1) << __PRETTY_FUNCTION__;
@@ -208,6 +218,24 @@
     session_store.erase(path);
 }
 
+void culs::Skeleton::on_state_changed(culs::State state)
+{
+    std::map<std::string, core::dbus::types::Variant> dict
+    {
+        {
+            culs::Interface::Properties::State::name(),
+            core::dbus::types::Variant::encode(state)
+        }
+    };
+
+    properties_changed->emit(
+                std::tie(
+                    core::dbus::traits::Service<culs::Interface>::interface_name(),
+                    dict,
+                    the_empty_array_of_invalidated_properties()));
+}
+
+
 void culs::Skeleton::on_does_satellite_based_positioning_changed(bool value)
 {
     std::map<std::string, core::dbus::types::Variant> dict
@@ -258,6 +286,11 @@
                 the_empty_array_of_invalidated_properties()));
 }
 
+const core::Property<culs::State>& culs::Skeleton::state() const
+{
+    return *properties.state;
+}
+
 core::Property<bool>& culs::Skeleton::does_satellite_based_positioning()
 {
     return *properties.does_satellite_based_positioning;

=== added file 'src/location_service/com/ubuntu/location/service/state.cpp'
--- src/location_service/com/ubuntu/location/service/state.cpp	1970-01-01 00:00:00 +0000
+++ src/location_service/com/ubuntu/location/service/state.cpp	2016-06-11 19:59:38 +0000
@@ -0,0 +1,71 @@
+/*
+ * 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: Thomas Voß <[e-mail]>
+ */
+
+#include <com/ubuntu/location/service/state.h>
+
+#include <map>
+#include <type_traits>
+
+namespace culs = com::ubuntu::location::service;
+
+namespace
+{
+typedef typename std::underlying_type<culs::State>::type UT;
+
+typedef std::pair<std::map<culs::State, std::string>, std::map<std::string, culs::State>> Lut;
+
+const Lut& lut()
+{
+    static const Lut instance
+    {
+        {
+            {culs::State::disabled, "disabled"},
+            {culs::State::enabled, "enabled"},
+            {culs::State::active, "active"}
+        },
+        {
+            {"disabled", culs::State::disabled},
+            {"enabled", culs::State::enabled},
+            {"active", culs::State::active}
+        }
+    };
+
+    return instance;
+}
+}
+
+bool culs::operator==(culs::State lhs, culs::State rhs)
+{
+    return static_cast<UT>(lhs) == static_cast<UT>(rhs);
+}
+
+bool culs::operator!=(culs::State lhs, culs::State rhs)
+{
+    return static_cast<UT>(lhs) != static_cast<UT>(rhs);
+}
+
+std::ostream& culs::operator<<(std::ostream& out, culs::State state)
+{
+    return out << lut().first.at(state);
+}
+
+std::istream& culs::operator>>(std::istream& in, culs::State& state)
+{
+    std::string s; in >> s; state = lut().second.at(s);
+    return in;
+}

=== modified file 'src/location_service/com/ubuntu/location/service/stub.cpp'
--- src/location_service/com/ubuntu/location/service/stub.cpp	2014-07-17 14:03:42 +0000
+++ src/location_service/com/ubuntu/location/service/stub.cpp	2016-06-11 19:59:38 +0000
@@ -34,6 +34,7 @@
             const dbus::Object::Ptr& object)
         : bus(connection),
           object(object),
+          state(object->get_property<culs::Interface::Properties::State>()),
           does_satellite_based_positioning(object->get_property<culs::Interface::Properties::DoesSatelliteBasedPositioning>()),
           does_report_cell_and_wifi_ids(object->get_property<culs::Interface::Properties::DoesReportCellAndWifiIds>()),
           is_online(object->get_property<culs::Interface::Properties::IsOnline>()),
@@ -43,6 +44,7 @@
 
     dbus::Bus::Ptr bus;
     dbus::Object::Ptr object;
+    std::shared_ptr<dbus::Property<culs::Interface::Properties::State>> state;
     std::shared_ptr<dbus::Property<culs::Interface::Properties::DoesSatelliteBasedPositioning>> does_satellite_based_positioning;
     std::shared_ptr<dbus::Property<culs::Interface::Properties::DoesReportCellAndWifiIds>> does_report_cell_and_wifi_ids;
     std::shared_ptr<dbus::Property<culs::Interface::Properties::IsOnline>> is_online;
@@ -74,6 +76,11 @@
     return culss::Interface::Ptr(new culss::Stub{d->bus, op.value()});
 }
 
+const core::Property<culs::State>& culs::Stub::state() const
+{
+    return *d->state;
+}
+
 core::Property<bool>& culs::Stub::does_satellite_based_positioning()
 {
     return *d->does_satellite_based_positioning;

=== added file 'src/location_service/com/ubuntu/location/state_tracking_provider.h'
--- src/location_service/com/ubuntu/location/state_tracking_provider.h	1970-01-01 00:00:00 +0000
+++ src/location_service/com/ubuntu/location/state_tracking_provider.h	2016-06-11 19:59:38 +0000
@@ -0,0 +1,162 @@
+/*
+ * 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: Thomas Voß <[e-mail]>
+ */
+#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_STATE_TRACKING_PROVIDER_H_
+#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_STATE_TRACKING_PROVIDER_H_
+
+#include <com/ubuntu/location/provider.h>
+
+#include <memory>
+
+namespace com
+{
+namespace ubuntu
+{
+namespace location
+{
+// StateTrackingProvider keeps track of the State of an individual provider instance.
+//
+// TODO(tvoss): Factor this functionality into the public interface once we find time
+// to break API/ABI of providers.
+class StateTrackingProvider : public Provider
+{
+public:
+    // Safe us some typing
+    typedef std::shared_ptr<StateTrackingProvider> Ptr;
+
+    // State models the different states of a provider.
+    enum class State
+    {
+        active, // The provider is actively delivering updates.
+        enabled // The provider is enabled but not actively delivering updates.
+    };
+
+    StateTrackingProvider(const Provider::Ptr& impl)
+        : impl_{impl},
+          connections
+          {
+              impl_->updates().position.connect(
+                  [this](const Update<Position>& u)
+                  {
+                      mutable_updates().position(u);
+                  }),
+              impl_->updates().heading.connect(
+                  [this](const Update<Heading>& u)
+                  {
+                      mutable_updates().heading(u);
+                  }),
+              impl_->updates().velocity.connect(
+                  [this](const Update<Velocity>& u)
+                  {
+                      mutable_updates().velocity(u);
+                  })
+           },
+          state_{State::enabled}
+    {
+    }
+
+    const core::Property<State>& state() const
+    {
+        return state_;
+    }
+
+    bool supports(const Features& f) const override
+    {
+        return impl_->supports(f);
+    }
+
+    bool requires(const Requirements& r) const override
+    {
+        return impl_->requires(r);
+    }
+
+    bool matches_criteria(const Criteria& criteria) override
+    {
+        return impl_->matches_criteria(criteria);
+    }
+
+    void on_wifi_and_cell_reporting_state_changed(WifiAndCellIdReportingState state) override
+    {
+        impl_->on_wifi_and_cell_reporting_state_changed(state);
+    }
+
+    void on_reference_location_updated(const Update<Position>& position) override
+    {
+        impl_->on_reference_location_updated(position);
+    }
+
+    void on_reference_velocity_updated(const Update<Velocity>& velocity) override
+    {
+        impl_->on_reference_velocity_updated(velocity);
+    }
+
+    void on_reference_heading_updated(const Update<Heading>& heading) override
+    {
+        impl_->on_reference_heading_updated(heading);
+    }
+
+    void start_position_updates() override
+    {
+        state_ = State::active;
+        impl_->state_controller()->start_position_updates();
+    }
+
+    void stop_position_updates() override
+    {
+        state_ = State::enabled;
+        impl_->state_controller()->stop_position_updates();
+    }
+
+    void start_velocity_updates() override
+    {
+        state_ = State::active;
+        impl_->state_controller()->start_velocity_updates();
+    }
+
+    void stop_velocity_updates() override
+    {
+        state_ = State::enabled;
+        impl_->state_controller()->stop_velocity_updates();
+    }
+
+    void start_heading_updates() override
+    {
+        state_ = State::active;
+        impl_->state_controller()->start_heading_updates();
+    }
+
+    void stop_heading_updates() override
+    {
+        state_ = State::enabled;
+        impl_->state_controller()->stop_heading_updates();
+    }
+
+private:
+    Provider::Ptr impl_;
+    struct
+    {
+        core::ScopedConnection position_updates;
+        core::ScopedConnection heading_updates;
+        core::ScopedConnection velocity_updates;
+    } connections;
+    core::Property<State> state_;
+};
+}
+}
+}
+
+#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_STATE_TRACKING_PROVIDER_H_

=== modified file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt	2016-03-31 10:15:53 +0000
+++ tests/CMakeLists.txt	2016-06-11 19:59:38 +0000
@@ -93,6 +93,7 @@
 LOCATION_SERVICE_ADD_TEST(wgs84_test wgs84_test.cpp)
 LOCATION_SERVICE_ADD_TEST(trust_store_permission_manager_test trust_store_permission_manager_test.cpp)
 LOCATION_SERVICE_ADD_TEST(runtime_test runtime_test.cpp)
+LOCATION_SERVICE_ADD_TEST(state_tracking_provider_test state_tracking_provider_test.cpp)
 
 # Provider-specific test-cases go here.
 if (LOCATION_SERVICE_ENABLE_GPS_PROVIDER)

=== modified file 'tests/daemon_and_cli_tests.cpp'
--- tests/daemon_and_cli_tests.cpp	2016-01-28 20:16:11 +0000
+++ tests/daemon_and_cli_tests.cpp	2016-06-11 19:59:38 +0000
@@ -122,6 +122,16 @@
 }
 }
 
+TEST_F(DaemonAndCli, QueryingStatePropertyWorks)
+{
+    EXPECT_EQ(core::testing::ForkAndRunResult::empty,
+              core::testing::fork_and_run(
+                  testing_daemon(*this),
+                  querying_cli_for_property(
+                      location::service::Daemon::Cli::Property::state,
+                      *this)));
+}
+
 TEST_F(DaemonAndCli, QueryingIsOnlinePropertyWorks)
 {
     EXPECT_EQ(core::testing::ForkAndRunResult::empty,

=== modified file 'tests/engine_test.cpp'
--- tests/engine_test.cpp	2016-03-02 15:26:21 +0000
+++ tests/engine_test.cpp	2016-06-11 19:59:38 +0000
@@ -72,26 +72,6 @@
 }
 }
 
-TEST(Engine, adding_and_removing_providers_inserts_and_erases_from_underlying_collection)
-{
-    location::Engine engine {std::make_shared<NullProviderSelectionPolicy>(), mock_settings()};
-
-    auto provider1 = std::make_shared<testing::NiceMock<MockProvider>>();
-    auto provider2 = std::make_shared<testing::NiceMock<MockProvider>>();
-
-    engine.add_provider(provider1);
-    EXPECT_TRUE(engine.has_provider(provider1));
-
-    engine.add_provider(provider2);
-    EXPECT_TRUE(engine.has_provider(provider2));
-
-    engine.remove_provider(provider1);
-    EXPECT_FALSE(engine.has_provider(provider1));
-
-    engine.remove_provider(provider2);
-    EXPECT_FALSE(engine.has_provider(provider2));
-}
-
 TEST(Engine, adding_a_null_provider_throws)
 {
     location::Engine engine {std::make_shared<NullProviderSelectionPolicy>(), mock_settings()};

=== added file 'tests/state_tracking_provider_test.cpp'
--- tests/state_tracking_provider_test.cpp	1970-01-01 00:00:00 +0000
+++ tests/state_tracking_provider_test.cpp	2016-06-11 19:59:38 +0000
@@ -0,0 +1,119 @@
+/*
+ * 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: Thomas Voß <[e-mail]>
+ */
+
+#include <com/ubuntu/location/state_tracking_provider.h>
+
+#include <com/ubuntu/location/clock.h>
+#include <com/ubuntu/location/update.h>
+#include <com/ubuntu/location/position.h>
+#include <com/ubuntu/location/units/units.h>
+#include <com/ubuntu/location/wgs84/latitude.h>
+#include <com/ubuntu/location/wgs84/longitude.h>
+#include <com/ubuntu/location/wgs84/altitude.h>
+
+#include "mock_provider.h"
+
+#include <gtest/gtest.h>
+
+namespace cul = com::ubuntu::location;
+
+namespace
+{
+auto timestamp = cul::Clock::now();
+
+// Create reference objects for injecting and validating updates.
+cul::Update<cul::Position> reference_position_update
+{
+    {
+        cul::wgs84::Latitude{9. * cul::units::Degrees},
+        cul::wgs84::Longitude{53. * cul::units::Degrees},
+        cul::wgs84::Altitude{-2. * cul::units::Meters}
+    },
+    timestamp
+};
+
+cul::Update<cul::Velocity> reference_velocity_update
+{
+    {5. * cul::units::MetersPerSecond},
+    timestamp
+};
+
+cul::Update<cul::Heading> reference_heading_update
+{
+    {120. * cul::units::Degrees},
+    timestamp
+};
+}
+TEST(StateTrackingProviderTest, forwards_calls_to_impl)
+{
+    using namespace testing;
+
+    auto impl = std::make_shared<MockProvider>();
+    EXPECT_CALL(*impl, supports(_)).Times(1).WillOnce(Return(true));
+    EXPECT_CALL(*impl, requires(_)).Times(1).WillOnce(Return(false));
+    EXPECT_CALL(*impl, matches_criteria(_)).Times(1).WillOnce(Return(false));
+    EXPECT_CALL(*impl, on_wifi_and_cell_reporting_state_changed(_)).Times(1);
+    EXPECT_CALL(*impl, on_reference_location_updated(_)).Times(1);
+    EXPECT_CALL(*impl, on_reference_velocity_updated(_)).Times(1);
+    EXPECT_CALL(*impl, on_reference_heading_updated(_)).Times(1);
+    EXPECT_CALL(*impl, start_position_updates()).Times(1);
+    EXPECT_CALL(*impl, stop_position_updates()).Times(1);
+    EXPECT_CALL(*impl, start_velocity_updates()).Times(1);
+    EXPECT_CALL(*impl, stop_velocity_updates()).Times(1);
+    EXPECT_CALL(*impl, start_heading_updates()).Times(1);
+    EXPECT_CALL(*impl, stop_heading_updates()).Times(1);
+
+    com::ubuntu::location::StateTrackingProvider stp{impl};
+    EXPECT_TRUE(stp.supports(com::ubuntu::location::Provider::Features::none));
+    EXPECT_FALSE(stp.requires(com::ubuntu::location::Provider::Requirements::none));
+    EXPECT_FALSE(stp.matches_criteria(com::ubuntu::location::Criteria{}));
+    stp.on_wifi_and_cell_reporting_state_changed(com::ubuntu::location::WifiAndCellIdReportingState::on);
+    stp.on_reference_location_updated(reference_position_update);
+    stp.on_reference_heading_updated(reference_heading_update);
+    stp.on_reference_velocity_updated(reference_velocity_update);
+    stp.start_position_updates();
+    stp.stop_position_updates();
+    stp.start_heading_updates();
+    stp.stop_heading_updates();
+    stp.start_velocity_updates();
+    stp.stop_velocity_updates();
+}
+
+TEST(StateTrackingProviderTest, state_after_construction_is_enabled)
+{
+    using namespace ::testing;
+    cul::StateTrackingProvider stp{std::make_shared<NiceMock<MockProvider>>()};
+    EXPECT_EQ(cul::StateTrackingProvider::State::enabled, stp.state());
+}
+
+TEST(StateTrackingProviderTest, state_after_start_is_active)
+{
+    using namespace ::testing;
+    cul::StateTrackingProvider stp{std::make_shared<NiceMock<MockProvider>>()};
+    stp.start_position_updates();
+    EXPECT_EQ(cul::StateTrackingProvider::State::active, stp.state());
+}
+
+TEST(StateTrackingProviderTest, stop_switches_to_enabled)
+{
+    using namespace ::testing;
+    cul::StateTrackingProvider stp{std::make_shared<NiceMock<MockProvider>>()};
+    stp.start_position_updates();
+    stp.stop_position_updates();
+    EXPECT_EQ(cul::StateTrackingProvider::State::enabled, stp.state());
+}