Merge lp:~bregma/geis/lp-1252447 into lp:geis

Proposed by Stephen M. Webb
Status: Merged
Approved by: Brandon Schaefer
Approved revision: 330
Merged at revision: 325
Proposed branch: lp:~bregma/geis/lp-1252447
Merge into: lp:geis
Diff against target: 457 lines (+201/-20)
9 files modified
include/geis/geis.h (+8/-1)
libgeis/backend/dbus/geis_dbus_backend.c (+3/-1)
libgeis/backend/grail/geis_grail_backend.c (+40/-7)
libgeis/backend/test_fixture/geis_backend_test_fixture.c (+3/-1)
libgeis/geis.c (+3/-1)
libgeis/geis_backend.c (+21/-1)
libgeis/geis_backend.h (+29/-1)
libgeis/geis_backend_protected.h (+3/-1)
testsuite/geis2/gtest_devices.cpp (+91/-6)
To merge this branch: bzr merge lp:~bregma/geis/lp-1252447
Reviewer Review Type Date Requested Status
Brandon Schaefer (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+196458@code.launchpad.net

Commit message

fix a synch problem when a subscription is activated in a callback on receipt of INIT_COMPLETE (lp: #1252447)

Description of the change

Fix a synchronization problem when a subscription is activated in a callback on receipt of the INIT_COMPLETE message before preexisting devices are recognized (lp: #1252447).

To post a comment you must log in.
lp:~bregma/geis/lp-1252447 updated
329. By Stephen M. Webb

fixed return status of new grail back end functions

330. By Stephen M. Webb

updated some copyright dates

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Testing are passing, and LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'include/geis/geis.h'
2--- include/geis/geis.h 2012-12-04 12:39:16 +0000
3+++ include/geis/geis.h 2013-11-25 02:22:49 +0000
4@@ -2,7 +2,7 @@
5 * @file geis/geis.h
6 * This is the public interface for the GEIS gesture API.
7 *
8- * Copyright 2010, 2011 Canonical Ltd.
9+ * Copyright 2010-2013 Canonical Ltd.
10 *
11 * This library is free software; you can redistribute it and/or modify it under
12 * the terms of version 3 of the GNU Lesser General Public License as published
13@@ -803,6 +803,13 @@
14 */
15 #define GEIS_CONFIG_TAP_TIMEOUT "com.canonical.oif.tap.timeout"
16
17+/**
18+ * @def GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS
19+ * The number of subscriptions currently active in the back end.
20+ * This config is query-only and intended for unit test validations.
21+ */
22+#define GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS "com.canonical.oif.debug.active_subs"
23+
24 /* @} */
25
26 /**
27
28=== modified file 'libgeis/backend/dbus/geis_dbus_backend.c'
29--- libgeis/backend/dbus/geis_dbus_backend.c 2012-12-04 12:39:16 +0000
30+++ libgeis/backend/dbus/geis_dbus_backend.c 2013-11-25 02:22:49 +0000
31@@ -4,7 +4,7 @@
32 */
33
34 /*
35- * Copyright 2011 Canonical Ltd.
36+ * Copyright 2011-2013 Canonical Ltd.
37 *
38 * This library is free software; you can redistribute it and/or modify it under
39 * the terms of version 3 of the GNU Lesser General Public License as published
40@@ -274,6 +274,8 @@
41 _geis_dbus_backend_create_token,
42 _geis_dbus_accept_gesture,
43 _geis_dbus_reject_gesture,
44+ NULL,
45+ NULL,
46 _geis_dbus_get_configuration,
47 _geis_dbus_set_configuration,
48 };
49
50=== modified file 'libgeis/backend/grail/geis_grail_backend.c'
51--- libgeis/backend/grail/geis_grail_backend.c 2013-11-20 15:45:43 +0000
52+++ libgeis/backend/grail/geis_grail_backend.c 2013-11-25 02:22:49 +0000
53@@ -4,7 +4,7 @@
54 */
55
56 /*
57- * Copyright 2011, 2012 Canonical Ltd.
58+ * Copyright 2011-2013 Canonical Ltd.
59 *
60 * This library is free software: you can redistribute it and/or modify it
61 * under the terms of the GNU Lesser General Public License version 3
62@@ -812,11 +812,14 @@
63 GeisBoolean device_applies = GEIS_TRUE;
64 for (GeisSize tindex = 0; tindex < geis_filter_term_count(*fit); ++tindex)
65 {
66- if (!geis_filter_term_match_device(geis_filter_term(*fit, tindex),
67- device))
68+ GeisFilterTerm term = geis_filter_term(*fit, tindex);
69+ if (geis_filter_term_facility(term) == GEIS_FILTER_DEVICE)
70 {
71- device_applies = GEIS_FALSE;
72- break;
73+ if (!geis_filter_term_match_device(term, device))
74+ {
75+ device_applies = GEIS_FALSE;
76+ break;
77+ }
78 }
79 }
80
81@@ -1013,7 +1016,6 @@
82 / sizeof(struct GeisFilterableAttribute);
83
84 geis_register_device(gbe->geis, geis_device, attr_count, attrs);
85- _grail_be_subscribe_new_device(gbe, geis_device);
86
87 /* We are not going to hold a pointer to this geis_device ourselves */
88 geis_device_unref(geis_device);
89@@ -1812,6 +1814,13 @@
90 return retval;
91 }
92
93+ if (0 == strcmp(item_name, GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS))
94+ {
95+ struct GeisGrailSubscriptionData *sub_data = geis_subscription_pdata(subscription);
96+ *((GeisSize*)item_value) = geis_ugsubscription_count(sub_data->ugstore);
97+ retval = GEIS_STATUS_SUCCESS;
98+ }
99+
100 #define GEIS_GRAIL_CHECK_GESTURE_CONFIG(gesture, Gesture, GESTURE) \
101 if (strcmp(item_name, GEIS_CONFIG_##GESTURE##_TIMEOUT) == 0) \
102 { \
103@@ -1830,7 +1839,7 @@
104 item_value); \
105 }
106
107- GEIS_GRAIL_CHECK_GESTURE_CONFIG(drag, Drag, DRAG)
108+ else GEIS_GRAIL_CHECK_GESTURE_CONFIG(drag, Drag, DRAG)
109 else GEIS_GRAIL_CHECK_GESTURE_CONFIG(pinch, Pinch, PINCH)
110 else GEIS_GRAIL_CHECK_GESTURE_CONFIG(rotate, Rotate, ROTATE)
111 else GEIS_GRAIL_CHECK_GESTURE_CONFIG(tap, Tap, TAP)
112@@ -1921,12 +1930,36 @@
113 return retval;
114 }
115
116+static GeisStatus
117+_grail_be_activate_device(GeisBackend be, GeisDevice device)
118+{
119+ GeisStatus status = GEIS_STATUS_SUCCESS;
120+ GeisGrailBackend gbe = (GeisGrailBackend)be;
121+ _grail_be_subscribe_new_device(gbe, device);
122+ return status;
123+}
124+
125+
126+static GeisStatus
127+_grail_be_deactivate_device(GeisBackend be, GeisDevice device)
128+{
129+ GeisStatus status = GEIS_STATUS_SUCCESS;
130+ GeisGrailBackend gbe = (GeisGrailBackend)be;
131+ UFDevice ufdevice = _grail_be_ufdevice_from_device_id(gbe,
132+ geis_device_id(device));
133+ _grail_be_unsubscribe_removed_device(gbe, ufdevice);
134+ return status;
135+}
136+
137+
138 static struct GeisBackendVtable gbe_vtbl = {
139 _geis_grail_backend_construct,
140 _geis_grail_backend_finalize,
141 geis_grail_token_new,
142 _grail_be_accept_gesture,
143 _grail_be_reject_gesture,
144+ _grail_be_activate_device,
145+ _grail_be_deactivate_device,
146 _grail_be_get_configuration,
147 _grail_be_set_configuration
148 };
149
150=== modified file 'libgeis/backend/test_fixture/geis_backend_test_fixture.c'
151--- libgeis/backend/test_fixture/geis_backend_test_fixture.c 2012-12-04 12:39:16 +0000
152+++ libgeis/backend/test_fixture/geis_backend_test_fixture.c 2013-11-25 02:22:49 +0000
153@@ -2,7 +2,7 @@
154 * @file geis_backend_test_fixture.c
155 * @brief GEIS mock back end test fixture implementation
156 *
157- * Copyright 2010 Canonical Ltd.
158+ * Copyright 2010-2013 Canonical Ltd.
159 *
160 * This library is free software; you can redistribute it and/or modify it under
161 * the terms of version 3 of the GNU Lesser General Public License as published
162@@ -274,6 +274,8 @@
163 _create_token,
164 _gmock_accept_gesture,
165 _gmock_reject_gesture,
166+ NULL,
167+ NULL,
168 _gmock_get_configuration,
169 _gmock_set_configuration
170 };
171
172=== modified file 'libgeis/geis.c'
173--- libgeis/geis.c 2012-12-04 12:39:16 +0000
174+++ libgeis/geis.c 2013-11-25 02:22:49 +0000
175@@ -9,7 +9,7 @@
176 */
177
178 /*
179- * Copyright 2010, 2011 Canonical Ltd.
180+ * Copyright 2010-2013 Canonical Ltd.
181 *
182 * This library is free software; you can redistribute it and/or modify it under
183 * the terms of version 3 of the GNU Lesser General Public License as published
184@@ -156,6 +156,7 @@
185 if (event_type == GEIS_EVENT_DEVICE_AVAILABLE)
186 {
187 geis_device_bag_insert(geis->devices, device);
188+ geis_backend_activate_device(geis->backend, device);
189 }
190
191 if (geis->device_event_callback != GEIS_DEFAULT_EVENT_CALLBACK)
192@@ -166,6 +167,7 @@
193
194 if (event_type == GEIS_EVENT_DEVICE_UNAVAILABLE)
195 {
196+ geis_backend_deactivate_device(geis->backend, device);
197 geis_device_bag_remove(geis->devices, device);
198 }
199
200
201=== modified file 'libgeis/geis_backend.c'
202--- libgeis/geis_backend.c 2012-12-04 12:39:16 +0000
203+++ libgeis/geis_backend.c 2013-11-25 02:22:49 +0000
204@@ -2,7 +2,7 @@
205 * @file geis_backend.c
206 * @brief internal GEIS back end base class implementation
207 *
208- * Copyright 2010 Canonical Ltd.
209+ * Copyright 2010-2013 Canonical Ltd.
210 *
211 * This library is free software; you can redistribute it and/or modify it under
212 * the terms of version 3 of the GNU Lesser General Public License as published
213@@ -199,6 +199,26 @@
214 }
215
216
217+GeisStatus
218+geis_backend_activate_device(GeisBackend be,
219+ GeisDevice device)
220+{
221+ if (be->be_class->vtbl->activate_device)
222+ return be->be_class->vtbl->activate_device(_data_from_be(be), device);
223+ return GEIS_STATUS_SUCCESS;
224+}
225+
226+
227+GeisStatus
228+geis_backend_deactivate_device(GeisBackend be,
229+ GeisDevice device)
230+{
231+ if (be->be_class->vtbl->deactivate_device)
232+ return be->be_class->vtbl->deactivate_device(_data_from_be(be), device);
233+ return GEIS_STATUS_SUCCESS;
234+}
235+
236+
237 /*
238 * Gets a back end configuration value.
239 */
240
241=== modified file 'libgeis/geis_backend.h'
242--- libgeis/geis_backend.h 2012-12-04 12:39:16 +0000
243+++ libgeis/geis_backend.h 2013-11-25 02:22:49 +0000
244@@ -2,7 +2,7 @@
245 * @file geis_backend.h
246 * @brief internal GEIS back end base class public interface
247 *
248- * Copyright 2010, 2012 Canonical Ltd.
249+ * Copyright 2010-2013 Canonical Ltd.
250 *
251 * This library is free software; you can redistribute it and/or modify it under
252 * the terms of version 3 of the GNU Lesser General Public License as published
253@@ -90,6 +90,34 @@
254 GeisGestureId gesture_id);
255
256 /**
257+ * Tells the back end to activate a device after it's been set up.
258+ *
259+ * @param[in] be The GEIS back end.
260+ * @param[in] device The GEIS device for which subscription wll activate.
261+ *
262+ * The back end sends and event to the middle end when a new device has been
263+ * detected. The middle end sets up some structures etc., then tells the back
264+ * end to activate the device (which may actually cause some subscriptions to be
265+ * activated), then forwards the new-device event to the front end where the
266+ * application can be made aware of it.
267+ */
268+GeisStatus
269+geis_backend_activate_device(GeisBackend be,
270+ GeisDevice device);
271+
272+/**
273+ * Tells the back end to activate a device after it's been set up.
274+ *
275+ * @param[in] be The GEIS back end.
276+ * @param[in] device The GEIS device for which subscription wll deactivate.
277+ *
278+ * See geis_backend_activate_device for more information.
279+ */
280+GeisStatus
281+geis_backend_deactivate_device(GeisBackend be,
282+ GeisDevice device);
283+
284+/**
285 * Gets a back end configuration value.
286 *
287 * @param[in] be The back end.
288
289=== modified file 'libgeis/geis_backend_protected.h'
290--- libgeis/geis_backend_protected.h 2012-12-04 12:39:16 +0000
291+++ libgeis/geis_backend_protected.h 2013-11-25 02:22:49 +0000
292@@ -7,7 +7,7 @@
293 */
294
295 /*
296- * Copyright 2010, 2012 Canonical Ltd.
297+ * Copyright 2010-2013 Canonical Ltd.
298 *
299 * This library is free software; you can redistribute it and/or modify it under
300 * the terms of version 3 of the GNU Lesser General Public License as published
301@@ -39,6 +39,8 @@
302 GeisBackendToken (* create_token)(GeisBackend, GeisBackendTokenInitState);
303 GeisStatus (* accept_gesture)(GeisBackend, GeisGroup, GeisGestureId);
304 GeisStatus (* reject_gesture)(GeisBackend, GeisGroup, GeisGestureId);
305+ GeisStatus (* activate_device)(GeisBackend, GeisDevice);
306+ GeisStatus (* deactivate_device)(GeisBackend, GeisDevice);
307 GeisStatus (* get_configuration)(GeisBackend, GeisSubscription, GeisString, GeisPointer);
308 GeisStatus (* set_configuration)(GeisBackend, GeisSubscription, GeisString, GeisPointer);
309 } *GeisBackendVtable;
310
311=== modified file 'testsuite/geis2/gtest_devices.cpp'
312--- testsuite/geis2/gtest_devices.cpp 2013-11-20 19:38:11 +0000
313+++ testsuite/geis2/gtest_devices.cpp 2013-11-25 02:22:49 +0000
314@@ -1,7 +1,7 @@
315 /**
316 * GTEst test suite for GEIS v2 device handling.
317 *
318- * Copyright 2012 Canonical Ltd.
319+ * Copyright 2012-2013 Canonical Ltd.
320 */
321 #include "geis/geis.h"
322 #include "gtest_evemu_device.h"
323@@ -22,7 +22,7 @@
324 * Fixture for testing device handling.
325 * This is a separate class because gtest uses Java reflection.
326 */
327-class GeisDeviceTests
328+class GeisBasicDeviceTests
329 : public GTestGeisFixture
330 {
331 void
332@@ -36,7 +36,33 @@
333 };
334
335
336-TEST_F(GeisDeviceTests, filterWithNoDevices)
337+/**
338+ * Fixture for testing device handling.
339+ * This is a separate class because gtest uses Java reflection.
340+ */
341+class GeisAdvancedDeviceTests
342+: public GTestGeisFixture
343+{
344+
345+ void
346+ setup_geis()
347+ {
348+ new_device_.reset(new Testsuite::EvemuDevice(TEST_DEVICE_PROP_FILE));
349+ geis_ = geis_new(GEIS_INIT_TRACK_DEVICES,
350+ NULL);
351+ geis_initialized_in_callback_ = false;
352+ device_count_ = 0;
353+ }
354+
355+public:
356+ std::unique_ptr<Testsuite::EvemuDevice> new_device_;
357+ bool geis_initialized_in_callback_;
358+ int device_count_;
359+ GeisSubscription subscription_;
360+};
361+
362+
363+TEST_F(GeisBasicDeviceTests, filterWithNoDevices)
364 {
365 GeisSubscription sub = geis_subscription_new(geis_,
366 "no devices",
367@@ -65,7 +91,7 @@
368 * device-added event is received, a recording is run through the device.
369 * Gesture events should be detected.
370 */
371-TEST_F(GeisDeviceTests, addDeviceSubscription)
372+TEST_F(GeisBasicDeviceTests, addDeviceSubscription)
373 {
374 std::unique_ptr<Testsuite::EvemuDevice> new_device;
375 GeisBoolean device_has_been_created = GEIS_FALSE;
376@@ -132,7 +158,7 @@
377 *
378 * This test really just makes sure nothing segfaults on device removal.
379 */
380-TEST_F(GeisDeviceTests, removeDeviceSubscription)
381+TEST_F(GeisBasicDeviceTests, removeDeviceSubscription)
382 {
383 std::unique_ptr<Testsuite::EvemuDevice> new_device;
384
385@@ -183,7 +209,7 @@
386 * This test creates a devicve with known X and Y extents and verifies the
387 * extent attributes are reported with the expected values.
388 */
389-TEST_F(GeisDeviceTests, deviceAttrs)
390+TEST_F(GeisBasicDeviceTests, deviceAttrs)
391 {
392 GeisBoolean off = GEIS_FALSE;
393 geis_set_configuration(geis_, GEIS_CONFIG_DISCARD_DEVICE_MESSAGES, &off);
394@@ -222,4 +248,63 @@
395 }
396
397
398+/**
399+ * GEIS event callback to verify lp:1252447.
400+ *
401+ * This bug reveals that subscriptions created in an event callback when the
402+ * INIT_COMPLETE event is received never get activated for devices that are
403+ * present at GEIS start time.
404+ */
405+static void
406+lp_1252447_event_callback(::Geis geis, GeisEvent event, void* context)
407+{
408+ GeisAdvancedDeviceTests* fixture = (GeisAdvancedDeviceTests*)context;
409+ switch (geis_event_type(event))
410+ {
411+ case GEIS_EVENT_INIT_COMPLETE:
412+ {
413+ fixture->geis_initialized_in_callback_ = true;
414+ fixture->subscription_ = geis_subscription_new(geis,
415+ "any device",
416+ GEIS_SUBSCRIPTION_NONE);
417+ GeisFilter filter = geis_filter_new(geis, "filter");
418+ geis_filter_add_term(filter,
419+ GEIS_FILTER_CLASS,
420+ GEIS_GESTURE_ATTRIBUTE_TOUCHES, GEIS_FILTER_OP_EQ, 2,
421+ NULL);
422+ EXPECT_EQ(geis_subscription_add_filter(fixture->subscription_, filter),
423+ GEIS_STATUS_SUCCESS);
424+ EXPECT_EQ(geis_subscription_activate(fixture->subscription_),
425+ GEIS_STATUS_SUCCESS);
426+ break;
427+ }
428+
429+ case GEIS_EVENT_DEVICE_AVAILABLE:
430+ {
431+ ++fixture->device_count_;
432+ GeisSize active_sub_count = 0;
433+ geis_subscription_get_configuration(fixture->subscription_,
434+ GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS,
435+ &active_sub_count);
436+ EXPECT_GT(active_sub_count, 0);
437+ break;
438+ }
439+
440+ default:
441+ break;
442+ }
443+}
444+
445+TEST_F(GeisAdvancedDeviceTests, initialDeviceRegistrationWithCallbacks)
446+{
447+ geis_register_event_callback(geis_, lp_1252447_event_callback, this);
448+ geis_register_device_callback(geis_, lp_1252447_event_callback, this);
449+
450+ set_geis_event_handler([&](Geis, GeisEvent event) { });
451+ geis_dispatch_loop();
452+ ASSERT_TRUE(geis_initialized_in_callback_);
453+ ASSERT_GT(device_count_, 0);
454+}
455+
456+
457 } // anonymous namespace

Subscribers

People subscribed via source and target branches