Merge lp:~mandel/location-service/tvoss-time-delta-fixes into lp:location-service/trunk

Proposed by Manuel de la Peña on 2015-04-27
Status: Approved
Approved by: Thomas Voß on 2015-05-04
Approved revision: 197
Proposed branch: lp:~mandel/location-service/tvoss-time-delta-fixes
Merge into: lp:location-service/trunk
Prerequisite: lp:~mandel/location-service/verbose-logging
Diff against target: 817 lines (+256/-212)
14 files modified
examples/service/service.cpp (+1/-1)
include/location_service/com/ubuntu/location/service/configuration.h (+4/-1)
include/location_service/com/ubuntu/location/update.h (+29/-1)
src/location_service/com/ubuntu/location/engine.cpp (+7/-1)
src/location_service/com/ubuntu/location/engine.h (+2/-2)
src/location_service/com/ubuntu/location/service/daemon.cpp (+1/-1)
src/location_service/com/ubuntu/location/service/default_configuration.cpp (+8/-1)
src/location_service/com/ubuntu/location/service/default_configuration.h (+4/-0)
src/location_service/com/ubuntu/location/time_based_update_policy.cpp (+52/-65)
src/location_service/com/ubuntu/location/time_based_update_policy.h (+18/-17)
src/location_service/com/ubuntu/location/update_policy.h (+5/-17)
tests/acceptance_tests.cpp (+5/-5)
tests/engine_test.cpp (+66/-8)
tests/time_based_update_policy_test.cpp (+54/-92)
To merge this branch: bzr merge lp:~mandel/location-service/tvoss-time-delta-fixes
Reviewer Review Type Date Requested Status
Thomas Voß (community) Approve on 2015-05-04
PS Jenkins bot continuous-integration Needs Fixing on 2015-04-29
Jim Hodapp (community) code 2015-04-27 Approve on 2015-04-27
Review via email: mp+257573@code.launchpad.net

Commit message

Improve heuristics and add them as parts of the config.

Description of the change

Improve the heuristics code and add it as part of the config.

To post a comment you must log in.
Jim Hodapp (jhodapp) wrote :

One minor change inline below.

review: Needs Fixing (code)
195. By Manuel de la Peña on 2015-04-27

Added code according to the reviews.

Jim Hodapp (jhodapp) wrote :

Looks good.

review: Approve (code)
196. By Manuel de la Peña on 2015-04-29

Merged verbose-logging into tvoss-time-delta-fixes.

197. By Manuel de la Peña on 2015-04-29

Merged verbose-logging into tvoss-time-delta-fixes.

Thomas Voß (thomas-voss) wrote :

LGTM.

review: Approve
198. By Manuel de la Peña on 2015-05-26

Merged verbose-logging into tvoss-time-delta-fixes.

Unmerged revisions

198. By Manuel de la Peña on 2015-05-26

Merged verbose-logging into tvoss-time-delta-fixes.

197. By Manuel de la Peña on 2015-04-29

Merged verbose-logging into tvoss-time-delta-fixes.

196. By Manuel de la Peña on 2015-04-29

Merged verbose-logging into tvoss-time-delta-fixes.

195. By Manuel de la Peña on 2015-04-27

Added code according to the reviews.

194. By Manuel de la Peña on 2015-04-27

Merged verbose-logging into tvoss-time-delta-fixes.

193. By Manuel de la Peña on 2015-04-27

Merged verbose-logging into tvoss-time-delta-fixes.

192. By Manuel de la Peña on 2015-04-27

Merge with tvoss fixes.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/service/service.cpp'
2--- examples/service/service.cpp 2014-11-14 11:26:45 +0000
3+++ examples/service/service.cpp 2015-05-26 16:07:17 +0000
4@@ -171,7 +171,7 @@
5 {
6 incoming,
7 outgoing,
8- config.the_engine(instantiated_providers, config.the_provider_selection_policy(), settings),
9+ config.the_engine(instantiated_providers, config.the_provider_selection_policy(), config.the_update_policy(), settings),
10 config.the_permission_manager(incoming),
11 culs::Harvester::Configuration
12 {
13
14=== modified file 'include/location_service/com/ubuntu/location/service/configuration.h'
15--- include/location_service/com/ubuntu/location/service/configuration.h 2014-11-14 11:33:05 +0000
16+++ include/location_service/com/ubuntu/location/service/configuration.h 2015-05-26 16:07:17 +0000
17@@ -22,6 +22,7 @@
18
19 #include <com/ubuntu/location/provider.h>
20 #include <com/ubuntu/location/provider_selection_policy.h>
21+#include <com/ubuntu/location/update_policy.h>
22
23 #include <set>
24
25@@ -43,9 +44,11 @@
26
27 virtual std::shared_ptr<Engine> the_engine(
28 const std::set<Provider::Ptr>& provider_set,
29- const ProviderSelectionPolicy::Ptr& provider_selection_policy) = 0;
30+ const ProviderSelectionPolicy::Ptr& provider_selection_policy,
31+ const UpdatePolicy::Ptr& update_policy) = 0;
32
33 virtual ProviderSelectionPolicy::Ptr the_provider_selection_policy() = 0;
34+ virtual UpdatePolicy::Ptr the_update_policy() = 0;
35 virtual std::set<Provider::Ptr> the_provider_set() = 0;
36 virtual PermissionManager::Ptr the_permission_manager() = 0;
37
38
39=== modified file 'include/location_service/com/ubuntu/location/update.h'
40--- include/location_service/com/ubuntu/location/update.h 2013-12-10 09:42:54 +0000
41+++ include/location_service/com/ubuntu/location/update.h 2015-05-26 16:07:17 +0000
42@@ -20,6 +20,7 @@
43
44 #include <com/ubuntu/location/clock.h>
45
46+#include <chrono>
47 #include <ostream>
48
49 namespace com
50@@ -41,7 +42,7 @@
51 * @param [in] when The timestamp when the value was measured.
52 */
53 inline Update(const T& value = T{},
54- const Clock::Timestamp& when = Clock::now())
55+ const Clock::Timestamp& when = Clock::Timestamp())
56 : value{value}, when{when}
57 {
58 }
59@@ -73,6 +74,33 @@
60 Clock::Timestamp when = Clock::beginning_of_time();
61 };
62
63+/** @brief TimeDifference enumerates all possible results of comparing two updates. */
64+enum class TimeDifference
65+{
66+ none, ///< There is no time difference.
67+ older, ///< The update is older than the reference update.
68+ newer ///< The update is newer than the reference update.
69+};
70+
71+/**
72+ * @brief compare_update_timestamps returns a classification of the time delta between two updates.
73+ * @returns TimeDifference::older if update - last_update exceeds limit.
74+ * @returns TimeDifference::newer if -(update - last-update) exceeds limit.
75+ * @returns TimeDifference::none if neither of the previous conditions holds true.
76+ */
77+template <typename T>
78+TimeDifference compare_update_timestamps(const location::Update<T> last_update, const location::Update<T> update, const std::chrono::minutes& limit)
79+{
80+ auto const delta = update.when - last_update.when;
81+ if (delta > limit)
82+ return TimeDifference::newer;
83+
84+ if (delta < (-1 * limit))
85+ return TimeDifference::older;
86+
87+ return TimeDifference::none;
88+}
89+
90 /**
91 * @brief Pretty-prints the update to the provided output stream.
92 * @param out The stream to write to.
93
94=== modified file 'src/location_service/com/ubuntu/location/engine.cpp'
95--- src/location_service/com/ubuntu/location/engine.cpp 2015-04-23 14:48:44 +0000
96+++ src/location_service/com/ubuntu/location/engine.cpp 2015-05-26 16:07:17 +0000
97@@ -33,16 +33,22 @@
98 const cul::Engine::Status cul::Engine::Configuration::Defaults::engine_state;
99
100 cul::Engine::Engine(const cul::ProviderSelectionPolicy::Ptr& provider_selection_policy,
101+ const cul::UpdatePolicy::Ptr& update_policy,
102 const cul::Settings::Ptr& settings)
103 : provider_selection_policy(provider_selection_policy),
104 settings(settings),
105- update_policy(std::make_shared<cul::TimeBasedUpdatePolicy>())
106+ update_policy(update_policy)
107 {
108 if (!provider_selection_policy) throw std::runtime_error
109 {
110 "Cannot construct an engine given a null ProviderSelectionPolicy"
111 };
112
113+ if (!update_policy) throw std::runtime_error
114+ {
115+ "Cannot construct an engine given a null UpdatePolicy"
116+ };
117+
118 if (!settings) throw std::runtime_error
119 {
120 "Cannot construct an engine given a null Settings instance"
121
122=== modified file 'src/location_service/com/ubuntu/location/engine.h'
123--- src/location_service/com/ubuntu/location/engine.h 2015-04-23 14:48:44 +0000
124+++ src/location_service/com/ubuntu/location/engine.h 2015-05-26 16:07:17 +0000
125@@ -24,6 +24,7 @@
126 #include <com/ubuntu/location/provider_selection_policy.h>
127 #include <com/ubuntu/location/satellite_based_positioning_state.h>
128 #include <com/ubuntu/location/space_vehicle.h>
129+#include <com/ubuntu/location/update_policy.h>
130 #include <com/ubuntu/location/wifi_and_cell_reporting_state.h>
131
132 #include <com/ubuntu/location/settings.h>
133@@ -33,8 +34,6 @@
134 #include <mutex>
135 #include <set>
136
137-#include "update_policy.h"
138-
139 namespace com
140 {
141 namespace ubuntu
142@@ -137,6 +136,7 @@
143 };
144
145 Engine(const ProviderSelectionPolicy::Ptr& provider_selection_policy,
146+ const UpdatePolicy::Ptr& update_policy,
147 const Settings::Ptr& settings);
148
149 Engine(const Engine&) = delete;
150
151=== modified file 'src/location_service/com/ubuntu/location/service/daemon.cpp'
152--- src/location_service/com/ubuntu/location/service/daemon.cpp 2015-05-26 16:07:17 +0000
153+++ src/location_service/com/ubuntu/location/service/daemon.cpp 2015-05-26 16:07:17 +0000
154@@ -211,7 +211,7 @@
155 {
156 config.incoming,
157 config.outgoing,
158- dc.the_engine(instantiated_providers, dc.the_provider_selection_policy(), config.settings),
159+ dc.the_engine(instantiated_providers, dc.the_provider_selection_policy(), dc.the_update_policy(), config.settings),
160 dc.the_permission_manager(config.outgoing),
161 location::service::Harvester::Configuration
162 {
163
164=== modified file 'src/location_service/com/ubuntu/location/service/default_configuration.cpp'
165--- src/location_service/com/ubuntu/location/service/default_configuration.cpp 2014-11-14 11:33:05 +0000
166+++ src/location_service/com/ubuntu/location/service/default_configuration.cpp 2015-05-26 16:07:17 +0000
167@@ -21,6 +21,7 @@
168
169 #include <com/ubuntu/location/engine.h>
170 #include <com/ubuntu/location/non_selecting_provider_selection_policy.h>
171+#include <com/ubuntu/location/time_based_update_policy.h>
172
173 namespace cul = com::ubuntu::location;
174 namespace culs = com::ubuntu::location::service;
175@@ -28,9 +29,10 @@
176 cul::Engine::Ptr culs::DefaultConfiguration::the_engine(
177 const std::set<cul::Provider::Ptr>& provider_set,
178 const cul::ProviderSelectionPolicy::Ptr& provider_selection_policy,
179+ const cul::UpdatePolicy::Ptr& update_policy,
180 const cul::Settings::Ptr& settings)
181 {
182- auto engine = std::make_shared<cul::Engine>(provider_selection_policy, settings);
183+ auto engine = std::make_shared<cul::Engine>(provider_selection_policy, update_policy, settings);
184 for (const auto& provider : provider_set)
185 engine->add_provider(provider);
186
187@@ -42,6 +44,11 @@
188 return std::make_shared<cul::NonSelectingProviderSelectionPolicy>();
189 }
190
191+cul::UpdatePolicy::Ptr culs::DefaultConfiguration::the_update_policy()
192+{
193+ return std::make_shared<cul::TimeBasedUpdatePolicy>();
194+}
195+
196 std::set<cul::Provider::Ptr> culs::DefaultConfiguration::the_provider_set(
197 const cul::Provider::Ptr& seed)
198 {
199
200=== modified file 'src/location_service/com/ubuntu/location/service/default_configuration.h'
201--- src/location_service/com/ubuntu/location/service/default_configuration.h 2014-11-14 11:33:05 +0000
202+++ src/location_service/com/ubuntu/location/service/default_configuration.h 2015-05-26 16:07:17 +0000
203@@ -45,6 +45,9 @@
204 // as specified by a client application.
205 virtual ProviderSelectionPolicy::Ptr the_provider_selection_policy();
206
207+ // Creates an update policy instance to filter incoming updates from providers.
208+ virtual UpdatePolicy::Ptr the_update_policy();
209+
210 // Returns a set of providers, seeded with the seed provider if it is not null.
211 virtual std::set<Provider::Ptr> the_provider_set(const Provider::Ptr& seed = Provider::Ptr {});
212
213@@ -53,6 +56,7 @@
214 virtual std::shared_ptr<Engine> the_engine(
215 const std::set<Provider::Ptr>& provider_set,
216 const ProviderSelectionPolicy::Ptr& provider_selection_policy,
217+ const UpdatePolicy::Ptr& update_policy,
218 const Settings::Ptr& settings);
219
220 // Instantiates an instance of the permission manager.
221
222=== modified file 'src/location_service/com/ubuntu/location/time_based_update_policy.cpp'
223--- src/location_service/com/ubuntu/location/time_based_update_policy.cpp 2015-04-23 21:30:04 +0000
224+++ src/location_service/com/ubuntu/location/time_based_update_policy.cpp 2015-05-26 16:07:17 +0000
225@@ -31,91 +31,78 @@
226 return default_limit;
227 }
228
229-TimeBasedUpdatePolicy::TimeBasedUpdatePolicy(std::chrono::minutes mins)
230+TimeBasedUpdatePolicy::TimeBasedUpdatePolicy(const std::chrono::minutes& mins)
231 : limit(mins)
232 {
233
234 }
235
236-const location::Update<location::Position>& TimeBasedUpdatePolicy::verify_update(const location::Update<location::Position>& update)
237+location::Update<location::Position> TimeBasedUpdatePolicy::verify_update(const location::Update<location::Position>& update)
238 {
239 std::lock_guard<std::mutex> guard(position_update_mutex);
240- bool use_new_update;
241- if (is_significantly_newer(last_position_update, update, limit))
242- {
243- use_new_update = true;
244- }
245- else if (is_significantly_older(last_position_update, update, limit))
246- {
247- use_new_update = false;
248- }
249- else
250- {
251- // if the update has happened within a reasonable amount of time we will just use it if it is more accurate
252- // that the previous one.
253- use_new_update = last_position_update.value.accuracy.horizontal && update.value.accuracy.horizontal
254- && *last_position_update.value.accuracy.horizontal >= *update.value.accuracy.horizontal;
255+ bool use_new_update{false};
256+ switch (compare_update_timestamps(last_update.position, update, limit))
257+ {
258+ case location::TimeDifference::newer:
259+ use_new_update = true; break;
260+ case location::TimeDifference::older:
261+ use_new_update = false; break;
262+ case location::TimeDifference::none:
263+ // if the update has happened within a reasonable amount of time we will just use it if it is more accurate
264+ // that the previous one.
265+ use_new_update =
266+ (last_update.position.value.accuracy.horizontal && update.value.accuracy.horizontal) &&
267+ *last_update.position.value.accuracy.horizontal >= *update.value.accuracy.horizontal;
268+ break;
269 }
270
271 if (use_new_update)
272- {
273- last_position_update = update;
274- return update;
275- }
276- else
277- {
278- return last_position_update;
279- }
280+ last_update.position = update;
281+
282+ return last_update.position;
283 }
284
285
286-const location::Update<location::Heading>& TimeBasedUpdatePolicy::verify_update(const location::Update<location::Heading>& update)
287+location::Update<location::Heading> TimeBasedUpdatePolicy::verify_update(const location::Update<location::Heading>& update)
288 {
289 std::lock_guard<std::mutex> guard(heading_update_mutex);
290- bool use_new_update;
291- if (is_significantly_newer(last_heading_update, update, limit))
292- {
293- use_new_update = true;
294- }
295- else if (is_significantly_older(last_heading_update, update, limit))
296- {
297- use_new_update = false;
298- }
299+ bool use_new_update{false};
300+ switch (compare_update_timestamps(last_update.heading, update, limit))
301+ {
302+ case location::TimeDifference::newer:
303+ use_new_update = true; break;
304+ case location::TimeDifference::older:
305+ use_new_update = false; break;
306+ case location::TimeDifference::none:
307+ use_new_update = false; break;
308+ }
309+
310 if (use_new_update)
311- {
312- last_heading_update = update;
313- return update;
314- }
315- else
316- {
317- return last_heading_update;
318- }
319+ last_update.heading = update;
320+
321+ return last_update.heading;
322 }
323
324-const location::Update<location::Velocity>& TimeBasedUpdatePolicy::verify_update(const location::Update<location::Velocity>& update)
325+location::Update<location::Velocity> TimeBasedUpdatePolicy::verify_update(const location::Update<location::Velocity>& update)
326 {
327 std::lock_guard<std::mutex> guard(velocity_update_mutex);
328- bool use_new_update;
329- if (is_significantly_newer(last_velocity_update, update, limit))
330- {
331- use_new_update = true;
332- }
333- else if (is_significantly_older(last_velocity_update, update, limit))
334- {
335- use_new_update = false;
336+ bool use_new_update{false};
337+ switch (compare_update_timestamps(last_update.velocity, update, limit))
338+ {
339+ case location::TimeDifference::newer:
340+ use_new_update = true; break;
341+ case location::TimeDifference::older:
342+ use_new_update = false; break;
343+ case location::TimeDifference::none:
344+ use_new_update = false; break;
345 }
346
347 if (use_new_update)
348- {
349- last_velocity_update = update;
350- return update;
351- }
352- else
353- {
354- return last_velocity_update;
355- }
356-}
357-
358-}
359-}
360-}
361\ No newline at end of file
362+ last_update.velocity = update;
363+
364+ return last_update.velocity;
365+}
366+
367+}
368+}
369+}
370
371=== modified file 'src/location_service/com/ubuntu/location/time_based_update_policy.h'
372--- src/location_service/com/ubuntu/location/time_based_update_policy.h 2015-04-23 21:14:50 +0000
373+++ src/location_service/com/ubuntu/location/time_based_update_policy.h 2015-05-26 16:07:17 +0000
374@@ -33,32 +33,33 @@
375 // An interface that can be implemented to add heuristics on how heading, position or velocity updates will be chosen.
376 // This class ensures that the best update possible is chosen within a reasonable time bracket.
377 class TimeBasedUpdatePolicy : public UpdatePolicy {
378+public:
379+ // Returns the default timeout value for the policy.
380+ static std::chrono::minutes default_timeout();
381
382- public:
383- TimeBasedUpdatePolicy(std::chrono::minutes mins=default_timeout());
384- TimeBasedUpdatePolicy(const TimeBasedUpdatePolicy&) = delete;
385- ~TimeBasedUpdatePolicy() = default;
386+ TimeBasedUpdatePolicy(const std::chrono::minutes& mins=default_timeout());
387
388 // Return if the given position update will be verified as the new position in the engine.
389- const location::Update<location::Position>& verify_update(const location::Update<location::Position>& update) override;
390+ location::Update<location::Position> verify_update(const location::Update<location::Position>& update);
391 // Return if the given heading update will be verified as the new heading in the engine.
392- const location::Update<location::Heading>& verify_update(const location::Update<location::Heading>& update) override;
393+ location::Update<location::Heading> verify_update(const location::Update<location::Heading>& update);
394 // Return if the given velocity update will be verified as the new velocity in the engine.
395- const location::Update<location::Velocity>& verify_update(const location::Update<location::Velocity>& update) override;
396-
397- static std::chrono::minutes default_timeout();
398-
399- protected:
400- // not private to simplify the testing but should be private
401- location::Update<location::Position> last_position_update;
402- location::Update<location::Heading> last_heading_update;
403- location::Update<location::Velocity> last_velocity_update;
404-
405- private:
406+ location::Update<location::Velocity> verify_update(const location::Update<location::Velocity>& update);
407+
408+private:
409 // callbacks can happen in diff threads, make sure multi-threading will work in this class
410 std::mutex position_update_mutex;
411 std::mutex heading_update_mutex;
412 std::mutex velocity_update_mutex;
413+
414+ // The updates we are tracking within the policy.
415+ struct
416+ {
417+ location::Update<location::Position> position;
418+ location::Update<location::Heading> heading;
419+ location::Update<location::Velocity> velocity;
420+ } last_update;
421+
422 // used to calculate the time accepted bracket
423 std::chrono::minutes limit;
424 };
425
426=== modified file 'src/location_service/com/ubuntu/location/update_policy.h'
427--- src/location_service/com/ubuntu/location/update_policy.h 2015-04-23 21:14:50 +0000
428+++ src/location_service/com/ubuntu/location/update_policy.h 2015-05-26 16:07:17 +0000
429@@ -42,30 +42,18 @@
430 UpdatePolicy(const UpdatePolicy&) = delete;
431 UpdatePolicy(UpdatePolicy&&) = delete;
432 UpdatePolicy& operator=(const UpdatePolicy&) = delete;
433+ UpdatePolicy& operator=(UpdatePolicy&&) = delete;
434 virtual ~UpdatePolicy() = default;
435
436 // Return if the given position update will be verified as the new position in the engine.
437- virtual const location::Update<location::Position>& verify_update(const location::Update<location::Position>& update) = 0;
438+ virtual location::Update<location::Position> verify_update(const location::Update<location::Position>& update) = 0;
439 // Return if the given heading update will be verified as the new heading in the engine.
440- virtual const location::Update<location::Heading>& verify_update(const location::Update<location::Heading>& update) = 0;
441+ virtual location::Update<location::Heading> verify_update(const location::Update<location::Heading>& update) = 0;
442 // Return if the given velocity update will be verified as the new velocity in the engine.
443- virtual const location::Update<location::Velocity>& verify_update(const location::Update<location::Velocity>& update) = 0;
444+ virtual location::Update<location::Velocity> verify_update(const location::Update<location::Velocity>& update) = 0;
445+
446 protected:
447 UpdatePolicy() = default;
448-
449- template <class T> bool is_significantly_newer(const location::Update<T> last_update, const location::Update<T> update, std::chrono::minutes limit) const
450- {
451- auto delta = update.when - last_update.when;
452- return delta > limit;
453- }
454-
455- template <class T> bool is_significantly_older(const location::Update<T> last_update, const location::Update<T> update, std::chrono::minutes limit) const
456- {
457- auto delta = update.when - last_update.when;
458- return delta < (-1 * limit);
459- }
460-
461-
462 };
463 }
464 }
465
466=== modified file 'tests/acceptance_tests.cpp'
467--- tests/acceptance_tests.cpp 2015-02-13 13:19:18 +0000
468+++ tests/acceptance_tests.cpp 2015-05-26 16:07:17 +0000
469@@ -240,7 +240,7 @@
470 {
471 incoming,
472 outgoing,
473- config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), null_settings()),
474+ config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), config.the_update_policy(), null_settings()),
475 config.the_permission_manager(incoming),
476 cul::service::Harvester::Configuration
477 {
478@@ -362,7 +362,7 @@
479 {
480 incoming,
481 outgoing,
482- config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), null_settings()),
483+ config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), config.the_update_policy(), null_settings()),
484 config.the_permission_manager(incoming),
485 cul::service::Harvester::Configuration
486 {
487@@ -443,7 +443,7 @@
488 {
489 incoming,
490 outgoing,
491- config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), null_settings()),
492+ config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), config.the_update_policy(), null_settings()),
493 config.the_permission_manager(incoming),
494 cul::service::Harvester::Configuration
495 {
496@@ -523,7 +523,7 @@
497 {
498 incoming,
499 outgoing,
500- config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), null_settings()),
501+ config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), config.the_update_policy(), null_settings()),
502 config.the_permission_manager(incoming),
503 cul::service::Harvester::Configuration
504 {
505@@ -611,7 +611,7 @@
506 {
507 incoming,
508 outgoing,
509- config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), null_settings()),
510+ config.the_engine(config.the_provider_set(helper), config.the_provider_selection_policy(), config.the_update_policy(), null_settings()),
511 config.the_permission_manager(incoming),
512 cul::service::Harvester::Configuration
513 {
514
515=== modified file 'tests/engine_test.cpp'
516--- tests/engine_test.cpp 2015-01-21 20:04:56 +0000
517+++ tests/engine_test.cpp 2015-05-26 16:07:17 +0000
518@@ -52,6 +52,10 @@
519 MOCK_METHOD1(on_reference_velocity_updated,
520 void(const location::Update<location::Velocity>&));
521
522+ void inject_position_update(const location::Update<location::Position>& update)
523+ {
524+ mutable_updates().position(update);
525+ }
526 };
527
528 struct MockSettings : public location::Settings
529@@ -70,11 +74,46 @@
530 {
531 return std::make_shared<::testing::NiceMock<MockSettings>>();
532 }
533+
534+struct NullUpdatePolicy : public location::UpdatePolicy
535+{
536+ location::Update<location::Position> verify_update(const location::Update<location::Position>& update)
537+ {
538+ return update;
539+ }
540+
541+ location::Update<location::Heading> verify_update(const location::Update<location::Heading>& update)
542+ {
543+ return update;
544+ }
545+
546+ location::Update<location::Velocity> verify_update(const location::Update<location::Velocity>& update)
547+ {
548+ return update;
549+ }
550+};
551+
552+struct MockUpdatePolicy : public location::UpdatePolicy
553+{
554+ MOCK_METHOD1(verify_update, location::Update<location::Position>(const location::Update<location::Position>&));
555+ MOCK_METHOD1(verify_update, location::Update<location::Heading>(const location::Update<location::Heading>&));
556+ MOCK_METHOD1(verify_update, location::Update<location::Velocity>(const location::Update<location::Velocity>&));
557+};
558+
559+const location::Update<location::Position> reference_position_update
560+{
561+ {
562+ location::wgs84::Latitude{9. * location::units::Degrees},
563+ location::wgs84::Longitude{53. * location::units::Degrees},
564+ location::wgs84::Altitude{-2. * location::units::Meters}
565+ },
566+ location::Clock::now()
567+};
568 }
569
570 TEST(Engine, adding_and_removing_providers_inserts_and_erases_from_underlying_collection)
571 {
572- location::Engine engine {std::make_shared<NullProviderSelectionPolicy>(), mock_settings()};
573+ location::Engine engine {std::make_shared<NullProviderSelectionPolicy>(), std::make_shared<NullUpdatePolicy>(), mock_settings()};
574
575 auto provider1 = std::make_shared<testing::NiceMock<MockProvider>>();
576 auto provider2 = std::make_shared<testing::NiceMock<MockProvider>>();
577@@ -94,7 +133,7 @@
578
579 TEST(Engine, adding_a_null_provider_throws)
580 {
581- location::Engine engine {std::make_shared<NullProviderSelectionPolicy>(), mock_settings()};
582+ location::Engine engine {std::make_shared<NullProviderSelectionPolicy>(), std::make_shared<NullUpdatePolicy>(), mock_settings()};
583
584 EXPECT_ANY_THROW(engine.add_provider(location::Provider::Ptr {}););
585 }
586@@ -126,6 +165,7 @@
587 &policy,
588 [](location::ProviderSelectionPolicy*) {}
589 },
590+ std::make_shared<NullUpdatePolicy>(),
591 mock_settings()
592 };
593
594@@ -144,7 +184,7 @@
595 using namespace ::testing;
596 auto provider = std::make_shared<NiceMock<MockProvider>>();
597 auto selection_policy = std::make_shared<NiceMock<MockProviderSelectionPolicy>>();
598- location::Engine engine{selection_policy, mock_settings()};
599+ location::Engine engine{selection_policy, std::make_shared<NullUpdatePolicy>(), mock_settings()};
600 engine.add_provider(provider);
601
602 EXPECT_CALL(*provider, on_wifi_and_cell_reporting_state_changed(_)).Times(1);
603@@ -154,9 +194,27 @@
604 EXPECT_CALL(*provider, on_reference_velocity_updated(_)).Times(1);
605
606 engine.configuration.wifi_and_cell_id_reporting_state = location::WifiAndCellIdReportingState::on;
607- engine.updates.reference_location = location::Update<location::Position>{};
608- engine.updates.reference_heading = location::Update<location::Heading>{};
609- engine.updates.reference_velocity = location::Update<location::Velocity>{};
610+ engine.updates.reference_location = location::Update<location::Position>{location::Position{}, location::Clock::now()};
611+ engine.updates.reference_heading = location::Update<location::Heading>{location::Heading{}, location::Clock::now()};
612+ engine.updates.reference_velocity = location::Update<location::Velocity>{location::Velocity{}, location::Clock::now()};
613+}
614+
615+TEST(Engine, a_provider_position_update_is_passed_through_the_update_policy)
616+{
617+ using namespace ::testing;
618+
619+ auto provider = std::make_shared<NiceMock<MockProvider>>();
620+ auto selection_policy = std::make_shared<NiceMock<MockProviderSelectionPolicy>>();
621+ auto update_policy = std::make_shared<NiceMock<MockUpdatePolicy>>();
622+
623+ EXPECT_CALL(*update_policy, verify_update(reference_position_update))
624+ .Times(1)
625+ .WillRepeatedly(Return(reference_position_update));
626+
627+ location::Engine engine{selection_policy, update_policy, mock_settings()};
628+ engine.add_provider(provider);
629+
630+ provider->inject_position_update(reference_position_update);
631 }
632
633 /* TODO(tvoss): We have to disable these tests as the MP is being refactored to not break ABI.
634@@ -288,7 +346,7 @@
635 .Times(1)
636 .WillRepeatedly(Return(ss_engine_state.str()));
637
638- location::Engine engine{selection_policy, settings};
639+ location::Engine engine{selection_policy, std::make_shared<NullUpdatePolicy>(), settings};
640 }
641
642 TEST(Engine, stores_state_from_settings_on_destruction)
643@@ -314,6 +372,6 @@
644 .Times(1)
645 .WillRepeatedly(Return(true));
646
647- {location::Engine engine{selection_policy, settings};}
648+ {location::Engine engine{selection_policy, std::make_shared<NullUpdatePolicy>(), settings};}
649 }
650
651
652=== modified file 'tests/time_based_update_policy_test.cpp'
653--- tests/time_based_update_policy_test.cpp 2015-04-23 17:04:09 +0000
654+++ tests/time_based_update_policy_test.cpp 2015-05-26 16:07:17 +0000
655@@ -24,108 +24,70 @@
656
657 namespace
658 {
659- auto timestamp = com::ubuntu::location::Clock::now();
660-
661- com::ubuntu::location::Update<com::ubuntu::location::Position> reference_position_update
662- {
663- {
664- com::ubuntu::location::wgs84::Latitude{9. * com::ubuntu::location::units::Degrees},
665- com::ubuntu::location::wgs84::Longitude{53. * com::ubuntu::location::units::Degrees},
666- com::ubuntu::location::wgs84::Altitude{-2. * com::ubuntu::location::units::Meters},
667- },
668- timestamp
669- };
670-}
671-
672-// make certain internal details public so that we can set the last update
673-class PublicTimeBasedUpdatePolicy : public cul::TimeBasedUpdatePolicy {
674- public:
675- PublicTimeBasedUpdatePolicy(std::chrono::minutes mins) : cul::TimeBasedUpdatePolicy(mins) {}
676- using cul::TimeBasedUpdatePolicy::last_position_update;
677- using cul::TimeBasedUpdatePolicy::last_heading_update;
678- using cul::TimeBasedUpdatePolicy::last_velocity_update;
679+const cul::Update<cul::Position> reference_position_update
680+{
681+ {
682+ cul::wgs84::Latitude{9. * cul::units::Degrees},
683+ cul::wgs84::Longitude{53. * cul::units::Degrees},
684+ cul::wgs84::Altitude{-2. * cul::units::Meters}
685+ },
686+ cul::Clock::now()
687 };
688+}
689
690 TEST(TimeBasedUpdatePolicy, policy_ignores_updates_that_are_too_old)
691 {
692- auto policy = std::make_shared<PublicTimeBasedUpdatePolicy>(std::chrono::minutes(2));
693- policy->last_position_update = reference_position_update;
694-
695- com::ubuntu::location::Update<com::ubuntu::location::Position> old_update
696- {
697- {
698- com::ubuntu::location::wgs84::Latitude{10. * com::ubuntu::location::units::Degrees},
699- com::ubuntu::location::wgs84::Longitude{60. * com::ubuntu::location::units::Degrees},
700- com::ubuntu::location::wgs84::Altitude{10. * com::ubuntu::location::units::Meters}
701- },
702- timestamp - std::chrono::minutes(5)
703- };
704- policy->verify_update(old_update);
705-
706- ASSERT_NE(policy->last_position_update.value.latitude, old_update.value.latitude);
707- ASSERT_EQ(policy->last_position_update.value.latitude, reference_position_update.value.latitude);
708-
709- ASSERT_NE(policy->last_position_update.value.longitude, old_update.value.longitude);
710- ASSERT_EQ(policy->last_position_update.value.longitude, reference_position_update.value.longitude);
711-
712- ASSERT_NE(policy->last_position_update.value.altitude, old_update.value.altitude);
713- ASSERT_EQ(policy->last_position_update.value.altitude, reference_position_update.value.altitude);
714+ auto policy = std::make_shared<cul::TimeBasedUpdatePolicy>(std::chrono::minutes(2));
715+ ASSERT_EQ(reference_position_update, policy->verify_update(reference_position_update));
716+
717+ cul::Update<cul::Position> old_update
718+ {
719+ {
720+ cul::wgs84::Latitude{10. * cul::units::Degrees},
721+ cul::wgs84::Longitude{60. * cul::units::Degrees},
722+ cul::wgs84::Altitude{10. * cul::units::Meters}
723+ },
724+ reference_position_update.when - std::chrono::minutes(5)
725+ };
726+ ASSERT_EQ(reference_position_update, policy->verify_update(old_update));
727 }
728
729 TEST(TimeBasedUpdatePolicy, policy_uses_very_recent_updates)
730 {
731- auto policy = std::make_shared<PublicTimeBasedUpdatePolicy>(std::chrono::minutes(2));
732-
733- policy->last_position_update = reference_position_update;
734-
735- com::ubuntu::location::Update<com::ubuntu::location::Position> new_update
736- {
737- {
738- com::ubuntu::location::wgs84::Latitude{10. * com::ubuntu::location::units::Degrees},
739- com::ubuntu::location::wgs84::Longitude{60. * com::ubuntu::location::units::Degrees},
740- com::ubuntu::location::wgs84::Altitude{10. * com::ubuntu::location::units::Meters}
741- },
742- timestamp + std::chrono::minutes(3)
743- };
744-
745- policy->verify_update(new_update);
746-
747- ASSERT_EQ(policy->last_position_update.value.latitude, new_update.value.latitude);
748- ASSERT_NE(policy->last_position_update.value.latitude, reference_position_update.value.latitude);
749-
750- ASSERT_EQ(policy->last_position_update.value.longitude, new_update.value.longitude);
751- ASSERT_NE(policy->last_position_update.value.longitude, reference_position_update.value.longitude);
752-
753- ASSERT_EQ(policy->last_position_update.value.altitude, new_update.value.altitude);
754- ASSERT_NE(policy->last_position_update.value.altitude, reference_position_update.value.altitude);
755+ auto policy = std::make_shared<cul::TimeBasedUpdatePolicy>(std::chrono::minutes(2));
756+ ASSERT_EQ(reference_position_update, policy->verify_update(reference_position_update));
757+
758+ cul::Update<cul::Position> new_update
759+ {
760+ {
761+ cul::wgs84::Latitude{10. * cul::units::Degrees},
762+ cul::wgs84::Longitude{60. * cul::units::Degrees},
763+ cul::wgs84::Altitude{10. * cul::units::Meters}
764+ },
765+ reference_position_update.when + std::chrono::minutes(3)
766+ };
767+
768+ ASSERT_EQ(new_update, policy->verify_update(new_update));
769 }
770
771 TEST(TimeBasedUpdatePolicy, policy_ignores_inaccurate_updates)
772 {
773- auto policy = std::make_shared<PublicTimeBasedUpdatePolicy>(std::chrono::minutes(2));
774- reference_position_update.value.accuracy.horizontal = 1. * com::ubuntu::location::units::Meters;
775- policy->last_position_update = reference_position_update;
776-
777- com::ubuntu::location::Update<com::ubuntu::location::Position> new_update
778- {
779- {
780- com::ubuntu::location::wgs84::Latitude{10. * com::ubuntu::location::units::Degrees},
781- com::ubuntu::location::wgs84::Longitude{60. * com::ubuntu::location::units::Degrees},
782- com::ubuntu::location::wgs84::Altitude{10. * com::ubuntu::location::units::Meters},
783- },
784- timestamp + std::chrono::minutes(1)
785- };
786- new_update.value.accuracy.horizontal = 8. * com::ubuntu::location::units::Meters;
787-
788- policy->verify_update(new_update);
789- ASSERT_TRUE(*new_update.value.accuracy.horizontal > *reference_position_update.value.accuracy.horizontal);
790-
791- ASSERT_NE(policy->last_position_update.value.latitude, new_update.value.latitude);
792- ASSERT_EQ(policy->last_position_update.value.latitude, reference_position_update.value.latitude);
793-
794- ASSERT_NE(policy->last_position_update.value.longitude, new_update.value.longitude);
795- ASSERT_EQ(policy->last_position_update.value.longitude, reference_position_update.value.longitude);
796-
797- ASSERT_NE(policy->last_position_update.value.altitude, new_update.value.altitude);
798- ASSERT_EQ(policy->last_position_update.value.altitude, reference_position_update.value.altitude);
799+ auto policy = std::make_shared<cul::TimeBasedUpdatePolicy>(std::chrono::minutes(2));
800+
801+ auto obfuscated_update = reference_position_update;
802+ obfuscated_update.value.accuracy.horizontal = 1. * cul::units::Meters;
803+ ASSERT_EQ(obfuscated_update, policy->verify_update(obfuscated_update));
804+
805+ cul::Update<cul::Position> new_update
806+ {
807+ {
808+ cul::wgs84::Latitude{10. * cul::units::Degrees},
809+ cul::wgs84::Longitude{60. * cul::units::Degrees},
810+ cul::wgs84::Altitude{10. * cul::units::Meters},
811+ },
812+ reference_position_update.when + std::chrono::minutes(1)
813+ };
814+
815+ new_update.value.accuracy.horizontal = 8. * cul::units::Meters;
816+ ASSERT_EQ(obfuscated_update, policy->verify_update(new_update));
817 }

Subscribers

People subscribed via source and target branches