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