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

[Scott Sweeny]

Ensure that we use a delayed provider when the espoo service is not running.

Ensure that the remote provider creates a delayed provider in the case in which the service is not ready. This allows the location service to be started without having the here blobs running.

[Thomas Voß (thomas-voss)]

A remark inline about splitting changes between espoo-delayed.provider.15.04 and delayed-provider.15.04.

One other remark: I would prefer a set of more sophisticated test cases that exercise the timing logic. Otherwise, we would have to invest significant manual testing efforts for each landing to ensure correct behavior. Would you mind introducing such tests into this MP?

review: Needs Fixing

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

200. By Scott Sweeny on 2015-10-27

Merge delayed-provider branch

[Scott Sweeny (ssweeny)]

I'm not clear on what tests you're looking for that aren't part of the other MP. Could you elaborate?

[Thomas Voß (thomas-voss)]

I was thinking about tests exercising the actual timing, something along the lines of a misbehaving provider that takes a configurable amount of time to come up. It might also be a good idea to test the overall location::Engine behavior with a couple of those providers registered. I hope that helps to clarify my line of thought.

Unmerged revisions

200. By Scott Sweeny on 2015-10-27

Merge delayed-provider branch

199. By Manuel de la Peña on 2015-10-22

Ensure that we use a delayed provider when the espoo service is not running.

198. By Manuel de la Peña on 2015-10-22

Added support for delayed providers.

=== modified file 'include/location_service/com/ubuntu/location/provider.h'
--- include/location_service/com/ubuntu/location/provider.h	2015-10-27 20:50:44 +0000
+++ include/location_service/com/ubuntu/location/provider.h	2015-10-27 20:50:44 +0000
@@ -241,7 +241,7 @@
      * @brief States the booting state of the provider.
      * @return The boot state of the provider.
      */
-    virtual BootState boot_state() const;
+    virtual BootState boot_state();
 
     /**
      * @brief Indicates to the provider that it should start the boot process in order to be able to

=== modified file 'src/location_service/com/ubuntu/location/provider.cpp'
--- src/location_service/com/ubuntu/location/provider.cpp	2015-10-27 20:50:44 +0000
+++ src/location_service/com/ubuntu/location/provider.cpp	2015-10-27 20:50:44 +0000
@@ -199,7 +199,7 @@
     return false;
 }
 
-cul::Provider::BootState cul::Provider::boot_state() const
+cul::Provider::BootState cul::Provider::boot_state()
 {
     return cul::Provider::BootState::BOOTED;
 }

=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.cpp'
--- src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2015-04-14 18:54:00 +0000
+++ src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2015-10-27 20:50:44 +0000
@@ -22,7 +22,9 @@
 
 #include <com/ubuntu/location/logging.h>
 
+#include <core/dbus/dbus.h>
 #include <core/dbus/object.h>
+#include <core/dbus/service_watcher.h>
 #include <core/dbus/signal.h>
 #include <core/dbus/asio/executor.h>
 
@@ -30,6 +32,8 @@
 
 #include <boost/asio.hpp>
 
+#include <functional>
+#include <mutex>
 #include <thread>
 
 namespace cul = com::ubuntu::location;
@@ -39,6 +43,124 @@
 
 namespace
 {
+// NotYet is a provider decoration that waits for a remote service to come up, before instantiating
+// the actual stub instance.
+class NotYet : public cul::Provider
+{
+public:
+    // Creates a new instance, watching fo the given service_name on the given bus, invoking the
+    // when_ready functor for constructing the provider instance whenever the service becomes ready.
+    NotYet(const core::dbus::Bus::Ptr& bus, const std::string& service_name, const std::function<cul::Provider::Ptr()>& when_ready)
+        : dbus{bus}, service_watcher{dbus.make_service_watcher(service_name)},
+          service_registered
+          {
+              service_watcher->service_registered().connect(std::bind(&NotYet::on_service_registered, this, when_ready))
+          }
+    {
+    }
+
+    void on_service_registered(const std::function<cul::Provider::Ptr()>& when_ready)
+    {
+        LOG(INFO) << "Remote location service appeared.";
+        impl = when_ready();
+        set_boot_state(cul::Provider::BootState::BOOTED);
+        // notify the system we are booted so that we get the current state (started etc..)
+        LOG(INFO) << "Emitting booted signal.";
+        booted_signal()(true);
+    }
+
+    bool matches_criteria(const cul::Criteria& criteria) override
+    {
+        if (impl) return impl->matches_criteria(criteria);
+
+        return false;
+    }
+
+    bool supports(const cul::Provider::Features& f) const override
+    {
+        if (impl) return impl->supports(f);
+        return false;
+    }
+
+    bool requires(const cul::Provider::Requirements& r) const override
+    {
+        if (impl) return impl->requires(r);
+        return true;
+    }
+
+    void on_wifi_and_cell_reporting_state_changed(cul::WifiAndCellIdReportingState state) override
+    {
+        if (impl) impl->on_wifi_and_cell_reporting_state_changed(state);
+    }
+
+    void on_reference_location_updated(const cul::Update<cul::Position>& position) override
+    {
+        if (impl) impl->on_reference_location_updated(position);
+    }
+
+    void on_reference_velocity_updated(const cul::Update<cul::Velocity>& velocity) override
+    {
+        if (impl) impl->on_reference_velocity_updated(velocity);
+    }
+
+    void on_reference_heading_updated(const cul::Update<cul::Heading>& heading) override
+    {
+        if (impl) impl->on_reference_heading_updated(heading);
+    }
+
+    void start_position_updates() override
+    {
+        if (impl) impl->state_controller()->start_position_updates();
+    }
+
+    void stop_position_updates() override
+    {
+        if (impl) impl->state_controller()->stop_position_updates();
+    }
+
+    void start_heading_updates() override
+    {
+        if (impl) impl->state_controller()->start_heading_updates();
+    }
+
+    void stop_heading_updates() override
+    {
+        if (impl) impl->state_controller()->stop_heading_updates();
+    }
+
+    void start_velocity_updates() override
+    {
+        if (impl) impl->state_controller()->start_velocity_updates();
+    }
+
+    void stop_velocity_updates() override
+    {
+        if (impl) impl->state_controller()->stop_velocity_updates();
+    }
+
+    BootState boot_state()
+    {
+        std::lock_guard<std::mutex> guard(state_mutex);
+        return state;
+    }
+
+    void set_boot_state(BootState new_state)
+    {
+        LOG(INFO) << "Setting boot state.";
+        std::lock_guard<std::mutex> guard(state_mutex);
+        state = new_state;
+        LOG(INFO) << "State was set.";
+    }
+
+private:
+    core::dbus::DBus dbus;
+    std::unique_ptr<core::dbus::ServiceWatcher> service_watcher;
+    core::ScopedConnection service_registered;
+    cul::Provider::Ptr impl;
+    cul::Provider::BootState state = cul::Provider::BootState::BOOTING;
+    std::mutex state_mutex;
+};
+
 struct Runtime
 {
     static Runtime& instance()
@@ -191,7 +313,20 @@
     auto service = dbus::Service::use_service(bus, name);
     auto object = service->object_for_path(path);
 
-    return create_instance_with_config(remote::stub::Configuration{object});
+    try
+    {
+        auto result = create_instance_with_config(remote::stub::Configuration{object});
+    } catch (const std::runtime_error& e)
+    {
+        std::cerr << "Error creating remote provider, creating facade for watching the service and delayed initialization" << std::endl;
+        std::cerr << e.what() << std::endl;
+    }
+
+    return std::make_shared<NotYet>(bus, name, [bus, name, path]()
+    {
+         auto object = dbus::Service::use_service(bus, name)->object_for_path(path);
+         return create_instance_with_config(remote::stub::Configuration{object});
+    });
 }
 
 cul::Provider::Ptr remote::Provider::Stub::create_instance_with_config(const remote::stub::Configuration& config)

=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.h'
--- src/location_service/com/ubuntu/location/providers/remote/provider.h	2015-01-26 19:11:46 +0000
+++ src/location_service/com/ubuntu/location/providers/remote/provider.h	2015-10-27 20:50:44 +0000
@@ -40,7 +40,8 @@
 {
     class Stub : public com::ubuntu::location::Provider, public std::enable_shared_from_this<Stub>
     {
-    public:
+    public:        
+
         // For integration with the Provider factory.
         static std::string class_name();
 

=== modified file 'tests/mock_delayed_provider.h'
--- tests/mock_delayed_provider.h	2015-10-27 20:50:44 +0000
+++ tests/mock_delayed_provider.h	2015-10-27 20:50:44 +0000
@@ -63,7 +63,7 @@
     MOCK_METHOD0(stop_velocity_updates, void());
 
     MOCK_CONST_METHOD0(has_delayed_boot, bool());
-    MOCK_CONST_METHOD0(boot_state, BootState());
+    MOCK_METHOD0(boot_state, BootState());
     MOCK_CONST_METHOD0(boot, void());