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

[Stephen M. Webb]

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

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).

[lp:~bregma/geis/lp-1252447]

329. By Stephen M. Webb on 2013-11-25

fixed return status of new grail back end functions

330. By Stephen M. Webb on 2013-11-25

updated some copyright dates

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:328
http://jenkins.qa.ubuntu.com/job/geis-ci/13/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/geis-trusty-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/geis-trusty-armhf-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/geis-trusty-i386-ci/8

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/geis-ci/13/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:330
http://jenkins.qa.ubuntu.com/job/geis-ci/14/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/geis-trusty-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/geis-trusty-armhf-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/geis-trusty-i386-ci/9

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/geis-ci/14/rebuild

review: Approve (continuous-integration)

[Brandon Schaefer (brandontschaefer)]

Testing are passing, and LGTM

review: Approve

=== modified file 'include/geis/geis.h'
--- include/geis/geis.h	2012-12-04 12:39:16 +0000
+++ include/geis/geis.h	2013-11-25 02:22:49 +0000
@@ -2,7 +2,7 @@
  * @file geis/geis.h
  * This is the public interface for the GEIS gesture API.
  *
- * Copyright 2010, 2011 Canonical Ltd.
+ * Copyright 2010-2013 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or modify it under
  * the terms of version 3 of the GNU Lesser General Public License as published
@@ -803,6 +803,13 @@
  */
 #define GEIS_CONFIG_TAP_TIMEOUT "com.canonical.oif.tap.timeout"
 
+/**
+ * @def GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS
+ * The number of subscriptions currently active in the back end.
+ * This config is query-only and intended for unit test validations.
+ */
+#define GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS "com.canonical.oif.debug.active_subs"
+
 /* @} */
 
 /**
 
=== modified file 'libgeis/backend/dbus/geis_dbus_backend.c'
--- libgeis/backend/dbus/geis_dbus_backend.c	2012-12-04 12:39:16 +0000
+++ libgeis/backend/dbus/geis_dbus_backend.c	2013-11-25 02:22:49 +0000
@@ -4,7 +4,7 @@
  */
 
 /*
- * Copyright 2011 Canonical Ltd.
+ * Copyright 2011-2013 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or modify it under
  * the terms of version 3 of the GNU Lesser General Public License as published
@@ -274,6 +274,8 @@
   _geis_dbus_backend_create_token,
   _geis_dbus_accept_gesture,
   _geis_dbus_reject_gesture,
+  NULL,
+  NULL,
   _geis_dbus_get_configuration,
   _geis_dbus_set_configuration,
 };
 
=== modified file 'libgeis/backend/grail/geis_grail_backend.c'
--- libgeis/backend/grail/geis_grail_backend.c	2013-11-20 15:45:43 +0000
+++ libgeis/backend/grail/geis_grail_backend.c	2013-11-25 02:22:49 +0000
@@ -4,7 +4,7 @@
  */
 
 /*
- * Copyright 2011, 2012 Canonical Ltd.
+ * Copyright 2011-2013 Canonical Ltd.
  *
  * This library is free software: you can redistribute it and/or modify it 
  * under the terms of the GNU Lesser General Public License version 3
@@ -812,11 +812,14 @@
       GeisBoolean device_applies = GEIS_TRUE;
       for (GeisSize tindex = 0; tindex < geis_filter_term_count(*fit); ++tindex)
       {
-        if (!geis_filter_term_match_device(geis_filter_term(*fit, tindex),
-                                           device))
+        GeisFilterTerm term = geis_filter_term(*fit, tindex);
+        if (geis_filter_term_facility(term) == GEIS_FILTER_DEVICE)
         {
-          device_applies = GEIS_FALSE;
-          break;
+          if (!geis_filter_term_match_device(term, device))
+          {
+            device_applies = GEIS_FALSE;
+            break;
+          }
         }
       }
 
@@ -1013,7 +1016,6 @@
                              / sizeof(struct GeisFilterableAttribute);
 
   geis_register_device(gbe->geis, geis_device, attr_count, attrs);
-  _grail_be_subscribe_new_device(gbe, geis_device);
 
   /* We are not going to hold a pointer to this geis_device ourselves  */
   geis_device_unref(geis_device);
@@ -1812,6 +1814,13 @@
     return retval;
   }
 
+  if (0 == strcmp(item_name, GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS))
+  {
+    struct GeisGrailSubscriptionData *sub_data = geis_subscription_pdata(subscription);
+    *((GeisSize*)item_value) = geis_ugsubscription_count(sub_data->ugstore);
+    retval =  GEIS_STATUS_SUCCESS;
+  }
+
   #define GEIS_GRAIL_CHECK_GESTURE_CONFIG(gesture, Gesture, GESTURE) \
   if (strcmp(item_name, GEIS_CONFIG_##GESTURE##_TIMEOUT) == 0) \
   { \
@@ -1830,7 +1839,7 @@
         item_value); \
   }
 
-  GEIS_GRAIL_CHECK_GESTURE_CONFIG(drag, Drag, DRAG)
+  else GEIS_GRAIL_CHECK_GESTURE_CONFIG(drag, Drag, DRAG)
   else GEIS_GRAIL_CHECK_GESTURE_CONFIG(pinch, Pinch, PINCH)
   else GEIS_GRAIL_CHECK_GESTURE_CONFIG(rotate, Rotate, ROTATE)
   else GEIS_GRAIL_CHECK_GESTURE_CONFIG(tap, Tap, TAP)
@@ -1921,12 +1930,36 @@
   return retval;
 }
 
+static GeisStatus
+_grail_be_activate_device(GeisBackend be, GeisDevice device)
+{
+  GeisStatus status = GEIS_STATUS_SUCCESS;
+  GeisGrailBackend gbe = (GeisGrailBackend)be;
+  _grail_be_subscribe_new_device(gbe, device);
+  return status;
+}
+
+
+static GeisStatus
+_grail_be_deactivate_device(GeisBackend be, GeisDevice device)
+{
+  GeisStatus status = GEIS_STATUS_SUCCESS;
+  GeisGrailBackend gbe = (GeisGrailBackend)be;
+  UFDevice ufdevice = _grail_be_ufdevice_from_device_id(gbe,
+                                                geis_device_id(device));
+  _grail_be_unsubscribe_removed_device(gbe, ufdevice);
+  return status;
+}
+
+
 static struct GeisBackendVtable gbe_vtbl = {
   _geis_grail_backend_construct,
   _geis_grail_backend_finalize,
   geis_grail_token_new,
   _grail_be_accept_gesture,
   _grail_be_reject_gesture,
+  _grail_be_activate_device,
+  _grail_be_deactivate_device,
   _grail_be_get_configuration,
   _grail_be_set_configuration
 };
 
=== modified file 'libgeis/backend/test_fixture/geis_backend_test_fixture.c'
--- libgeis/backend/test_fixture/geis_backend_test_fixture.c	2012-12-04 12:39:16 +0000
+++ libgeis/backend/test_fixture/geis_backend_test_fixture.c	2013-11-25 02:22:49 +0000
@@ -2,7 +2,7 @@
  * @file geis_backend_test_fixture.c
  * @brief GEIS mock back end test fixture implementation
  *
- * Copyright 2010 Canonical Ltd.
+ * Copyright 2010-2013 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or modify it under
  * the terms of version 3 of the GNU Lesser General Public License as published
@@ -274,6 +274,8 @@
   _create_token,
   _gmock_accept_gesture,
   _gmock_reject_gesture,
+  NULL,
+  NULL,
   _gmock_get_configuration,
   _gmock_set_configuration
 };
 
=== modified file 'libgeis/geis.c'
--- libgeis/geis.c	2012-12-04 12:39:16 +0000
+++ libgeis/geis.c	2013-11-25 02:22:49 +0000
@@ -9,7 +9,7 @@
  */
 
 /*
- * Copyright 2010, 2011 Canonical Ltd.
+ * Copyright 2010-2013 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or modify it under
  * the terms of version 3 of the GNU Lesser General Public License as published
@@ -156,6 +156,7 @@
   if (event_type == GEIS_EVENT_DEVICE_AVAILABLE)
   {
     geis_device_bag_insert(geis->devices, device);
+    geis_backend_activate_device(geis->backend, device);
   }
 
   if (geis->device_event_callback != GEIS_DEFAULT_EVENT_CALLBACK)
@@ -166,6 +167,7 @@
 
   if (event_type == GEIS_EVENT_DEVICE_UNAVAILABLE)
   {
+    geis_backend_deactivate_device(geis->backend, device);
     geis_device_bag_remove(geis->devices, device);
   }
 
 
=== modified file 'libgeis/geis_backend.c'
--- libgeis/geis_backend.c	2012-12-04 12:39:16 +0000
+++ libgeis/geis_backend.c	2013-11-25 02:22:49 +0000
@@ -2,7 +2,7 @@
  * @file geis_backend.c
  * @brief internal GEIS back end base class implementation
  *
- * Copyright 2010 Canonical Ltd.
+ * Copyright 2010-2013 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or modify it under
  * the terms of version 3 of the GNU Lesser General Public License as published
@@ -199,6 +199,26 @@
 }
 
 
+GeisStatus
+geis_backend_activate_device(GeisBackend  be,
+                             GeisDevice   device)
+{
+  if (be->be_class->vtbl->activate_device)
+    return  be->be_class->vtbl->activate_device(_data_from_be(be), device);
+  return GEIS_STATUS_SUCCESS;
+}
+
+
+GeisStatus
+geis_backend_deactivate_device(GeisBackend  be,
+                               GeisDevice   device)
+{
+  if (be->be_class->vtbl->deactivate_device)
+    return  be->be_class->vtbl->deactivate_device(_data_from_be(be), device);
+  return GEIS_STATUS_SUCCESS;
+}
+
+
 /*
  * Gets a back end configuration value.
  */
 
=== modified file 'libgeis/geis_backend.h'
--- libgeis/geis_backend.h	2012-12-04 12:39:16 +0000
+++ libgeis/geis_backend.h	2013-11-25 02:22:49 +0000
@@ -2,7 +2,7 @@
  * @file geis_backend.h
  * @brief internal GEIS back end base class public interface
  *
- * Copyright 2010, 2012 Canonical Ltd.
+ * Copyright 2010-2013 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or modify it under
  * the terms of version 3 of the GNU Lesser General Public License as published
@@ -90,6 +90,34 @@
                             GeisGestureId gesture_id);
 
 /**
+ * Tells the back end to activate a device after it's been set up.
+ *
+ * @param[in] be          The GEIS back end.
+ * @param[in] device      The GEIS device for which subscription wll activate.
+ *
+ * The back end sends and event to the middle end when a new device has been
+ * detected.  The middle end sets up some structures etc., then tells the back
+ * end to activate the device (which may actually cause some subscriptions to be
+ * activated), then forwards the new-device event to the front end where the
+ * application can be made aware of it.
+ */
+GeisStatus
+geis_backend_activate_device(GeisBackend  be,
+                             GeisDevice   device);
+
+/**
+ * Tells the back end to activate a device after it's been set up.
+ *
+ * @param[in] be          The GEIS back end.
+ * @param[in] device      The GEIS device for which subscription wll deactivate.
+ *
+ * See geis_backend_activate_device for more information.
+ */
+GeisStatus
+geis_backend_deactivate_device(GeisBackend  be,
+                               GeisDevice   device);
+
+/**
  * Gets a back end configuration value.
  *
  * @param[in]  be                       The back end.
 
=== modified file 'libgeis/geis_backend_protected.h'
--- libgeis/geis_backend_protected.h	2012-12-04 12:39:16 +0000
+++ libgeis/geis_backend_protected.h	2013-11-25 02:22:49 +0000
@@ -7,7 +7,7 @@
  */
 
 /*
- * Copyright 2010, 2012 Canonical Ltd.
+ * Copyright 2010-2013 Canonical Ltd.
  *
  * This library is free software; you can redistribute it and/or modify it under
  * the terms of version 3 of the GNU Lesser General Public License as published
@@ -39,6 +39,8 @@
   GeisBackendToken (* create_token)(GeisBackend, GeisBackendTokenInitState);
   GeisStatus       (* accept_gesture)(GeisBackend, GeisGroup, GeisGestureId);
   GeisStatus       (* reject_gesture)(GeisBackend, GeisGroup, GeisGestureId);
+  GeisStatus       (* activate_device)(GeisBackend, GeisDevice);
+  GeisStatus       (* deactivate_device)(GeisBackend, GeisDevice);
   GeisStatus       (* get_configuration)(GeisBackend, GeisSubscription, GeisString, GeisPointer);
   GeisStatus       (* set_configuration)(GeisBackend, GeisSubscription, GeisString, GeisPointer);
 } *GeisBackendVtable;
 
=== modified file 'testsuite/geis2/gtest_devices.cpp'
--- testsuite/geis2/gtest_devices.cpp	2013-11-20 19:38:11 +0000
+++ testsuite/geis2/gtest_devices.cpp	2013-11-25 02:22:49 +0000
@@ -1,7 +1,7 @@
 /**
  * GTEst test suite for GEIS v2 device handling.
  *
- * Copyright 2012 Canonical Ltd.
+ * Copyright 2012-2013 Canonical Ltd.
  */
 #include "geis/geis.h"
 #include "gtest_evemu_device.h"
@@ -22,7 +22,7 @@
  * Fixture for testing device handling.
  * This is a separate class because gtest uses Java reflection.
  */
-class GeisDeviceTests
+class GeisBasicDeviceTests
 : public GTestGeisFixture
 {
   void
@@ -36,7 +36,33 @@
 };
 
 
-TEST_F(GeisDeviceTests, filterWithNoDevices)
+/**
+ * Fixture for testing device handling.
+ * This is a separate class because gtest uses Java reflection.
+ */
+class GeisAdvancedDeviceTests
+: public GTestGeisFixture
+{
+
+  void
+  setup_geis()
+  {
+    new_device_.reset(new Testsuite::EvemuDevice(TEST_DEVICE_PROP_FILE));
+    geis_ = geis_new(GEIS_INIT_TRACK_DEVICES,
+                     NULL);
+    geis_initialized_in_callback_ = false;
+    device_count_ = 0;
+  }
+
+public:
+  std::unique_ptr<Testsuite::EvemuDevice> new_device_;
+  bool                                    geis_initialized_in_callback_;
+  int                                     device_count_;
+  GeisSubscription                        subscription_;
+};
+
+
+TEST_F(GeisBasicDeviceTests, filterWithNoDevices)
 {
   GeisSubscription sub = geis_subscription_new(geis_,
                                                "no devices",
@@ -65,7 +91,7 @@
  * device-added event is received, a recording is run through the device.
  * Gesture events should be detected.
  */
-TEST_F(GeisDeviceTests, addDeviceSubscription)
+TEST_F(GeisBasicDeviceTests, addDeviceSubscription)
 {
   std::unique_ptr<Testsuite::EvemuDevice> new_device;
   GeisBoolean device_has_been_created = GEIS_FALSE;
@@ -132,7 +158,7 @@
  *
  * This test really just makes sure nothing segfaults on device removal.
  */
-TEST_F(GeisDeviceTests, removeDeviceSubscription)
+TEST_F(GeisBasicDeviceTests, removeDeviceSubscription)
 {
   std::unique_ptr<Testsuite::EvemuDevice> new_device;
 
@@ -183,7 +209,7 @@
  * This test creates a devicve with known X and Y extents and verifies the
  * extent attributes are reported with the expected values.
  */
-TEST_F(GeisDeviceTests, deviceAttrs)
+TEST_F(GeisBasicDeviceTests, deviceAttrs)
 {
   GeisBoolean off = GEIS_FALSE;
   geis_set_configuration(geis_, GEIS_CONFIG_DISCARD_DEVICE_MESSAGES, &off);
@@ -222,4 +248,63 @@
 }
 
 
+/**
+ * GEIS event callback to verify lp:1252447.
+ *
+ * This bug reveals that subscriptions created in an event callback when the
+ * INIT_COMPLETE event is received never get activated for devices that are
+ * present at GEIS start time.
+ */
+static void
+lp_1252447_event_callback(::Geis geis, GeisEvent event, void* context)
+{
+  GeisAdvancedDeviceTests* fixture = (GeisAdvancedDeviceTests*)context;
+  switch (geis_event_type(event))
+  {
+    case GEIS_EVENT_INIT_COMPLETE:
+    {
+      fixture->geis_initialized_in_callback_ = true;
+      fixture->subscription_ = geis_subscription_new(geis,
+                                                     "any device",
+                                                     GEIS_SUBSCRIPTION_NONE);
+      GeisFilter filter = geis_filter_new(geis, "filter");
+      geis_filter_add_term(filter,
+                           GEIS_FILTER_CLASS,
+                           GEIS_GESTURE_ATTRIBUTE_TOUCHES, GEIS_FILTER_OP_EQ, 2,
+                           NULL);
+      EXPECT_EQ(geis_subscription_add_filter(fixture->subscription_, filter),
+                GEIS_STATUS_SUCCESS);
+      EXPECT_EQ(geis_subscription_activate(fixture->subscription_),
+                GEIS_STATUS_SUCCESS);
+      break;
+    }
+
+    case GEIS_EVENT_DEVICE_AVAILABLE:
+    {
+      ++fixture->device_count_;
+      GeisSize active_sub_count = 0;
+      geis_subscription_get_configuration(fixture->subscription_,
+                                          GEIS_CONFIG_NUM_ACTIVE_SUBSCRIPTIONS,
+                                          &active_sub_count);
+      EXPECT_GT(active_sub_count, 0);
+      break;
+    }
+
+    default:
+      break;
+  }
+}
+
+TEST_F(GeisAdvancedDeviceTests, initialDeviceRegistrationWithCallbacks)
+{
+  geis_register_event_callback(geis_, lp_1252447_event_callback, this);
+  geis_register_device_callback(geis_, lp_1252447_event_callback, this);
+
+  set_geis_event_handler([&](Geis, GeisEvent event) { });
+  geis_dispatch_loop();
+  ASSERT_TRUE(geis_initialized_in_callback_);
+  ASSERT_GT(device_count_, 0);
+}
+
+
 } // anonymous namespace