location-service

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

  1. espoo-delayed-provider
  2. Merge into trunk
Proposed by Manuel de la Peña on 2015-06-16
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 on 2015-06-26
Alfonso Sanchez-Beato 2015-06-16 Needs Fixing on 2015-06-18
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.
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Reply
lp:~mandel/location-service/espoo-delayed-provider updated on 2015-06-16
210. By Manuel de la Peña on 2015-06-16

Add extra logging to keep track of the remote daemon.

PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Reply
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

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

review: Needs Fixing
Reply
lp:~mandel/location-service/espoo-delayed-provider updated on 2015-06-24
211. By Manuel de la Peña on 2015-06-24

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

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Reply
lp:~mandel/location-service/espoo-delayed-provider updated on 2015-06-25
212. By Manuel de la Peña on 2015-06-25

Add some extra logging for debugging purposes.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Reply
lp:~mandel/location-service/espoo-delayed-provider updated on 2015-06-26
213. By Manuel de la Peña on 2015-06-26

Increase logging to know what the provider is not started.

PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Reply

Unmerged revisions

213. By Manuel de la Peña on 2015-06-26

Increase logging to know what the provider is not started.

212. By Manuel de la Peña on 2015-06-25

Add some extra logging for debugging purposes.

211. By Manuel de la Peña on 2015-06-24

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

210. By Manuel de la Peña on 2015-06-16

Add extra logging to keep track of the remote daemon.

209. By Manuel de la Peña on 2015-06-16

Use the new method to get the signal.

208. By Manuel de la Peña on 2015-06-16

Merged delayed-providers into espoo-delayed-provider.

207. By Manuel de la Peña on 2015-06-16

Ensure that we connect to the booted signal.

206. By Manuel de la Peña on 2015-06-15

Merged delayed-providers into espoo-delayed-provider.

205. By Manuel de la Peña on 2015-06-15

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

204. By Manuel de la Peña on 2015-06-15

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