[Merge] lp:~thomas-voss/location-service/fix-1415029 into lp:location-service

[Loïc Minier]

Diff comments:

> === modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.cpp'
> --- src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2015-01-26 19:13:52 +0000
> +++ src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2015-01-27 14:28:08 +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>
>  
> @@ -39,6 +41,101 @@
>  
>  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([this, when_ready]()
> +              {
> +                  impl = when_ready();
> +              })
> +          }
> +    {
> +    }
> +
> +    bool matches_criteria(const cul::Criteria& criteria) override

This kind of lies about matches_criteria/supports/requires; perhaps it would be best to throw and catch that higher up the stack to retry the matches_criteria/supports/requires calls when the provider is avail?

> +    {
> +        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();
> +    }
> +
> +private:
> +    core::dbus::DBus dbus;
> +    std::unique_ptr<core::dbus::ServiceWatcher> service_watcher;
> +    core::ScopedConnection service_registered;
> +    cul::Provider::Ptr impl;
> +};
> +
>  struct Runtime
>  {
>      static Runtime& instance()
> @@ -190,7 +287,19 @@
>      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;
> +    }
> +
> +    return std::make_shared<NotYet>(bus, name, [bus, name, path]()

Shouldn't we simply use this code path all the time?

> +    {
> +         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-01-27 14:28:08 +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();
>  
> 


-- 
https://code.launchpad.net/~thomas-voss/location-service/fix-1415029/+merge/247712
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~thomas-voss/location-service/fix-1415029 into lp:location-service.