Merge lp:~mandel/location-service/espoo-delayed-provider into lp:location-service/trunk

Proposed by Manuel de la Peña
Status: Needs review
Proposed branch: lp:~mandel/location-service/espoo-delayed-provider
Merge into: lp:location-service/trunk
Prerequisite: lp:~mandel/location-service/delayed-providers
Diff against target: 248 lines (+151/-8)
5 files modified
include/location_service/com/ubuntu/location/provider.h (+1/-1)
src/location_service/com/ubuntu/location/provider.cpp (+11/-4)
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:~mandel/location-service/espoo-delayed-provider
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Alfonso Sanchez-Beato Needs Fixing
Review via email: mp+262056@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.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
210. By Manuel de la Peña

Add extra logging to keep track of the remote daemon.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM, just have some minor nits. See comments below.

review: Needs Fixing
211. By Manuel de la Peña

Track ownership change of the service to check if the name changed.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
212. By Manuel de la Peña

Add some extra logging for debugging purposes.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
213. By Manuel de la Peña

Increase logging to know what the provider is not started.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)

Unmerged revisions

213. By Manuel de la Peña

Increase logging to know what the provider is not started.

212. By Manuel de la Peña

Add some extra logging for debugging purposes.

211. By Manuel de la Peña

Track ownership change of the service to check if the name changed.

210. By Manuel de la Peña

Add extra logging to keep track of the remote daemon.

209. By Manuel de la Peña

Use the new method to get the signal.

208. By Manuel de la Peña

Merged delayed-providers into espoo-delayed-provider.

207. By Manuel de la Peña

Ensure that we connect to the booted signal.

206. By Manuel de la Peña

Merged delayed-providers into espoo-delayed-provider.

205. By Manuel de la Peña

Make the remote provider work when the daemons have not been launched.

204. By Manuel de la Peña

Merged with tvoss work to make the boot more reliable.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/location_service/com/ubuntu/location/provider.h'
2--- include/location_service/com/ubuntu/location/provider.h 2015-06-26 08:28:45 +0000
3+++ include/location_service/com/ubuntu/location/provider.h 2015-06-26 08:28:45 +0000
4@@ -241,7 +241,7 @@
5 * @brief States the booting state of the provider.
6 * @return The boot state of the provider.
7 */
8- virtual BootState boot_state() const;
9+ virtual BootState boot_state();
10
11 /**
12 * @brief Indicates to the provider that it should start the boot process in order to be able to
13
14=== modified file 'src/location_service/com/ubuntu/location/provider.cpp'
15--- src/location_service/com/ubuntu/location/provider.cpp 2015-06-26 08:28:45 +0000
16+++ src/location_service/com/ubuntu/location/provider.cpp 2015-06-26 08:28:45 +0000
17@@ -142,17 +142,24 @@
18
19 void cul::Provider::Controller::on_provider_booted(bool was_booted)
20 {
21+ LOG(INFO) << "Provider was booted " << was_booted;
22 if (!was_booted)
23 return;
24
25 // we need to propagate the state of the provider using the internal counters as references
26- if (position_updates_counter > 0) {
27+ if (position_updates_counter > 0)
28+ {
29+ LOG(INFO) << "Starting position updates";
30 instance.start_position_updates();
31 }
32- if (heading_updates_counter > 0) {
33+ if (heading_updates_counter > 0)
34+ {
35+ LOG(INFO) << "Starting heading updates";
36 instance.start_heading_updates();
37 }
38- if (velocity_updates_counter > 0) {
39+ if (velocity_updates_counter > 0)
40+ {
41+ LOG(INFO) << "Starting velocity updates";
42 instance.start_velocity_updates();
43 }
44 }
45@@ -194,7 +201,7 @@
46 return false;
47 }
48
49-cul::Provider::BootState cul::Provider::boot_state() const
50+cul::Provider::BootState cul::Provider::boot_state()
51 {
52 return cul::Provider::BootState::BOOTED;
53 }
54
55=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.cpp'
56--- src/location_service/com/ubuntu/location/providers/remote/provider.cpp 2015-04-14 18:54:00 +0000
57+++ src/location_service/com/ubuntu/location/providers/remote/provider.cpp 2015-06-26 08:28:45 +0000
58@@ -22,7 +22,9 @@
59
60 #include <com/ubuntu/location/logging.h>
61
62+#include <core/dbus/dbus.h>
63 #include <core/dbus/object.h>
64+#include <core/dbus/service_watcher.h>
65 #include <core/dbus/signal.h>
66 #include <core/dbus/asio/executor.h>
67
68@@ -30,6 +32,8 @@
69
70 #include <boost/asio.hpp>
71
72+#include <functional>
73+#include <mutex>
74 #include <thread>
75
76 namespace cul = com::ubuntu::location;
77@@ -39,6 +43,124 @@
78
79 namespace
80 {
81+// NotYet is a provider decoration that waits for a remote service to come up, before instantiating
82+// the actual stub instance.
83+class NotYet : public cul::Provider
84+{
85+public:
86+ // Creates a new instance, watching fo the given service_name on the given bus, invoking the
87+ // when_ready functor for constructing the provider instance whenever the service becomes ready.
88+ NotYet(const core::dbus::Bus::Ptr& bus, const std::string& service_name, const std::function<cul::Provider::Ptr()>& when_ready)
89+ : dbus{bus}, service_watcher{dbus.make_service_watcher(service_name)},
90+ service_registered
91+ {
92+ service_watcher->service_registered().connect(std::bind(&NotYet::on_service_registered, this, when_ready))
93+ }
94+ {
95+ }
96+
97+ void on_service_registered(const std::function<cul::Provider::Ptr()>& when_ready)
98+ {
99+ LOG(INFO) << "Remote location service appeared.";
100+ impl = when_ready();
101+ set_boot_state(cul::Provider::BootState::BOOTED);
102+ // notify the system we are booted so that we get the current state (started etc..)
103+ LOG(INFO) << "Emitting booted signal.";
104+ booted_signal()(true);
105+ }
106+
107+ bool matches_criteria(const cul::Criteria& criteria) override
108+ {
109+ if (impl) return impl->matches_criteria(criteria);
110+
111+ return false;
112+ }
113+
114+ bool supports(const cul::Provider::Features& f) const override
115+ {
116+ if (impl) return impl->supports(f);
117+ return false;
118+ }
119+
120+ bool requires(const cul::Provider::Requirements& r) const override
121+ {
122+ if (impl) return impl->requires(r);
123+ return true;
124+ }
125+
126+ void on_wifi_and_cell_reporting_state_changed(cul::WifiAndCellIdReportingState state) override
127+ {
128+ if (impl) impl->on_wifi_and_cell_reporting_state_changed(state);
129+ }
130+
131+ void on_reference_location_updated(const cul::Update<cul::Position>& position) override
132+ {
133+ if (impl) impl->on_reference_location_updated(position);
134+ }
135+
136+ void on_reference_velocity_updated(const cul::Update<cul::Velocity>& velocity) override
137+ {
138+ if (impl) impl->on_reference_velocity_updated(velocity);
139+ }
140+
141+ void on_reference_heading_updated(const cul::Update<cul::Heading>& heading) override
142+ {
143+ if (impl) impl->on_reference_heading_updated(heading);
144+ }
145+
146+ void start_position_updates() override
147+ {
148+ if (impl) impl->state_controller()->start_position_updates();
149+ }
150+
151+ void stop_position_updates() override
152+ {
153+ if (impl) impl->state_controller()->stop_position_updates();
154+ }
155+
156+ void start_heading_updates() override
157+ {
158+ if (impl) impl->state_controller()->start_heading_updates();
159+ }
160+
161+ void stop_heading_updates() override
162+ {
163+ if (impl) impl->state_controller()->stop_heading_updates();
164+ }
165+
166+ void start_velocity_updates() override
167+ {
168+ if (impl) impl->state_controller()->start_velocity_updates();
169+ }
170+
171+ void stop_velocity_updates() override
172+ {
173+ if (impl) impl->state_controller()->stop_velocity_updates();
174+ }
175+
176+ BootState boot_state()
177+ {
178+ std::lock_guard<std::mutex> guard(state_mutex);
179+ return state;
180+ }
181+
182+ void set_boot_state(BootState new_state)
183+ {
184+ LOG(INFO) << "Setting boot state.";
185+ std::lock_guard<std::mutex> guard(state_mutex);
186+ state = new_state;
187+ LOG(INFO) << "State was set.";
188+ }
189+
190+private:
191+ core::dbus::DBus dbus;
192+ std::unique_ptr<core::dbus::ServiceWatcher> service_watcher;
193+ core::ScopedConnection service_registered;
194+ cul::Provider::Ptr impl;
195+ cul::Provider::BootState state = cul::Provider::BootState::BOOTING;
196+ std::mutex state_mutex;
197+};
198+
199 struct Runtime
200 {
201 static Runtime& instance()
202@@ -191,7 +313,20 @@
203 auto service = dbus::Service::use_service(bus, name);
204 auto object = service->object_for_path(path);
205
206- return create_instance_with_config(remote::stub::Configuration{object});
207+ try
208+ {
209+ auto result = create_instance_with_config(remote::stub::Configuration{object});
210+ } catch (const std::runtime_error& e)
211+ {
212+ std::cerr << "Error creating remote provider, creating facade for watching the service and delayed initialization" << std::endl;
213+ std::cerr << e.what() << std::endl;
214+ }
215+
216+ return std::make_shared<NotYet>(bus, name, [bus, name, path]()
217+ {
218+ auto object = dbus::Service::use_service(bus, name)->object_for_path(path);
219+ return create_instance_with_config(remote::stub::Configuration{object});
220+ });
221 }
222
223 cul::Provider::Ptr remote::Provider::Stub::create_instance_with_config(const remote::stub::Configuration& config)
224
225=== modified file 'src/location_service/com/ubuntu/location/providers/remote/provider.h'
226--- src/location_service/com/ubuntu/location/providers/remote/provider.h 2015-01-26 19:11:46 +0000
227+++ src/location_service/com/ubuntu/location/providers/remote/provider.h 2015-06-26 08:28:45 +0000
228@@ -40,7 +40,8 @@
229 {
230 class Stub : public com::ubuntu::location::Provider, public std::enable_shared_from_this<Stub>
231 {
232- public:
233+ public:
234+
235 // For integration with the Provider factory.
236 static std::string class_name();
237
238
239=== modified file 'tests/mock_delayed_provider.h'
240--- tests/mock_delayed_provider.h 2015-06-26 08:28:45 +0000
241+++ tests/mock_delayed_provider.h 2015-06-26 08:28:45 +0000
242@@ -63,7 +63,7 @@
243 MOCK_METHOD0(stop_velocity_updates, void());
244
245 MOCK_CONST_METHOD0(has_delayed_boot, bool());
246- MOCK_CONST_METHOD0(boot_state, BootState());
247+ MOCK_METHOD0(boot_state, BootState());
248 MOCK_CONST_METHOD0(boot, void());
249
250

Subscribers

People subscribed via source and target branches