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

Proposed by Manuel de la Peña on 2015-06-15
Status: Needs review
Proposed branch: lp:~mandel/location-service/delayed-providers
Merge into: lp:location-service/trunk
Diff against target: 562 lines (+406/-9)
4 files modified
include/location_service/com/ubuntu/location/provider.h (+43/-1)
src/location_service/com/ubuntu/location/provider.cpp (+61/-8)
tests/controller_test.cpp (+213/-0)
tests/mock_delayed_provider.h (+89/-0)
To merge this branch: bzr merge lp:~mandel/location-service/delayed-providers
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2015-06-16
Jim Hodapp (community) code Needs Fixing on 2015-06-15
Alfonso Sanchez-Beato 2015-06-15 Approve on 2015-06-15
Review via email: mp+261944@code.launchpad.net

Commit message

Add the concept of a delayed providers. This will allow providers that are slow to start the chance to be started and later be connected accordingly.

Description of the change

Add the concept of a delayed providers. This will allow providers that are slow to start the chance to be started and later be connected accordingly.

To post a comment you must log in.

Please check inline comments.

review: Needs Fixing

See comment below

LGTM

review: Approve
Jim Hodapp (jhodapp) wrote :

Some issues to address, see comments inline below.

review: Needs Fixing (code)
206. By Manuel de la Peña on 2015-06-16

Made changes according to reviews.

Unmerged revisions

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

Made changes according to reviews.

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

Update docs.

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

Made chages as per review.

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

Added support for delayed providers.

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-01-21 20:13:28 +0000
3+++ include/location_service/com/ubuntu/location/provider.h 2015-06-16 08:14:07 +0000
4@@ -150,6 +150,8 @@
5 friend class Provider;
6 explicit Controller(Provider& instance);
7
8+ virtual void on_provider_booted(bool was_booted);
9+
10 private:
11 Provider& instance;
12 std::atomic<int> position_updates_counter;
13@@ -172,6 +174,20 @@
14 core::Signal<Update<std::set<SpaceVehicle>>> svs;
15 };
16
17+ /**
18+ * @brief Wraps al those sigals that are related to the boot of the provider.
19+ */
20+ struct Signals
21+ {
22+ core::Signal<bool> booted;
23+ };
24+
25+ enum class BootState {
26+ NOT_BOOTED,
27+ BOOTING,
28+ BOOTED
29+ };
30+
31 virtual ~Provider() = default;
32
33 Provider(const Provider&) = delete;
34@@ -189,6 +205,18 @@
35 virtual const Controller::Ptr& state_controller() const;
36
37 /**
38+ * @brief Returns the booted signal to track the booting state of the provider.
39+ * @return The signal that will be triggered once the provider has booted.
40+ */
41+ virtual core::Signal<bool>& booted_signal();
42+
43+ /**
44+ * @brief Returns the booted signal to track the booting state of the provider.
45+ * @return The signal that will be triggered once the provider has booted.
46+ */
47+ virtual const core::Signal<bool>& booted_signal() const;
48+
49+ /**
50 * @brief Checks if the provider supports a specific feature.
51 * @param f Feature to test for
52 * @return true iff the provider supports the feature.
53@@ -210,6 +238,18 @@
54 virtual bool matches_criteria(const Criteria& criteria);
55
56 /**
57+ * @brief States the booting state of the provider.
58+ * @return The boot state of the provider.
59+ */
60+ virtual BootState boot_state() const;
61+
62+ /**
63+ * @brief Indicates to the provider that it should start the boot process in order to be able to
64+ * be used by the engine.
65+ */
66+ virtual void boot();
67+
68+ /**
69 * @brief Called by the engine whenever the wifi and cell ID reporting state changes.
70 * @param state The new state.
71 */
72@@ -270,12 +310,14 @@
73 */
74 virtual void stop_velocity_updates();
75
76-private:
77+protected:
78+ // protected to allow better white box testing
79 struct
80 {
81 Features features = Features::none;
82 Requirements requirements = Requirements::none;
83 Updates updates;
84+ Signals signals;
85 Controller::Ptr controller = Controller::Ptr{};
86 } d;
87 };
88
89=== modified file 'src/location_service/com/ubuntu/location/provider.cpp'
90--- src/location_service/com/ubuntu/location/provider.cpp 2015-01-23 19:07:09 +0000
91+++ src/location_service/com/ubuntu/location/provider.cpp 2015-06-16 08:14:07 +0000
92@@ -16,9 +16,11 @@
93 * Authored by: Thomas Voß <thomas.voss@canonical.com>
94 */
95 #include <com/ubuntu/location/provider.h>
96+#include <com/ubuntu/location/logging.h>
97
98 #include <atomic>
99 #include <bitset>
100+#include <functional>
101 #include <memory>
102
103 namespace cul = com::ubuntu::location;
104@@ -51,12 +53,15 @@
105
106 void cul::Provider::Controller::start_position_updates()
107 {
108- if (position_updates_counter < 0)
109+ if (position_updates_counter < 0) {
110 return;
111+ }
112
113 if (++position_updates_counter == 1)
114 {
115- instance.start_position_updates();
116+ if (instance.boot_state() == BootState::BOOTED) {
117+ instance.start_position_updates();
118+ }
119 }
120 }
121
122@@ -67,7 +72,8 @@
123
124 if (--position_updates_counter == 0)
125 {
126- instance.stop_position_updates();
127+ if (instance.boot_state() == BootState::BOOTED)
128+ instance.stop_position_updates();
129 }
130 }
131
132@@ -83,7 +89,8 @@
133
134 if (++heading_updates_counter == 1)
135 {
136- instance.start_heading_updates();
137+ if (instance.boot_state() == BootState::BOOTED)
138+ instance.start_heading_updates();
139 }
140 }
141
142@@ -94,7 +101,8 @@
143
144 if (--heading_updates_counter == 0)
145 {
146- instance.stop_heading_updates();
147+ if (instance.boot_state() == BootState::BOOTED)
148+ instance.stop_heading_updates();
149 }
150 }
151
152@@ -110,7 +118,8 @@
153
154 if (++velocity_updates_counter == 1)
155 {
156- instance.start_velocity_updates();
157+ if (instance.boot_state() == BootState::BOOTED)
158+ instance.start_velocity_updates();
159 }
160 }
161
162@@ -121,7 +130,8 @@
163
164 if (--velocity_updates_counter == 0)
165 {
166- instance.stop_velocity_updates();
167+ if (instance.boot_state() == BootState::BOOTED)
168+ instance.stop_velocity_updates();
169 }
170 }
171
172@@ -130,12 +140,38 @@
173 return velocity_updates_counter > 0;
174 }
175
176+void cul::Provider::Controller::on_provider_booted(bool was_booted)
177+{
178+ if (!was_booted)
179+ return;
180+
181+ // we need to propagate the state of the provider using the internal counters as references
182+ if (position_updates_counter > 0) {
183+ instance.start_position_updates();
184+ }
185+ if (heading_updates_counter > 0) {
186+ instance.start_heading_updates();
187+ }
188+ if (velocity_updates_counter > 0) {
189+ instance.start_velocity_updates();
190+ }
191+}
192+
193 cul::Provider::Controller::Controller(cul::Provider& instance)
194 : instance(instance),
195 position_updates_counter(0),
196 heading_updates_counter(0),
197 velocity_updates_counter(0)
198-{
199+{
200+ using namespace std::placeholders;
201+
202+ if (instance.boot_state() != BootState::BOOTED) {
203+ // if the instance has not booted and is delayed we need to keep track of its booting state
204+ instance.booted_signal().connect(std::bind(&Controller::on_provider_booted, this, _1));
205+ if (instance.boot_state() != BootState::BOOTING) {
206+ instance.boot();
207+ }
208+ }
209 }
210
211 const cul::Provider::Controller::Ptr& cul::Provider::state_controller() const
212@@ -158,11 +194,28 @@
213 return false;
214 }
215
216+cul::Provider::BootState cul::Provider::boot_state() const
217+{
218+ return cul::Provider::BootState::BOOTED;
219+}
220+
221+void cul::Provider::boot() { }
222+
223 const cul::Provider::Updates& cul::Provider::updates() const
224 {
225 return d.updates;
226 }
227
228+core::Signal<bool>& cul::Provider::booted_signal()
229+{
230+ return d.signals.booted;
231+}
232+
233+const core::Signal<bool>& cul::Provider::booted_signal() const
234+{
235+ return d.signals.booted;
236+}
237+
238 cul::Provider::Provider(
239 const cul::Provider::Features& features,
240 const cul::Provider::Requirements& requirements)
241
242=== modified file 'tests/controller_test.cpp'
243--- tests/controller_test.cpp 2014-09-10 19:34:09 +0000
244+++ tests/controller_test.cpp 2015-06-16 08:14:07 +0000
245@@ -18,6 +18,7 @@
246 #include <com/ubuntu/location/provider.h>
247
248 #include "mock_provider.h"
249+#include "mock_delayed_provider.h"
250
251 #include <gmock/gmock.h>
252 #include <gtest/gtest.h>
253@@ -130,3 +131,215 @@
254 EXPECT_FALSE(controller->are_velocity_updates_running());
255 EXPECT_FALSE(controller->are_heading_updates_running());
256 }
257+
258+TEST(Controller, controller_resumes_state_after_boot) {
259+ using namespace ::testing;
260+ using ::testing::Mock;
261+
262+ MockDelayedProvider provider;
263+
264+ // ignore what ever was performed on construction
265+ Mock::VerifyAndClear(&provider);
266+
267+}
268+
269+TEST(Controller, controller_does_not_call_start_position_if_not_booted) {
270+ using namespace ::testing;
271+ using ::testing::Mock;
272+
273+ MockDelayedProvider provider;
274+
275+ // ignore what ever was performed on construction
276+ Mock::VerifyAndClear(&provider);
277+ // use a mock controller
278+ auto controller = provider.state_controller();
279+
280+ EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
281+ .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
282+ EXPECT_CALL(provider, start_position_updates()).Times(Exactly(0));
283+
284+ controller->start_position_updates();
285+ controller->start_position_updates();
286+}
287+
288+TEST(Controller, controller_does_not_call_start_position_if_booting) {
289+ using namespace ::testing;
290+ using ::testing::Mock;
291+
292+ MockDelayedProvider provider;
293+
294+ // ignore what ever was performed on construction
295+ Mock::VerifyAndClear(&provider);
296+ // use a mock controller
297+ auto controller = provider.state_controller();
298+
299+ EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
300+ .WillOnce(Return(cul::Provider::BootState::BOOTING));
301+ EXPECT_CALL(provider, start_position_updates()).Times(Exactly(0));
302+
303+ controller->start_position_updates();
304+}
305+
306+TEST(Controller, controller_does_not_call_start_heading_if_not_booted) {
307+ using namespace ::testing;
308+ using ::testing::Mock;
309+
310+ MockDelayedProvider provider;
311+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
312+
313+ // ignore what ever was performed on construction
314+ Mock::VerifyAndClear(&provider);
315+ // use a mock controller
316+ auto controller = provider.state_controller();
317+
318+ EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
319+ .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
320+ EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
321+
322+ controller->start_heading_updates();
323+}
324+
325+TEST(Controller, controller_does_not_call_start_heading_if_booting) {
326+ using namespace ::testing;
327+ using ::testing::Mock;
328+
329+ MockDelayedProvider provider;
330+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
331+
332+ // ignore what ever was performed on construction
333+ Mock::VerifyAndClear(&provider);
334+ // use a mock controller
335+ auto controller = provider.state_controller();
336+
337+ EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
338+ .WillOnce(Return(cul::Provider::BootState::BOOTING));
339+ EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
340+
341+ controller->start_heading_updates();
342+}
343+
344+TEST(Controller, controller_does_not_call_start_velocity_if_not_booted) {
345+ using namespace ::testing;
346+ using ::testing::Mock;
347+
348+ MockDelayedProvider provider;
349+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
350+
351+ // ignore what ever was performed on construction
352+ Mock::VerifyAndClear(&provider);
353+ // use a mock controller
354+ auto controller = provider.state_controller();
355+
356+ EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
357+ .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
358+ EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
359+
360+ controller->start_velocity_updates();
361+}
362+
363+TEST(Controller, controller_does_not_call_start_velocity_if_booting) {
364+ using namespace ::testing;
365+ using ::testing::Mock;
366+
367+ MockDelayedProvider provider;
368+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
369+
370+ // ignore what ever was performed on construction
371+ Mock::VerifyAndClear(&provider);
372+ // use a mock controller
373+ auto controller = provider.state_controller();
374+
375+ EXPECT_CALL(provider, boot_state()).Times(Exactly(1))
376+ .WillOnce(Return(cul::Provider::BootState::BOOTING));
377+ EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(0));
378+
379+ controller->start_velocity_updates();
380+}
381+
382+TEST(Controller, controller_does_not_call_stop_position_if_not_booted) {
383+ using namespace ::testing;
384+ using ::testing::Mock;
385+
386+ MockDelayedProvider provider;
387+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
388+
389+ // ignore what ever was performed on construction
390+ Mock::VerifyAndClear(&provider);
391+ // use a mock controller
392+ auto controller = provider.state_controller();
393+
394+ EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
395+ .WillOnce(Return(cul::Provider::BootState::BOOTED))
396+ .WillOnce(Return(cul::Provider::BootState::NOT_BOOTED));
397+ EXPECT_CALL(provider, start_position_updates()).Times(Exactly(1));
398+ EXPECT_CALL(provider, stop_position_updates()).Times(Exactly(0));
399+
400+ controller->start_position_updates();
401+ controller->stop_position_updates();
402+}
403+
404+TEST(Controller, controller_does_not_call_stop_position_if_booting) {
405+ using namespace ::testing;
406+ using ::testing::Mock;
407+
408+ MockDelayedProvider provider;
409+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
410+
411+ // ignore what ever was performed on construction
412+ Mock::VerifyAndClear(&provider);
413+ // use a mock controller
414+ auto controller = provider.state_controller();
415+
416+ EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
417+ .WillOnce(Return(cul::Provider::BootState::BOOTED))
418+ .WillOnce(Return(cul::Provider::BootState::BOOTING));
419+ EXPECT_CALL(provider, start_position_updates()).Times(Exactly(1));
420+ EXPECT_CALL(provider, stop_position_updates()).Times(Exactly(0));
421+
422+ controller->start_position_updates();
423+ controller->stop_position_updates();
424+}
425+
426+TEST(Controller, controller_does_not_call_stop_heading_if_not_booted) {
427+ using namespace ::testing;
428+ using ::testing::Mock;
429+
430+ MockDelayedProvider provider;
431+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
432+
433+ // ignore what ever was performed on construction
434+ Mock::VerifyAndClear(&provider);
435+ // use a mock controller
436+ auto controller = provider.state_controller();
437+
438+ EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
439+ .WillOnce(Return(cul::Provider::BootState::BOOTED))
440+ .WillOnce(Return(cul::Provider::BootState::BOOTING));
441+ EXPECT_CALL(provider, start_heading_updates()).Times(Exactly(1));
442+ EXPECT_CALL(provider, stop_heading_updates()).Times(Exactly(0));
443+
444+ controller->start_heading_updates();
445+ controller->stop_heading_updates();
446+}
447+
448+TEST(Controller, controller_does_not_call_stop_velocity_if_not_booted) {
449+ using namespace ::testing;
450+ using ::testing::Mock;
451+
452+ MockDelayedProvider provider;
453+ EXPECT_CALL(provider, has_delayed_boot()).WillRepeatedly(Return(true));
454+
455+ // ignore what ever was performed on construction
456+ Mock::VerifyAndClear(&provider);
457+ // use a mock controller
458+ auto controller = provider.state_controller();
459+
460+ EXPECT_CALL(provider, boot_state()).Times(Exactly(2))
461+ .WillOnce(Return(cul::Provider::BootState::BOOTED))
462+ .WillOnce(Return(cul::Provider::BootState::BOOTING));
463+ EXPECT_CALL(provider, start_velocity_updates()).Times(Exactly(1));
464+ EXPECT_CALL(provider, stop_velocity_updates()).Times(Exactly(0));
465+
466+ controller->start_velocity_updates();
467+ controller->stop_velocity_updates();
468+}
469
470=== added file 'tests/mock_delayed_provider.h'
471--- tests/mock_delayed_provider.h 1970-01-01 00:00:00 +0000
472+++ tests/mock_delayed_provider.h 2015-06-16 08:14:07 +0000
473@@ -0,0 +1,89 @@
474+/*
475+ * Copyright © 2014 Canonical Ltd.
476+ *
477+ * This program is free software: you can redistribute it and/or modify it
478+ * under the terms of the GNU Lesser General Public License version 3,
479+ * as published by the Free Software Foundation.
480+ *
481+ * This program is distributed in the hope that it will be useful,
482+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
483+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
484+ * GNU Lesser General Public License for more details.
485+ *
486+ * You should have received a copy of the GNU Lesser General Public License
487+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
488+ *
489+ * Authored by: M
490+ */
491+#ifndef MOCK_DELAYED_PROVIDER_H_
492+#define MOCK_DELAYED_PROVIDER_H_
493+
494+#include <com/ubuntu/location/provider.h>
495+#include <com/ubuntu/location/logging.h>
496+
497+#include <gmock/gmock.h>
498+
499+struct PublicController : public com::ubuntu::location::Provider::Controller
500+{
501+
502+ explicit PublicController(com::ubuntu::location::Provider& instance)
503+ : com::ubuntu::location::Provider::Controller(instance) {}
504+};
505+
506+struct MockDelayedProvider : public com::ubuntu::location::Provider
507+{
508+ MockDelayedProvider() : com::ubuntu::location::Provider()
509+ {
510+ }
511+
512+ MOCK_METHOD1(matches_criteria, bool(const com::ubuntu::location::Criteria&));
513+
514+ MOCK_CONST_METHOD1(supports, bool(const com::ubuntu::location::Provider::Features&));
515+ MOCK_CONST_METHOD1(requires, bool(const com::ubuntu::location::Provider::Requirements&));
516+
517+ // Called by the engine whenever the wifi and cell ID reporting state changes.
518+ MOCK_METHOD1(on_wifi_and_cell_reporting_state_changed, void(com::ubuntu::location::WifiAndCellIdReportingState state));
519+
520+ // Called by the engine whenever the reference location changed.
521+ MOCK_METHOD1(on_reference_location_updated, void(const com::ubuntu::location::Update<com::ubuntu::location::Position>& position));
522+
523+ // Called by the engine whenever the reference velocity changed.
524+ MOCK_METHOD1(on_reference_velocity_updated, void(const com::ubuntu::location::Update<com::ubuntu::location::Velocity>& velocity));
525+
526+ // Called by the engine whenever the reference heading changed.
527+ MOCK_METHOD1(on_reference_heading_updated, void(const com::ubuntu::location::Update<com::ubuntu::location::Heading>& heading));
528+
529+ MOCK_METHOD0(start_position_updates, void());
530+ MOCK_METHOD0(stop_position_updates, void());
531+
532+ MOCK_METHOD0(start_heading_updates, void());
533+ MOCK_METHOD0(stop_heading_updates, void());
534+
535+ MOCK_METHOD0(start_velocity_updates, void());
536+ MOCK_METHOD0(stop_velocity_updates, void());
537+
538+ MOCK_CONST_METHOD0(has_delayed_boot, bool());
539+ MOCK_CONST_METHOD0(boot_state, BootState());
540+ MOCK_CONST_METHOD0(boot, void());
541+
542+
543+ // Inject a position update from the outside.
544+ void inject_update(const com::ubuntu::location::Update<com::ubuntu::location::Position>& update)
545+ {
546+ mutable_updates().position(update);
547+ }
548+
549+ // Inject a velocity update from the outside.
550+ void inject_update(const com::ubuntu::location::Update<com::ubuntu::location::Velocity>& update)
551+ {
552+ mutable_updates().velocity(update);
553+ }
554+
555+ // Inject a heading update from the outside.
556+ void inject_update(const com::ubuntu::location::Update<com::ubuntu::location::Heading>& update)
557+ {
558+ mutable_updates().heading(update);
559+ }
560+};
561+
562+#endif // MOCK_PROVIDER_H_

Subscribers

People subscribed via source and target branches