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

Proposed by Scott Sweeny on 2015-10-22
Status: Needs review
Proposed branch: lp:~ssweeny/location-service/espoo-delayed-provider.15.04
Merge into: lp:location-service/15.04
Prerequisite: lp:~ssweeny/location-service/delayed-providers.15.04
Diff against target: 220 lines (+141/-5)
5 files modified
include/location_service/com/ubuntu/location/provider.h (+1/-1)
src/location_service/com/ubuntu/location/provider.cpp (+1/-1)
src/location_service/com/ubuntu/location/providers/remote/provider.cpp (+136/-1)
src/location_service/com/ubuntu/location/providers/remote/provider.h (+2/-1)
tests/mock_delayed_provider.h (+1/-1)
To merge this branch: bzr merge lp:~ssweeny/location-service/espoo-delayed-provider.15.04
Reviewer Review Type Date Requested Status
Thomas Voß (community) 2015-10-22 Needs Fixing on 2015-10-26
Review via email: mp+275424@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Thomas Voß (thomas-voss) wrote :

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

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

review: Needs Fixing
200. By Scott Sweeny on 2015-10-27

Merge delayed-provider branch

Scott Sweeny (ssweeny) wrote :

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

Thomas Voß (thomas-voss) wrote :

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

Unmerged revisions

200. By Scott Sweeny on 2015-10-27

Merge delayed-provider branch

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

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

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

Added support for delayed providers.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'include/location_service/com/ubuntu/location/provider.h'
--- include/location_service/com/ubuntu/location/provider.h 2015-10-27 20:50:44 +0000
+++ include/location_service/com/ubuntu/location/provider.h 2015-10-27 20:50:44 +0000
@@ -241,7 +241,7 @@
241 * @brief States the booting state of the provider.241 * @brief States the booting state of the provider.
242 * @return The boot state of the provider.242 * @return The boot state of the provider.
243 */243 */
244 virtual BootState boot_state() const;244 virtual BootState boot_state();
245245
246 /**246 /**
247 * @brief Indicates to the provider that it should start the boot process in order to be able to247 * @brief Indicates to the provider that it should start the boot process in order to be able to
248248
=== modified file 'src/location_service/com/ubuntu/location/provider.cpp'
--- src/location_service/com/ubuntu/location/provider.cpp 2015-10-27 20:50:44 +0000
+++ src/location_service/com/ubuntu/location/provider.cpp 2015-10-27 20:50:44 +0000
@@ -199,7 +199,7 @@
199 return false;199 return false;
200}200}
201201
202cul::Provider::BootState cul::Provider::boot_state() const202cul::Provider::BootState cul::Provider::boot_state()
203{203{
204 return cul::Provider::BootState::BOOTED;204 return cul::Provider::BootState::BOOTED;
205}205}
206206
=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.cpp'
--- src/location_service/com/ubuntu/location/providers/remote/provider.cpp 2015-04-14 18:54:00 +0000
+++ src/location_service/com/ubuntu/location/providers/remote/provider.cpp 2015-10-27 20:50:44 +0000
@@ -22,7 +22,9 @@
2222
23#include <com/ubuntu/location/logging.h>23#include <com/ubuntu/location/logging.h>
2424
25#include <core/dbus/dbus.h>
25#include <core/dbus/object.h>26#include <core/dbus/object.h>
27#include <core/dbus/service_watcher.h>
26#include <core/dbus/signal.h>28#include <core/dbus/signal.h>
27#include <core/dbus/asio/executor.h>29#include <core/dbus/asio/executor.h>
2830
@@ -30,6 +32,8 @@
3032
31#include <boost/asio.hpp>33#include <boost/asio.hpp>
3234
35#include <functional>
36#include <mutex>
33#include <thread>37#include <thread>
3438
35namespace cul = com::ubuntu::location;39namespace cul = com::ubuntu::location;
@@ -39,6 +43,124 @@
3943
40namespace44namespace
41{45{
46// NotYet is a provider decoration that waits for a remote service to come up, before instantiating
47// the actual stub instance.
48class NotYet : public cul::Provider
49{
50public:
51 // Creates a new instance, watching fo the given service_name on the given bus, invoking the
52 // when_ready functor for constructing the provider instance whenever the service becomes ready.
53 NotYet(const core::dbus::Bus::Ptr& bus, const std::string& service_name, const std::function<cul::Provider::Ptr()>& when_ready)
54 : dbus{bus}, service_watcher{dbus.make_service_watcher(service_name)},
55 service_registered
56 {
57 service_watcher->service_registered().connect(std::bind(&NotYet::on_service_registered, this, when_ready))
58 }
59 {
60 }
61
62 void on_service_registered(const std::function<cul::Provider::Ptr()>& when_ready)
63 {
64 LOG(INFO) << "Remote location service appeared.";
65 impl = when_ready();
66 set_boot_state(cul::Provider::BootState::BOOTED);
67 // notify the system we are booted so that we get the current state (started etc..)
68 LOG(INFO) << "Emitting booted signal.";
69 booted_signal()(true);
70 }
71
72 bool matches_criteria(const cul::Criteria& criteria) override
73 {
74 if (impl) return impl->matches_criteria(criteria);
75
76 return false;
77 }
78
79 bool supports(const cul::Provider::Features& f) const override
80 {
81 if (impl) return impl->supports(f);
82 return false;
83 }
84
85 bool requires(const cul::Provider::Requirements& r) const override
86 {
87 if (impl) return impl->requires(r);
88 return true;
89 }
90
91 void on_wifi_and_cell_reporting_state_changed(cul::WifiAndCellIdReportingState state) override
92 {
93 if (impl) impl->on_wifi_and_cell_reporting_state_changed(state);
94 }
95
96 void on_reference_location_updated(const cul::Update<cul::Position>& position) override
97 {
98 if (impl) impl->on_reference_location_updated(position);
99 }
100
101 void on_reference_velocity_updated(const cul::Update<cul::Velocity>& velocity) override
102 {
103 if (impl) impl->on_reference_velocity_updated(velocity);
104 }
105
106 void on_reference_heading_updated(const cul::Update<cul::Heading>& heading) override
107 {
108 if (impl) impl->on_reference_heading_updated(heading);
109 }
110
111 void start_position_updates() override
112 {
113 if (impl) impl->state_controller()->start_position_updates();
114 }
115
116 void stop_position_updates() override
117 {
118 if (impl) impl->state_controller()->stop_position_updates();
119 }
120
121 void start_heading_updates() override
122 {
123 if (impl) impl->state_controller()->start_heading_updates();
124 }
125
126 void stop_heading_updates() override
127 {
128 if (impl) impl->state_controller()->stop_heading_updates();
129 }
130
131 void start_velocity_updates() override
132 {
133 if (impl) impl->state_controller()->start_velocity_updates();
134 }
135
136 void stop_velocity_updates() override
137 {
138 if (impl) impl->state_controller()->stop_velocity_updates();
139 }
140
141 BootState boot_state()
142 {
143 std::lock_guard<std::mutex> guard(state_mutex);
144 return state;
145 }
146
147 void set_boot_state(BootState new_state)
148 {
149 LOG(INFO) << "Setting boot state.";
150 std::lock_guard<std::mutex> guard(state_mutex);
151 state = new_state;
152 LOG(INFO) << "State was set.";
153 }
154
155private:
156 core::dbus::DBus dbus;
157 std::unique_ptr<core::dbus::ServiceWatcher> service_watcher;
158 core::ScopedConnection service_registered;
159 cul::Provider::Ptr impl;
160 cul::Provider::BootState state = cul::Provider::BootState::BOOTING;
161 std::mutex state_mutex;
162};
163
42struct Runtime164struct Runtime
43{165{
44 static Runtime& instance()166 static Runtime& instance()
@@ -191,7 +313,20 @@
191 auto service = dbus::Service::use_service(bus, name);313 auto service = dbus::Service::use_service(bus, name);
192 auto object = service->object_for_path(path);314 auto object = service->object_for_path(path);
193315
194 return create_instance_with_config(remote::stub::Configuration{object});316 try
317 {
318 auto result = create_instance_with_config(remote::stub::Configuration{object});
319 } catch (const std::runtime_error& e)
320 {
321 std::cerr << "Error creating remote provider, creating facade for watching the service and delayed initialization" << std::endl;
322 std::cerr << e.what() << std::endl;
323 }
324
325 return std::make_shared<NotYet>(bus, name, [bus, name, path]()
326 {
327 auto object = dbus::Service::use_service(bus, name)->object_for_path(path);
328 return create_instance_with_config(remote::stub::Configuration{object});
329 });
195}330}
196331
197cul::Provider::Ptr remote::Provider::Stub::create_instance_with_config(const remote::stub::Configuration& config)332cul::Provider::Ptr remote::Provider::Stub::create_instance_with_config(const remote::stub::Configuration& config)
198333
=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.h'
--- src/location_service/com/ubuntu/location/providers/remote/provider.h 2015-01-26 19:11:46 +0000
+++ src/location_service/com/ubuntu/location/providers/remote/provider.h 2015-10-27 20:50:44 +0000
@@ -40,7 +40,8 @@
40{40{
41 class Stub : public com::ubuntu::location::Provider, public std::enable_shared_from_this<Stub>41 class Stub : public com::ubuntu::location::Provider, public std::enable_shared_from_this<Stub>
42 {42 {
43 public:43 public:
44
44 // For integration with the Provider factory.45 // For integration with the Provider factory.
45 static std::string class_name();46 static std::string class_name();
4647
4748
=== modified file 'tests/mock_delayed_provider.h'
--- tests/mock_delayed_provider.h 2015-10-27 20:50:44 +0000
+++ tests/mock_delayed_provider.h 2015-10-27 20:50:44 +0000
@@ -63,7 +63,7 @@
63 MOCK_METHOD0(stop_velocity_updates, void());63 MOCK_METHOD0(stop_velocity_updates, void());
6464
65 MOCK_CONST_METHOD0(has_delayed_boot, bool());65 MOCK_CONST_METHOD0(has_delayed_boot, bool());
66 MOCK_CONST_METHOD0(boot_state, BootState());66 MOCK_METHOD0(boot_state, BootState());
67 MOCK_CONST_METHOD0(boot, void());67 MOCK_CONST_METHOD0(boot, void());
6868
6969

Subscribers

People subscribed via source and target branches