[Merge] lp:~mandel/location-service/remove-pimpl into lp:location-service
Thomas Voß
thomas.voss at canonical.com
Wed Apr 22 10:05:12 UTC 2015
Review: Needs Fixing
Diff comments:
> === removed directory 'include/location_service/com/ubuntu/location/providers/geoclue'
> === removed directory 'include/location_service/com/ubuntu/location/providers/skyhook'
> === renamed file 'include/location_service/com/ubuntu/location/providers/geoclue/geoclue.h' => 'src/location_service/com/ubuntu/location/providers/geoclue/geoclue.h'
> === modified file 'src/location_service/com/ubuntu/location/providers/geoclue/provider.cpp'
> --- src/location_service/com/ubuntu/location/providers/geoclue/provider.cpp 2014-02-03 13:52:21 +0000
> +++ src/location_service/com/ubuntu/location/providers/geoclue/provider.cpp 2015-04-22 08:43:39 +0000
> @@ -1,5 +1,5 @@
> /*
> - * Copyright © 2012-2013 Canonical Ltd.
> + * Copyright © 2012-2015 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,
> @@ -14,18 +14,10 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> *
> * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + * Manuel de la Pena <manuel.delapena at canonial.com>
> */
> -#include <com/ubuntu/location/providers/geoclue/provider.h>
> -
> -#include <com/ubuntu/location/providers/geoclue/geoclue.h>
> -
> -#include <core/dbus/object.h>
> -#include <core/dbus/signal.h>
> -
> -#include "core/dbus/object.h"
> -#include "core/dbus/signal.h"
> -
> -#include <thread>
> +
> +#include "provider.h"
>
> namespace cul = com::ubuntu::location;
> namespace culpg = com::ubuntu::location::providers::geoclue;
> @@ -41,128 +33,103 @@
> }
> }
>
> -struct culpg::Provider::Private
> -{
> - typedef dbus::Signal<
> - org::freedesktop::Geoclue::Position::Signals::PositionChanged,
> - org::freedesktop::Geoclue::Position::Signals::PositionChanged::ArgumentType
> - > PositionChanged;
> -
> - typedef dbus::Signal<
> - org::freedesktop::Geoclue::Velocity::Signals::VelocityChanged,
> - org::freedesktop::Geoclue::Velocity::Signals::VelocityChanged::ArgumentType
> - > VelocityChanged;
> -
> - Private(const culpg::Provider::Configuration& config)
> - : bus(the_session_bus()),
> - service(dbus::Service::use_service(bus, config.name)),
> - object(service->object_for_path(config.path)),
> - signal_position_changed(object->get_signal<org::freedesktop::Geoclue::Position::Signals::PositionChanged>()),
> - signal_velocity_changed(object->get_signal<org::freedesktop::Geoclue::Velocity::Signals::VelocityChanged>())
> - {
> - }
> -
> - void start()
> - {
> - if (!worker.joinable())
> - worker = std::move(std::thread{std::bind(&dbus::Bus::run, bus)});
> - }
> -
> - void stop()
> - {
> - bus->stop();
> - if (worker.joinable())
> - worker.join();
> - }
> -
> - dbus::Bus::Ptr bus;
> - dbus::Service::Ptr service;
> - dbus::Object::Ptr object;
> - PositionChanged::Ptr signal_position_changed;
> - VelocityChanged::Ptr signal_velocity_changed;
> - PositionChanged::SubscriptionToken position_updates_connection;
> - VelocityChanged::SubscriptionToken velocity_updates_connection;
> -
> - std::thread worker;
> -};
> +void culpg::Provider::start()
> +{
> + if (!worker.joinable())
> + worker = std::move(std::thread{std::bind(&dbus::Bus::run, bus)});
> +}
> +
> +void culpg::Provider::stop()
> +{
> + bus->stop();
> + if (worker.joinable())
> + worker.join();
> +}
> +
> +void culpg::Provider::on_position_changed(const fd::Geoclue::Position::Signals::PositionChanged::ArgumentType& arg)
> +{
> + fd::Geoclue::Position::FieldFlags flags{static_cast<unsigned long>(std::get<0>(arg))};
> + cul::Position pos
> + {
> + flags.test(fd::Geoclue::Position::Field::latitude) ?
> + cul::wgs84::Latitude{std::get<2>(arg)* cul::units::Degrees} : cul::wgs84::Latitude{},
> + flags.test(fd::Geoclue::Position::Field::longitude) ?
> + cul::wgs84::Longitude{std::get<3>(arg)* cul::units::Degrees} : cul::wgs84::Longitude{}
> + };
> +
> + if (flags.test(fd::Geoclue::Position::Field::altitude))
> + pos.altitude = cul::wgs84::Altitude{std::get<4>(arg)* cul::units::Meters};
> +
> + cul::Update<cul::Position> update(pos);
> + mutable_updates().position(update);
> +}
> +
> +void culpg::Provider::on_velocity_changed(const fd::Geoclue::Velocity::Signals::VelocityChanged::ArgumentType& arg)
> +{
> + fd::Geoclue::Velocity::FieldFlags flags{static_cast<unsigned long>(std::get<0>(arg))};
> + if (flags.none())
> + return;
> + if (flags.test(fd::Geoclue::Velocity::Field::speed))
> + {
> + cul::Update<cul::Velocity> update
> + {
> + std::get<2>(arg) * cul::units::MetersPerSecond,
> + cul::Clock::now()
> + };
> + mutable_updates().velocity(update);
> + }
> +
> + if (flags.test(fd::Geoclue::Velocity::Field::direction))
> + {
> + cul::Update<cul::Heading> update
> + {
> + std::get<3>(arg) * cul::units::Degrees,
> + cul::Clock::now()
> + };
> +
> + mutable_updates().heading(update);
> + }
> +}
>
> cul::Provider::Ptr culpg::Provider::create_instance(const cul::ProviderFactory::Configuration& config)
> {
> culpg::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.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");
> return cul::Provider::Ptr{new culpg::Provider{pConfig}};
> }
>
> culpg::Provider::Provider(const culpg::Provider::Configuration& config)
> : com::ubuntu::location::Provider(config.features, config.requirements),
> - d(new Private(config))
> + bus(the_session_bus()),
> + service(dbus::Service::use_service(bus, config.name)),
> + object(service->object_for_path(config.path)),
> + signal_position_changed(object->get_signal<fd::Geoclue::Position::Signals::PositionChanged>()),
> + signal_velocity_changed(object->get_signal<fd::Geoclue::Velocity::Signals::VelocityChanged>())
> {
> - d->position_updates_connection =
> - d->signal_position_changed->connect(
> - [this](const org::freedesktop::Geoclue::Position::Signals::PositionChanged::ArgumentType& arg)
> - {
> - org::freedesktop::Geoclue::Position::FieldFlags flags{static_cast<unsigned long>(std::get<0>(arg))};
> - cul::Position pos
> - {
> - flags.test(org::freedesktop::Geoclue::Position::Field::latitude) ?
> - cul::wgs84::Latitude{std::get<2>(arg)* cul::units::Degrees} : cul::wgs84::Latitude{},
> - flags.test(org::freedesktop::Geoclue::Position::Field::longitude) ?
> - cul::wgs84::Longitude{std::get<3>(arg)* cul::units::Degrees} : cul::wgs84::Longitude{}
> - };
> -
> - if (flags.test(org::freedesktop::Geoclue::Position::Field::altitude))
> - pos.altitude = cul::wgs84::Altitude{std::get<4>(arg)* cul::units::Meters};
> -
> - cul::Update<cul::Position> update(pos);
> - this->mutable_updates().position(update);
> - });
> -
> - d->velocity_updates_connection =
> - d->signal_velocity_changed->connect(
> - [this](const org::freedesktop::Geoclue::Velocity::Signals::VelocityChanged::ArgumentType& arg)
> - {
> - org::freedesktop::Geoclue::Velocity::FieldFlags flags{static_cast<unsigned long>(std::get<0>(arg))};
> - if (flags.none())
> - return;
> - if (flags.test(org::freedesktop::Geoclue::Velocity::Field::speed))
> - {
> - cul::Update<cul::Velocity> update
> - {
> - std::get<2>(arg) * cul::units::MetersPerSecond,
> - cul::Clock::now()
> - };
> - this->mutable_updates().velocity(update);
> - }
> -
> - if (flags.test(org::freedesktop::Geoclue::Velocity::Field::direction))
> - {
> - cul::Update<cul::Heading> update
> - {
> - std::get<3>(arg) * cul::units::Degrees,
> - cul::Clock::now()
> - };
> -
> - this->mutable_updates().heading(update);
> - }
> - });
> -
> - auto info = d->object->invoke_method_synchronously<
> - org::freedesktop::Geoclue::GetProviderInfo,
> - org::freedesktop::Geoclue::GetProviderInfo::ResultType>();
> - auto status = d->object->invoke_method_synchronously<
> - org::freedesktop::Geoclue::GetStatus,
> - org::freedesktop::Geoclue::GetStatus::ResultType>();
> + position_updates_connection = signal_position_changed->connect(
> + std::bind(&culpg::Provider::on_position_changed, this, std::placeholders::_1));
> + velocity_updates_connection = signal_velocity_changed->connect(
> + std::bind(&culpg::Provider::on_velocity_changed, this, std::placeholders::_1));
> +
> + auto info = object->invoke_method_synchronously<
> + fd::Geoclue::GetProviderInfo,
> + fd::Geoclue::GetProviderInfo::ResultType>();
> + auto status = object->invoke_method_synchronously<
> + fd::Geoclue::GetStatus,
> + fd::Geoclue::GetStatus::ResultType>();
>
> std::cout << "GeoclueProvider: ["
> << std::get<0>(info.value()) << ", "
> << std::get<1>(info.value()) << ","
> - << static_cast<org::freedesktop::Geoclue::Status>(status.value()) << "]" <<std::endl;
> + << static_cast<fd::Geoclue::Status>(status.value()) << "]" <<std::endl;
> }
>
> culpg::Provider::~Provider() noexcept
> {
> - d->stop();
> + stop();
> }
>
> bool culpg::Provider::matches_criteria(const cul::Criteria&)
> @@ -172,30 +139,30 @@
>
> void culpg::Provider::start_position_updates()
> {
> - d->start();
> + start();
> }
>
> void culpg::Provider::stop_position_updates()
> {
> - d->stop();
> + stop();
> }
>
> void culpg::Provider::start_velocity_updates()
> {
> - d->start();
> + start();
> }
>
> void culpg::Provider::stop_velocity_updates()
> {
> - d->stop();
> + stop();
> }
>
> void culpg::Provider::start_heading_updates()
> {
> - d->start();
> + start();
> }
>
> void culpg::Provider::stop_heading_updates()
> {
> - d->stop();
> + stop();
> }
>
> === renamed file 'include/location_service/com/ubuntu/location/providers/geoclue/provider.h' => 'src/location_service/com/ubuntu/location/providers/geoclue/provider.h'
> --- include/location_service/com/ubuntu/location/providers/geoclue/provider.h 2013-12-10 09:42:54 +0000
> +++ src/location_service/com/ubuntu/location/providers/geoclue/provider.h 2015-04-22 08:43:39 +0000
> @@ -14,13 +14,26 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> *
> * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + * Manuel de la Pena <manuel.delapena at canonical.com>
> */
> -#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_GEOCLUE_PROVIDER_H_
> -#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_GEOCLUE_PROVIDER_H_
> +
> +#pragma once
> +
> +#include <core/dbus/object.h>
> +#include <core/dbus/signal.h>
> +
> +#include "core/dbus/object.h"
> +#include "core/dbus/signal.h"
>
> #include <com/ubuntu/location/provider.h>
> #include <com/ubuntu/location/provider_factory.h>
>
> +#include <thread>
> +
> +#include "geoclue.h"
> +
> +namespace fd = org::freedesktop;
> +
> namespace com
> {
> namespace ubuntu
> @@ -33,6 +46,17 @@
> {
> class Provider : public com::ubuntu::location::Provider
> {
> +
> + typedef dbus::Signal<
> + fd::Geoclue::Position::Signals::PositionChanged,
> + fd::Geoclue::Position::Signals::PositionChanged::ArgumentType
> + > PositionChanged;
> +
> + typedef dbus::Signal<
> + fd::Geoclue::Velocity::Signals::VelocityChanged,
> + fd::Geoclue::Velocity::Signals::VelocityChanged::ArgumentType
> + > VelocityChanged;
> +
> public:
> static Provider::Ptr create_instance(const ProviderFactory::Configuration&);
>
> @@ -52,22 +76,36 @@
>
> virtual bool matches_criteria(const Criteria&);
>
> - virtual void start_position_updates();
> - virtual void stop_position_updates();
> -
> - virtual void start_velocity_updates();
> - virtual void stop_velocity_updates();
> -
> - virtual void start_heading_updates();
> - virtual void stop_heading_updates();
> + virtual void start_position_updates() override;
> + virtual void stop_position_updates() override;
> +
> + virtual void start_velocity_updates() override;
> + virtual void stop_velocity_updates() override;
> +
> + virtual void start_heading_updates() override;
> + virtual void stop_heading_updates() override;
> +
> + protected:
> + // set to be protected instead of private so that they can be exposed for testing
> + void start();
> + void stop();
> +
> + void on_position_changed(const fd::Geoclue::Position::Signals::PositionChanged::ArgumentType& arg);
> + void on_velocity_changed(const fd::Geoclue::Velocity::Signals::VelocityChanged::ArgumentType& arg);
>
> private:
> - struct Private;
> - std::unique_ptr<Private> d;
> + dbus::Bus::Ptr bus;
> + dbus::Service::Ptr service;
> + dbus::Object::Ptr object;
> + PositionChanged::Ptr signal_position_changed;
> + VelocityChanged::Ptr signal_velocity_changed;
> + PositionChanged::SubscriptionToken position_updates_connection;
> + VelocityChanged::SubscriptionToken velocity_updates_connection;
> +
> + std::thread worker;
> };
> }
> }
> }
> }
> }
> -#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_GEOCLUE_PROVIDER_H_
>
> === modified file 'src/location_service/com/ubuntu/location/providers/gps/provider.cpp'
> --- src/location_service/com/ubuntu/location/providers/gps/provider.cpp 2014-10-27 21:58:16 +0000
> +++ src/location_service/com/ubuntu/location/providers/gps/provider.cpp 2015-04-22 08:43:39 +0000
> @@ -28,11 +28,6 @@
> namespace cul = com::ubuntu::location;
> namespace culg = com::ubuntu::location::providers::gps;
>
> -struct culg::Provider::Private
> -{
> - std::shared_ptr<HardwareAbstractionLayer> hal;
> -};
> -
> std::string culg::Provider::class_name()
> {
> return "gps::Provider";
> @@ -47,26 +42,25 @@
> : cul::Provider(
> cul::Provider::Features::position | cul::Provider::Features::velocity | cul::Provider::Features::heading,
> cul::Provider::Requirements::satellites),
> - d(new Private())
> + hal(hal)
> {
> - d->hal = hal;
>
> - d->hal->position_updates().connect([this](const location::Position& pos)
> + hal->position_updates().connect([this](const location::Position& pos)
> {
> mutable_updates().position(Update<Position>(pos));
> });
>
> - d->hal->heading_updates().connect([this](const location::Heading& heading)
> + hal->heading_updates().connect([this](const location::Heading& heading)
> {
> mutable_updates().heading(Update<Heading>(heading));
> });
>
> - d->hal->velocity_updates().connect([this](const location::Velocity& velocity)
> + hal->velocity_updates().connect([this](const location::Velocity& velocity)
> {
> mutable_updates().velocity(Update<Velocity>(velocity));
> });
>
> - d->hal->space_vehicle_updates().connect([this](const std::set<location::SpaceVehicle>& svs)
> + hal->space_vehicle_updates().connect([this](const std::set<location::SpaceVehicle>& svs)
> {
> mutable_updates().svs(Update<std::set<location::SpaceVehicle>>(svs));
> });
> @@ -83,12 +77,12 @@
>
> void culg::Provider::start_position_updates()
> {
> - d->hal->start_positioning();
> + hal->start_positioning();
> }
>
> void culg::Provider::stop_position_updates()
> {
> - d->hal->stop_positioning();
> + hal->stop_positioning();
> }
>
> void culg::Provider::start_velocity_updates()
> @@ -109,6 +103,6 @@
>
> void culg::Provider::on_reference_location_updated(const cul::Update<cul::Position>& position)
> {
> - d->hal->inject_reference_position(position.value);
> + hal->inject_reference_position(position.value);
> }
>
>
> === modified file 'src/location_service/com/ubuntu/location/providers/gps/provider.h'
> --- src/location_service/com/ubuntu/location/providers/gps/provider.h 2014-01-20 13:03:19 +0000
> +++ src/location_service/com/ubuntu/location/providers/gps/provider.h 2015-04-22 08:43:39 +0000
> @@ -14,9 +14,10 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> *
> * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + * Manuel de la Pena <manuel.delapena at canonical.com>
> */
> -#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_GPS_PROVIDER_H_
> -#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_GPS_PROVIDER_H_
> +
> +#pragma once
This conflicts with the remaining code base, please stick to what is the standard right now or do a bulk adjustment.
>
> #include <com/ubuntu/location/provider.h>
> #include <com/ubuntu/location/provider_factory.h>
> @@ -60,12 +61,10 @@
> void on_reference_location_updated(const Update<Position>& position);
>
> private:
> - struct Private;
> - std::unique_ptr<Private> d;
> + std::shared_ptr<HardwareAbstractionLayer> hal;
> };
> }
> }
> }
> }
> }
> -#endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_GPS_PROVIDER_H_
>
> === modified file 'src/location_service/com/ubuntu/location/providers/skyhook/provider.cpp'
> --- src/location_service/com/ubuntu/location/providers/skyhook/provider.cpp 2013-12-10 09:42:54 +0000
> +++ src/location_service/com/ubuntu/location/providers/skyhook/provider.cpp 2015-04-22 08:43:39 +0000
> @@ -53,71 +53,6 @@
> };
> }
>
> -struct culs::Provider::Private
> -{
> - enum class State
> - {
> - stopped,
> - started,
> - stop_requested
> - };
> -
> - static WPS_Continuation periodic_callback(
> - void* context,
> - WPS_ReturnCode code,
> - const WPS_Location* location,
> - const void*);
> -
> - Private(
> - const culs::Provider::Configuration& config,
> - culs::Provider* parent)
> - : parent(parent),
> - config(config),
> - state(State::stopped)
> - {
> - }
> -
> - void start()
> - {
> - if (state != State::stopped)
> - return;
> -
> - if (worker.joinable())
> - worker.join();
> -
> - static const unsigned infinite_iterations = 0;
> -
> - authentication.username = config.user_name.c_str();
> - authentication.realm = config.realm.c_str();
> -
> - worker = std::move(std::thread([&]()
> - {
> - int rc = WPS_periodic_location(
> - &authentication,
> - WPS_NO_STREET_ADDRESS_LOOKUP,
> - config.period.count(),
> - infinite_iterations,
> - culs::Provider::Private::periodic_callback,
> - this);
> -
> - if (rc != WPS_OK)
> - LOG(ERROR) << return_code_lut.at(rc);
> - }));
> -
> - state = State::started;
> - }
> -
> - void request_stop()
> - {
> - state = State::stop_requested;
> - }
> -
> - culs::Provider* parent;
> - Configuration config;
> - State state;
> - WPS_SimpleAuthentication authentication;
> - std::thread worker;
> -};
>
> WPS_Continuation culs::Provider::Private::periodic_callback(void* context,
> WPS_ReturnCode code,
> @@ -195,13 +130,44 @@
>
> culs::Provider::Provider(const culs::Provider::Configuration& config)
> : com::ubuntu::location::Provider(culs::Provider::default_feature_flags(), culs::Provider::default_requirement_flags()),
> - d(new Private(config, this))
> + config(config),
> + state(State::stopped)
> {
> }
>
> culs::Provider::~Provider() noexcept
> {
> - d->request_stop();
> + request_stop();
> +}
> +
> +void culs::Provider::start()
> +{
> + if (state != State::stopped)
> + return;
> +
> + if (worker.joinable())
> + worker.join();
> +
> + static const unsigned infinite_iterations = 0;
> +
> + authentication.username = config.user_name.c_str();
> + authentication.realm = config.realm.c_str();
> +
> + worker = std::move(std::thread([&]()
> + {
> + int rc = WPS_periodic_location(&authentication, WPS_NO_STREET_ADDRESS_LOOKUP, config.period.count(),
> + infinite_iterations, culs::Provider::Private::periodic_callback, this);
> +
> + if (rc != WPS_OK)
> + LOG(ERROR) << return_code_lut.at(rc);
> + }));
> +
> + state = State::started;
> +}
> +
> +void culs::Provider::request_stop()
> +{
> + state = State::stop_requested;
> }
>
> bool culs::Provider::matches_criteria(const cul::Criteria&)
> @@ -211,30 +177,30 @@
>
> void culs::Provider::start_position_updates()
> {
> - d->start();
> + start();
> }
>
> void culs::Provider::stop_position_updates()
> {
> - d->request_stop();
> + request_stop();
> }
>
> void culs::Provider::start_velocity_updates()
> {
> - d->start();
> + start();
> }
>
> void culs::Provider::stop_velocity_updates()
> {
> - d->request_stop();
> + request_stop();
> }
>
> void culs::Provider::start_heading_updates()
> {
> - d->start();
> + start();
> }
>
> void culs::Provider::stop_heading_updates()
> {
> - d->request_stop();
> + request_stop();
> }
>
> === renamed file 'include/location_service/com/ubuntu/location/providers/skyhook/provider.h' => 'src/location_service/com/ubuntu/location/providers/skyhook/provider.h'
> --- include/location_service/com/ubuntu/location/providers/skyhook/provider.h 2013-12-10 09:42:54 +0000
> +++ src/location_service/com/ubuntu/location/providers/skyhook/provider.h 2015-04-22 08:43:39 +0000
> @@ -14,9 +14,10 @@
> * along with this program. If not, see <http://www.gnu.org/licenses/>.
> *
> * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + * Manuel de la Pena <manuel.delapena at canonical.com>
> */
> -#ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_SKYHOOK_PROVIDER_H_
> -#define LOCATION_SERVICE_COM_UBUNTU_LOCATION_PROVIDERS_SKYHOOK_PROVIDER_H_
> +
> +#pragma once
See before.
>
> #include <com/ubuntu/location/provider.h>
> #include <com/ubuntu/location/provider_factory.h>
> @@ -35,6 +36,13 @@
> {
> class Provider : public com::ubuntu::location::Provider
> {
> + enum class State
> + {
> + stopped,
> + started,
> + stop_requested
> + };
> +
> public:
> static Provider::Ptr create_instance(const ProviderFactory::Configuration& config);
>
> @@ -57,20 +65,34 @@
> Provider& operator=(const Provider&) = delete;
> ~Provider() noexcept;
>
> - virtual bool matches_criteria(const Criteria&);
> -
> - virtual void start_position_updates();
> - virtual void stop_position_updates();
> -
> - virtual void start_velocity_updates();
> - virtual void stop_velocity_updates();
> -
> - virtual void start_heading_updates();
> - virtual void stop_heading_updates();
> + virtual bool matches_criteria(const Criteria&) override;
> +
> + virtual void start_position_updates() override;
> + virtual void stop_position_updates() override;
> +
> + virtual void start_velocity_updates() override;
> + virtual void stop_velocity_updates() override;
> +
> + virtual void start_heading_updates() override;
> + virtual void stop_heading_updates() override;
> +
> + protected:
I don't see this being used in tests right now. Please postpone introducing the change until we really use it. I see your intention, but we should rather bundle together impl and test changes.
> + // visible to simplify exposing it to the tests
> + static WPS_Continuation periodic_callback(
> + void* context,
> + WPS_ReturnCode code,
> + const WPS_Location* location,
> + const void*);
> +
> + void start();
> + void stop();
>
> private:
> - struct Private;
> - std::unique_ptr<Private> d;
> +
> + Configuration config;
> + State state;
> + WPS_SimpleAuthentication authentication;
> + std::thread worker;
> };
> }
> }
>
--
https://code.launchpad.net/~mandel/location-service/remove-pimpl/+merge/257036
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service.
More information about the Ubuntu-reviews
mailing list