Merge lp:~thomas-voss/location-service/robustify-event-propagation-in-case-of-multiple-providers-running into lp:location-service/15.04
- robustify-event-propagation-in-case-of-multiple-providers-running
- Merge into 15.04
Status: | Needs review |
---|---|
Proposed branch: | lp:~thomas-voss/location-service/robustify-event-propagation-in-case-of-multiple-providers-running |
Merge into: | lp:location-service/15.04 |
Diff against target: |
671 lines (+513/-51) 5 files modified
src/location_service/com/ubuntu/location/CMakeLists.txt (+1/-0) src/location_service/com/ubuntu/location/dispatching_provider.cpp (+325/-0) src/location_service/com/ubuntu/location/dispatching_provider.h (+90/-0) src/location_service/com/ubuntu/location/service/daemon.cpp (+91/-41) tests/acceptance_tests.cpp (+6/-10) |
To merge this branch: | bzr merge lp:~thomas-voss/location-service/robustify-event-propagation-in-case-of-multiple-providers-running |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alberto Mardegan (community) | Approve | ||
Loïc Minier | Pending | ||
PS Jenkins bot | continuous-integration | Pending | |
Review via email: mp+277789@code.launchpad.net |
This proposal supersedes a proposal from 2014-10-16.
Commit message
Introduce a dispatching provider that leverages a functor to hand over updates and invocations to/from providers.
Adjust acceptance tests to account for multiple providers.
Description of the change
Introduce a dispatching provider that leverages a functor to hand over updates and invocations to/from providers.
Adjust acceptance tests to account for multiple providers.
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal | # |
PASSED: Continuous integration, rev:130
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Loïc Minier (lool) wrote : Posted in a previous version of this proposal | # |
LGTM
- 131. By Thomas Voß
-
[ Alberto Mardegan ]
* Make sure that injected time is given in milliseconds
[ Thomas Voß ]
* Cherry-pick rev. 196 and 199 from lp:location-service. The changes
got accidentally removed by merging the outstanding documentation
branch.
* Handle responses of clients to updates asynchronously. Rely on
dummy::ConnectivityMa nager as harvesting is disabled anyway. (LP:
#1462664, #1387643)
[ Thomas Voß ]
* Add documentation for debugging, hacking and debugging the location
service. Pull manual testing instructions over from the wiki. Add
tools for formatting the source.
[ thomas-voss ]
* Add documentation for debugging, hacking and debugging the location
service. Pull manual testing instructions over from the wiki. Add
tools for formatting the source.
[ CI Train Bot ]
* New rebuild forced.
[ Manuel de la Pena ]
* Make sure that cached modems are considered as well when calculating
connection characteristics.
[ CI Train Bot ]
* New rebuild forced.
[ Manuel de la Pena ]
* Improve the selection of the bag of providers to ensure that the
locations used are within a reasonable time margin.
* Remove the pimpl implementation from the providers and hide their
public headers because they should only be used within the project.
[ Thomas Voß ]
* Increase default timeout for downloading gps xtra data. (LP:
#1447161)
[ CI Train Bot ]
* New rebuild forced.
[ Manuel de la Pena ]
* If an exception is thrown from the io_executor run method it must be
caught, logger and continued with the main loop.
[ CI Train Bot ]
* Launchpad automatic translations update. added: po/af.po po/bg.po
po/sk.po
* New rebuild forced.
[ thomas-voss ]
* Account for dbus interface breakage in NM from 0.9.8.8 -> 0.9.10.0.
[ thomas-voss ]
* Automatically clean up session store for dead clients. (LP:
#1418033)
[ thomas-voss ]
* Make the remote::Provider: :Stub fail loudly on construction if the
other side is not reachable. Relax the exception in
location::Daemon: :main and do not exit if instantiating a provider
fails. (LP: #1414591)
[ CI Train Bot ]
* Resync trunk
[ thomas-voss ]
* Add an interface for querying settings by key. Add an implementation
leveraging boost::property_ tree to provide settings. (LP: #1362765)
* Allow for enabling/disabling providers. Wire up engine state changes
to enabling/disabling of providers. (LP: #1392399)
[ thomas-voss ]
* Print details about visible space vehicles to the gps provider test
case. (LP: #1408984)
[ thomas-voss ]
* Fix #1394204 by: (LP: #1394204)
[ Ubuntu daily release ]
* New rebuild forced
[ thomas-voss ]
* Make sure that devices being added/removed by NetworkManager are
handled correctly. (LP: #1390490)
[ CI bot ]
* Resync trunk
[ Kevin DuBois ]
* The headers shipped in libubuntu-location- service- dev contain
includes that are provided in the libboost-dev package (specifically
headers like boost/units/cmath.hpp) . Make the dev package depend on
libboost-dev so the downstreams get what they need to compile
against the libubuntu-location- service- dev headers
* New rebuild forced
[ thomas-voss ]
* Bump build dependency.
* Disconnect event connections for bag of providers. (LP: #1387572)
[ thomas-voss ]
* Prevent multiple invocations of start positioning on android GPS HAL
to prevent buggy HAL implementations from blocking. Allow for
decorated provider names to enable moving providers OOP. (LP:
#1382501) - 132. By Thomas Voß
-
Revert accidental change to debian/
source/ format.
Alberto Mardegan (mardy) wrote : | # |
The code looks good to me, but I feel that this makes the logic even more complicated.
I would much rather force all providers to be out of process. This would lead to a simpler design, and might also make it possible to free some memory when the GPS is not in use.
I'm approving this because code-wise it's fine, but if it were for me, I would not merge it.
- 133. By Thomas Voß
-
Explicitly stop bus instances on shut down.
Michał Karnicki (karni) wrote : | # |
Was browsing this out of curiosity and to learn. Tiny comment - the license headers should probably contain 2015?
Unmerged revisions
- 133. By Thomas Voß
-
Explicitly stop bus instances on shut down.
Preview Diff
1 | === modified file 'src/location_service/com/ubuntu/location/CMakeLists.txt' | |||
2 | --- src/location_service/com/ubuntu/location/CMakeLists.txt 2015-04-23 14:48:44 +0000 | |||
3 | +++ src/location_service/com/ubuntu/location/CMakeLists.txt 2015-11-25 08:29:55 +0000 | |||
4 | @@ -19,6 +19,7 @@ | |||
5 | 19 | non_selecting_provider_selection_policy.cpp | 19 | non_selecting_provider_selection_policy.cpp |
6 | 20 | 20 | ||
7 | 21 | criteria.cpp | 21 | criteria.cpp |
8 | 22 | dispatching_provider.cpp | ||
9 | 22 | engine.cpp | 23 | engine.cpp |
10 | 23 | init_and_shutdown.cpp | 24 | init_and_shutdown.cpp |
11 | 24 | position.cpp | 25 | position.cpp |
12 | 25 | 26 | ||
13 | === added file 'src/location_service/com/ubuntu/location/dispatching_provider.cpp' | |||
14 | --- src/location_service/com/ubuntu/location/dispatching_provider.cpp 1970-01-01 00:00:00 +0000 | |||
15 | +++ src/location_service/com/ubuntu/location/dispatching_provider.cpp 2015-11-25 08:29:55 +0000 | |||
16 | @@ -0,0 +1,325 @@ | |||
17 | 1 | /* | ||
18 | 2 | * Copyright © 2014 Canonical Ltd. | ||
19 | 3 | * | ||
20 | 4 | * This program is free software: you can redistribute it and/or modify it | ||
21 | 5 | * under the terms of the GNU Lesser General Public License version 3, | ||
22 | 6 | * as published by the Free Software Foundation. | ||
23 | 7 | * | ||
24 | 8 | * This program is distributed in the hope that it will be useful, | ||
25 | 9 | * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
26 | 10 | * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
27 | 11 | * GNU Lesser General Public License for more details. | ||
28 | 12 | * | ||
29 | 13 | * You should have received a copy of the GNU Lesser General Public License | ||
30 | 14 | * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
31 | 15 | * | ||
32 | 16 | * Authored by: Thomas Voß <thomas.voss@canonical.com> | ||
33 | 17 | */ | ||
34 | 18 | |||
35 | 19 | #include <com/ubuntu/location/dispatching_provider.h> | ||
36 | 20 | |||
37 | 21 | #include <com/ubuntu/location/logging.h> | ||
38 | 22 | |||
39 | 23 | #include <future> | ||
40 | 24 | |||
41 | 25 | namespace location = com::ubuntu::location; | ||
42 | 26 | |||
43 | 27 | location::DispatchingProvider::Ptr location::DispatchingProvider::create( | ||
44 | 28 | const location::DispatchingProvider::Dispatcher& dispatcher, | ||
45 | 29 | const location::Provider::Ptr& fwd) | ||
46 | 30 | { | ||
47 | 31 | std::shared_ptr<location::DispatchingProvider> sp | ||
48 | 32 | { | ||
49 | 33 | new location::DispatchingProvider{dispatcher, fwd} | ||
50 | 34 | }; | ||
51 | 35 | return sp->init(); | ||
52 | 36 | } | ||
53 | 37 | |||
54 | 38 | location::DispatchingProvider::DispatchingProvider(const location::DispatchingProvider::Dispatcher& dispatcher, const location::Provider::Ptr& fwd) | ||
55 | 39 | : dispatcher{dispatcher}, | ||
56 | 40 | fwd{fwd} | ||
57 | 41 | { | ||
58 | 42 | if (not dispatcher) throw std::logic_error | ||
59 | 43 | { | ||
60 | 44 | "com::ubuntu::location::DispatchingProvider: Cannot operate without valid dispatcher" | ||
61 | 45 | }; | ||
62 | 46 | |||
63 | 47 | if (not fwd) throw std::logic_error | ||
64 | 48 | { | ||
65 | 49 | "com::ubuntu::location::DispatchingProvider: Cannot operate without valid Provider" | ||
66 | 50 | }; | ||
67 | 51 | } | ||
68 | 52 | |||
69 | 53 | location::DispatchingProvider::~DispatchingProvider() | ||
70 | 54 | { | ||
71 | 55 | } | ||
72 | 56 | |||
73 | 57 | bool location::DispatchingProvider::supports(const location::Provider::Features& f) const | ||
74 | 58 | { | ||
75 | 59 | std::promise<bool> promise; | ||
76 | 60 | std::future<bool> future = promise.get_future(); | ||
77 | 61 | |||
78 | 62 | dispatcher([&]() | ||
79 | 63 | { | ||
80 | 64 | try | ||
81 | 65 | { | ||
82 | 66 | promise.set_value(fwd->supports(f)); | ||
83 | 67 | } catch(const std::exception& e) | ||
84 | 68 | { | ||
85 | 69 | LOG(WARNING) << e.what(); | ||
86 | 70 | promise.set_exception(std::current_exception()); | ||
87 | 71 | } catch(...) | ||
88 | 72 | { | ||
89 | 73 | promise.set_exception(std::current_exception()); | ||
90 | 74 | } | ||
91 | 75 | }); | ||
92 | 76 | |||
93 | 77 | return future.get(); | ||
94 | 78 | } | ||
95 | 79 | |||
96 | 80 | bool location::DispatchingProvider::requires(const location::Provider::Requirements& r) const | ||
97 | 81 | { | ||
98 | 82 | std::promise<bool> promise; | ||
99 | 83 | std::future<bool> future = promise.get_future(); | ||
100 | 84 | |||
101 | 85 | dispatcher([&]() | ||
102 | 86 | { | ||
103 | 87 | try | ||
104 | 88 | { | ||
105 | 89 | promise.set_value(fwd->requires(r)); | ||
106 | 90 | } catch(const std::exception& e) | ||
107 | 91 | { | ||
108 | 92 | LOG(WARNING) << e.what(); | ||
109 | 93 | promise.set_exception(std::current_exception()); | ||
110 | 94 | } catch(...) | ||
111 | 95 | { | ||
112 | 96 | promise.set_exception(std::current_exception()); | ||
113 | 97 | } | ||
114 | 98 | }); | ||
115 | 99 | |||
116 | 100 | return future.get(); | ||
117 | 101 | } | ||
118 | 102 | |||
119 | 103 | bool location::DispatchingProvider::matches_criteria(const location::Criteria& criteria) | ||
120 | 104 | { | ||
121 | 105 | std::promise<bool> promise; | ||
122 | 106 | std::future<bool> future = promise.get_future(); | ||
123 | 107 | |||
124 | 108 | dispatcher([&]() | ||
125 | 109 | { | ||
126 | 110 | try | ||
127 | 111 | { | ||
128 | 112 | promise.set_value(fwd->matches_criteria(criteria)); | ||
129 | 113 | } catch(const std::exception& e) | ||
130 | 114 | { | ||
131 | 115 | LOG(WARNING) << e.what(); | ||
132 | 116 | promise.set_exception(std::current_exception()); | ||
133 | 117 | } catch(...) | ||
134 | 118 | { | ||
135 | 119 | promise.set_exception(std::current_exception()); | ||
136 | 120 | } | ||
137 | 121 | }); | ||
138 | 122 | |||
139 | 123 | return future.get(); | ||
140 | 124 | } | ||
141 | 125 | |||
142 | 126 | // We forward all events to the other providers. | ||
143 | 127 | void location::DispatchingProvider::on_wifi_and_cell_reporting_state_changed(location::WifiAndCellIdReportingState state) | ||
144 | 128 | { | ||
145 | 129 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
146 | 130 | dispatcher([wp, state]() | ||
147 | 131 | { | ||
148 | 132 | auto sp = wp.lock(); | ||
149 | 133 | |||
150 | 134 | if (not sp) | ||
151 | 135 | return; | ||
152 | 136 | |||
153 | 137 | sp->fwd->on_wifi_and_cell_reporting_state_changed(state); | ||
154 | 138 | }); | ||
155 | 139 | } | ||
156 | 140 | |||
157 | 141 | void location::DispatchingProvider::on_reference_location_updated(const location::Update<location::Position>& position) | ||
158 | 142 | { | ||
159 | 143 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
160 | 144 | dispatcher([wp, position]() | ||
161 | 145 | { | ||
162 | 146 | auto sp = wp.lock(); | ||
163 | 147 | |||
164 | 148 | if (not sp) | ||
165 | 149 | return; | ||
166 | 150 | |||
167 | 151 | sp->fwd->on_reference_location_updated(position); | ||
168 | 152 | }); | ||
169 | 153 | } | ||
170 | 154 | |||
171 | 155 | void location::DispatchingProvider::on_reference_velocity_updated(const location::Update<location::Velocity>& velocity) | ||
172 | 156 | { | ||
173 | 157 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
174 | 158 | dispatcher([wp, velocity]() | ||
175 | 159 | { | ||
176 | 160 | auto sp = wp.lock(); | ||
177 | 161 | |||
178 | 162 | if (not sp) | ||
179 | 163 | return; | ||
180 | 164 | |||
181 | 165 | sp->fwd->on_reference_velocity_updated(velocity); | ||
182 | 166 | }); | ||
183 | 167 | } | ||
184 | 168 | |||
185 | 169 | void location::DispatchingProvider::on_reference_heading_updated(const location::Update<location::Heading>& heading) | ||
186 | 170 | { | ||
187 | 171 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
188 | 172 | dispatcher([wp, heading]() | ||
189 | 173 | { | ||
190 | 174 | auto sp = wp.lock(); | ||
191 | 175 | |||
192 | 176 | if (not sp) | ||
193 | 177 | sp->fwd->on_reference_heading_updated(heading); | ||
194 | 178 | }); | ||
195 | 179 | } | ||
196 | 180 | |||
197 | 181 | // As well as the respective state change requests. | ||
198 | 182 | void location::DispatchingProvider::start_position_updates() | ||
199 | 183 | { | ||
200 | 184 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
201 | 185 | dispatcher([wp]() | ||
202 | 186 | { | ||
203 | 187 | auto sp = wp.lock(); | ||
204 | 188 | |||
205 | 189 | if (not sp) | ||
206 | 190 | return; | ||
207 | 191 | |||
208 | 192 | sp->fwd->state_controller()->start_position_updates(); | ||
209 | 193 | }); | ||
210 | 194 | } | ||
211 | 195 | |||
212 | 196 | void location::DispatchingProvider::stop_position_updates() | ||
213 | 197 | { | ||
214 | 198 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
215 | 199 | dispatcher([wp]() | ||
216 | 200 | { | ||
217 | 201 | auto sp = wp.lock(); | ||
218 | 202 | |||
219 | 203 | if (not sp) | ||
220 | 204 | return; | ||
221 | 205 | |||
222 | 206 | sp->fwd->state_controller()->stop_position_updates(); | ||
223 | 207 | }); | ||
224 | 208 | } | ||
225 | 209 | |||
226 | 210 | void location::DispatchingProvider::start_heading_updates() | ||
227 | 211 | { | ||
228 | 212 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
229 | 213 | dispatcher([wp]() | ||
230 | 214 | { | ||
231 | 215 | auto sp = wp.lock(); | ||
232 | 216 | |||
233 | 217 | if (not sp) | ||
234 | 218 | return; | ||
235 | 219 | |||
236 | 220 | sp->fwd->state_controller()->start_heading_updates(); | ||
237 | 221 | }); | ||
238 | 222 | } | ||
239 | 223 | |||
240 | 224 | void location::DispatchingProvider::stop_heading_updates() | ||
241 | 225 | { | ||
242 | 226 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
243 | 227 | dispatcher([wp]() | ||
244 | 228 | { | ||
245 | 229 | auto sp = wp.lock(); | ||
246 | 230 | |||
247 | 231 | if (not sp) | ||
248 | 232 | return; | ||
249 | 233 | |||
250 | 234 | sp->fwd->state_controller()->stop_heading_updates(); | ||
251 | 235 | }); | ||
252 | 236 | } | ||
253 | 237 | |||
254 | 238 | void location::DispatchingProvider::start_velocity_updates() | ||
255 | 239 | { | ||
256 | 240 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
257 | 241 | dispatcher([wp]() | ||
258 | 242 | { | ||
259 | 243 | auto sp = wp.lock(); | ||
260 | 244 | |||
261 | 245 | if (not sp) | ||
262 | 246 | return; | ||
263 | 247 | |||
264 | 248 | sp->fwd->state_controller()->start_velocity_updates(); | ||
265 | 249 | }); | ||
266 | 250 | } | ||
267 | 251 | |||
268 | 252 | void location::DispatchingProvider::stop_velocity_updates() | ||
269 | 253 | { | ||
270 | 254 | std::weak_ptr<location::DispatchingProvider> wp{shared_from_this()}; | ||
271 | 255 | dispatcher([wp]() | ||
272 | 256 | { | ||
273 | 257 | auto sp = wp.lock(); | ||
274 | 258 | |||
275 | 259 | if (not sp) | ||
276 | 260 | return; | ||
277 | 261 | |||
278 | 262 | sp->fwd->state_controller()->stop_velocity_updates(); | ||
279 | 263 | }); | ||
280 | 264 | } | ||
281 | 265 | |||
282 | 266 | location::DispatchingProvider::Ptr location::DispatchingProvider::init() | ||
283 | 267 | { | ||
284 | 268 | auto sp = shared_from_this(); | ||
285 | 269 | std::weak_ptr<location::DispatchingProvider> wp{sp}; | ||
286 | 270 | connections.push_back(fwd->updates().position.connect([wp](const location::Update<location::Position>& update) | ||
287 | 271 | { | ||
288 | 272 | auto sp = wp.lock(); | ||
289 | 273 | |||
290 | 274 | if (not sp) | ||
291 | 275 | return; | ||
292 | 276 | |||
293 | 277 | sp->dispatcher([wp, update]() | ||
294 | 278 | { | ||
295 | 279 | auto sp = wp.lock(); | ||
296 | 280 | |||
297 | 281 | if (not sp) | ||
298 | 282 | return; | ||
299 | 283 | |||
300 | 284 | sp->mutable_updates().position(update); | ||
301 | 285 | }); | ||
302 | 286 | })); | ||
303 | 287 | |||
304 | 288 | connections.push_back(fwd->updates().heading.connect([wp](const location::Update<location::Heading>& update) | ||
305 | 289 | { | ||
306 | 290 | auto sp = wp.lock(); | ||
307 | 291 | |||
308 | 292 | if (not sp) | ||
309 | 293 | return; | ||
310 | 294 | |||
311 | 295 | sp->dispatcher([wp, update]() | ||
312 | 296 | { | ||
313 | 297 | auto sp = wp.lock(); | ||
314 | 298 | |||
315 | 299 | if (not sp) | ||
316 | 300 | return; | ||
317 | 301 | |||
318 | 302 | sp->mutable_updates().heading(update); | ||
319 | 303 | }); | ||
320 | 304 | })); | ||
321 | 305 | |||
322 | 306 | connections.push_back(fwd->updates().velocity.connect([wp](const location::Update<location::Velocity>& update) | ||
323 | 307 | { | ||
324 | 308 | auto sp = wp.lock(); | ||
325 | 309 | |||
326 | 310 | if (not sp) | ||
327 | 311 | return; | ||
328 | 312 | |||
329 | 313 | sp->dispatcher([wp, update]() | ||
330 | 314 | { | ||
331 | 315 | auto sp = wp.lock(); | ||
332 | 316 | |||
333 | 317 | if (not sp) | ||
334 | 318 | return; | ||
335 | 319 | |||
336 | 320 | sp->mutable_updates().velocity(update); | ||
337 | 321 | }); | ||
338 | 322 | })); | ||
339 | 323 | |||
340 | 324 | return sp; | ||
341 | 325 | } | ||
342 | 0 | 326 | ||
343 | === added file 'src/location_service/com/ubuntu/location/dispatching_provider.h' | |||
344 | --- src/location_service/com/ubuntu/location/dispatching_provider.h 1970-01-01 00:00:00 +0000 | |||
345 | +++ src/location_service/com/ubuntu/location/dispatching_provider.h 2015-11-25 08:29:55 +0000 | |||
346 | @@ -0,0 +1,90 @@ | |||
347 | 1 | /* | ||
348 | 2 | * Copyright © 2014 Canonical Ltd. | ||
349 | 3 | * | ||
350 | 4 | * This program is free software: you can redistribute it and/or modify it | ||
351 | 5 | * under the terms of the GNU Lesser General Public License version 3, | ||
352 | 6 | * as published by the Free Software Foundation. | ||
353 | 7 | * | ||
354 | 8 | * This program is distributed in the hope that it will be useful, | ||
355 | 9 | * but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
356 | 10 | * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
357 | 11 | * GNU Lesser General Public License for more details. | ||
358 | 12 | * | ||
359 | 13 | * You should have received a copy of the GNU Lesser General Public License | ||
360 | 14 | * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
361 | 15 | * | ||
362 | 16 | * Authored by: Thomas Voß <thomas.voss@canonical.com> | ||
363 | 17 | */ | ||
364 | 18 | |||
365 | 19 | #ifndef LOCATION_SERVICE_COM_UBUNTU_LOCATION_DISPATCHING_PROVIDER_H_ | ||
366 | 20 | #define LOCATION_SERVICE_COM_UBUNTU_LOCATION_DISPATCHING_PROVIDER_H_ | ||
367 | 21 | |||
368 | 22 | #include <com/ubuntu/location/provider.h> | ||
369 | 23 | |||
370 | 24 | #include <functional> | ||
371 | 25 | #include <memory> | ||
372 | 26 | |||
373 | 27 | namespace com | ||
374 | 28 | { | ||
375 | 29 | namespace ubuntu | ||
376 | 30 | { | ||
377 | 31 | namespace location | ||
378 | 32 | { | ||
379 | 33 | // A Provider implementation that wraps another provider implementation | ||
380 | 34 | // dispatching events/invocations via a Dispatcher functor. The dispatcher | ||
381 | 35 | // can either immediately process the given task or hand it over to a runtime | ||
382 | 36 | // with an associated event loop. | ||
383 | 37 | class DispatchingProvider : public Provider, public std::enable_shared_from_this<DispatchingProvider> | ||
384 | 38 | { | ||
385 | 39 | public: | ||
386 | 40 | // To safe us some typing. | ||
387 | 41 | typedef std::shared_ptr<DispatchingProvider> Ptr; | ||
388 | 42 | |||
389 | 43 | // The Dispatcher functor that is invoked for all incoming | ||
390 | 44 | // invocations and for all events, with both of them being | ||
391 | 45 | // wrapped as a task. | ||
392 | 46 | typedef std::function<void()> Task; | ||
393 | 47 | typedef std::function<void(Task)> Dispatcher; | ||
394 | 48 | |||
395 | 49 | // Create a new instance wired up to the given Provider instance. | ||
396 | 50 | static DispatchingProvider::Ptr create(const Dispatcher& dispatcher, const Provider::Ptr& fwd); | ||
397 | 51 | |||
398 | 52 | ~DispatchingProvider() noexcept; | ||
399 | 53 | |||
400 | 54 | bool supports(const location::Provider::Features& f) const override; | ||
401 | 55 | bool requires(const location::Provider::Requirements& r) const override; | ||
402 | 56 | bool matches_criteria(const location::Criteria&) override; | ||
403 | 57 | |||
404 | 58 | // We forward all events to the other providers. | ||
405 | 59 | void on_wifi_and_cell_reporting_state_changed(WifiAndCellIdReportingState state) override; | ||
406 | 60 | void on_reference_location_updated(const location::Update<location::Position>& position) override; | ||
407 | 61 | void on_reference_velocity_updated(const location::Update<location::Velocity>& velocity) override; | ||
408 | 62 | void on_reference_heading_updated(const location::Update<location::Heading>& heading) override; | ||
409 | 63 | |||
410 | 64 | // As well as the respective state change requests. | ||
411 | 65 | void start_position_updates() override; | ||
412 | 66 | void stop_position_updates() override; | ||
413 | 67 | void start_heading_updates() override; | ||
414 | 68 | void stop_heading_updates() override; | ||
415 | 69 | void start_velocity_updates() override; | ||
416 | 70 | void stop_velocity_updates() override; | ||
417 | 71 | |||
418 | 72 | private: | ||
419 | 73 | // We want to pass ourselves around. | ||
420 | 74 | DispatchingProvider(const Dispatcher& dispatcher, const Provider::Ptr& fwd); | ||
421 | 75 | |||
422 | 76 | // Two stage initialization is evil, but we are somewhat forced to do it. | ||
423 | 77 | DispatchingProvider::Ptr init(); | ||
424 | 78 | |||
425 | 79 | // The dispatcher we rely on to dispatch events/invocations. | ||
426 | 80 | Dispatcher dispatcher; | ||
427 | 81 | // The provider that we relay to/from. | ||
428 | 82 | Provider::Ptr fwd; | ||
429 | 83 | // We store all connections that should be cut on destruction. | ||
430 | 84 | std::vector<core::ScopedConnection> connections; | ||
431 | 85 | }; | ||
432 | 86 | } | ||
433 | 87 | } | ||
434 | 88 | } | ||
435 | 89 | |||
436 | 90 | #endif // LOCATION_SERVICE_COM_UBUNTU_LOCATION_DISPATCHING_PROVIDER_H_ | ||
437 | 0 | 91 | ||
438 | === modified file 'src/location_service/com/ubuntu/location/service/daemon.cpp' | |||
439 | --- src/location_service/com/ubuntu/location/service/daemon.cpp 2015-10-23 13:43:32 +0000 | |||
440 | +++ src/location_service/com/ubuntu/location/service/daemon.cpp 2015-11-25 08:29:55 +0000 | |||
441 | @@ -20,6 +20,8 @@ | |||
442 | 20 | #include <com/ubuntu/location/boost_ptree_settings.h> | 20 | #include <com/ubuntu/location/boost_ptree_settings.h> |
443 | 21 | #include <com/ubuntu/location/provider_factory.h> | 21 | #include <com/ubuntu/location/provider_factory.h> |
444 | 22 | 22 | ||
445 | 23 | #include <com/ubuntu/location/logging.h> | ||
446 | 24 | #include <com/ubuntu/location/dispatching_provider.h> | ||
447 | 23 | #include <com/ubuntu/location/connectivity/dummy_connectivity_manager.h> | 25 | #include <com/ubuntu/location/connectivity/dummy_connectivity_manager.h> |
448 | 24 | 26 | ||
449 | 25 | #include <com/ubuntu/location/service/default_configuration.h> | 27 | #include <com/ubuntu/location/service/default_configuration.h> |
450 | @@ -39,6 +41,8 @@ | |||
451 | 39 | 41 | ||
452 | 40 | #include <core/posix/signal.h> | 42 | #include <core/posix/signal.h> |
453 | 41 | 43 | ||
454 | 44 | #include <boost/asio.hpp> | ||
455 | 45 | |||
456 | 42 | #include <system_error> | 46 | #include <system_error> |
457 | 43 | #include <thread> | 47 | #include <thread> |
458 | 44 | 48 | ||
459 | @@ -74,6 +78,88 @@ | |||
460 | 74 | } | 78 | } |
461 | 75 | }; | 79 | }; |
462 | 76 | 80 | ||
463 | 81 | // We bundle our "global" runtime dependencies here, specifically | ||
464 | 82 | // a dispatcher to decouple multiple in-process providers from one | ||
465 | 83 | // another , forcing execution to a well known set of threads. | ||
466 | 84 | struct Runtime | ||
467 | 85 | { | ||
468 | 86 | // Our default concurrency setup. | ||
469 | 87 | static constexpr const std::uint32_t worker_threads = 2; | ||
470 | 88 | |||
471 | 89 | // Our global singleton instance. | ||
472 | 90 | static Runtime& instance() | ||
473 | 91 | { | ||
474 | 92 | static Runtime runtime; | ||
475 | 93 | return runtime; | ||
476 | 94 | } | ||
477 | 95 | |||
478 | 96 | Runtime() | ||
479 | 97 | : running{true}, | ||
480 | 98 | service{worker_threads}, | ||
481 | 99 | strand{service}, | ||
482 | 100 | keep_alive{service} | ||
483 | 101 | { | ||
484 | 102 | for (unsigned int i = 0; i < worker_threads; i++) | ||
485 | 103 | workers.push_back(std::thread | ||
486 | 104 | { | ||
487 | 105 | [this]() | ||
488 | 106 | { | ||
489 | 107 | while(running) | ||
490 | 108 | { | ||
491 | 109 | try | ||
492 | 110 | { | ||
493 | 111 | service.run(); | ||
494 | 112 | break; | ||
495 | 113 | } | ||
496 | 114 | catch (const std::exception& e) | ||
497 | 115 | { | ||
498 | 116 | LOG(WARNING) << e.what(); | ||
499 | 117 | } | ||
500 | 118 | catch (...) | ||
501 | 119 | { | ||
502 | 120 | LOG(WARNING) << "Unknown exception caught while executing boost::asio::io_service"; | ||
503 | 121 | } | ||
504 | 122 | } | ||
505 | 123 | } | ||
506 | 124 | }); | ||
507 | 125 | } | ||
508 | 126 | |||
509 | 127 | ~Runtime() | ||
510 | 128 | { | ||
511 | 129 | stop(); | ||
512 | 130 | } | ||
513 | 131 | |||
514 | 132 | void stop() | ||
515 | 133 | { | ||
516 | 134 | VLOG(1) << __PRETTY_FUNCTION__; | ||
517 | 135 | running = false; | ||
518 | 136 | service.stop(); | ||
519 | 137 | VLOG(1) << "\t Service stopped."; | ||
520 | 138 | |||
521 | 139 | for (auto& worker : workers) | ||
522 | 140 | if (worker.joinable()) | ||
523 | 141 | worker.join(); | ||
524 | 142 | |||
525 | 143 | VLOG(1) << "\t Worker threads joined."; | ||
526 | 144 | } | ||
527 | 145 | |||
528 | 146 | // Allows for reusing the runtime in components that require a dispatcher | ||
529 | 147 | // to control execution of tasks. | ||
530 | 148 | std::function<void(std::function<void()>)> to_dispatcher_functional() | ||
531 | 149 | { | ||
532 | 150 | return [this](std::function<void()> task) | ||
533 | 151 | { | ||
534 | 152 | strand.post(task); | ||
535 | 153 | }; | ||
536 | 154 | } | ||
537 | 155 | |||
538 | 156 | bool running; | ||
539 | 157 | boost::asio::io_service service; | ||
540 | 158 | boost::asio::io_service::strand strand; | ||
541 | 159 | boost::asio::io_service::work keep_alive; | ||
542 | 160 | std::vector<std::thread> workers; | ||
543 | 161 | }; | ||
544 | 162 | |||
545 | 77 | location::ProgramOptions init_daemon_options() | 163 | location::ProgramOptions init_daemon_options() |
546 | 78 | { | 164 | { |
547 | 79 | location::ProgramOptions options; | 165 | location::ProgramOptions options; |
548 | @@ -193,7 +279,9 @@ | |||
549 | 193 | config.provider_options.at(provider) : empty_provider_configuration); | 279 | config.provider_options.at(provider) : empty_provider_configuration); |
550 | 194 | 280 | ||
551 | 195 | if (p) | 281 | if (p) |
553 | 196 | instantiated_providers.insert(p); | 282 | instantiated_providers.insert( |
554 | 283 | location::DispatchingProvider::create( | ||
555 | 284 | Runtime::instance().to_dispatcher_functional(), p)); | ||
556 | 197 | else | 285 | else |
557 | 198 | throw std::runtime_error("Problem instantiating provider"); | 286 | throw std::runtime_error("Problem instantiating provider"); |
558 | 199 | 287 | ||
559 | @@ -203,8 +291,8 @@ | |||
560 | 203 | } | 291 | } |
561 | 204 | } | 292 | } |
562 | 205 | 293 | ||
565 | 206 | config.incoming->install_executor(dbus::asio::make_executor(config.incoming)); | 294 | config.incoming->install_executor(dbus::asio::make_executor(config.incoming, Runtime::instance().service)); |
566 | 207 | config.outgoing->install_executor(dbus::asio::make_executor(config.outgoing)); | 295 | config.outgoing->install_executor(dbus::asio::make_executor(config.outgoing, Runtime::instance().service)); |
567 | 208 | 296 | ||
568 | 209 | location::service::DefaultConfiguration dc; | 297 | location::service::DefaultConfiguration dc; |
569 | 210 | 298 | ||
570 | @@ -222,50 +310,12 @@ | |||
571 | 222 | }; | 310 | }; |
572 | 223 | 311 | ||
573 | 224 | auto location_service = std::make_shared<location::service::Implementation>(configuration); | 312 | auto location_service = std::make_shared<location::service::Implementation>(configuration); |
574 | 225 | // We need to ensure that any exception raised by the executor does not crash the app | ||
575 | 226 | // and also gets logged. | ||
576 | 227 | auto execute = [] (std::shared_ptr<core::dbus::Bus> bus) { | ||
577 | 228 | while(true) | ||
578 | 229 | { | ||
579 | 230 | try | ||
580 | 231 | { | ||
581 | 232 | VLOG(10) << "Starting a bus executor"; | ||
582 | 233 | bus->run(); | ||
583 | 234 | break; // run() exited normally | ||
584 | 235 | } | ||
585 | 236 | catch (const std::exception& e) | ||
586 | 237 | { | ||
587 | 238 | LOG(WARNING) << e.what(); | ||
588 | 239 | } | ||
589 | 240 | catch (...) | ||
590 | 241 | { | ||
591 | 242 | LOG(WARNING) << "Unexpected exception was raised by the bus executor"; | ||
592 | 243 | } | ||
593 | 244 | } | ||
594 | 245 | }; | ||
595 | 246 | |||
596 | 247 | std::thread t1{execute, config.incoming}; | ||
597 | 248 | std::thread t2{execute, config.incoming}; | ||
598 | 249 | std::thread t3{execute, config.incoming}; | ||
599 | 250 | std::thread t4{execute, config.outgoing}; | ||
600 | 251 | 313 | ||
601 | 252 | trap->run(); | 314 | trap->run(); |
602 | 253 | 315 | ||
603 | 254 | config.incoming->stop(); | 316 | config.incoming->stop(); |
604 | 255 | config.outgoing->stop(); | 317 | config.outgoing->stop(); |
605 | 256 | 318 | ||
606 | 257 | if (t1.joinable()) | ||
607 | 258 | t1.join(); | ||
608 | 259 | |||
609 | 260 | if (t2.joinable()) | ||
610 | 261 | t2.join(); | ||
611 | 262 | |||
612 | 263 | if (t3.joinable()) | ||
613 | 264 | t3.join(); | ||
614 | 265 | |||
615 | 266 | if (t4.joinable()) | ||
616 | 267 | t4.join(); | ||
617 | 268 | |||
618 | 269 | return EXIT_SUCCESS; | 319 | return EXIT_SUCCESS; |
619 | 270 | } | 320 | } |
620 | 271 | 321 | ||
621 | 272 | 322 | ||
622 | === modified file 'tests/acceptance_tests.cpp' | |||
623 | --- tests/acceptance_tests.cpp 2015-02-13 13:19:18 +0000 | |||
624 | +++ tests/acceptance_tests.cpp 2015-11-25 08:29:55 +0000 | |||
625 | @@ -708,7 +708,7 @@ | |||
626 | 708 | 708 | ||
627 | 709 | options.add(Keys::update_period, | 709 | options.add(Keys::update_period, |
628 | 710 | "Update period length for dummy::Provider setup.", | 710 | "Update period length for dummy::Provider setup.", |
630 | 711 | std::uint32_t{100}); | 711 | std::uint32_t{10}); |
631 | 712 | 712 | ||
632 | 713 | options.add(Keys::client_count, | 713 | options.add(Keys::client_count, |
633 | 714 | "Number of clients that should be fired up.", | 714 | "Number of clients that should be fired up.", |
634 | @@ -761,6 +761,8 @@ | |||
635 | 761 | }; | 761 | }; |
636 | 762 | } | 762 | } |
637 | 763 | 763 | ||
638 | 764 | #include "did_finish_successfully.h" | ||
639 | 765 | |||
640 | 764 | TEST_F(LocationServiceStandaloneLoad, MultipleClientsConnectingAndDisconnectingWorks) | 766 | TEST_F(LocationServiceStandaloneLoad, MultipleClientsConnectingAndDisconnectingWorks) |
641 | 765 | { | 767 | { |
642 | 766 | EXPECT_TRUE(trust_store_is_set_up_for_testing); | 768 | EXPECT_TRUE(trust_store_is_set_up_for_testing); |
643 | @@ -829,7 +831,7 @@ | |||
644 | 829 | status; | 831 | status; |
645 | 830 | }, core::posix::StandardStream::empty); | 832 | }, core::posix::StandardStream::empty); |
646 | 831 | 833 | ||
648 | 832 | std::this_thread::sleep_for(std::chrono::seconds{2}); | 834 | std::this_thread::sleep_for(std::chrono::seconds{15}); |
649 | 833 | 835 | ||
650 | 834 | auto client = [this]() | 836 | auto client = [this]() |
651 | 835 | { | 837 | { |
652 | @@ -957,17 +959,11 @@ | |||
653 | 957 | { | 959 | { |
654 | 958 | VLOG(1) << "Stopping client...: " << client.pid(); | 960 | VLOG(1) << "Stopping client...: " << client.pid(); |
655 | 959 | client.send_signal_or_throw(core::posix::Signal::sig_term); | 961 | client.send_signal_or_throw(core::posix::Signal::sig_term); |
660 | 960 | auto result = client.wait_for(core::posix::wait::Flags::untraced); | 962 | EXPECT_TRUE(did_finish_successfully(client.wait_for(core::posix::wait::Flags::untraced))); |
657 | 961 | |||
658 | 962 | EXPECT_EQ(core::posix::wait::Result::Status::exited, result.status); | ||
659 | 963 | EXPECT_EQ(core::posix::exit::Status::success, result.detail.if_exited.status); | ||
661 | 964 | } | 963 | } |
662 | 965 | 964 | ||
663 | 966 | VLOG(1) << "Cleaned up clients, shutting down the service..."; | 965 | VLOG(1) << "Cleaned up clients, shutting down the service..."; |
664 | 967 | 966 | ||
665 | 968 | server.send_signal_or_throw(core::posix::Signal::sig_term); | 967 | server.send_signal_or_throw(core::posix::Signal::sig_term); |
670 | 969 | auto result = server.wait_for(core::posix::wait::Flags::untraced); | 968 | EXPECT_TRUE(did_finish_successfully(server.wait_for(core::posix::wait::Flags::untraced))); |
667 | 970 | |||
668 | 971 | EXPECT_EQ(core::posix::wait::Result::Status::exited, result.status); | ||
669 | 972 | EXPECT_EQ(core::posix::exit::Status::success, result.detail.if_exited.status); | ||
671 | 973 | } | 969 | } |
FAILED: Continuous integration, rev:129 jenkins. qa.ubuntu. com/job/ location- service- ci/316/ jenkins. qa.ubuntu. com/job/ location- service- utopic- amd64-ci/ 223/console jenkins. qa.ubuntu. com/job/ location- service- utopic- armhf-ci/ 223/console jenkins. qa.ubuntu. com/job/ location- service- utopic- i386-ci/ 223/console
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/location- service- ci/316/ rebuild
http://