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:
|
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
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.