[Merge] lp:~thomas-voss/location-service/expose-remote-provider-interface-as-part-of-dev-package into lp:location-service

Manuel de la Peña manuel.delapena at canonical.com
Mon Sep 15 15:54:37 UTC 2014


Review: Needs Fixing



Diff comments:

> === modified file 'include/location_service/com/ubuntu/location/codec.h'
> --- include/location_service/com/ubuntu/location/codec.h	2014-02-03 13:52:21 +0000
> +++ include/location_service/com/ubuntu/location/codec.h	2014-09-15 08:59:26 +0000
> @@ -21,6 +21,7 @@
>  #include <com/ubuntu/location/criteria.h>
>  #include <com/ubuntu/location/heading.h>
>  #include <com/ubuntu/location/position.h>
> +#include <com/ubuntu/location/provider.h>
>  #include <com/ubuntu/location/space_vehicle.h>
>  #include <com/ubuntu/location/update.h>
>  #include <com/ubuntu/location/velocity.h>
> @@ -341,6 +342,49 @@
>          Codec<com::ubuntu::location::Optional<HeadingAccuracy>>::decode_argument(reader, in.accuracy.heading);
>      }
>  };
> +
> +template<>
> +struct Codec<com::ubuntu::location::Provider::Features>
> +{
> +    static void encode_argument(Message::Writer& writer, const com::ubuntu::location::Provider::Features& in)
> +    {
> +        writer.push_int32(static_cast<std::int32_t>(in));
> +    }
> +
> +    static void decode_argument(Message::Reader& reader, com::ubuntu::location::Provider::Features& in)
> +    {
> +        in = static_cast<com::ubuntu::location::Provider::Features>(reader.pop_int32());
> +    }
> +};
> +
> +template<>
> +struct Codec<com::ubuntu::location::Provider::Requirements>
> +{
> +    static void encode_argument(Message::Writer& writer, const com::ubuntu::location::Provider::Requirements& in)
> +    {
> +        writer.push_int32(static_cast<std::int32_t>(in));
> +    }
> +
> +    static void decode_argument(Message::Reader& reader, com::ubuntu::location::Provider::Requirements& in)
> +    {
> +        in = static_cast<com::ubuntu::location::Provider::Requirements>(reader.pop_int32());
> +    }
> +};
> +
> +template<>
> +struct Codec<com::ubuntu::location::WifiAndCellIdReportingState>
> +{
> +    static void encode_argument(Message::Writer& writer, const com::ubuntu::location::WifiAndCellIdReportingState& in)
> +    {
> +        writer.push_int32(static_cast<std::int32_t>(in));
> +    }
> +
> +    static void decode_argument(Message::Reader& reader, com::ubuntu::location::WifiAndCellIdReportingState& in)
> +    {
> +        in = static_cast<com::ubuntu::location::WifiAndCellIdReportingState>(reader.pop_int32());
> +    }
> +};
> +
>  namespace helper
>  {
>  template<typename T>
> 
> === added directory 'include/location_service/com/ubuntu/location/providers/remote'
> === renamed file 'src/location_service/com/ubuntu/location/providers/remote/remote_interface.h' => 'include/location_service/com/ubuntu/location/providers/remote/interface.h'
> --- src/location_service/com/ubuntu/location/providers/remote/remote_interface.h	2014-09-15 08:59:26 +0000
> +++ include/location_service/com/ubuntu/location/providers/remote/interface.h	2014-09-15 08:59:26 +0000
> @@ -16,8 +16,8 @@
>   * Authored by: Manuel de la Pena <manuel.delapena at canonical.com>
>   */
>  
> -#ifndef CORE_UBUNTU_ESPOO_PROVIDER_P_H_
> -#define CORE_UBUNTU_ESPOO_PROVIDER_P_H_
> +#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_INTERFACE_H_
> +#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_INTERFACE_H_
>  
>  #include <core/dbus/macros.h>
>  #include <core/dbus/object.h>
> @@ -33,15 +33,17 @@
>  #include <com/ubuntu/location/position.h>
>  #include <com/ubuntu/location/velocity.h>
>  
> -namespace cul = com::ubuntu::location;
> -
>  namespace com
>  {
>  namespace ubuntu
>  {
> +namespace location
> +{
> +namespace providers
> +{
>  namespace remote
>  {
> -struct RemoteInterface
> +struct Interface
>  {
>      static const std::string& name()
>      {
> @@ -49,32 +51,47 @@
>          return s;
>      }
>  
> -    DBUS_CPP_METHOD_DEF(StartPositionUpdates, RemoteInterface)
> -    DBUS_CPP_METHOD_DEF(StopPositionUpdates, RemoteInterface)
> -    DBUS_CPP_METHOD_DEF(StartHeadingUpdates, RemoteInterface)
> -    DBUS_CPP_METHOD_DEF(StopHeadingUpdates, RemoteInterface)
> -    DBUS_CPP_METHOD_DEF(StartVelocityUpdates, RemoteInterface)
> -    DBUS_CPP_METHOD_DEF(StopVelocityUpdates, RemoteInterface)
> +    // Checks if a provider satisfies a set of accuracy criteria.
> +    DBUS_CPP_METHOD_DEF(MatchesCriteria, remote::Interface)
> +    // Checks if the provider has got a specific requirement.
> +    DBUS_CPP_METHOD_DEF(Requires, remote::Interface)
> +    // Checks if the provider supports a specific feature.
> +    DBUS_CPP_METHOD_DEF(Supports, remote::Interface)
> +    // Called by the engine whenever the wifi and cell ID reporting state changes.
> +    DBUS_CPP_METHOD_DEF(OnWifiAndCellIdReportingStateChanged, remote::Interface)
> +    // Called by the engine whenever the reference location changed.
> +    DBUS_CPP_METHOD_DEF(OnReferenceLocationChanged, remote::Interface)
> +    // Called by the engine whenever the reference heading changed.
> +    DBUS_CPP_METHOD_DEF(OnReferenceHeadingChanged, remote::Interface)
> +    // Called by the engine whenever the reference velocity changed.
> +    DBUS_CPP_METHOD_DEF(OnReferenceVelocityChanged, remote::Interface)
> +
> +    DBUS_CPP_METHOD_DEF(StartPositionUpdates, remote::Interface)
> +    DBUS_CPP_METHOD_DEF(StopPositionUpdates, remote::Interface)
> +    DBUS_CPP_METHOD_DEF(StartHeadingUpdates, remote::Interface)
> +    DBUS_CPP_METHOD_DEF(StopHeadingUpdates, remote::Interface)
> +    DBUS_CPP_METHOD_DEF(StartVelocityUpdates, remote::Interface)
> +    DBUS_CPP_METHOD_DEF(StopVelocityUpdates, remote::Interface)
>  
>      struct Signals
>      {
> -        DBUS_CPP_SIGNAL_DEF(PositionChanged, RemoteInterface, cul::Position)
> -        DBUS_CPP_SIGNAL_DEF(HeadingChanged, RemoteInterface, cul::Heading)
> -        DBUS_CPP_SIGNAL_DEF(VelocityChanged, RemoteInterface, cul::Velocity)
> +        DBUS_CPP_SIGNAL_DEF(PositionChanged, remote::Interface, com::ubuntu::location::Position)
> +        DBUS_CPP_SIGNAL_DEF(HeadingChanged, remote::Interface, com::ubuntu::location::Heading)
> +        DBUS_CPP_SIGNAL_DEF(VelocityChanged, remote::Interface, com::ubuntu::location::Velocity)
>      };
>  
>      struct Properties
>      {
> -        DBUS_CPP_READABLE_PROPERTY_DEF(HasPosition, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(HasVelocity, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(HasHeading, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresSatellites, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresCellNetwork, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresDataNetwork, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresMonetarySpending, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(ArePositionUpdatesRunning, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(AreHeadingUpdatesRunning, RemoteInterface, bool)
> -        DBUS_CPP_READABLE_PROPERTY_DEF(AreVelocityUpdatesRunning, RemoteInterface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(HasPosition, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(HasVelocity, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(HasHeading, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresSatellites, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresCellNetwork, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresDataNetwork, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(RequiresMonetarySpending, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(ArePositionUpdatesRunning, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(AreHeadingUpdatesRunning, remote::Interface, bool)
> +        DBUS_CPP_READABLE_PROPERTY_DEF(AreVelocityUpdatesRunning, remote::Interface, bool)
>      };
>  
>      struct Skeleton
> @@ -206,8 +223,9 @@
>      };
>  
>  };
> -} // remote
> -} // ubuntu
> -}  // core
> -
> -#endif
> +}
> +}
> +}
> +}
> +}
> +#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_INTERFACE_H_
> 
> === added file 'include/location_service/com/ubuntu/location/providers/remote/skeleton.h'
> --- include/location_service/com/ubuntu/location/providers/remote/skeleton.h	1970-01-01 00:00:00 +0000
> +++ include/location_service/com/ubuntu/location/providers/remote/skeleton.h	2014-09-15 08:59:26 +0000
> @@ -0,0 +1,59 @@
> +/*
> + * Copyright © 2014 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ß <thomas.voss at canonical.com>
> + */
> +#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_SKELETON_H_
> +#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_SKELETON_H_
> +
> +#include <com/ubuntu/location/provider.h>
> +
> +namespace core { namespace dbus {
> +class Bus;
> +class Object;
> +}}
> +
> +namespace com
> +{
> +namespace ubuntu
> +{
> +namespace location
> +{
> +namespace providers
> +{
> +namespace remote
> +{
> +namespace skeleton
> +{
> +/** @brief All creation time arguments go here. */
> +struct Configuration
> +{
> +    /** @brief Remote object that should implement remote::Interface. */
> +    std::shared_ptr<core::dbus::Object> object;
> +    /** @brief The bus connection for handling incoming requests. */
> +    std::shared_ptr<core::dbus::Bus> bus;
> +    /** @brief The actual provider implementation. */
> +    Provider::Ptr provider;
> +};
> +
> +/** @brief Create a stub instance referring to a remote provider instance. */
> +Provider::Ptr create_with_configuration(const Configuration& configuration);
> +}
> +}
> +}
> +}
> +}
> +}
> +#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_SKELETON_H_
> 
> === added file 'include/location_service/com/ubuntu/location/providers/remote/stub.h'
> --- include/location_service/com/ubuntu/location/providers/remote/stub.h	1970-01-01 00:00:00 +0000
> +++ include/location_service/com/ubuntu/location/providers/remote/stub.h	2014-09-15 08:59:26 +0000
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright © 2014 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ß <thomas.voss at canonical.com>
> + */
> +#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_STUB_H_
> +#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_STUB_H_
> +
> +#include <com/ubuntu/location/provider.h>
> +
> +namespace core { namespace dbus {
> +class Bus;
> +class Object;
> +}}
> +
> +namespace com
> +{
> +namespace ubuntu
> +{
> +namespace location
> +{
> +namespace providers
> +{
> +namespace remote
> +{
> +namespace stub
> +{
> +/** @brief All creation time arguments go here. */
> +struct Configuration
> +{
> +    /** @brief Remote object implementing remote::Interface. */
> +    std::shared_ptr<core::dbus::Object> object;
> +};
> +
> +/** @brief Create a stub instance referring to a remote provider instance. */
> +Provider::Ptr create_with_configuration(const Configuration& configuration);
> +}
> +}
> +}
> +}
> +}
> +}
> +#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE_STUB_H_
> 
> === modified file 'src/location_service/com/ubuntu/location/CMakeLists.txt'
> --- src/location_service/com/ubuntu/location/CMakeLists.txt	2014-08-18 08:11:34 +0000
> +++ src/location_service/com/ubuntu/location/CMakeLists.txt	2014-09-15 08:59:26 +0000
> @@ -44,6 +44,10 @@
>  
>    providers/config.cpp
>  
> +  providers/remote/provider.cpp
> +  providers/remote/skeleton.cpp
> +  providers/remote/stub.cpp
> +
>    ${ICHNAEA_REPORTER_SRCS}
>  )
>  
> 
> === modified file 'src/location_service/com/ubuntu/location/providers/config.cpp'
> --- src/location_service/com/ubuntu/location/providers/config.cpp	2014-08-26 10:03:09 +0000
> +++ src/location_service/com/ubuntu/location/providers/config.cpp	2014-09-15 08:59:26 +0000
> @@ -41,6 +41,13 @@
>      com::ubuntu::location::providers::dummy::Provider::create_instance
>  };
>  
> +#include <com/ubuntu/location/providers/remote/provider.h>
> +static FactoryInjector remote_injector
> +{
> +    "remote::Provider",
> +    com::ubuntu::location::providers::remote::Provider::Stub::create_instance
> +};
> +
>  #if defined(COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_GEOCLUE)
>  #include <com/ubuntu/location/providers/geoclue/provider.h>
>  static FactoryInjector geoclue_injector
> @@ -67,12 +74,3 @@
>      com::ubuntu::location::providers::skyhook::Provider::create_instance
>  };
>  #endif // COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_SKYHOOK
> -
> -#if defined(COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE)
> -#include <com/ubuntu/location/providers/remote/provider.h>
> -static FactoryInjector remote_injector
> -{
> -    "remote::Provider", 
> -    com::ubuntu::location::providers::remote::Provider::create_instance
> -};
> -#endif // COM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE
> 
> === modified file 'src/location_service/com/ubuntu/location/providers/remote/CMakeLists.txt'
> --- src/location_service/com/ubuntu/location/providers/remote/CMakeLists.txt	2014-08-26 14:56:44 +0000
> +++ src/location_service/com/ubuntu/location/providers/remote/CMakeLists.txt	2014-09-15 08:59:26 +0000
> @@ -1,37 +1,13 @@
> -option(
> -    LOCATION_SERVICE_ENABLE_REMOTE_PROVIDER
> -    "Enable location provider relying on the remote provider SDK"
> -    ON
> +add_library(
> +    remote
> +    provider.h provider.cpp
> +    stub.cpp skeleton.cpp
>  )
>  
> -if (LOCATION_SERVICE_ENABLE_REMOTE_PROVIDER)
> -
> -    message(STATUS "Enabling support for the remote location providers")
> -
> -    set(REMOTE_SOURCES
> -    	provider.cpp
> -    )
> -
> -    set(REMOTE_HEADERS
> -    	remote_interface.h
> -    	provider.h
> -    )
> -
> -    add_library(remote
> -    	${REMOTE_HEADERS}
> -    	${REMOTE_SOURCES}
> -    )
> -
> -    set(
> -            ENABLED_PROVIDER_TARGETS
> -            ${ENABLED_PROVIDER_TARGETS} remote
> -            PARENT_SCOPE
> -    )
> -
> -   set(
> -            ENABLED_PROVIDER_TARGETS_DEFINITIONS
> -            -DCOM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE ${ENABLED_PROVIDER_TARGETS_DEFINITIONS}
> -            PARENT_SCOPE
> -   )
> -
> -endif (LOCATION_SERVICE_ENABLE_REMOTE_PROVIDER)
> +#set(ENABLED_PROVIDER_TARGETS
> +#    ${ENABLED_PROVIDER_TARGETS} remote
> +#    PARENT_SCOPE)
> +
> +#set(ENABLED_PROVIDER_TARGETS_DEFINITIONS
> +#    -DCOM_UBUNTU_LOCATION_SERVICE_PROVIDERS_REMOTE ${ENABLED_PROVIDER_TARGETS_DEFINITIONS}
> +#    PARENT_SCOPE)

This are commented out, is there a reason?

> 
> === modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.cpp'
> --- src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2014-09-15 08:59:26 +0000
> +++ src/location_service/com/ubuntu/location/providers/remote/provider.cpp	2014-09-15 08:59:26 +0000
> @@ -15,8 +15,11 @@
>   *
>   * Authored by: Manuel de la Peña <manuel.delapena at canonical.com>
>   */
> +
>  #include <com/ubuntu/location/providers/remote/provider.h>
>  
> +#include <com/ubuntu/location/providers/remote/interface.h>
> +
>  #include <com/ubuntu/location/logging.h>
>  
>  #include <core/dbus/object.h>
> @@ -26,19 +29,28 @@
>  #include <thread>
>  
>  namespace cul = com::ubuntu::location;
> -namespace culpr = com::ubuntu::location::providers::remote;
> -namespace cur = com::ubuntu::remote;
> +namespace remote = com::ubuntu::location::providers::remote;
> +
>  namespace dbus = core::dbus;
>  
>  namespace
>  {
> +void throw_if_error(const dbus::Result<void>& result)
> +{
> +    if (result.is_error()) throw std::runtime_error
> +    {
> +        result.error().print()
> +    };
> +}
> +
>  template<typename T>
> -void throw_if_error(const dbus::Result<T>& result)
> +T throw_if_error_or_return(const dbus::Result<T>& result)
>  {
>      if (result.is_error()) throw std::runtime_error
>      {
>          result.error().print()
>      };
> +    return result.value();
>  }
>  
>  dbus::Bus::Ptr the_system_bus()
> @@ -49,115 +61,397 @@
>  }
>  }
>  
> -struct culpr::Provider::Private
> +struct remote::Provider::Stub::Private
>  {
> -    Private(const culpr::Provider::Configuration& config)
> -            : bus(config.connection),
> -              service(dbus::Service::use_service(bus, config.name)),
> -              object(service->object_for_path(config.path)),
> -              stub(object),
> -              worker([this]() { bus->run(); })
> +    Private(const remote::stub::Configuration& config)
> +            : object(config.object),
> +              stub(object)
>      {
>      }
>  
>      ~Private()
>      {
> -        bus->stop();
> -
> -        if (worker.joinable())
> -            worker.join();
>      }
>  
> -    dbus::Bus::Ptr bus;
> -    dbus::Service::Ptr service;
>      dbus::Object::Ptr object;
> -
> -    com::ubuntu::remote::RemoteInterface::Stub stub;
> -
> -    std::thread worker;
> +    remote::Interface::Stub stub;
>  };
>  
> -std::string culpr::Provider::class_name()
> +std::string remote::Provider::Stub::class_name()
>  {
>      return "remote::Provider";
>  }
>  
> -cul::Provider::Ptr culpr::Provider::create_instance(const cul::ProviderFactory::Configuration& config)
> +cul::Provider::Ptr remote::Provider::Stub::create_instance(const cul::ProviderFactory::Configuration& config)
>  {
> -    culpr::Provider::Configuration pConfig;
> -    pConfig.name = config.count(Configuration::key_name()) > 0 ? config.get<std::string>(Configuration::key_name()) : throw std::runtime_error("Missing bus-name");
> -    pConfig.path = config.count(Configuration::key_path()) > 0 ? config.get<std::string>(Configuration::key_path()) : throw std::runtime_error("Missing bus-path");
> -
> -    pConfig.connection = the_system_bus();
> -
> -    return cul::Provider::Ptr{new culpr::Provider{pConfig}};
> +    auto name = config.count(Stub::key_name) > 0 ? config.get<std::string>(Stub::key_name) :
> +                                                   throw std::runtime_error("Missing bus-name");
> +    auto path = config.count(Stub::key_path) > 0 ? config.get<std::string>(Stub::key_path) :
> +                                                   throw std::runtime_error("Missing bus-path");
> +
> +    auto bus = the_system_bus();
> +    auto service = dbus::Service::use_service(bus, name);
> +    auto object = service->object_for_path(path);
> +
> +    return cul::Provider::Ptr{new remote::Provider::Stub{remote::stub::Configuration{object}}};
>  }
>  
> -culpr::Provider::Provider(const culpr::Provider::Configuration& config)
> -        : com::ubuntu::location::Provider(config.features, config.requirements),
> +remote::Provider::Stub::Stub(const remote::stub::Configuration& config)
> +        : com::ubuntu::location::Provider(/* TODO(tvoss) Features should be all initially*/),
>            d(new Private(config))
>  {
>      d->stub.signals.position_changed->connect(
> -        [this](const cur::RemoteInterface::Signals::PositionChanged::ArgumentType& arg)
> +        [this](const remote::Interface::Signals::PositionChanged::ArgumentType& arg)
>          {
> -            VLOG(50) << "culpr::Provider::PositionChanged: " << arg;
> +            VLOG(50) << "remote::Provider::Stub::PositionChanged: " << arg;
>              mutable_updates().position(arg);
>          });
>      d->stub.signals.heading_changed->connect(
> -        [this](const cur::RemoteInterface::Signals::HeadingChanged::ArgumentType& arg)
> +        [this](const remote::Interface::Signals::HeadingChanged::ArgumentType& arg)
>          {
> -            VLOG(50) << "culpr::Provider::HeadingChanged: " << arg;
> +            VLOG(50) << "remote::Provider::Stub::HeadingChanged: " << arg;
>              mutable_updates().heading(arg);
>          });
>      d->stub.signals.velocity_changed->connect(
> -        [this](const cur::RemoteInterface::Signals::VelocityChanged::ArgumentType& arg)
> +        [this](const remote::Interface::Signals::VelocityChanged::ArgumentType& arg)
>          {
> -            VLOG(50) << "culpr::Provider::VelocityChanged: " << arg;
> +            VLOG(50) << "remote::Provider::Stub::VelocityChanged: " << arg;
>              mutable_updates().velocity(arg);
>          });
>  }
>  
> -culpr::Provider::~Provider() noexcept
> -{
> -}
> -
> -bool culpr::Provider::matches_criteria(const cul::Criteria&)
> -{
> -    return true;
> -}
> -
> -void culpr::Provider::start_position_updates()
> -{
> -    VLOG(10) << __PRETTY_FUNCTION__;
> -    throw_if_error(d->stub.object->transact_method<cur::RemoteInterface::StartPositionUpdates, void>());
> -}
> -
> -void culpr::Provider::stop_position_updates()
> -{
> -    VLOG(10) << __PRETTY_FUNCTION__;
> -    throw_if_error(d->stub.object->transact_method<cur::RemoteInterface::StopPositionUpdates, void>());
> -}
> -
> -void culpr::Provider::start_heading_updates()
> -{
> -    VLOG(10) << __PRETTY_FUNCTION__;
> -    throw_if_error(d->stub.object->transact_method<cur::RemoteInterface::StartHeadingUpdates, void>());
> -}
> -
> -void culpr::Provider::stop_heading_updates()
> -{
> -    VLOG(10) << __PRETTY_FUNCTION__;
> -    throw_if_error(d->stub.object->transact_method<cur::RemoteInterface::StopHeadingUpdates, void>());
> -}
> -
> -void culpr::Provider::start_velocity_updates()
> -{
> -    VLOG(10) << __PRETTY_FUNCTION__;
> -    throw_if_error(d->stub.object->transact_method<cur::RemoteInterface::StartVelocityUpdates, void>());
> -}
> -
> -void culpr::Provider::stop_velocity_updates()
> -{
> -    VLOG(10) << __PRETTY_FUNCTION__;
> -    throw_if_error(d->stub.object->transact_method<cur::RemoteInterface::StopVelocityUpdates, void>());
> +remote::Provider::Stub::~Stub() noexcept
> +{
> +}
> +
> +bool remote::Provider::Stub::matches_criteria(const cul::Criteria& criteria)
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__ << std::endl;
> +    return throw_if_error_or_return(d->stub.object->transact_method<remote::Interface::MatchesCriteria, bool>(criteria));
> +}
> +
> +bool remote::Provider::Stub::supports(const cul::Provider::Features& f) const
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    return throw_if_error_or_return(d->stub.object->transact_method<remote::Interface::Supports, bool>(f));
> +}
> +
> +bool remote::Provider::Stub::requires(const cul::Provider::Requirements& r) const
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    return throw_if_error_or_return(d->stub.object->transact_method<remote::Interface::Requires, bool>(r));
> +}
> +
> +void remote::Provider::Stub::on_wifi_and_cell_reporting_state_changed(cul::WifiAndCellIdReportingState state)
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::OnWifiAndCellIdReportingStateChanged, void>(state));
> +}
> +
> +void remote::Provider::Stub::on_reference_location_updated(const cul::Update<cul::Position>& position)
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::OnReferenceLocationChanged, void>(position));
> +}
> +
> +void remote::Provider::Stub::on_reference_velocity_updated(const cul::Update<cul::Velocity>& velocity)
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::OnReferenceVelocityChanged, void>(velocity));
> +}
> +
> +void remote::Provider::Stub::on_reference_heading_updated(const cul::Update<cul::Heading>& heading)
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::OnReferenceHeadingChanged, void>(heading));
> +}
> +
> +void remote::Provider::Stub::start_position_updates()
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::StartPositionUpdates, void>());
> +}
> +
> +void remote::Provider::Stub::stop_position_updates()
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::StopPositionUpdates, void>());
> +}
> +
> +void remote::Provider::Stub::start_heading_updates()
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::StartHeadingUpdates, void>());
> +}
> +
> +void remote::Provider::Stub::stop_heading_updates()
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::StopHeadingUpdates, void>());
> +}
> +
> +void remote::Provider::Stub::start_velocity_updates()
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::StartVelocityUpdates, void>());
> +}
> +
> +void remote::Provider::Stub::stop_velocity_updates()
> +{
> +    VLOG(10) << __PRETTY_FUNCTION__;
> +    throw_if_error(d->stub.object->transact_method<remote::Interface::StopVelocityUpdates, void>());
> +}
> +
> +struct remote::Provider::Skeleton::Private
> +{
> +    Private(const remote::skeleton::Configuration& config)
> +            : bus(config.bus),
> +              skeleton(config.object),
> +              impl(config.provider),
> +              connections
> +              {
> +                  impl->updates().position.connect([this](const cul::Update<cul::Position>& position)
> +                  {
> +                      skeleton.signals.position_changed->emit(position.value);
> +                  }),
> +                  impl->updates().heading.connect([this](const cul::Update<cul::Heading>& heading)
> +                  {
> +                      skeleton.signals.heading_changed->emit(heading.value);
> +                  }),
> +                  impl->updates().velocity.connect([this](const cul::Update<cul::Velocity>& velocity)
> +                  {
> +                      skeleton.signals.velocity_changed->emit(velocity.value);
> +                  })
> +              }
> +    {
> +    }
> +
> +    core::dbus::Bus::Ptr bus;
> +    remote::Interface::Skeleton skeleton;
> +    cul::Provider::Ptr impl;
> +
> +    // All connections to signals go here.
> +    struct
> +    {
> +        core::ScopedConnection position_changed;
> +        core::ScopedConnection heading_changed;
> +        core::ScopedConnection velocity_changed;
> +    } connections;
> +};
> +
> +remote::Provider::Skeleton::Skeleton(const remote::skeleton::Configuration& config)
> +    : cul::Provider(),
> +      d(new Private(config))
> +{
> +    // Wire up to updates.
> +    d->impl->updates().position.connect([this](const cul::Update<cul::Position>& position)
> +    {
> +        d->skeleton.signals.position_changed->emit(position.value);
> +    });
> +
> +    d->impl->updates().heading.connect([this](const cul::Update<cul::Heading>& heading)
> +    {
> +        d->skeleton.signals.heading_changed->emit(heading.value);
> +    });
> +
> +    d->impl->updates().velocity.connect([this](const cul::Update<cul::Velocity>& velocity)
> +    {
> +        d->skeleton.signals.velocity_changed->emit(velocity.value);
> +    });
> +
> +    // And install method handlers.
> +    d->skeleton.object->install_method_handler<remote::Interface::MatchesCriteria>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "MatchesCriteria";
> +
> +        cul::Criteria criteria; msg->reader() >> criteria;
> +        auto reply = dbus::Message::make_method_return(msg);
> +        reply->writer() << matches_criteria(criteria);
> +
> +        d->bus->send(reply);
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::Supports>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "Supports";
> +
> +        cul::Provider::Features f; msg->reader() >> f;
> +        auto reply = dbus::Message::make_method_return(msg);
> +        reply->writer() << supports(f);
> +
> +        d->bus->send(reply);
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::Requires>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "Requires";
> +
> +        cul::Provider::Requirements r; msg->reader() >> r;
> +        auto reply = dbus::Message::make_method_return(msg);
> +        reply->writer() << requires(r);
> +
> +        d->bus->send(reply);
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::OnWifiAndCellIdReportingStateChanged>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "OnWifiAndCellIdReportingStateChanged";
> +
> +        cul::WifiAndCellIdReportingState s; msg->reader() >> s;
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +
> +        on_wifi_and_cell_reporting_state_changed(s);
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::OnReferenceLocationChanged>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "OnReferenceLocationChanged";
> +
> +        cul::Update<cul::Position> u; msg->reader() >> u;
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +
> +        on_reference_location_updated(u);
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::OnReferenceHeadingChanged>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "OnReferenceHeadingChanged";
> +
> +        cul::Update<cul::Heading> u; msg->reader() >> u;
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +
> +        on_reference_heading_updated(u);
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::OnReferenceVelocityChanged>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "OnReferenceVelocityChanged";
> +
> +        cul::Update<cul::Velocity> u; msg->reader() >> u;
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +
> +        on_reference_velocity_updated(u);
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::StartPositionUpdates>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "StartPositionUpdates";
> +        start_position_updates();
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::StopPositionUpdates>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "StopPositionUpdates";
> +        stop_position_updates();
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::StartHeadingUpdates>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "StartHeadingUpdates";
> +        start_heading_updates();
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::StopHeadingUpdates>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "StopHeadingUpdates";
> +        stop_heading_updates();
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::StartVelocityUpdates>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "StartVelocityUpdates";
> +        start_velocity_updates();
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +    });
> +
> +    d->skeleton.object->install_method_handler<remote::Interface::StopVelocityUpdates>([this](const dbus::Message::Ptr & msg)
> +    {
> +        VLOG(1) << "StartVelocityUpdates";
> +        stop_velocity_updates();
> +        d->bus->send(dbus::Message::make_method_return(msg));
> +    });
> +}
> +
> +remote::Provider::Skeleton::~Skeleton() noexcept
> +{
> +    d->skeleton.object->uninstall_method_handler<remote::Interface::MatchesCriteria>();
> +
> +    d->skeleton.object->uninstall_method_handler<remote::Interface::StartPositionUpdates>();
> +    d->skeleton.object->uninstall_method_handler<remote::Interface::StopPositionUpdates>();
> +
> +    d->skeleton.object->uninstall_method_handler<remote::Interface::StartHeadingUpdates>();
> +    d->skeleton.object->uninstall_method_handler<remote::Interface::StopHeadingUpdates>();
> +
> +    d->skeleton.object->uninstall_method_handler<remote::Interface::StartVelocityUpdates>();
> +    d->skeleton.object->uninstall_method_handler<remote::Interface::StopVelocityUpdates>();
> +}
> +
> +// We just forward calls to the actual implementation
> +bool remote::Provider::Skeleton::matches_criteria(const cul::Criteria& criteria)
> +{
> +    return d->impl->matches_criteria(criteria);
> +}
> +
> +bool remote::Provider::Skeleton::supports(const cul::Provider::Features& f) const
> +{
> +    return d->impl->supports(f);
> +}
> +
> +bool remote::Provider::Skeleton::requires(const cul::Provider::Requirements& r) const
> +{
> +    return d->impl->requires(r);
> +}
> +
> +void remote::Provider::Skeleton::on_wifi_and_cell_reporting_state_changed(cul::WifiAndCellIdReportingState state)
> +{
> +    d->impl->on_wifi_and_cell_reporting_state_changed(state);
> +}
> +
> +void remote::Provider::Skeleton::on_reference_location_updated(const cul::Update<cul::Position>& position)
> +{
> +    d->impl->on_reference_location_updated(position);
> +}
> +
> +void remote::Provider::Skeleton::on_reference_velocity_updated(const cul::Update<cul::Velocity>& velocity)
> +{
> +    d->impl->on_reference_velocity_updated(velocity);
> +}
> +
> +void remote::Provider::Skeleton::on_reference_heading_updated(const cul::Update<cul::Heading>& heading)
> +{
> +    d->impl->on_reference_heading_updated(heading);
> +}
> +
> +void remote::Provider::Skeleton::start_position_updates()
> +{
> +    d->impl->state_controller()->start_position_updates();
> +}
> +
> +void remote::Provider::Skeleton::stop_position_updates()
> +{
> +    d->impl->state_controller()->stop_position_updates();
> +}
> +
> +void remote::Provider::Skeleton::start_heading_updates()
> +{
> +    d->impl->state_controller()->start_heading_updates();
> +}
> +
> +void remote::Provider::Skeleton::stop_heading_updates()
> +{
> +    d->impl->state_controller()->stop_heading_updates();
> +}
> +
> +void remote::Provider::Skeleton::start_velocity_updates()
> +{
> +    d->impl->state_controller()->start_velocity_updates();
> +}
> +
> +void remote::Provider::Skeleton::stop_velocity_updates()
> +{
> +    d->impl->state_controller()->stop_velocity_updates();
>  }
> 
> === modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.h'
> --- src/location_service/com/ubuntu/location/providers/remote/provider.h	2014-09-15 08:59:26 +0000
> +++ src/location_service/com/ubuntu/location/providers/remote/provider.h	2014-09-15 08:59:26 +0000
> @@ -15,12 +15,14 @@
>   *
>   * Authored by: Manuel de la Peña <manuel.delapena at canonical.com>
>   */
> -#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_ESPOO_PROVIDER_H_
> -#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_ESPOO_PROVIDER_H_
> +#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_REMOTE_PROVIDER_H_
> +#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_REMOTE_PROVIDER_H_
>  
>  #include <com/ubuntu/location/provider.h>
>  #include <com/ubuntu/location/provider_factory.h>
> -#include <com/ubuntu/location/providers/remote/remote_interface.h>
> +
> +#include <com/ubuntu/location/providers/remote/skeleton.h>
> +#include <com/ubuntu/location/providers/remote/stub.h>
>  
>  #include <core/dbus/bus.h>
>  
> @@ -34,55 +36,82 @@
>  {
>  namespace remote
>  {
> -
> -class Provider : public com::ubuntu::location::Provider
> +struct Provider
>  {
> -  public:
> -    // For integration with the Provider factory.
> -    static std::string class_name();
> -
> -    // Instantiates a new provider instance, populating the configuration object
> -    // from the provided property bundle.
> -    static Provider::Ptr create_instance(const ProviderFactory::Configuration&);
> -
> -    // structure that represents the configuration used in the remote provider
> -    struct Configuration
> -    {
> -        static std::string key_name() { return "name"; }
> -        static std::string key_path() { return "path"; }
> -
> -        std::string name;
> -        std::string path;
> -
> -        core::dbus::Bus::Ptr connection;
> -
> -        Provider::Features features = Provider::Features::position;
> -        Provider::Requirements requirements = Provider::Requirements::cell_network |
> -            Provider::Requirements::data_network | Provider::Requirements::monetary_spending;
> -    };
> -
> -    Provider(const Configuration& config);
> -    ~Provider() noexcept;
> -
> -    virtual bool matches_criteria(const Criteria&);
> -
> -    virtual void start_position_updates() override;
> -    virtual void stop_position_updates() override;
> -
> -    virtual void start_heading_updates() override;
> -    virtual void stop_heading_updates() override;
> -
> -    virtual void start_velocity_updates() override;
> -    virtual void stop_velocity_updates() override;
> -
> -  private:
> -    struct Private;
> -    std::unique_ptr<Private> d;
> +    class Stub : public com::ubuntu::location::Provider
> +    {
> +    public:
> +        // For integration with the Provider factory.
> +        static std::string class_name();
> +
> +        // Instantiates a new provider instance, populating the configuration object
> +        // from the provided property bundle.
> +        static Provider::Ptr create_instance(const ProviderFactory::Configuration&);
> +
> +        // Name of the command line parameter for passing in the remote service name.
> +        static constexpr const char* key_name{"name"};
> +        // Name of the command line parameter for passing in the path of the remote provider impl.
> +        static constexpr const char* key_path{"path"};
> +
> +        Stub(const stub::Configuration& config);
> +        ~Stub() noexcept;
> +
> +        virtual bool matches_criteria(const Criteria&) override;
> +        virtual bool supports(const Features& f) const override;
> +        virtual bool requires(const Requirements& r) const override;
> +
> +        virtual void on_wifi_and_cell_reporting_state_changed(WifiAndCellIdReportingState state) override;
> +        virtual void on_reference_location_updated(const Update<Position>& position) override;
> +        virtual void on_reference_velocity_updated(const Update<Velocity>& velocity) override;
> +        virtual void on_reference_heading_updated(const Update<Heading>& heading) override;
> +
> +        virtual void start_position_updates() override;
> +        virtual void stop_position_updates() override;
> +
> +        virtual void start_heading_updates() override;
> +        virtual void stop_heading_updates() override;
> +
> +        virtual void start_velocity_updates() override;
> +        virtual void stop_velocity_updates() override;
> +
> +    private:
> +        struct Private;
> +        std::unique_ptr<Private> d;
> +    };
> +
> +    class Skeleton : public com::ubuntu::location::Provider
> +    {
> +    public:
> +        Skeleton(const remote::skeleton::Configuration& config);
> +        ~Skeleton() noexcept;
> +
> +        virtual bool matches_criteria(const Criteria&) override;
> +
> +        virtual bool supports(const Features& f) const override;
> +        virtual bool requires(const Requirements& r) const override;
> +
> +        virtual void on_wifi_and_cell_reporting_state_changed(WifiAndCellIdReportingState state) override;
> +        virtual void on_reference_location_updated(const Update<Position>& position) override;
> +        virtual void on_reference_velocity_updated(const Update<Velocity>& velocity) override;
> +        virtual void on_reference_heading_updated(const Update<Heading>& heading) override;
> +
> +        virtual void start_position_updates() override;
> +        virtual void stop_position_updates() override;
> +
> +        virtual void start_heading_updates() override;
> +        virtual void stop_heading_updates() override;
> +
> +        virtual void start_velocity_updates() override;
> +        virtual void stop_velocity_updates() override;
> +
> +    private:
> +        struct Private;
> +        std::unique_ptr<Private> d;
> +    };
>  };
> -
> -}  // remote
> -}  // providers
> -}  // location
> -}  // ubuntu
> -}  // com
> -#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_GEOCLUE_PROVIDER_H_
> +}
> +}
> +}
> +}
> +}
> +#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_REMOTE_PROVIDER_H_
> 
> === added file 'src/location_service/com/ubuntu/location/providers/remote/skeleton.cpp'
> --- src/location_service/com/ubuntu/location/providers/remote/skeleton.cpp	1970-01-01 00:00:00 +0000
> +++ src/location_service/com/ubuntu/location/providers/remote/skeleton.cpp	2014-09-15 08:59:26 +0000
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright © 2014 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <com/ubuntu/location/providers/remote/skeleton.h>
> +
> +#include "provider.h"
> +
> +namespace location = com::ubuntu::location;
> +namespace remote = com::ubuntu::location::providers::remote;
> +
> +location::Provider::Ptr remote::skeleton::create_with_configuration(const remote::skeleton::Configuration& configuration)
> +{
> +    return std::make_shared<remote::Provider::Skeleton>(configuration);
> +}
> 
> === added file 'src/location_service/com/ubuntu/location/providers/remote/stub.cpp'
> --- src/location_service/com/ubuntu/location/providers/remote/stub.cpp	1970-01-01 00:00:00 +0000
> +++ src/location_service/com/ubuntu/location/providers/remote/stub.cpp	2014-09-15 08:59:26 +0000
> @@ -0,0 +1,29 @@
> +/*
> + * Copyright © 2014 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ß <thomas.voss at canonical.com>
> + */
> +
> +#include <com/ubuntu/location/providers/remote/stub.h>
> +
> +#include "provider.h"
> +
> +namespace location = com::ubuntu::location;
> +namespace remote = com::ubuntu::location::providers::remote;
> +
> +location::Provider::Ptr remote::stub::create_with_configuration(const remote::stub::Configuration& configuration)
> +{
> +    return std::make_shared<remote::Provider::Stub>(configuration);
> +}
> 
> === modified file 'tests/CMakeLists.txt'
> --- tests/CMakeLists.txt	2014-09-15 08:59:26 +0000
> +++ tests/CMakeLists.txt	2014-09-15 08:59:26 +0000
> @@ -117,7 +117,5 @@
>    endif (LOCATION_SERVICE_ENABLE_DBUS_TEST_RUNNER)
>  endif (LOCATION_SERVICE_ENABLE_GEOCLUE_PROVIDERS)
>  
> -if (LOCATION_SERVICE_ENABLE_REMOTE_PROVIDER)
> -  LOCATION_SERVICE_ADD_TEST(remote_provider_test remote_provider_test.cpp)
> -  LOCATION_SERVICE_ADD_TEST(espoo_provider_test espoo_provider_test.cpp)
> -endif (LOCATION_SERVICE_ENABLE_REMOTE_PROVIDER)
> +LOCATION_SERVICE_ADD_TEST(remote_provider_test remote_provider_test.cpp)
> +LOCATION_SERVICE_ADD_TEST(espoo_provider_test espoo_provider_test.cpp)
> 
> === modified file 'tests/espoo_provider_test.cpp'
> --- tests/espoo_provider_test.cpp	2014-09-15 08:59:26 +0000
> +++ tests/espoo_provider_test.cpp	2014-09-15 08:59:26 +0000
> @@ -22,6 +22,7 @@
>  
>  #include <com/ubuntu/location/connectivity/manager.h>
>  
> +#include <core/dbus/service.h>
>  #include <core/dbus/asio/executor.h>
>  
>  #include <core/posix/signal.h>
> @@ -345,10 +346,16 @@
>          trap->stop();
>      });
>  
> -    remote::Provider::Configuration config;
> -    config.name = EspooProviderTest::service_name;
> -    config.path = EspooProviderTest::path;
> -    config.connection = bus;
> +    std::thread worker([this]()
> +    {
> +        bus->run();
> +    });
> +
> +    remote::stub::Configuration config;
> +    config.object = core::dbus::Service::use_service(
> +                bus,
> +                EspooProviderTest::service_name)->object_for_path(
> +                    core::dbus::types::ObjectPath{EspooProviderTest::path});
>  
>      // We keep track of some characteristics.
>      struct Stats
> @@ -359,7 +366,7 @@
>          Counter position_updates_counter{"Position updates"}; // The number of position updates we received.
>      } stats;
>  
> -    remote::Provider provider{config};
> +    remote::Provider::Stub provider{config};
>  
>      provider.updates().position.connect([&stats](const cul::Update<cul::Position>& update)
>      {
> @@ -376,6 +383,11 @@
>      trap->run();
>      // provider.stop_position_updates();
>  
> +    bus->stop();
> +
> +    if (worker.joinable())
> +        worker.join();
> +
>      // Finally printing some statistics
>      std::cout << "Total execution time: "
>                << std::chrono::duration_cast<std::chrono::seconds>(stats.execution_time.stop()).count() << " [s]" << std::endl;
> 
> === modified file 'tests/mock_provider.h'
> --- tests/mock_provider.h	2014-09-15 08:59:26 +0000
> +++ tests/mock_provider.h	2014-09-15 08:59:26 +0000
> @@ -28,6 +28,11 @@
>      {
>      }
>  
> +    MOCK_METHOD1(matches_criteria, bool(const com::ubuntu::location::Criteria&));
> +
> +    MOCK_CONST_METHOD1(supports, bool(const com::ubuntu::location::Provider::Features&));
> +    MOCK_CONST_METHOD1(requires, bool(const com::ubuntu::location::Provider::Requirements&));
> +
>      // Called by the engine whenever the wifi and cell ID reporting state changes.
>      MOCK_METHOD1(on_wifi_and_cell_reporting_state_changed, void(com::ubuntu::location::WifiAndCellIdReportingState state));
>  
> 
> === modified file 'tests/remote_provider_test.cpp'
> --- tests/remote_provider_test.cpp	2014-09-15 08:59:26 +0000
> +++ tests/remote_provider_test.cpp	2014-09-15 08:59:26 +0000
> @@ -18,11 +18,17 @@
>  
>  #include <com/ubuntu/location/logging.h>
>  #include <com/ubuntu/location/provider.h>
> +
> +#include <com/ubuntu/location/providers/remote/interface.h>
> +#include <com/ubuntu/location/providers/remote/skeleton.h>
> +#include <com/ubuntu/location/providers/remote/stub.h>
> +
>  #include <com/ubuntu/location/providers/remote/provider.h>
>  
>  #include "mock_provider.h"
>  
>  #include <core/dbus/fixture.h>
> +#include <core/dbus/service.h>
>  #include <core/dbus/asio/executor.h>
>  
>  #include <core/posix/fork.h>
> @@ -33,8 +39,9 @@
>  #include <gmock/gmock.h>
>  #include <gtest/gtest.h>
>  
> +#include <condition_variable>
> +
>  namespace cul = com::ubuntu::location;
> -namespace cur = com::ubuntu::remote;
>  namespace dbus = core::dbus;
>  namespace remote = com::ubuntu::location::providers::remote;
>  
> @@ -43,11 +50,7 @@
>  
>  MATCHER_P(PositionUpdatesAreEqualExceptForTiming, value, "Returns true if the positions in both updates are equal.")
>  {
> -    auto pos = arg.value;
> -
> -    return value.longitude == pos.longitude && value.latitude == pos.latitude && value.altitude == pos.altitude
> -        && pos.accuracy.horizontal == value.accuracy.horizontal
> -        && pos.accuracy.vertical == value.accuracy.vertical;
> +    return arg.value == value;
>  }
>  
>  MATCHER_P(HeadingUpdatesAreEqualExceptForTiming, value, "Returns true if the heading in both updates are equal.")
> @@ -230,83 +233,56 @@
>                              dbus::types::ObjectPath{RemoteProvider::stub_remote_provider_path});
>  
>          // We use this instance to capture incoming requests.
> -        NiceMock<MockProvider> mock_provider;
> -
> -        EXPECT_CALL(mock_provider, start_position_updates()).Times(1);
> -        EXPECT_CALL(mock_provider, start_heading_updates()).Times(1);
> -        EXPECT_CALL(mock_provider, start_velocity_updates()).Times(1);
> -        EXPECT_CALL(mock_provider, stop_position_updates()).Times(1);
> -        EXPECT_CALL(mock_provider, stop_heading_updates()).Times(1);
> -        EXPECT_CALL(mock_provider, stop_velocity_updates()).Times(1);
> -
> -        cur::RemoteInterface::Skeleton remote_provider{object};
> -
> -        remote_provider.object->install_method_handler<cur::RemoteInterface::StartPositionUpdates>([bus, &mock_provider](const dbus::Message::Ptr & msg)
> -        {
> -            VLOG(1) << "StartPositionUpdates";
> -            mock_provider.start_position_updates();
> -            bus->send(dbus::Message::make_method_return(msg));
> -        });
> -
> -        remote_provider.object->install_method_handler<cur::RemoteInterface::StopPositionUpdates>([bus, &mock_provider](const dbus::Message::Ptr & msg)
> -        {
> -            VLOG(1) << "StopPositionUpdates";
> -            mock_provider.stop_position_updates();
> -            bus->send(dbus::Message::make_method_return(msg));
> -        });
> -
> -        remote_provider.object->install_method_handler<cur::RemoteInterface::StartHeadingUpdates>([bus, &mock_provider](const dbus::Message::Ptr & msg)
> -        {
> -            VLOG(1) << "StartHeadingUpdates";
> -            mock_provider.start_heading_updates();
> -            bus->send(dbus::Message::make_method_return(msg));
> -        });
> -
> -        remote_provider.object->install_method_handler<cur::RemoteInterface::StopHeadingUpdates>([bus, &mock_provider](const dbus::Message::Ptr & msg)
> -        {
> -            VLOG(1) << "StopHeadingUpdates";
> -            mock_provider.stop_heading_updates();
> -            bus->send(dbus::Message::make_method_return(msg));
> -        });
> -
> -        remote_provider.object->install_method_handler<cur::RemoteInterface::StartVelocityUpdates>([bus, &mock_provider](const dbus::Message::Ptr & msg)
> -        {
> -            VLOG(1) << "StartVelocityUpdates";
> -            mock_provider.start_velocity_updates();
> -            bus->send(dbus::Message::make_method_return(msg));
> -        });
> -
> -        remote_provider.object->install_method_handler<cur::RemoteInterface::StopVelocityUpdates>([bus, &mock_provider](const dbus::Message::Ptr & msg)
> -        {
> -            VLOG(1) << "StartVelocityUpdates";
> -            mock_provider.stop_velocity_updates();
> -            bus->send(dbus::Message::make_method_return(msg));
> -        });
> -
> -        std::thread position_updates_injector{[&remote_provider, &running]()
> -        {
> -            while (running)
> -            {
> -                remote_provider.signals.position_changed->emit(position);
> -                std::this_thread::sleep_for(std::chrono::milliseconds{10});
> -            }
> -        }};
> -
> -        std::thread heading_updates_injector{[&remote_provider, &running]()
> -        {
> -            while (running)
> -            {
> -                remote_provider.signals.heading_changed->emit(heading);
> -                std::this_thread::sleep_for(std::chrono::milliseconds{10});
> -            }
> -        }};
> -
> -        std::thread velocity_updates_injector{[&remote_provider, &running]()
> -        {
> -            while (running)
> -            {
> -                remote_provider.signals.velocity_changed->emit(velocity);
> -                 std::this_thread::sleep_for(std::chrono::milliseconds{10});
> +        auto mock_provider = std::make_shared<NiceMock<MockProvider>>();
> +
> +        EXPECT_CALL(*mock_provider, matches_criteria(_)).Times(1).WillRepeatedly(Return(false));
> +
> +        EXPECT_CALL(*mock_provider, supports(_)).Times(4).WillRepeatedly(Return(true));
> +        EXPECT_CALL(*mock_provider, requires(_)).Times(5).WillRepeatedly(Return(true));
> +
> +        EXPECT_CALL(*mock_provider, on_wifi_and_cell_reporting_state_changed(_)).Times(1);
> +        EXPECT_CALL(*mock_provider, on_reference_location_updated(_)).Times(1);
> +        EXPECT_CALL(*mock_provider, on_reference_velocity_updated(_)).Times(1);
> +        EXPECT_CALL(*mock_provider, on_reference_heading_updated(_)).Times(1);
> +
> +        EXPECT_CALL(*mock_provider, start_position_updates()).Times(1);
> +        EXPECT_CALL(*mock_provider, start_heading_updates()).Times(1);
> +        EXPECT_CALL(*mock_provider, start_velocity_updates()).Times(1);
> +        EXPECT_CALL(*mock_provider, stop_position_updates()).Times(1);
> +        EXPECT_CALL(*mock_provider, stop_heading_updates()).Times(1);
> +        EXPECT_CALL(*mock_provider, stop_velocity_updates()).Times(1);
> +
> +        auto provider = remote::skeleton::create_with_configuration(remote::skeleton::Configuration
> +        {
> +            object,
> +            bus,
> +            mock_provider
> +        });
> +
> +        std::thread position_updates_injector{[mock_provider, &running]()
> +        {
> +            while (running)
> +            {
> +                mock_provider->inject_update(cul::Update<cul::Position>{position});
> +                std::this_thread::sleep_for(std::chrono::milliseconds{10});
> +            }
> +        }};
> +
> +        std::thread heading_updates_injector{[mock_provider, &running]()
> +        {
> +            while (running)
> +            {
> +                mock_provider->inject_update(cul::Update<cul::Heading>{heading});
> +                std::this_thread::sleep_for(std::chrono::milliseconds{10});
> +            }
> +        }};
> +
> +        std::thread velocity_updates_injector{[mock_provider, &running]()
> +        {
> +            while (running)
> +            {
> +                mock_provider->inject_update(cul::Update<cul::Velocity>{velocity});
> +                std::this_thread::sleep_for(std::chrono::milliseconds{10});
>              }
>          }};
>  
> @@ -334,16 +310,44 @@
>  
>      auto stub = core::posix::fork([this]()
>      {
> -        auto conf = remote::Provider::Configuration{};
> -        conf.name = RemoteProvider::stub_remote_provider_service_name;
> -        conf.path = RemoteProvider::stub_remote_provider_path;
> -        conf.connection = session_bus();
> -        conf.connection->install_executor(dbus::asio::make_executor(conf.connection));
> -
> -        remote::Provider provider{conf};
> -        provider.start_position_updates();
> -        provider.start_heading_updates();
> -        provider.start_velocity_updates();
> +        auto bus = session_bus();
> +        bus->install_executor(dbus::asio::make_executor(bus));
> +
> +        std::thread worker([bus]()
> +        {
> +            bus->run();
> +        });
> +
> +        remote::stub::Configuration conf
> +        {
> +            core::dbus::Service::use_service(
> +                bus,
> +                RemoteProvider::stub_remote_provider_service_name)->object_for_path(
> +                    core::dbus::types::ObjectPath{RemoteProvider::stub_remote_provider_path})
> +        };
> +
> +        auto provider = remote::stub::create_with_configuration(conf);
> +
> +        EXPECT_FALSE(provider->matches_criteria(cul::Criteria{}));
> +
> +        EXPECT_TRUE(provider->supports(cul::Provider::Features::none));
> +        EXPECT_TRUE(provider->supports(cul::Provider::Features::position));
> +        EXPECT_TRUE(provider->supports(cul::Provider::Features::heading));
> +        EXPECT_TRUE(provider->supports(cul::Provider::Features::velocity));
> +
> +        EXPECT_TRUE(provider->requires(cul::Provider::Requirements::cell_network));
> +        EXPECT_TRUE(provider->requires(cul::Provider::Requirements::data_network));
> +        EXPECT_TRUE(provider->requires(cul::Provider::Requirements::monetary_spending));
> +        EXPECT_TRUE(provider->requires(cul::Provider::Requirements::none));
> +        EXPECT_TRUE(provider->requires(cul::Provider::Requirements::satellites));
> +
> +        provider->on_wifi_and_cell_reporting_state_changed(cul::WifiAndCellIdReportingState::on);
> +        provider->on_reference_location_updated(cul::Update<cul::Position>{});
> +        provider->on_reference_heading_updated(cul::Update<cul::Heading>{});
> +        provider->on_reference_velocity_updated(cul::Update<cul::Velocity>{});
> +        provider->state_controller()->start_position_updates();
> +        provider->state_controller()->start_heading_updates();
> +        provider->state_controller()->start_velocity_updates();
>  
>          MockEventConsumer mec;
>          EXPECT_CALL(mec, on_new_position(PositionUpdatesAreEqualExceptForTiming(position))).Times(AtLeast(1));
> @@ -352,7 +356,7 @@
>  
>          core::ScopedConnection sc1
>          {
> -            provider.updates().position.connect([&mec](const cul::Update<cul::Position>& p)
> +            provider->updates().position.connect([&mec](const cul::Update<cul::Position>& p)
>              {
>                  mec.on_new_position(p);
>              })
> @@ -360,7 +364,7 @@
>  
>          core::ScopedConnection sc2
>          {
> -            provider.updates().heading.connect([&mec](const cul::Update<cul::Heading>& h)
> +            provider->updates().heading.connect([&mec](const cul::Update<cul::Heading>& h)
>              {
>                  mec.on_new_heading(h);
>              })
> @@ -368,7 +372,7 @@
>  
>          core::ScopedConnection sc3
>          {
> -            provider.updates().velocity.connect([&mec](const cul::Update<cul::Velocity>& v)
> +            provider->updates().velocity.connect([&mec](const cul::Update<cul::Velocity>& v)
>              {
>                  mec.on_new_velocity(v);
>              })
> @@ -378,9 +382,14 @@
>          EXPECT_TRUE(mec.wait_for_heading_update_for(std::chrono::milliseconds{1000}));
>          EXPECT_TRUE(mec.wait_for_velocity_update_for(std::chrono::milliseconds{1000}));
>  
> -        provider.stop_position_updates();
> -        provider.stop_heading_updates();
> -        provider.stop_velocity_updates();
> +        provider->state_controller()->stop_position_updates();
> +        provider->state_controller()->stop_heading_updates();
> +        provider->state_controller()->stop_velocity_updates();
> +
> +        bus->stop();
> +
> +        if (worker.joinable())
> +            worker.join();
>  
>          return ::testing::Test::HasFailure() ? core::posix::exit::Status::failure :
>                                                 core::posix::exit::Status::success;
> @@ -390,18 +399,3 @@
>      skeleton.send_signal_or_throw(core::posix::Signal::sig_term);
>      EXPECT_TRUE(did_finish_successfully(skeleton.wait_for(core::posix::wait::Flags::untraced)));
>  }
> -
> -TESTP_F(RemoteProvider, matches_criteria,
> -{
> -    auto conf = remote::Provider::Configuration{};
> -    conf.name = RemoteProvider::stub_remote_provider_service_name;
> -    conf.path = RemoteProvider::stub_remote_provider_path;
> -    conf.connection = session_bus();
> -    conf.connection->install_executor(dbus::asio::make_executor(conf.connection));
> -    remote::Provider provider(conf);
> -
> -    EXPECT_FALSE(provider.requires(com::ubuntu::location::Provider::Requirements::satellites));
> -    EXPECT_TRUE(provider.requires(com::ubuntu::location::Provider::Requirements::cell_network));
> -    EXPECT_TRUE(provider.requires(com::ubuntu::location::Provider::Requirements::data_network));
> -    EXPECT_TRUE(provider.requires(com::ubuntu::location::Provider::Requirements::monetary_spending));
> -})
> 


-- 
https://code.launchpad.net/~thomas-voss/location-service/expose-remote-provider-interface-as-part-of-dev-package/+merge/234464
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service.



More information about the Ubuntu-reviews mailing list