[Merge] lp:~mandel/location-service/delayed-providers into lp:location-service
Alfonso Sanchez-Beato
alfonso.sanchez-beato at canonical.com
Mon Jun 15 15:10:59 UTC 2015
See comment below
Diff comments:
> === modified file 'include/location_service/com/ubuntu/location/provider.h'
> --- include/location_service/com/ubuntu/location/provider.h 2015-01-21 20:13:28 +0000
> +++ include/location_service/com/ubuntu/location/provider.h 2015-06-15 14:58:06 +0000
> @@ -150,6 +150,8 @@
> friend class Provider;
> explicit Controller(Provider& instance);
>
> + virtual void on_provider_booted(bool was_booted);
> +
> private:
> Provider& instance;
> std::atomic<int> position_updates_counter;
> @@ -172,6 +174,20 @@
> core::Signal<Update<std::set<SpaceVehicle>>> svs;
> };
>
> + /**
> + * @brief Wraps al those sigals that are related to the boot of the provider.
> + */
> + struct Signals
> + {
> + core::Signal<bool> booted;
> + };
> +
> + enum class BootState {
> + NOT_BOOTED,
> + BOOTING,
> + BOOTED
> + };
> +
> virtual ~Provider() = default;
>
> Provider(const Provider&) = delete;
> @@ -189,6 +205,12 @@
> virtual const Controller::Ptr& state_controller() const;
>
> /**
> + * @brief Provides non-mutable access to this provider's signals.
> + * @return A non-mutable reference to the signals.
> + */
> + virtual const Signals& signals() const;
> +
> + /**
> * @brief Checks if the provider supports a specific feature.
> * @param f Feature to test for
> * @return true iff the provider supports the feature.
> @@ -210,6 +232,19 @@
> virtual bool matches_criteria(const Criteria& criteria);
>
> /**
> + * @brief States the booting state of the provider.
> + * @return The boot state of the provider.
> + */
> + virtual BootState boot_state() const;
> +
> + /**
> + * @brief States the booting state of the provider. This method must be implemented in a thread safe mode
Is the comment right? Then it would fo the same as boot_state()
> + * since it can be called more than one.
> + * @return The boot state of the provider.
> + */
> + virtual void boot();
> +
> + /**
> * @brief Called by the engine whenever the wifi and cell ID reporting state changes.
> * @param state The new state.
> */
> @@ -270,12 +305,14 @@
> */
> virtual void stop_velocity_updates();
>
> -private:
> +protected:
> + // protected to allow better white box testing
> struct
> {
> Features features = Features::none;
> Requirements requirements = Requirements::none;
> Updates updates;
> + Signals signals;
> Controller::Ptr controller = Controller::Ptr{};
> } d;
> };
>
> === modified file 'src/location_service/com/ubuntu/location/provider.cpp'
> --- src/location_service/com/ubuntu/location/provider.cpp 2015-01-23 19:07:09 +0000
> +++ src/location_service/com/ubuntu/location/provider.cpp 2015-06-15 14:58:06 +0000
> @@ -16,9 +16,11 @@
> * Authored by: Thomas Voß <thomas.voss at canonical.com>
> */
> #include <com/ubuntu/location/provider.h>
> +#include <com/ubuntu/location/logging.h>
>
> #include <atomic>
> #include <bitset>
> +#include <functional>
> #include <memory>
>
> namespace cul = com::ubuntu::location;
> @@ -51,12 +53,18 @@
>
> void cul::Provider::Controller::start_position_updates()
> {
> - if (position_updates_counter < 0)
> + if (position_updates_counter < 0) {
> + LOG(WARNING) << "Counter smaller than 0";
> return;
> + }
>
> if (++position_updates_counter == 1)
> {
> - instance.start_position_updates();
> + LOG(WARNING) << "Should we start";
> + if (instance.boot_state() == BootState::BOOTED) {
> + LOG(WARNING) << "Check boot state.";
> + instance.start_position_updates();
> + }
> }
> }
>
> @@ -67,7 +75,8 @@
>
> if (--position_updates_counter == 0)
> {
> - instance.stop_position_updates();
> + if (instance.boot_state() == BootState::BOOTED)
> + instance.stop_position_updates();
> }
> }
>
> @@ -83,7 +92,8 @@
>
> if (++heading_updates_counter == 1)
> {
> - instance.start_heading_updates();
> + if (instance.boot_state() == BootState::BOOTED)
> + instance.start_heading_updates();
> }
> }
>
> @@ -94,7 +104,8 @@
>
> if (--heading_updates_counter == 0)
> {
> - instance.stop_heading_updates();
> + if (instance.boot_state() == BootState::BOOTED)
> + instance.stop_heading_updates();
> }
> }
>
> @@ -110,7 +121,8 @@
>
> if (++velocity_updates_counter == 1)
> {
> - instance.start_velocity_updates();
> + if (instance.boot_state() == BootState::BOOTED)
> + instance.start_velocity_updates();
> }
> }
>
> @@ -121,7 +133,8 @@
>
> if (--velocity_updates_counter == 0)
> {
> - instance.stop_velocity_updates();
> + if (instance.boot_state() == BootState::BOOTED)
> + instance.stop_velocity_updates();
> }
> }
>
> @@ -130,12 +143,38 @@
> return velocity_updates_counter > 0;
> }
>
> +void cul::Provider::Controller::on_provider_booted(bool was_booted)
> +{
> + if (!was_booted)
> + return;
> +
> + // we need to propagate the state of the provider using the internal counters as references
> + if (position_updates_counter > 0) {
> + instance.start_position_updates();
> + }
> + if (heading_updates_counter > 0) {
> + instance.start_heading_updates();
> + }
> + if (velocity_updates_counter > 0) {
> + instance.start_velocity_updates();
> + }
> +}
> +
> cul::Provider::Controller::Controller(cul::Provider& instance)
> : instance(instance),
> position_updates_counter(0),
> heading_updates_counter(0),
> velocity_updates_counter(0)
> -{
> +{
> + using namespace std::placeholders;
> +
> + if (instance.boot_state() != BootState::BOOTED) {
> + // if the instance has not booted and is delayed we need to keep track of its booting state
> + instance.signals().booted.connect(std::bind(&Controller::on_provider_booted, this, _1));
> + if (instance.boot_state() != BootState::BOOTING) {
> + instance.boot();
> + }
> + }
> }
>
> const cul::Provider::Controller::Ptr& cul::Provider::state_controller() const
> @@ -158,11 +197,23 @@
> return false;
> }
>
> +cul::Provider::BootState cul::Provider::boot_state() const
> +{
> + return cul::Provider::BootState::BOOTED;
> +}
> +
> +void cul::Provider::boot() { }
> +
> const cul::Provider::Updates& cul::Provider::updates() const
> {
> return d.updates;
> }
>
> +const cul::Provider::Signals& cul::Provider::signals() const
> +{
> + return d.signals;
> +}
> +
> cul::Provider::Provider(
> const cul::Provider::Features& features,
> const cul::Provider::Requirements& requirements)
>
> === modified file 'tests/controller_test.cpp'
> --- tests/controller_test.cpp 2014-09-10 19:34:09 +0000
> +++ tests/controller_test.cpp 2015-06-15 14:58:06 +0000
> @@ -18,6 +18,7 @@
> #include <com/ubuntu/location/provider.h>
>
> #include "mock_provider.h"
> +#include "mock_delayed_provider.h"
>
> #include <gmock/gmock.h>
> #include <gtest/gtest.h>
> @@ -130,3 +131,215 @@
> EXPECT_FALSE(controller->are_velocity_updates_running());
> EXPECT_FALSE(controller->are_heading_updates_running());
> }
> +
> +TEST(Controller, controller_resumes_state_after_boot) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> +
> +}
> +
> +TEST(Controller, controller_does_not_call_start_position_if_not_booted) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
> + .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
> + EXPECT_CALL(provider, start_position_updates()).Times(Exactly(0));
> +
> + controller->start_position_updates();
> + controller->start_position_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_start_position_if_booting) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
> + .WillOnce(Return(cul::Provider::BootState::BOOTING));
> + EXPECT_CALL(provider, start_position_updates()).Times(Exactly(0));
> +
> + controller->start_position_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_start_heading_if_not_booted) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
> + .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
> + EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
> +
> + controller->start_heading_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_start_heading_if_booting) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
> + .WillOnce(Return(cul::Provider::BootState::BOOTING));
> + EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
> +
> + controller->start_heading_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_start_velocity_if_not_booted) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
> + .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
> + EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
> +
> + controller->start_velocity_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_start_velocity_if_booting) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
> + .WillOnce(Return(cul::Provider::BootState::BOOTING));
> + EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
> +
> + controller->start_velocity_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_stop_position_if_not_booted) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
> + .WillOnce(Return(cul::Provider::BootState::BOOTED))
> + .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
> + EXPECT_CALL(provider, start_position_updates()).Times(Exactly(1));
> + EXPECT_CALL(provider, stop_position_updates()).Times(Exactly(0));
> +
> + controller->start_position_updates();
> + controller->stop_position_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_stop_position_if_booting) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
> + .WillOnce(Return(cul::Provider::BootState::BOOTED))
> + .WillOnce(Return(cul::Provider::BootState::BOOTING));
> + EXPECT_CALL(provider, start_position_updates()).Times(Exactly(1));
> + EXPECT_CALL(provider, stop_position_updates()).Times(Exactly(0));
> +
> + controller->start_position_updates();
> + controller->stop_position_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_stop_heading_if_not_booted) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
> + .WillOnce(Return(cul::Provider::BootState::BOOTED))
> + .WillOnce(Return(cul::Provider::BootState::BOOTING));
> + EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(1));
> + EXPECT_CALL(provider, stop_heading_updates()).Times(Exactly(0));
> +
> + controller->start_heading_updates();
> + controller->stop_heading_updates();
> +}
> +
> +TEST(Controller, controller_does_not_call_stop_velocity_if_not_booted) {
> + using namespace ::testing;
> + using ::testing::Mock;
> +
> + MockDelayedProvider provider;
> + EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
> +
> + // ignore what ever was performed on construction
> + Mock::VerifyAndClear(&provider);
> + // use a mock controller
> + auto controller = provider.state_controller();
> +
> + EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
> + .WillOnce(Return(cul::Provider::BootState::BOOTED))
> + .WillOnce(Return(cul::Provider::BootState::BOOTING));
> + EXPECT_CALL(provider, start_velocity_updates()).Times(Exactly(1));
> + EXPECT_CALL(provider, stop_velocity_updates()).Times(Exactly(0));
> +
> + controller->start_velocity_updates();
> + controller->stop_velocity_updates();
> +}
>
> === added file 'tests/mock_delayed_provider.h'
> --- tests/mock_delayed_provider.h 1970-01-01 00:00:00 +0000
> +++ tests/mock_delayed_provider.h 2015-06-15 14:58:06 +0000
> @@ -0,0 +1,89 @@
> +/*
> + * 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: M
> + */
> +#ifndef MOCK_DELAYED_PROVIDER_H_
> +#define MOCK_DELAYED_PROVIDER_H_
> +
> +#include <com/ubuntu/location/provider.h>
> +#include <com/ubuntu/location/logging.h>
> +
> +#include <gmock/gmock.h>
> +
> +struct PublicController : public com::ubuntu::location::Provider::Controller
> +{
> +
> + explicit PublicController(com::ubuntu::location::Provider& instance)
> + : com::ubuntu::location::Provider::Controller(instance) {}
> +};
> +
> +struct MockDelayedProvider : public com::ubuntu::location::Provider
> +{
> + MockDelayedProvider() : com::ubuntu::location::Provider()
> + {
> + }
> +
> + 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));
> +
> + // Called by the engine whenever the reference location changed.
> + MOCK_METHOD1(on_reference_location_updated, void(const com::ubuntu::location::Update<com::ubuntu::location::Position>& position));
> +
> + // Called by the engine whenever the reference velocity changed.
> + MOCK_METHOD1(on_reference_velocity_updated, void(const com::ubuntu::location::Update<com::ubuntu::location::Velocity>& velocity));
> +
> + // Called by the engine whenever the reference heading changed.
> + MOCK_METHOD1(on_reference_heading_updated, void(const com::ubuntu::location::Update<com::ubuntu::location::Heading>& heading));
> +
> + MOCK_METHOD0(start_position_updates, void());
> + MOCK_METHOD0(stop_position_updates, void());
> +
> + MOCK_METHOD0(start_heading_updates, void());
> + MOCK_METHOD0(stop_heading_updates, void());
> +
> + MOCK_METHOD0(start_velocity_updates, void());
> + MOCK_METHOD0(stop_velocity_updates, void());
> +
> + MOCK_CONST_METHOD0(has_delayed_boot, bool());
> + MOCK_CONST_METHOD0(boot_state, BootState());
> + MOCK_CONST_METHOD0(boot, void());
> +
> +
> + // Inject a position update from the outside.
> + void inject_update(const com::ubuntu::location::Update<com::ubuntu::location::Position>& update)
> + {
> + mutable_updates().position(update);
> + }
> +
> + // Inject a velocity update from the outside.
> + void inject_update(const com::ubuntu::location::Update<com::ubuntu::location::Velocity>& update)
> + {
> + mutable_updates().velocity(update);
> + }
> +
> + // Inject a heading update from the outside.
> + void inject_update(const com::ubuntu::location::Update<com::ubuntu::location::Heading>& update)
> + {
> + mutable_updates().heading(update);
> + }
> +};
> +
> +#endif // MOCK_PROVIDER_H_
>
--
https://code.launchpad.net/~mandel/location-service/delayed-providers/+merge/261944
Your team Ubuntu Phablet Team is subscribed to branch lp:location-service.
More information about the Ubuntu-reviews
mailing list