[PATCH v3 0/7] Add GPS atom

[PATCH v3 0/7] Add GPS atom

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 2227 bytes --]

These patches add GPS atom to ofono, and implement it for mbm modem.

Changes since V2: there is a new API
 - removed Settings property
 - added gpsagent
 - removed additional properties emitted once Powered=True
 - removed gps_set_property and gps_signal_ which are not needed
   anymore
 - removed enable-gps and disable-gps scripts. Add
   test-location-reporting script
 - adapted gps atom, gps mbm driver, and documentation to the new API
 
Changes since V1:
 - renamed ofono gps interface ".Gps" to "LocationReporting"
 - split set_gps method (in ofono_gps_driver) into enable and disable
   methods
 - additional information is sent by enable method to the core :
   device node and type
 - dropped query_gps which is not needed anymore
 - changed the port used by the gps driver to the proper one (dev port)
 - additional properties is emitted once Powered=True inside a 
   Settings dictionary

Rafael Ignacio Zurita (7):
  gps: add public header
  gps: add gps atom and gpsagent implementations
  mbmmodem: add gps atom
  plugins: add gps atom to mbm
  plugins/udev.c: add gps comparison for add_mbm registered modem
  gps: add test script
  gps: add documentation

 Makefile.am                  |   14 ++-
 doc/gps-api.txt              |   49 +++++++
 drivers/mbmmodem/gps.c       |  247 ++++++++++++++++++++++++++++++++
 drivers/mbmmodem/mbmmodem.c  |    2 +
 drivers/mbmmodem/mbmmodem.h  |    3 +
 include/gps.h                |   76 ++++++++++
 plugins/mbm.c                |    8 +
 plugins/udev.c               |    5 +-
 src/gps.c                    |  326 ++++++++++++++++++++++++++++++++++++++++++
 src/gpsagent.c               |  145 +++++++++++++++++++
 src/gpsagent.h               |   37 +++++
 src/ofono.conf               |    1 +
 src/ofono.h                  |    2 +
 test/test-location-reporting |   49 +++++++
 14 files changed, 957 insertions(+), 7 deletions(-)
 create mode 100644 doc/gps-api.txt
 create mode 100644 drivers/mbmmodem/gps.c
 create mode 100644 include/gps.h
 create mode 100644 src/gps.c
 create mode 100644 src/gpsagent.c
 create mode 100644 src/gpsagent.h
 create mode 100644 test/test-location-reporting

-- 
1.7.2.3

[PATCH v3 1/7] gps: add public header

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 3230 bytes --]

---
 Makefile.am   |    2 +-
 include/gps.h |   76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 1 deletions(-)
 create mode 100644 include/gps.h

diff --git a/Makefile.am b/Makefile.am
index 9933e32..7374f70 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -13,7 +13,7 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/radio-settings.h include/stk.h \
 			include/audio-settings.h include/nettime.h \
 			include/ctm.h include/cdma-voicecall.h \
-			include/cdma-sms.h include/sim-auth.h
+			include/cdma-sms.h include/sim-auth.h include/gps.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
diff --git a/include/gps.h b/include/gps.h
new file mode 100644
index 0000000..c70e90f
--- /dev/null
+++ b/include/gps.h
@@ -0,0 +1,76 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *  Copyright (C) 2010 ProFUSION embedded systems.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __OFONO_GPS_H
+#define __OFONO_GPS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <ofono/types.h>
+
+struct ofono_gps;
+
+enum ofono_gps_device_type {
+	OFONO_GPS_DEVICE_TYPE_NMEA = 0,
+};
+
+typedef void (*ofono_gps_enable_cb_t)(const struct ofono_error *error,
+					int fd,
+					void *data);
+typedef void (*ofono_gps_disable_cb_t)(const struct ofono_error *error,
+								void *data);
+
+struct ofono_gps_driver {
+	const char *name;
+	enum ofono_gps_device_type type;
+	int (*probe)(struct ofono_gps *gps, unsigned int vendor, void *data);
+	void (*remove)(struct ofono_gps *gps);
+	void (*enable)(struct ofono_gps *gps,
+					ofono_gps_enable_cb_t cb, void *data);
+	void (*disable)(struct ofono_gps *gps,
+					ofono_gps_disable_cb_t cb, void *data);
+};
+
+int ofono_gps_driver_register(const struct ofono_gps_driver *d);
+void ofono_gps_driver_unregister(const struct ofono_gps_driver *d);
+
+struct ofono_gps *ofono_gps_create(struct ofono_modem *modem,
+					unsigned int vendor, const char *driver,
+					void *data);
+
+void ofono_gps_register(struct ofono_gps *gps);
+void ofono_gps_remove(struct ofono_gps *gps);
+
+void ofono_gps_set_data(struct ofono_gps *gps, void *data);
+void *ofono_gps_get_data(struct ofono_gps *gps);
+
+int ofono_gps_get_fd(struct ofono_gps *gps);
+
+struct ofono_modem *ofono_gps_get_modem(struct ofono_gps *gps);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_GPS_H */
-- 
1.7.2.3

[PATCH v3 2/7] gps: add gps atom and gpsagent implementations

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 15649 bytes --]

---
 Makefile.am    |    3 +-
 src/gps.c      |  326 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/gpsagent.c |  145 +++++++++++++++++++++++++
 src/gpsagent.h |   37 +++++++
 src/ofono.conf |    1 +
 src/ofono.h    |    2 +
 6 files changed, 513 insertions(+), 1 deletions(-)
 create mode 100644 src/gps.c
 create mode 100644 src/gpsagent.c
 create mode 100644 src/gpsagent.h

diff --git a/Makefile.am b/Makefile.am
index 7374f70..de70976 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -355,7 +355,8 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
 			src/simfs.c src/simfs.h src/audio-settings.c \
 			src/smsagent.c src/smsagent.h src/ctm.c \
 			src/cdma-voicecall.c src/sim-auth.c \
-			src/message.h src/message.c
+			src/message.h src/message.c src/gps.c \
+			src/gpsagent.c src/gpsagent.h
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
 
diff --git a/src/gps.c b/src/gps.c
new file mode 100644
index 0000000..b5916ee
--- /dev/null
+++ b/src/gps.c
@@ -0,0 +1,326 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2010 ProFUSION embedded systems.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <gdbus.h>
+
+#include "ofono.h"
+#include "common.h"
+#include "gpsagent.h"
+
+static GSList *g_drivers = NULL;
+
+struct ofono_gps {
+	DBusMessage *pending;
+	const struct ofono_gps_driver *driver;
+	void *driver_data;
+	struct ofono_atom *atom;
+	int fd;
+	struct gps_agent *agent;
+};
+
+static const char *gps_device_type_to_string(enum ofono_gps_device_type type)
+{
+	switch (type) {
+	case OFONO_GPS_DEVICE_TYPE_NMEA:
+		return "nmea";
+	};
+
+	return NULL;
+}
+
+static DBusMessage *gps_get_properties(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+
+{
+	struct ofono_gps *gps = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	const char *type;
+
+	reply = dbus_message_new_method_return(msg);
+	if (reply == NULL)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					OFONO_PROPERTIES_ARRAY_SIGNATURE,
+					&dict);
+
+	type = gps_device_type_to_string(gps->driver->type);
+	ofono_dbus_dict_append(&dict, "Type", DBUS_TYPE_STRING, &type);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
+static void gps_enable_callback(const struct ofono_error *error,
+					int fd,
+					void *data)
+{
+	struct ofono_gps *gps = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Powering ON gps failed");
+
+		gps_agent_free(gps->agent);
+
+		reply = __ofono_error_failed(gps->pending);
+		__ofono_dbus_pending_reply(&gps->pending, reply);
+
+		return;
+	}
+
+	gps->fd = fd;
+	gps_agent_send_noreply(gps->agent, "ReceiveGpsFileDescriptor", fd);
+}
+
+static void gps_disable_callback(const struct ofono_error *error,
+						void *data)
+{
+	struct ofono_gps *gps = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Powering OFF gps failed");
+
+		reply = __ofono_error_failed(gps->pending);
+		__ofono_dbus_pending_reply(&gps->pending, reply);
+
+		return;
+	}
+
+	gps->fd = -1;
+}
+
+static void agent_exited(gpointer user_data)
+{
+	struct ofono_gps *gps = user_data;
+
+	gps->agent = NULL;
+
+	gps->driver->disable(gps, gps_disable_callback, gps);
+}
+
+static DBusMessage *gps_register_agent(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_gps *gps = data;
+	const char *agent_path;
+
+	if (gps->agent)
+		return __ofono_error_busy(msg);
+
+	if (dbus_message_get_args(msg, NULL,
+					DBUS_TYPE_OBJECT_PATH, &agent_path,
+					DBUS_TYPE_INVALID) == FALSE)
+		return __ofono_error_invalid_args(msg);
+
+	if (!__ofono_dbus_valid_object_path(agent_path))
+		return __ofono_error_invalid_format(msg);
+
+	gps->agent = gps_agent_new(dbus_message_get_sender(msg), agent_path);
+	if (gps->agent == NULL)
+		return __ofono_error_failed(msg);
+
+	gps_agent_set_removed_notify(gps->agent, agent_exited, gps);
+
+	gps->driver->enable(gps, gps_enable_callback, gps);
+
+	return dbus_message_new_method_return(msg);
+}
+
+static DBusMessage *gps_unregister_agent(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct ofono_gps *gps = data;
+	const char *agent_path;
+	const char *agent_bus = dbus_message_get_sender(msg);
+
+	if (dbus_message_get_args(msg, NULL,
+					DBUS_TYPE_OBJECT_PATH, &agent_path,
+					DBUS_TYPE_INVALID) == FALSE)
+		return __ofono_error_invalid_args(msg);
+
+	if (gps->agent == NULL)
+		return __ofono_error_failed(msg);
+
+	if (gps_agent_matches(gps->agent, agent_bus, agent_path) == FALSE)
+		return __ofono_error_failed(msg);
+
+	gps_agent_free(gps->agent);
+
+	return dbus_message_new_method_return(msg);
+}
+
+static GDBusMethodTable gps_methods[] = {
+	{ "GetProperties",  "",    "a{sv}",  gps_get_properties,
+						G_DBUS_METHOD_FLAG_ASYNC },
+	{ "RegisterAgent",		"o",	"",	gps_register_agent },
+	{ "UnregisterAgent",		"o",	"",	gps_unregister_agent },
+
+	{ }
+};
+
+static GDBusSignalTable gps_signals[] = {
+	{ "PropertyChanged",	"sv" },
+	{ }
+};
+
+int ofono_gps_driver_register(const struct ofono_gps_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (d == NULL || d->probe == NULL)
+		return -EINVAL;
+
+	g_drivers = g_slist_prepend(g_drivers, (void *) d);
+
+	return 0;
+}
+
+void ofono_gps_driver_unregister(const struct ofono_gps_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (d == NULL)
+		return;
+
+	g_drivers = g_slist_remove(g_drivers, (void *) d);
+}
+
+struct ofono_modem *ofono_gps_get_modem(struct ofono_gps *g)
+{
+	return __ofono_atom_get_modem(g->atom);
+}
+
+static void gps_unregister(struct ofono_atom *atom)
+{
+	struct ofono_gps *gps = __ofono_atom_get_data(atom);
+	const char *path = __ofono_atom_get_path(gps->atom);
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(gps->atom);
+
+	ofono_modem_remove_interface(modem, OFONO_LOCATION_REPORTING_INTERFACE);
+	g_dbus_unregister_interface(conn, path,
+					OFONO_LOCATION_REPORTING_INTERFACE);
+}
+
+static void gps_remove(struct ofono_atom *atom)
+{
+	struct ofono_gps *gps = __ofono_atom_get_data(atom);
+
+	DBG("atom: %p", atom);
+
+	if (gps == NULL)
+		return;
+
+	if (gps->driver && gps->driver->remove)
+		gps->driver->remove(gps);
+
+	g_free(gps);
+}
+
+struct ofono_gps *ofono_gps_create(struct ofono_modem *modem,
+					 unsigned int vendor,
+					 const char *driver, void *data)
+{
+	struct ofono_gps *gps;
+	GSList *l;
+
+	if (driver == NULL)
+		return NULL;
+
+	gps = g_try_new0(struct ofono_gps, 1);
+	if (gps == NULL)
+		return NULL;
+
+	gps->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_GPS,
+						gps_remove, gps);
+
+	for (l = g_drivers; l; l = l->next) {
+		const struct ofono_gps_driver *drv = l->data;
+
+		if (g_strcmp0(drv->name, driver) != 0)
+			continue;
+
+		if (drv->probe(gps, vendor, data) < 0)
+			continue;
+
+		gps->driver = drv;
+		break;
+	}
+
+	return gps;
+}
+
+void ofono_gps_register(struct ofono_gps *gps)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(gps->atom);
+	const char *path = __ofono_atom_get_path(gps->atom);
+
+	if (!g_dbus_register_interface(conn, path,
+					OFONO_LOCATION_REPORTING_INTERFACE,
+					gps_methods, gps_signals, NULL, gps,
+					NULL)) {
+		ofono_error("Could not create %s interface",
+					OFONO_LOCATION_REPORTING_INTERFACE);
+
+		return;
+	}
+
+	ofono_modem_add_interface(modem, OFONO_LOCATION_REPORTING_INTERFACE);
+	__ofono_atom_register(gps->atom, gps_unregister);
+}
+
+void ofono_gps_remove(struct ofono_gps *gps)
+{
+	__ofono_atom_free(gps->atom);
+}
+
+void ofono_gps_set_data(struct ofono_gps *gps, void *data)
+{
+	gps->driver_data = data;
+}
+
+int ofono_gps_get_fd(struct ofono_gps *gps)
+{
+	return gps->fd;
+}
+
+void *ofono_gps_get_data(struct ofono_gps *gps)
+{
+	return gps->driver_data;
+}
diff --git a/src/gpsagent.c b/src/gpsagent.c
new file mode 100644
index 0000000..bc7d44f
--- /dev/null
+++ b/src/gpsagent.c
@@ -0,0 +1,145 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2009-2010 Nokia Corporation and/or its subsidiary(-ies).
+ *  Copyright (C) 2010 ProFUSION embedded systems.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <stdint.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <gdbus.h>
+
+#include <ofono/dbus.h>
+
+#include "ofono.h"
+
+#include "common.h"
+#include "gpsagent.h"
+
+#define AGENT_INTERFACE "org.ofono.LocationReportingAgent"
+
+#ifndef DBUS_TYPE_UNIX_FD
+#define DBUS_TYPE_UNIX_FD -1
+#endif
+
+struct gps_agent {
+	char *path;
+	char *service;
+	guint disconnect_watch;
+	ofono_destroy_func removed_cb;
+	void *removed_data;
+};
+
+void gps_agent_send_noreply(struct gps_agent *agent, const char *method, int fd)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *message;
+
+	message = dbus_message_new_method_call(agent->service, agent->path,
+						AGENT_INTERFACE, method);
+	if (message == NULL)
+		return;
+
+	if (fd != -1)
+		dbus_message_append_args(message, DBUS_TYPE_UNIX_FD, &fd,
+							DBUS_TYPE_INVALID);
+
+	dbus_message_set_no_reply(message, TRUE);
+
+	g_dbus_send_message(conn, message);
+}
+
+static inline void gps_agent_send_release(struct gps_agent *agent)
+{
+	gps_agent_send_noreply(agent, "Release", -1);
+}
+
+static void gps_agent_disconnect_cb(DBusConnection *conn, void *data)
+{
+	struct gps_agent *agent = data;
+
+	agent->disconnect_watch = 0;
+
+	gps_agent_free(agent);
+}
+
+struct gps_agent *gps_agent_new(const char *service, const char *path)
+{
+	struct gps_agent *agent = g_try_new0(struct gps_agent, 1);
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	if (agent == NULL)
+		return NULL;
+
+	agent->service = g_strdup(service);
+	agent->path = g_strdup(path);
+
+	agent->disconnect_watch = g_dbus_add_disconnect_watch(conn, service,
+							gps_agent_disconnect_cb,
+							agent, NULL);
+
+	return agent;
+}
+
+void gps_agent_set_removed_notify(struct gps_agent *agent,
+					ofono_destroy_func destroy,
+					void *user_data)
+{
+	agent->removed_cb = destroy;
+	agent->removed_data = user_data;
+}
+
+void gps_agent_free(struct gps_agent *agent)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	if (agent == NULL)
+		return;
+
+	if (agent->disconnect_watch) {
+		gps_agent_send_release(agent);
+
+		g_dbus_remove_watch(conn, agent->disconnect_watch);
+		agent->disconnect_watch = 0;
+	}
+
+	if (agent->removed_cb)
+		agent->removed_cb(agent->removed_data);
+
+	g_free(agent->path);
+	g_free(agent->service);
+	g_free(agent);
+}
+
+ofono_bool_t gps_agent_matches(struct gps_agent *agent, const char *service,
+				const char *path)
+{
+	if (path == NULL || service == NULL)
+		return FALSE;
+
+	return g_str_equal(agent->path, path) &&
+			g_str_equal(agent->service, service);
+}
diff --git a/src/gpsagent.h b/src/gpsagent.h
new file mode 100644
index 0000000..0ff5bf4
--- /dev/null
+++ b/src/gpsagent.h
@@ -0,0 +1,37 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2009-2010 Nokia Corporation and/or its subsidiary(-ies).
+ *  Copyright (C) 2010 ProFUSION embedded systems.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+struct gps_agent;
+
+struct gps_agent *gps_agent_new(const char *service, const char *path);
+
+void gps_agent_set_removed_notify(struct gps_agent *agent,
+					ofono_destroy_func destroy,
+					void *user_data);
+
+ofono_bool_t gps_agent_matches(struct gps_agent *agent, const char *service,
+				const char *path);
+
+void gps_agent_free(struct gps_agent *agent);
+
+void gps_agent_send_noreply(struct gps_agent *agent,
+				 const char *method, int fd);
diff --git a/src/ofono.conf b/src/ofono.conf
index 0dfa038..b06ea63 100644
--- a/src/ofono.conf
+++ b/src/ofono.conf
@@ -13,6 +13,7 @@
     <allow send_interface="org.ofono.SimToolkitAgent"/>
     <allow send_interface="org.ofono.PushNotificationAgent"/>
     <allow send_interface="org.ofono.SmartMessagingAgent"/>
+    <allow send_interface="org.ofono.LocationReportingAgent"/>
   </policy>
 
   <policy at_console="true">
diff --git a/src/ofono.h b/src/ofono.h
index e48dbf6..7c34062 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -128,6 +128,7 @@ enum ofono_atom_type {
 	OFONO_ATOM_TYPE_CTM,
 	OFONO_ATOM_TYPE_CDMA_VOICECALL_MANAGER,
 	OFONO_ATOM_TYPE_SIM_AUTH,
+	OFONO_ATOM_TYPE_GPS,
 };
 
 enum ofono_atom_watch_condition {
@@ -209,6 +210,7 @@ gboolean __ofono_call_settings_is_busy(struct ofono_call_settings *cs);
 #include <ofono/radio-settings.h>
 #include <ofono/audio-settings.h>
 #include <ofono/ctm.h>
+#include <ofono/gps.h>
 
 #include <ofono/voicecall.h>
 
-- 
1.7.2.3

[PATCH v3 3/7] mbmmodem: add gps atom

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 7612 bytes --]

---
 Makefile.am                 |    3 +-
 drivers/mbmmodem/gps.c      |  247 +++++++++++++++++++++++++++++++++++++++++++
 drivers/mbmmodem/mbmmodem.c |    2 +
 drivers/mbmmodem/mbmmodem.h |    3 +
 4 files changed, 254 insertions(+), 1 deletions(-)
 create mode 100644 drivers/mbmmodem/gps.c

diff --git a/Makefile.am b/Makefile.am
index de70976..4c00f16 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -207,7 +207,8 @@ builtin_sources += drivers/atmodem/atutil.h \
 			drivers/mbmmodem/mbmmodem.h \
 			drivers/mbmmodem/mbmmodem.c \
 			drivers/mbmmodem/gprs-context.c \
-			drivers/mbmmodem/stk.c
+			drivers/mbmmodem/stk.c \
+			drivers/mbmmodem/gps.c
 
 builtin_modules += hsomodem
 builtin_sources += drivers/atmodem/atutil.h \
diff --git a/drivers/mbmmodem/gps.c b/drivers/mbmmodem/gps.c
new file mode 100644
index 0000000..aa7b7bb
--- /dev/null
+++ b/drivers/mbmmodem/gps.c
@@ -0,0 +1,247 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2010 ProFUSION embedded systems.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define _GNU_SOURCE
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+
+#include <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/gps.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+#include "gattty.h"
+
+#include "mbmmodem.h"
+
+static const char *none_prefix[] = { NULL };
+static const char *e2gpsctl_prefix[] = { "*E2GPSCTL:", NULL };
+
+struct gps_data {
+	GAtChat *chat;
+};
+
+struct e2gpsnpd_data {
+	GIOChannel *channel;
+	GAtChat *chat;
+};
+
+static void mbm_e2gpsctl_disable_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_gps_disable_cb_t cb = cbd->cb;
+	struct ofono_error error;
+
+	decode_at_error(&error, g_at_result_final_response(result));
+	cb(&error, cbd->data);
+}
+
+static void mbm_gps_disable(struct ofono_gps *g,
+				ofono_gps_disable_cb_t cb,
+				void *data)
+{
+	struct gps_data *gd = ofono_gps_get_data(g);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	int fd = ofono_gps_get_fd(g);
+
+	/* close the Gps FileDescriptor */
+	close(fd);
+
+	if (g_at_chat_send(gd->chat, "AT*E2GPSCTL=0,5,1", none_prefix,
+				mbm_e2gpsctl_disable_cb, cbd, g_free) > 0)
+		return;
+
+	/* error */
+	CALLBACK_WITH_FAILURE(cb, data);
+	g_free(cbd);
+}
+
+static void mbm_gps_send_e2gpsnpd_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct e2gpsnpd_data *e2d = user_data;
+
+	g_io_channel_unref(e2d->channel);
+	g_at_chat_unref(e2d->chat);
+}
+
+static void mbm_gps_send_e2gpsnpd(const char *gps_dev, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_gps_enable_cb_t cb = cbd->cb;
+	GAtSyntax *syntax;
+	struct e2gpsnpd_data *e2d;
+	int fd;
+
+
+	e2d = g_try_new0(struct e2gpsnpd_data, 1);
+	if (e2d == NULL)
+		goto error;
+
+	e2d->channel = g_at_tty_open(gps_dev, NULL);
+	if (e2d->channel == NULL)
+		goto error;
+
+	syntax = g_at_syntax_new_gsm_permissive();
+	e2d->chat = g_at_chat_new(e2d->channel, syntax);
+	g_at_syntax_unref(syntax);
+
+	if (e2d->chat == NULL)
+		goto error;
+
+	if (g_at_chat_send(e2d->chat, "AT*E2GPSNPD", none_prefix,
+				 mbm_gps_send_e2gpsnpd_cb, e2d, g_free) > 0) {
+		fd = g_io_channel_unix_get_fd(e2d->channel);
+		CALLBACK_WITH_SUCCESS(cb, fd, cbd->data);
+		return;
+	}
+
+error:
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+
+	if (e2d->chat)
+		g_at_chat_unref(e2d->chat);
+
+	if (e2d->channel)
+		g_io_channel_unref(e2d->channel);
+
+	if (e2d)
+		g_free(e2d);
+}
+
+static void mbm_e2gpsctl_enable_cb(gboolean ok, GAtResult *result,
+							 gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	struct ofono_gps *gps = cbd->user;
+	ofono_gps_enable_cb_t cb = cbd->cb;
+	struct ofono_modem *modem;
+	const char *gps_dev;
+
+	if (!ok) {
+		struct ofono_error error;
+
+		decode_at_error(&error, g_at_result_final_response(result));
+		cb(&error, -1, cbd->data);
+		return;
+	}
+
+	modem = ofono_gps_get_modem(gps);
+	gps_dev = ofono_modem_get_string(modem, "GPSDevice");
+
+	/* NMEA stream is sent to gps_dev */
+	mbm_gps_send_e2gpsnpd(gps_dev, user_data);
+}
+
+static void mbm_gps_enable(struct ofono_gps *g,
+				ofono_gps_enable_cb_t cb,
+				void *data)
+{
+	struct gps_data *gd = ofono_gps_get_data(g);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	if (cbd == NULL)
+		goto error;
+
+	cbd->user = g;
+
+	if (g_at_chat_send(gd->chat, "AT*E2GPSCTL=1,5,1", none_prefix,
+				mbm_e2gpsctl_enable_cb, cbd, g_free) > 0)
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, -1, data);
+	g_free(cbd);
+}
+
+static void mbm_gps_support_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct ofono_gps *gps = user_data;
+
+	if (!ok) {
+		ofono_gps_remove(gps);
+		return;
+	}
+
+	ofono_gps_register(gps);
+}
+
+static int mbm_gps_probe(struct ofono_gps *g,
+					unsigned int vendor, void *data)
+{
+	GAtChat *chat = data;
+	struct gps_data *gd;
+
+	gd = g_try_new0(struct gps_data, 1);
+	if (gd == NULL)
+		return -ENOMEM;
+
+	gd->chat = g_at_chat_clone(chat);
+
+	ofono_gps_set_data(g, gd);
+
+	g_at_chat_send(gd->chat, "AT*E2GPSCTL=?", e2gpsctl_prefix,
+					mbm_gps_support_cb, g, NULL);
+
+	return 0;
+}
+
+static void mbm_gps_remove(struct ofono_gps *g)
+{
+	struct gps_data *gd = ofono_gps_get_data(g);
+
+	ofono_gps_set_data(g, NULL);
+
+	g_at_chat_unref(gd->chat);
+	g_free(gd);
+}
+
+static struct ofono_gps_driver driver = {
+	.name			= "mbmmodem",
+	.type			= OFONO_GPS_DEVICE_TYPE_NMEA,
+	.probe			= mbm_gps_probe,
+	.remove			= mbm_gps_remove,
+	.enable			= mbm_gps_enable,
+	.disable		= mbm_gps_disable,
+};
+
+void mbm_gps_init()
+{
+	ofono_gps_driver_register(&driver);
+}
+
+void mbm_gps_exit()
+{
+	ofono_gps_driver_unregister(&driver);
+}
diff --git a/drivers/mbmmodem/mbmmodem.c b/drivers/mbmmodem/mbmmodem.c
index 03b61b3..ea56cd4 100644
--- a/drivers/mbmmodem/mbmmodem.c
+++ b/drivers/mbmmodem/mbmmodem.c
@@ -36,12 +36,14 @@ static int mbmmodem_init(void)
 {
 	mbm_gprs_context_init();
 	mbm_stk_init();
+	mbm_gps_init();
 
 	return 0;
 }
 
 static void mbmmodem_exit(void)
 {
+	mbm_gps_exit();
 	mbm_stk_exit();
 	mbm_gprs_context_exit();
 }
diff --git a/drivers/mbmmodem/mbmmodem.h b/drivers/mbmmodem/mbmmodem.h
index 4c6f263..0393fcb 100644
--- a/drivers/mbmmodem/mbmmodem.h
+++ b/drivers/mbmmodem/mbmmodem.h
@@ -26,3 +26,6 @@ extern void mbm_gprs_context_exit(void);
 
 extern void mbm_stk_init(void);
 extern void mbm_stk_exit(void);
+
+extern void mbm_gps_init();
+extern void mbm_gps_exit();
-- 
1.7.2.3

[PATCH v3 4/7] plugins: add gps atom to mbm

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]

---
 plugins/mbm.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/plugins/mbm.c b/plugins/mbm.c
index 4048f6a..3732735 100644
--- a/plugins/mbm.c
+++ b/plugins/mbm.c
@@ -45,6 +45,7 @@
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
 #include <ofono/log.h>
+#include <ofono/gps.h>
 
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
@@ -66,6 +67,7 @@ struct mbm_data {
 	gboolean have_sim;
 	struct ofono_gprs *gprs;
 	struct ofono_gprs_context *gc;
+	struct ofono_gps *gps;
 	guint reopen_source;
 	enum mbm_variant variant;
 };
@@ -510,9 +512,15 @@ static void mbm_post_online(struct ofono_modem *modem)
 {
 	struct mbm_data *data = ofono_modem_get_data(modem);
 	struct ofono_gprs_context *gc;
+	const char *gps_dev;
 
 	DBG("%p", modem);
 
+	gps_dev = ofono_modem_get_string(modem, "GPSDevice");
+	if (gps_dev)
+		data->gps = ofono_gps_create(modem, 0,
+					"mbmmodem", data->modem_port);
+
 	ofono_netreg_create(modem, OFONO_VENDOR_MBM,
 					"atmodem", data->modem_port);
 
-- 
1.7.2.3

[PATCH v3 5/7] plugins/udev.c: add gps comparison for add_mbm registered modem

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 1074 bytes --]

---
 plugins/udev.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/plugins/udev.c b/plugins/udev.c
index 202b225..ca09a30 100644
--- a/plugins/udev.c
+++ b/plugins/udev.c
@@ -105,7 +105,7 @@ static void add_mbm(struct ofono_modem *modem,
 					struct udev_device *udev_device)
 {
 	const char *desc, *devnode;
-	const char *device, *data, *network;
+	const char *device, *data, *network, *gps;
 	int registered;
 
 	desc = udev_device_get_sysattr_value(udev_device, "device/interface");
@@ -152,8 +152,9 @@ static void add_mbm(struct ofono_modem *modem,
 	device  = ofono_modem_get_string(modem, MODEM_DEVICE);
 	data = ofono_modem_get_string(modem, DATA_DEVICE);
 	network = ofono_modem_get_string(modem, NETWORK_INTERFACE);
+	gps = ofono_modem_get_string(modem, GPS_DEVICE);
 
-	if (device != NULL && data != NULL && network != NULL) {
+	if (device != NULL && data != NULL && network != NULL && gps != NULL) {
 		ofono_modem_set_integer(modem, "Registered", 1);
 		ofono_modem_register(modem);
 	}
-- 
1.7.2.3

[PATCH v3 6/7] gps: add test script

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 2071 bytes --]

---
 Makefile.am                  |    3 +-
 test/test-location-reporting |   49 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletions(-)
 create mode 100755 test/test-location-reporting

diff --git a/Makefile.am b/Makefile.am
index 4c00f16..49bf8f3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -472,7 +472,8 @@ test_scripts = test/backtrace \
 		test/cdma-list-call \
 		test/cdma-dial-number \
 		test/cdma-hangup \
-		test/disable-call-forwarding
+		test/disable-call-forwarding \
+		test/test-location-reporting
 
 if TEST
 testdir = $(pkglibdir)/test
diff --git a/test/test-location-reporting b/test/test-location-reporting
new file mode 100755
index 0000000..fe478c6
--- /dev/null
+++ b/test/test-location-reporting
@@ -0,0 +1,49 @@
+#!/usr/bin/python
+
+import gobject
+
+import sys
+import dbus
+import dbus.service
+import dbus.mainloop.glib
+
+class LocationReportingAgent(dbus.service.Object):
+	@dbus.service.method("org.ofono.LocationReportingAgent",
+					in_signature="", out_signature="")
+	def Release(self):
+		print "Release"
+		mainloop.quit()
+
+	@dbus.service.method("org.ofono.LocationReportingAgent",
+				in_signature="i", out_signature="")
+	def ReceiveGpsFileDescriptor(self, fd):
+		print "FD: %d" % (fd, )
+
+if __name__ == '__main__':
+	dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
+
+	bus = dbus.SystemBus()
+	manager = dbus.Interface(bus.get_object("org.ofono", "/"),
+							"org.ofono.Manager")
+
+	modems = manager.GetModems()
+
+	for path, properties in modems:
+		if "org.ofono.LocationReporting" not in properties["Interfaces"]:
+			continue
+
+		pn = dbus.Interface(bus.get_object('org.ofono', path),
+					'org.ofono.LocationReporting')
+
+	path = "/test/agent"
+	agent = LocationReportingAgent(bus, path)
+	pn.RegisterAgent(path)
+	print "Agent registered"
+
+	mainloop = gobject.MainLoop()
+
+	try:
+		mainloop.run()
+	except KeyboardInterrupt:
+		pn.UnregisterAgent(path)
+		mainloop.run()
-- 
1.7.2.3

[PATCH v3 7/7] gps: add documentation

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 2358 bytes --]

---
 Makefile.am                  |    3 +-
 doc/gps-api.txt              |   49 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletions(-)
 create mode 100644 doc/gps-api.txt
 mode change 100755 => 100644 test/test-location-reporting

diff --git a/Makefile.am b/Makefile.am
index 49bf8f3..2ebf9e6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -399,7 +399,8 @@ doc_files = doc/overview.txt doc/ofono-paper.txt doc/release-faq.txt \
 			doc/phonebook-api.txt doc/radio-settings-api.txt \
 			doc/sim-api.txt doc/stk-api.txt \
 			doc/audio-settings-api.txt doc/text-telephony-api.txt \
-			doc/calypso-modem.txt doc/message-api.txt
+			doc/calypso-modem.txt doc/message-api.txt \
+			doc/gps-api.txt
 
 
 test_scripts = test/backtrace \
diff --git a/doc/gps-api.txt b/doc/gps-api.txt
new file mode 100644
index 0000000..85b0669
--- /dev/null
+++ b/doc/gps-api.txt
@@ -0,0 +1,49 @@
+Location Reporting hierarchy
+=================
+
+Service		org.ofono
+Interface	org.ofono.LocationReporting
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		dict GetProperties()
+
+			Returns all Gps properties. See the
+			properties section for available properties.
+
+			Possible Errors: [service].Error.InProgress
+					 [service].Error.Failed
+
+		void RegisterAgent(object path)
+
+			Registers an agent which will be called with the
+			gps file descriptor.
+
+		void UnregisterAgent(object path)
+
+			Unregisters an agent.
+
+Properties	string Type [readonly]
+
+			Holds the type of the device (e.g. "nmea")
+
+LocationReportingAgent Hierarchy [experimental]
+===============
+
+Service		unique name
+Interface	org.ofono.LocationReportingAgent
+Object path	freely definable
+
+Methods		void ReceiveGpsFileDescriptor(int32 fd)
+
+			Supplies the current gps device open file
+			descriptor. The file descriptor should be used by
+			the external client to receive the NMEA data.
+
+			Possible Errors: None
+
+		void Release() [noreply]
+
+			Agent is being released, possibly because of oFono
+			terminating, Location Reporting interface is being
+			torn down or modem off.  No UnregisterAgent call is
+			needed.
diff --git a/test/test-location-reporting b/test/test-location-reporting
old mode 100755
new mode 100644
-- 
1.7.2.3

Re: [PATCH v3 7/7] gps: add documentation

[Gustavo F. Padovan]

[-- Attachment #1: Type: text/plain, Size: 2473 bytes --]

Hi Rafael,

* Rafael Ignacio Zurita <rafael.zurita@profusion.mobi> [2011-01-26 12:32:54 -0300]:

> ---
>  Makefile.am                  |    3 +-
>  doc/gps-api.txt              |   49 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 1 deletions(-)
>  create mode 100644 doc/gps-api.txt
>  mode change 100755 => 100644 test/test-location-reporting
> 
> diff --git a/Makefile.am b/Makefile.am
> index 49bf8f3..2ebf9e6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -399,7 +399,8 @@ doc_files = doc/overview.txt doc/ofono-paper.txt doc/release-faq.txt \
>  			doc/phonebook-api.txt doc/radio-settings-api.txt \
>  			doc/sim-api.txt doc/stk-api.txt \
>  			doc/audio-settings-api.txt doc/text-telephony-api.txt \
> -			doc/calypso-modem.txt doc/message-api.txt
> +			doc/calypso-modem.txt doc/message-api.txt \
> +			doc/gps-api.txt

What about call this location-report-api.txt?

>  
>  
>  test_scripts = test/backtrace \
> diff --git a/doc/gps-api.txt b/doc/gps-api.txt
> new file mode 100644
> index 0000000..85b0669
> --- /dev/null
> +++ b/doc/gps-api.txt
> @@ -0,0 +1,49 @@
> +Location Reporting hierarchy
> +=================
> +
> +Service		org.ofono
> +Interface	org.ofono.LocationReporting
> +Object path	[variable prefix]/{modem0,modem1,...}
> +
> +Methods		dict GetProperties()
> +
> +			Returns all Gps properties. See the
> +			properties section for available properties.
> +
> +			Possible Errors: [service].Error.InProgress
> +					 [service].Error.Failed
> +
> +		void RegisterAgent(object path)
> +
> +			Registers an agent which will be called with the
> +			gps file descriptor.
> +
> +		void UnregisterAgent(object path)
> +
> +			Unregisters an agent.
> +

You are missing Enable() and Disable() methods here, you can't just enable the
GPS device when registering the Agent.

> +Properties	string Type [readonly]
> +
> +			Holds the type of the device (e.g. "nmea")

Then you also need a Enabled property here.

> +
> +LocationReportingAgent Hierarchy [experimental]
> +===============
> +
> +Service		unique name
> +Interface	org.ofono.LocationReportingAgent
> +Object path	freely definable
> +
> +Methods		void ReceiveGpsFileDescriptor(int32 fd)

I think you get rid of this method an make Enable() return the file descriptor
you want. Input from Marcel and Denis here would be good.

-- 
Gustavo F. Padovan
http://profusion.mobi

Re: [PATCH v3 2/7] gps: add gps atom and gpsagent implementations

[Gustavo F. Padovan]

[-- Attachment #1: Type: text/plain, Size: 1302 bytes --]

Hi Rafael,

* Rafael Ignacio Zurita <rafael.zurita@profusion.mobi> [2011-01-26 12:32:49 -0300]:

> ---
>  Makefile.am    |    3 +-
>  src/gps.c      |  326 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/gpsagent.c |  145 +++++++++++++++++++++++++
>  src/gpsagent.h |   37 +++++++
>  src/ofono.conf |    1 +
>  src/ofono.h    |    2 +
>  6 files changed, 513 insertions(+), 1 deletions(-)
>  create mode 100644 src/gps.c
>  create mode 100644 src/gpsagent.c
>  create mode 100644 src/gpsagent.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index 7374f70..de70976 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -355,7 +355,8 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
>  			src/simfs.c src/simfs.h src/audio-settings.c \
>  			src/smsagent.c src/smsagent.h src/ctm.c \
>  			src/cdma-voicecall.c src/sim-auth.c \
> -			src/message.h src/message.c
> +			src/message.h src/message.c src/gps.c \
> +			src/gpsagent.c src/gpsagent.h

gpsagent.c should not be in src/, it should be in test/. But I saw you already
have python implementation for the Agent. So just discard the c one and keep
only the python one. oFono does not really care about the Agent
implementation.

-- 
Gustavo F. Padovan
http://profusion.mobi

Re: [PATCH v3 7/7] gps: add documentation

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 3248 bytes --]

On Wed, Jan 26, 2011 at 07:47:46PM -0200, Gustavo F. Padovan wrote:
> Hi Rafael,
> 
> * Rafael Ignacio Zurita <rafael.zurita@profusion.mobi> [2011-01-26 12:32:54 -0300]:
> 
> > ---
> >  Makefile.am                  |    3 +-
> >  doc/gps-api.txt              |   49 ++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 51 insertions(+), 1 deletions(-)
> >  create mode 100644 doc/gps-api.txt
> >  mode change 100755 => 100644 test/test-location-reporting
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 49bf8f3..2ebf9e6 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -399,7 +399,8 @@ doc_files = doc/overview.txt doc/ofono-paper.txt doc/release-faq.txt \
> >  			doc/phonebook-api.txt doc/radio-settings-api.txt \
> >  			doc/sim-api.txt doc/stk-api.txt \
> >  			doc/audio-settings-api.txt doc/text-telephony-api.txt \
> > -			doc/calypso-modem.txt doc/message-api.txt
> > +			doc/calypso-modem.txt doc/message-api.txt \
> > +			doc/gps-api.txt
> 
> What about call this location-report-api.txt?

Yes, that could be more consistent.

> 
> >  
> >  
> >  test_scripts = test/backtrace \
> > diff --git a/doc/gps-api.txt b/doc/gps-api.txt
> > new file mode 100644
> > index 0000000..85b0669
> > --- /dev/null
> > +++ b/doc/gps-api.txt
> > @@ -0,0 +1,49 @@
> > +Location Reporting hierarchy
> > +=================
> > +
> > +Service		org.ofono
> > +Interface	org.ofono.LocationReporting
> > +Object path	[variable prefix]/{modem0,modem1,...}
> > +
> > +Methods		dict GetProperties()
> > +
> > +			Returns all Gps properties. See the
> > +			properties section for available properties.
> > +
> > +			Possible Errors: [service].Error.InProgress
> > +					 [service].Error.Failed
> > +
> > +		void RegisterAgent(object path)
> > +
> > +			Registers an agent which will be called with the
> > +			gps file descriptor.
> > +
> > +		void UnregisterAgent(object path)
> > +
> > +			Unregisters an agent.
> > +
> 
> You are missing Enable() and Disable() methods here, you can't just enable the
> GPS device when registering the Agent.

Why no? I followed the comments for the new API :
http://lists.ofono.org/pipermail/ofono/2011-January/007477.html
which suggests :

An external client registers an Agent with this
interface.  Internally this triggers the driver function to bring up the
GPS device and return the file descriptor to the core.  The core takes
care of calling the Agent with the file descriptor using DBus FD passing
mechanisms.

> 
> > +Properties	string Type [readonly]
> > +
> > +			Holds the type of the device (e.g. "nmea")
> 
> Then you also need a Enabled property here.
> 
> > +
> > +LocationReportingAgent Hierarchy [experimental]
> > +===============
> > +
> > +Service		unique name
> > +Interface	org.ofono.LocationReportingAgent
> > +Object path	freely definable
> > +
> > +Methods		void ReceiveGpsFileDescriptor(int32 fd)
> 
> I think you get rid of this method an make Enable() return the file descriptor
> you want. Input from Marcel and Denis here would be good.

If so then you would like to see the previous proposal, my version 2 of
the gps patches, which does you like.


Rafael Zurita

Re: [PATCH v3 2/7] gps: add gps atom and gpsagent implementations

[Rafael Ignacio Zurita]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

Hi Gustavo,

On Wed, Jan 26, 2011 at 07:51:57PM -0200, Gustavo F. Padovan wrote:
> Hi Rafael,
> 
> * Rafael Ignacio Zurita <rafael.zurita@profusion.mobi> [2011-01-26 12:32:49 -0300]:
> 
> > ---
> >  Makefile.am    |    3 +-
> >  src/gps.c      |  326 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/gpsagent.c |  145 +++++++++++++++++++++++++
> >  src/gpsagent.h |   37 +++++++
> >  src/ofono.conf |    1 +
> >  src/ofono.h    |    2 +
> >  6 files changed, 513 insertions(+), 1 deletions(-)
> >  create mode 100644 src/gps.c
> >  create mode 100644 src/gpsagent.c
> >  create mode 100644 src/gpsagent.h
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 7374f70..de70976 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -355,7 +355,8 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
> >  			src/simfs.c src/simfs.h src/audio-settings.c \
> >  			src/smsagent.c src/smsagent.h src/ctm.c \
> >  			src/cdma-voicecall.c src/sim-auth.c \
> > -			src/message.h src/message.c
> > +			src/message.h src/message.c src/gps.c \
> > +			src/gpsagent.c src/gpsagent.h
> 
> gpsagent.c should not be in src/, it should be in test/. But I saw you already
> have python implementation for the Agent. So just discard the c one and keep
> only the python one. oFono does not really care about the Agent
> implementation.

I am not understanding your comments. gpsagent.c here is not a test. 
It is similar to stkagent or smsagent.

Rafael Zurita

Re: [PATCH v3 2/7] gps: add gps atom and gpsagent implementations

[Gustavo F. Padovan]

[-- Attachment #1: Type: text/plain, Size: 1762 bytes --]

* Rafael Ignacio Zurita <rafael.zurita@profusion.mobi> [2011-01-26 19:11:46 -0300]:

> Hi Gustavo,
> 
> On Wed, Jan 26, 2011 at 07:51:57PM -0200, Gustavo F. Padovan wrote:
> > Hi Rafael,
> > 
> > * Rafael Ignacio Zurita <rafael.zurita@profusion.mobi> [2011-01-26 12:32:49 -0300]:
> > 
> > > ---
> > >  Makefile.am    |    3 +-
> > >  src/gps.c      |  326 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  src/gpsagent.c |  145 +++++++++++++++++++++++++
> > >  src/gpsagent.h |   37 +++++++
> > >  src/ofono.conf |    1 +
> > >  src/ofono.h    |    2 +
> > >  6 files changed, 513 insertions(+), 1 deletions(-)
> > >  create mode 100644 src/gps.c
> > >  create mode 100644 src/gpsagent.c
> > >  create mode 100644 src/gpsagent.h
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 7374f70..de70976 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -355,7 +355,8 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
> > >  			src/simfs.c src/simfs.h src/audio-settings.c \
> > >  			src/smsagent.c src/smsagent.h src/ctm.c \
> > >  			src/cdma-voicecall.c src/sim-auth.c \
> > > -			src/message.h src/message.c
> > > +			src/message.h src/message.c src/gps.c \
> > > +			src/gpsagent.c src/gpsagent.h
> > 
> > gpsagent.c should not be in src/, it should be in test/. But I saw you already
> > have python implementation for the Agent. So just discard the c one and keep
> > only the python one. oFono does not really care about the Agent
> > implementation.
> 
> I am not understanding your comments. gpsagent.c here is not a test. 
> It is similar to stkagent or smsagent.

Yeah, because they are wrong. :(

-- 
Gustavo F. Padovan
http://profusion.mobi