On Mon, 2010-06-21 at 13:32 +0200, Javier Fernandez Garcia-Boente wrote:
> On Thu, 2010-06-17 at 11:25 +0200, Javier Fernandez Garcia-Boente wrote:

> 
> Anyway, getting back to the WebKit Geolocation unit tests topic, i think
> we should provide a way of forcing one provider as the selected one,
> even though if several are available. When programming unit tests, not
> every functionality have to be enabled; even more, some of them could be
> dangerous or inefficient for the automatic testing framework. The usual
> way of programming unit tests by setting a dummy GPS coordinates before
> executing the test and retrieving such dummy coordinates as the result
> of the operation, so the Manual provider seems to be the more
> appropriated for this way of use the Geolocation API.
> 
> Do you think this is the right way to proceed ? Implementing a way of
> selecting a fixed provider.
> 

New version of the patch attached. Besides that, after further
investigations in the WebKit part, it seems that the position-changed
notifications were not being received correctly in the WebKit unit
tests. I initially implemented the mockup calls this way:

GeocluePosition *pos = NULL;
const char *service = "org.freedesktop.Geoclue.Providers.Manual";
const char *path = "/org/freedesktop/Geoclue/Providers/Manual";
const char *iface = "org.freedesktop.Geoclue.Manual";

pos = geoclue_position_iface_new (service, path, iface);
geoclue_position_set_position (pos,accuracy,longitude,latitude,0,NULL);
g_object_unref (pos);

That way, the GeoclueManual provider stores the dummy position, but
there is no client connected to the postition-changed signal; since the
GeoclueManual has the ProvidesUpdate flag (already set because of the
Address iface), the master provider started afterwards, during the
actual unit test call, does not retrieve the correct info, but the
initial cached values. So, after figure out how to proceed I decided to
use the MasterClient instance to build the Position provider, so that
the client could be connected to the position-changed signal.

However, in the implementation of the SetPosition method i considered
better not to expose this method in the PositionIface, mainly because i
see such method as a way of performing unit tests. The problem if this
approach is that since the PositionIface does not implement itself the
SetPosition method the GeolcuePosition object created by the
MasterClient can be used to set the dummy position. The solution i chose
was to define a new API method in the MasterClient class, called
create_iface-osition, which internally calls to the
geoclue_position_iface_new for accessing directly to the selected
position provider. See bellow:

GError *error = NULL;
GeoclueMaster* master = geoclue_master_get_default();
GeoclueMasterClient* client = geoclue_master_create_client(master,0,0);
if (geoclue_master_client_set_requirements(client,
                                        GEOCLUE_ACCURACY_LEVEL_LOCALITY,
                                        0, false, GEOCLUE_RESOURCE_ALL,
                                        &error)) {

        pos = geoclue_master_client_create_iface_position (client,
                                        service, path, iface, &error);
        geoclue_position_set_position (pos, accuracy, longitude, 
                                        latitude, 0, &error);
        g_object_unref (pos);
}

/* FIXME: it can be unreferenced because it will receive the DBus 
   signal asynchronously, some time after unreferencing the
   client instance. */
//     g_object_unref(client);
g_object_unref(master);


Sorry for this long email, but at the end, i think I've got a solution
to at least enabling the success.html unit test in the WebKitGtk port.

Thanks you all, and greetings.


-- 
Javier Fernández García-Boente
http://blogs.igalia.com/jfernandez/
www.igalia.com
From 24dcea7bea1bea81a3d94e892a6f2e40fe6f0713 Mon Sep 17 00:00:00 2001
From: Javier Fernandez <[email protected]>
Date: Tue, 22 Jun 2010 23:00:24 +0000
Subject: [PATCH 1/3] * The Manual provider implements the Position interface.
 * The Manual provider implements a DBus method, called
 SetPosition, to set a dummy position for unit testing
 purposes.
 * The GeocluePosition class exposes a new constructor to
 pass as argument the specific Provider interface to be
 used when the remote operations are invoked.

---
 geoclue/geoclue-position.c               |   56 ++++++++++++++++++++
 geoclue/geoclue-position.h               |   11 ++++
 providers/manual/geoclue-manual.c        |   84 ++++++++++++++++++++++++++++++
 providers/manual/geoclue-manual.provider |    4 +-
 providers/manual/geoclue-manual.xml      |    6 ++
 5 files changed, 159 insertions(+), 2 deletions(-)

diff --git a/geoclue/geoclue-position.c b/geoclue/geoclue-position.c
index ea4d53f..2c768ce 100644
--- a/geoclue/geoclue-position.c
+++ b/geoclue/geoclue-position.c
@@ -168,6 +168,23 @@ geoclue_position_new (const char *service,
 			     NULL);
 }
 
+GeocluePosition *
+geoclue_position_iface_new (const char *service,
+                            const char *path,
+                            const char *iface)
+{
+        const char *interface = GEOCLUE_POSITION_INTERFACE_NAME;
+        if (iface != NULL) {
+                interface = iface;
+        }
+
+       return g_object_new (GEOCLUE_TYPE_POSITION,
+                            "service", service,
+                            "path", path,
+                            "interface", interface,
+                            NULL);
+}
+
 /**
  * geoclue_position_get_position:
  * @position: A #GeocluePosition object
@@ -232,6 +249,45 @@ geoclue_position_get_position (GeocluePosition  *position,
 }
 
 
+/**
+ * geoclue_position_get_position:
+ * @position: A #GeocluePosition object
+ * @accuracy: The expected accuracy for the operation.
+ * @longitude: The longitude in degrees or %NULL
+ * @latitude: The latitude in degrees or %NULL
+ * @altitude: The altitude in meters or %NULL
+ * @error: Pointer to returned #Gerror or %NULL
+ *
+ * Sets a Dummy GPS coordinates to be used for unit testing
+ * purposes.
+ */
+void
+geoclue_position_set_position (GeocluePosition  *position,
+                              int              accuracy,
+                              double           longitude,
+                              double           latitude,
+                              double           altitude,
+                              GError          **error)
+{
+       GeoclueProvider *provider = GEOCLUE_PROVIDER (position);
+        gboolean result = FALSE;
+
+        result = dbus_g_proxy_call (provider->proxy,
+                                    "SetPosition", error,
+                                    G_TYPE_INT, accuracy,
+                                    G_TYPE_DOUBLE, longitude,
+                                    G_TYPE_DOUBLE, latitude,
+                                    G_TYPE_DOUBLE, altitude,
+                                    G_TYPE_INVALID, G_TYPE_INVALID);
+
+       if (!result) {
+               g_warning ("provider: failed to set position from: %s",
+                          (*error)->message);
+       }
+
+}
+
+
 typedef struct _GeocluePositionAsyncData {
 	GeocluePosition *position;
 	GCallback callback;
diff --git a/geoclue/geoclue-position.h b/geoclue/geoclue-position.h
index 3d95672..d0f7e28 100644
--- a/geoclue/geoclue-position.h
+++ b/geoclue/geoclue-position.h
@@ -58,6 +58,17 @@ GType geoclue_position_get_type (void);
 GeocluePosition *geoclue_position_new (const char *service,
 				       const char *path);
 
+GeocluePosition *geoclue_position_iface_new (const char *service,
+                                             const char *path,
+                                             const char *iface);
+
+void geoclue_position_set_position (GeocluePosition  *position,
+                                    int              accuracy,
+                                    double           longitude,
+                                    double           latitude,
+                                    double           altitude,
+                                    GError          **error);
+
 GeocluePositionFields geoclue_position_get_position (GeocluePosition  *position,
 						     int              *timestamp,
 						     double           *latitude,
diff --git a/providers/manual/geoclue-manual.c b/providers/manual/geoclue-manual.c
index 3640c4b..c328a4a 100644
--- a/providers/manual/geoclue-manual.c
+++ b/providers/manual/geoclue-manual.c
@@ -60,6 +60,7 @@
 
 #include <geoclue/gc-provider.h>
 #include <geoclue/gc-iface-address.h>
+#include <geoclue/gc-iface-position.h>
 
 typedef struct {
 	GcProvider parent;
@@ -71,6 +72,11 @@ typedef struct {
 	int timestamp;
 	GHashTable *address;
 	GeoclueAccuracy *accuracy;
+
+        int acc;
+        double latitude;
+        double longitude;
+        double altitude;
 } GeoclueManual;
 
 typedef struct {
@@ -81,12 +87,23 @@ typedef struct {
 #define GEOCLUE_MANUAL(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), GEOCLUE_TYPE_MANUAL, GeoclueManual))
 
 static void geoclue_manual_address_init (GcIfaceAddressClass *iface);
+static void geoclue_manual_position_init (GcIfacePositionClass *iface);
 
 G_DEFINE_TYPE_WITH_CODE (GeoclueManual, geoclue_manual, GC_TYPE_PROVIDER,
+                         G_IMPLEMENT_INTERFACE (GC_TYPE_IFACE_POSITION,
+                                                geoclue_manual_position_init)
                          G_IMPLEMENT_INTERFACE (GC_TYPE_IFACE_ADDRESS,
                                                 geoclue_manual_address_init))
 
 static gboolean
+geoclue_manual_set_position (GeoclueManual *manual,
+                             gint acc,
+                             gdouble longitude,
+                             gdouble latitude,
+                             gdouble altitude,
+                             GError **error);
+
+static gboolean
 geoclue_manual_set_address (GeoclueManual *manual,
                             int valid_until,
                             GHashTable *address,
@@ -258,6 +275,22 @@ geoclue_manual_set_address_fields (GeoclueManual *manual,
 	return TRUE;
 }
 
+static gboolean
+geoclue_manual_set_position (GeoclueManual *manual,
+                             gint acc,
+                             gdouble longitude,
+                             gdouble latitude,
+                             gdouble altitude,
+                             GError **error)
+{
+        manual->acc = acc;
+        manual->longitude = longitude;
+        manual->latitude = latitude;
+        manual->altitude = altitude;
+
+        return TRUE;
+}
+
 
 static void
 finalize (GObject *object)
@@ -298,8 +331,53 @@ geoclue_manual_init (GeoclueManual *manual)
 	manual->address = geoclue_address_details_new ();
 	manual->accuracy = 
 		geoclue_accuracy_new (GEOCLUE_ACCURACY_LEVEL_NONE, 0, 0);
+
+        manual->longitude = G_MAXDOUBLE;
+        manual->latitude = G_MAXDOUBLE;
+        manual->altitude = G_MAXDOUBLE;
+}
+
+static gboolean
+get_position (GcIfacePosition        *iface,
+              GeocluePositionFields  *fields,
+              int                    *timestamp,
+              double                 *latitude,
+              double                 *longitude,
+              double                 *altitude,
+              GeoclueAccuracy       **accuracy,
+              GError                **error)
+{
+       GeoclueManual *obj = (GEOCLUE_MANUAL (iface));
+
+       *fields = GEOCLUE_POSITION_FIELDS_NONE;
+
+        /* Get previously stored coordinates.*/
+       if (latitude && (obj->latitude < G_MAXDOUBLE)) {
+                *latitude = obj->latitude;
+                *fields |= GEOCLUE_POSITION_FIELDS_LATITUDE;
+       }
+       if (longitude && (obj->longitude < G_MAXDOUBLE)) {
+                *longitude = obj->longitude;
+                *fields |= GEOCLUE_POSITION_FIELDS_LONGITUDE;
+       }
+       if (altitude && (obj->altitude < G_MAXDOUBLE)) {
+               *altitude = obj->altitude;
+                *fields |= GEOCLUE_POSITION_FIELDS_ALTITUDE;
+       }
+
+       time ((time_t *)timestamp);
+
+       if (*fields == GEOCLUE_POSITION_FIELDS_NONE) {
+               *accuracy = geoclue_accuracy_new (GEOCLUE_ACCURACY_LEVEL_NONE,
+                                                 0, 0);
+       } else {
+               *accuracy = geoclue_accuracy_new (GEOCLUE_ACCURACY_LEVEL_LOCALITY,
+                                                 obj->acc, 0);
+       }
+       return TRUE;
 }
 
+
 static gboolean
 get_address (GcIfaceAddress   *gc,
              int              *timestamp,
@@ -329,6 +407,12 @@ geoclue_manual_address_init (GcIfaceAddressClass *iface)
 	iface->get_address = get_address;
 }
 
+static void
+geoclue_manual_position_init (GcIfacePositionClass *iface)
+{
+       iface->get_position = get_position;
+}
+
 int
 main (int    argc,
       char **argv)
diff --git a/providers/manual/geoclue-manual.provider b/providers/manual/geoclue-manual.provider
index c45a9da..c6e3e39 100644
--- a/providers/manual/geoclue-manual.provider
+++ b/providers/manual/geoclue-manual.provider
@@ -2,6 +2,6 @@
 Name=Manual
 Service=org.freedesktop.Geoclue.Providers.Manual
 Path=/org/freedesktop/Geoclue/Providers/Manual
-Interfaces=org.freedesktop.Geoclue.Address
+Interfaces=org.freedesktop.Geoclue.Position;org.freedesktop.Geoclue.Address
 Provides=ProvidesUpdates
-Accuracy=Street
+Accuracy=Locality
diff --git a/providers/manual/geoclue-manual.xml b/providers/manual/geoclue-manual.xml
index 9569a46..5a0575e 100644
--- a/providers/manual/geoclue-manual.xml
+++ b/providers/manual/geoclue-manual.xml
@@ -16,5 +16,11 @@
       <arg type="s" name="postalcode" direction="in"/>
       <arg type="s" name="street" direction="in"/>
     </method>
+    <method name="SetPosition">
+      <arg type="i" name="acc" direction="in"/>
+      <arg type="d" name="longitude" direction="in"/>
+      <arg type="d" name="latitude" direction="in"/>
+      <arg type="d" name="altitude" direction="in"/>
+    </method>
   </interface>
 </node>
-- 
1.7.0.4

From 9a8db73c1be9fc1a1a97351974c58b8e72b7d48a Mon Sep 17 00:00:00 2001
From: Javier Fernandez <[email protected]>
Date: Thu, 24 Jun 2010 16:36:19 +0000
Subject: [PATCH 2/3] The GeoclueManual emits the postition-changed signal after
 updating the stored position info.

---
 providers/manual/geoclue-manual.c |   89 ++++++++++++++++++++++++++++++-------
 1 files changed, 72 insertions(+), 17 deletions(-)

diff --git a/providers/manual/geoclue-manual.c b/providers/manual/geoclue-manual.c
index c328a4a..da3bc97 100644
--- a/providers/manual/geoclue-manual.c
+++ b/providers/manual/geoclue-manual.c
@@ -104,6 +104,13 @@ geoclue_manual_set_position (GeoclueManual *manual,
                              GError **error);
 
 static gboolean
+geoclue_manual_set_position_fields (GeoclueManual         *manual,
+                                    GeocluePositionFields *fields,
+                                    gint                  *timestamp,
+                                    GeoclueAccuracy       **accuracy,
+                                    GError                **error);
+
+static gboolean
 geoclue_manual_set_address (GeoclueManual *manual,
                             int valid_until,
                             GHashTable *address,
@@ -121,6 +128,7 @@ geoclue_manual_set_address_fields (GeoclueManual *manual,
                                    char *street,
                                    GError **error);
 
+
 #include "geoclue-manual-glue.h"
 
 
@@ -276,6 +284,45 @@ geoclue_manual_set_address_fields (GeoclueManual *manual,
 }
 
 static gboolean
+geoclue_manual_set_position_fields (GeoclueManual         *manual,
+                                    GeocluePositionFields *fields,
+                                    gint                  *timestamp,
+                                    GeoclueAccuracy       **accuracy,
+                                    GError                **error)
+{
+        g_assert (fields != NULL);
+
+        *fields = GEOCLUE_POSITION_FIELDS_NONE;
+
+        /* Get previously stored coordinates.*/
+        if (manual->latitude < G_MAXDOUBLE) {
+                *fields |= GEOCLUE_POSITION_FIELDS_LATITUDE;
+        }
+        if (manual->longitude < G_MAXDOUBLE) {
+                *fields |= GEOCLUE_POSITION_FIELDS_LONGITUDE;
+        }
+        if (manual->altitude < G_MAXDOUBLE) {
+                *fields |= GEOCLUE_POSITION_FIELDS_ALTITUDE;
+        }
+
+        if (timestamp) {
+                time ((time_t *)timestamp);
+        }
+
+        if (accuracy != NULL) {
+                if (*fields == GEOCLUE_POSITION_FIELDS_NONE) {
+                        *accuracy = geoclue_accuracy_new (GEOCLUE_ACCURACY_LEVEL_NONE,
+                                                          0, 0);
+                } else {
+                        *accuracy = geoclue_accuracy_new (GEOCLUE_ACCURACY_LEVEL_LOCALITY,
+                                                          manual->acc, 0);
+                }
+        }
+
+        return TRUE;
+}
+
+static gboolean
 geoclue_manual_set_position (GeoclueManual *manual,
                              gint acc,
                              gdouble longitude,
@@ -283,11 +330,28 @@ geoclue_manual_set_position (GeoclueManual *manual,
                              gdouble altitude,
                              GError **error)
 {
+        GeocluePositionFields fields;
+        GeoclueAccuracy *accuracy = NULL;
+        gint timestamp = 0;
+
         manual->acc = acc;
         manual->longitude = longitude;
         manual->latitude = latitude;
         manual->altitude = altitude;
 
+        /* Get fields and accuracy. */
+        geoclue_manual_set_position_fields (manual, &fields,
+                                            &timestamp,
+                                            &accuracy,
+                                            error);
+
+        /* Emit "position-changed" signal. */
+	gc_iface_position_emit_position_changed
+		(GC_IFACE_POSITION (manual), fields,
+		 (int) timestamp,
+		 latitude, longitude, altitude,
+		 accuracy);
+
         return TRUE;
 }
 
@@ -349,31 +413,22 @@ get_position (GcIfacePosition        *iface,
 {
        GeoclueManual *obj = (GEOCLUE_MANUAL (iface));
 
-       *fields = GEOCLUE_POSITION_FIELDS_NONE;
-
         /* Get previously stored coordinates.*/
-       if (latitude && (obj->latitude < G_MAXDOUBLE)) {
+       if (latitude) {
                 *latitude = obj->latitude;
-                *fields |= GEOCLUE_POSITION_FIELDS_LATITUDE;
        }
-       if (longitude && (obj->longitude < G_MAXDOUBLE)) {
+       if (longitude) {
                 *longitude = obj->longitude;
-                *fields |= GEOCLUE_POSITION_FIELDS_LONGITUDE;
        }
-       if (altitude && (obj->altitude < G_MAXDOUBLE)) {
+       if (altitude) {
                *altitude = obj->altitude;
-                *fields |= GEOCLUE_POSITION_FIELDS_ALTITUDE;
        }
 
-       time ((time_t *)timestamp);
-
-       if (*fields == GEOCLUE_POSITION_FIELDS_NONE) {
-               *accuracy = geoclue_accuracy_new (GEOCLUE_ACCURACY_LEVEL_NONE,
-                                                 0, 0);
-       } else {
-               *accuracy = geoclue_accuracy_new (GEOCLUE_ACCURACY_LEVEL_LOCALITY,
-                                                 obj->acc, 0);
-       }
+       /* Get fields and accuracy. */
+       geoclue_manual_set_position_fields (obj, fields,
+                                           timestamp,
+                                           accuracy,
+                                           error);
        return TRUE;
 }
 
-- 
1.7.0.4

From 17ac7191f82a74fb0993d21274210a3c6618d930 Mon Sep 17 00:00:00 2001
From: Javier Fernandez <[email protected]>
Date: Thu, 24 Jun 2010 16:36:57 +0000
Subject: [PATCH 3/3] The GeoclueMasterClient object has a new GeocluePosition instance
 constructor to use the geoclue_position_iface_new, allowing calls
 to the selected positoin provider directly and still receive the
 position-changed notifications.

---
 geoclue/geoclue-master-client.c |   16 ++++++++++++++++
 geoclue/geoclue-master-client.h |    1 +
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/geoclue/geoclue-master-client.c b/geoclue/geoclue-master-client.c
index 248bc32..eccbb32 100644
--- a/geoclue/geoclue-master-client.c
+++ b/geoclue/geoclue-master-client.c
@@ -527,6 +527,22 @@ geoclue_master_client_create_position (GeoclueMasterClient *client,
 	return geoclue_position_new (GEOCLUE_MASTER_DBUS_SERVICE, priv->object_path);
 }
 
+GeocluePosition *
+geoclue_master_client_create_iface_position (GeoclueMasterClient *client,
+                                             const char *service,
+                                             const char *path,
+                                             const char *iface,
+                                             GError **error)
+{
+	GeoclueMasterClientPrivate *priv;
+	
+	priv = GET_PRIVATE (client);
+	
+	if (!org_freedesktop_Geoclue_MasterClient_position_start (priv->proxy, error)) {
+		return NULL;
+	}
+	return geoclue_position_iface_new (service, path, iface);
+}
 
 static void
 position_start_async_callback (DBusGProxy                   *proxy, 
diff --git a/geoclue/geoclue-master-client.h b/geoclue/geoclue-master-client.h
index 043f26c..ecd6f06 100644
--- a/geoclue/geoclue-master-client.h
+++ b/geoclue/geoclue-master-client.h
@@ -88,6 +88,7 @@ void geoclue_master_client_create_address_async (GeoclueMasterClient   *client,
 
 
 GeocluePosition *geoclue_master_client_create_position (GeoclueMasterClient *client, GError **error);
+GeocluePosition *geoclue_master_client_create_iface_position (GeoclueMasterClient *client, const char *service, const char *object, const char *iface, GError **error);
 typedef void (*CreatePositionCallback) (GeoclueMasterClient *client,
 					GeocluePosition     *position,
 					GError              *error,
-- 
1.7.0.4

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
GeoClue mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/geoclue

Reply via email to