[PATCH v4 0/6] Add location-reporting atom

[PATCH v4 0/6] Add location-reporting atom

[Lucas De Marchi]

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

From: Lucas De Marchi <lucas.de.marchi@gmail.com>

This is the 4th version of the location-reporting patches (previous title was
"Add GPS atom"). I worked with Rafael Ignacio Zurita to implement this last
version, making some changes in order to fit the new design as discussed on irc.

Now when a client calls 'Request()' we give it a file
descriptor in which the location reporting will be written. By checking the
'Type' property the client can decide what to do with such information. As of
now only NMEA is supported and it's implemented on mbm modem.

If client exits or calls 'Release()', the location reporting is disabled.

Rafael Ignacio Zurita (6):
  location-reporting: add public header
  location-reporting: add atom implementation
  mbmmodem: add location-reporting driver implementation
  mbm: add location-reporting atom
  udev: add location-reporting device to mbm
  location-reporting: add documentation

 Makefile.am                           |   11 +-
 doc/location-reporting-api.txt        |   39 ++++
 drivers/mbmmodem/location-reporting.c |  247 +++++++++++++++++++++
 drivers/mbmmodem/mbmmodem.c           |    2 +
 drivers/mbmmodem/mbmmodem.h           |    3 +
 include/location-reporting.h          |   81 +++++++
 plugins/mbm.c                         |    8 +
 plugins/udev.c                        |    5 +-
 src/location-reporting.c              |  393 +++++++++++++++++++++++++++++++++
 src/ofono.h                           |    2 +
 10 files changed, 785 insertions(+), 6 deletions(-)
 create mode 100644 doc/location-reporting-api.txt
 create mode 100644 drivers/mbmmodem/location-reporting.c
 create mode 100644 include/location-reporting.h
 create mode 100644 src/location-reporting.c

-- 
1.7.4.1

[PATCH v4 1/6] location-reporting: add public header

[Lucas De Marchi]

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

From: Rafael Ignacio Zurita <rafael.zurita@profusion.mobi>

---
 Makefile.am                  |    3 +-
 include/location-reporting.h |   81 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 1 deletions(-)
 create mode 100644 include/location-reporting.h

diff --git a/Makefile.am b/Makefile.am
index 7bd7f4f..f9a1bd9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -14,7 +14,8 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/audio-settings.h include/nettime.h \
 			include/ctm.h include/cdma-voicecall.h \
 			include/cdma-sms.h include/sim-auth.h \
-			include/gprs-provision.h include/emulator.h
+			include/gprs-provision.h include/emulator.h \
+			include/location-reporting.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
diff --git a/include/location-reporting.h b/include/location-reporting.h
new file mode 100644
index 0000000..d932d9d
--- /dev/null
+++ b/include/location-reporting.h
@@ -0,0 +1,81 @@
+/*
+ *
+ *  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_LOCATION_REPORTING_H
+#define __OFONO_LOCATION_REPORTING_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <ofono/types.h>
+
+struct ofono_location_reporting;
+
+enum ofono_location_reporting_type {
+	OFONO_LOCATION_REPORTING_TYPE_NMEA = 0,
+};
+
+typedef void (*ofono_location_reporting_enable_cb_t)(
+						const struct ofono_error *error,
+						int fd, void *data);
+typedef void (*ofono_location_reporting_disable_cb_t)(
+						const struct ofono_error *error,
+						void *data);
+
+struct ofono_location_reporting_driver {
+	const char *name;
+	enum ofono_location_reporting_type type;
+	int (*probe)(struct ofono_location_reporting *lr, unsigned int vendor,
+								void *data);
+	void (*remove)(struct ofono_location_reporting *lr);
+	void (*enable)(struct ofono_location_reporting *lr,
+			ofono_location_reporting_enable_cb_t cb, void *data);
+	void (*disable)(struct ofono_location_reporting *lr,
+			ofono_location_reporting_disable_cb_t cb, void *data);
+};
+
+int ofono_location_reporting_driver_register(
+			const struct ofono_location_reporting_driver *d);
+void ofono_location_reporting_driver_unregister(
+			const struct ofono_location_reporting_driver *d);
+
+struct ofono_location_reporting *ofono_location_reporting_create(
+						struct ofono_modem *modem,
+						unsigned int vendor,
+						const char *driver, void *data);
+
+void ofono_location_reporting_register(struct ofono_location_reporting *lr);
+void ofono_location_reporting_remove(struct ofono_location_reporting *lr);
+
+void ofono_location_reporting_set_data(struct ofono_location_reporting *lr,
+								void *data);
+void *ofono_location_reporting_get_data(struct ofono_location_reporting *lr);
+
+struct ofono_modem *ofono_location_reporting_get_modem(
+					struct ofono_location_reporting *lr);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_LOCATION_REPORTING_H */
-- 
1.7.4.1

[PATCH v4 2/6] location-reporting: add atom implementation

[Lucas De Marchi]

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

From: Rafael Ignacio Zurita <rafael.zurita@profusion.mobi>

---
 Makefile.am              |    2 +-
 src/location-reporting.c |  393 ++++++++++++++++++++++++++++++++++++++++++++++
 src/ofono.h              |    2 +
 3 files changed, 396 insertions(+), 1 deletions(-)
 create mode 100644 src/location-reporting.c

diff --git a/Makefile.am b/Makefile.am
index f9a1bd9..48d84c8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -377,7 +377,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
 			src/smsagent.c src/smsagent.h src/ctm.c \
 			src/cdma-voicecall.c src/sim-auth.c \
 			src/message.h src/message.c src/gprs-provision.c \
-			src/emulator.c
+			src/emulator.c src/location-reporting.c
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
 
diff --git a/src/location-reporting.c b/src/location-reporting.c
new file mode 100644
index 0000000..6ab0914
--- /dev/null
+++ b/src/location-reporting.c
@@ -0,0 +1,393 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010  Nokia Corporation and/or its subsidiary(-ies).
+ *  Copyright (C) 2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2011  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"
+
+#ifndef DBUS_TYPE_UNIX_FD
+#define DBUS_TYPE_UNIX_FD -1
+#endif
+
+static GSList *g_drivers = NULL;
+
+struct ofono_location_reporting {
+	DBusMessage *pending;
+	const struct ofono_location_reporting_driver *driver;
+	void *driver_data;
+	struct ofono_atom *atom;
+	ofono_bool_t enabled;
+	char *client_owner;
+	guint disconnect_watch;
+};
+
+static const char *location_reporting_type_to_string(
+					enum ofono_location_reporting_type type)
+{
+	switch (type) {
+	case OFONO_LOCATION_REPORTING_TYPE_NMEA:
+		return "nmea";
+	};
+
+	return NULL;
+}
+
+static DBusMessage *location_reporting_get_properties(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+
+{
+	struct ofono_location_reporting *lr = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+	const char *type;
+	int value;
+
+	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);
+
+	value = lr->enabled;
+	ofono_dbus_dict_append(&dict, "Enabled", DBUS_TYPE_BOOLEAN, &value);
+
+	type = location_reporting_type_to_string(lr->driver->type);
+	ofono_dbus_dict_append(&dict, "Type", DBUS_TYPE_STRING, &type);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
+static void client_remove(struct ofono_location_reporting *lr)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	if (lr->disconnect_watch) {
+		g_dbus_remove_watch(conn, lr->disconnect_watch);
+		lr->disconnect_watch = 0;
+	}
+
+	g_free(lr->client_owner);
+}
+
+static void signal_enabled(const struct ofono_location_reporting *lr)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(lr->atom);
+	int value = lr->enabled;
+
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_LOCATION_REPORTING_INTERFACE,
+					"Enabled", DBUS_TYPE_BOOLEAN, &value);
+}
+
+static void client_exited_disable_cb(const struct ofono_error *error,
+								void *data)
+{
+	struct ofono_location_reporting *lr = data;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Disabling location-reporting failed");
+
+		return;
+	}
+
+	client_remove(lr);
+	lr->enabled = FALSE;
+
+	signal_enabled(lr);
+}
+
+static void client_exited(DBusConnection *conn, void *data)
+{
+	struct ofono_location_reporting *lr = data;
+
+	lr->disconnect_watch = 0;
+
+	lr->driver->disable(lr, client_exited_disable_cb , lr);
+}
+
+static void location_reporting_disable_cb(const struct ofono_error *error,
+								void *data)
+{
+	struct ofono_location_reporting *lr = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Disabling location-reporting failed");
+
+		reply = __ofono_error_failed(lr->pending);
+		__ofono_dbus_pending_reply(&lr->pending, reply);
+
+		return;
+	}
+
+	client_remove(lr);
+	lr->enabled = FALSE;
+
+	reply = dbus_message_new_method_return(lr->pending);
+	__ofono_dbus_pending_reply(&lr->pending, reply);
+
+	signal_enabled(lr);
+}
+
+static void location_reporting_enable_cb(const struct ofono_error *error,
+							int fd,	void *data)
+{
+	struct ofono_location_reporting *lr = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Enabling location-reporting failed");
+
+		client_remove(lr);
+
+		reply = __ofono_error_failed(lr->pending);
+		__ofono_dbus_pending_reply(&lr->pending, reply);
+
+		return;
+	}
+
+	lr->enabled = TRUE;
+
+	reply = dbus_message_new_method_return(lr->pending);
+	dbus_message_append_args(reply, DBUS_TYPE_UNIX_FD, &fd,
+							DBUS_TYPE_INVALID);
+
+	__ofono_dbus_pending_reply(&lr->pending, reply);
+
+	signal_enabled(lr);
+}
+
+static DBusMessage *location_reporting_request(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct ofono_location_reporting *lr = data;
+	const char *caller = dbus_message_get_sender(msg);
+
+	if (lr->pending != NULL)
+		return __ofono_error_busy(msg);
+
+	if (lr->enabled)
+		return __ofono_error_in_use(msg);
+
+	lr->client_owner = g_strdup(caller);
+	lr->disconnect_watch = g_dbus_add_disconnect_watch(conn,
+				lr->client_owner, client_exited, lr, NULL);
+	lr->pending = dbus_message_ref(msg);
+
+	lr->driver->enable(lr, location_reporting_enable_cb, lr);
+
+	return NULL;
+}
+
+static DBusMessage *location_reporting_release(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct ofono_location_reporting *lr = data;
+	const char *caller = dbus_message_get_sender(msg);
+
+
+	/*
+	 * Avoid a race by not trying to release the device if there is a
+	 * pending message or client already signaled it's exiting. In the
+	 * later case, the device will eventually be released in
+	 * client_exited_disable_cb().
+	 */
+	if (lr->pending != NULL || (lr->enabled && !lr->disconnect_watch))
+		return __ofono_error_busy(msg);
+
+	if (lr->enabled == FALSE)
+		return __ofono_error_not_available(msg);
+
+	if (g_strcmp0(caller, lr->client_owner))
+		return __ofono_error_access_denied(msg);
+
+	lr->pending = dbus_message_ref(msg);
+
+	lr->driver->disable(lr, location_reporting_disable_cb, lr);
+
+	return NULL;
+}
+
+static GDBusMethodTable location_reporting_methods[] = {
+	{ "GetProperties",  "",    "a{sv}", location_reporting_get_properties },
+	{ "Request",        "",    "h",     location_reporting_request,
+						G_DBUS_METHOD_FLAG_ASYNC },
+	{ "Release",        "",    "",      location_reporting_release,
+						G_DBUS_METHOD_FLAG_ASYNC },
+	{ }
+};
+
+static GDBusSignalTable location_reporting_signals[] = {
+	{ "PropertyChanged",	"sv" },
+	{ }
+};
+
+int ofono_location_reporting_driver_register(
+				const struct ofono_location_reporting_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_location_reporting_driver_unregister(
+				const struct ofono_location_reporting_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_location_reporting_get_modem(
+					struct ofono_location_reporting *lr)
+{
+	return __ofono_atom_get_modem(lr->atom);
+}
+
+static void location_reporting_unregister(struct ofono_atom *atom)
+{
+	struct ofono_location_reporting *lr = __ofono_atom_get_data(atom);
+	const char *path = __ofono_atom_get_path(lr->atom);
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(lr->atom);
+
+	ofono_modem_remove_interface(modem, OFONO_LOCATION_REPORTING_INTERFACE);
+	g_dbus_unregister_interface(conn, path,
+					OFONO_LOCATION_REPORTING_INTERFACE);
+}
+
+static void location_reporting_remove(struct ofono_atom *atom)
+{
+	struct ofono_location_reporting *lr = __ofono_atom_get_data(atom);
+
+	DBG("atom: %p", atom);
+
+	if (lr == NULL)
+		return;
+
+	if (lr->driver && lr->driver->remove)
+		lr->driver->remove(lr);
+
+	g_free(lr);
+}
+
+struct ofono_location_reporting *ofono_location_reporting_create(
+						struct ofono_modem *modem,
+						unsigned int vendor,
+						const char *driver, void *data)
+{
+	struct ofono_location_reporting *lr;
+	GSList *l;
+
+	if (driver == NULL)
+		return NULL;
+
+	/* Only D-Bus >= 1.3 supports fd-passing */
+	if (DBUS_TYPE_UNIX_FD == -1)
+		return NULL;
+
+	lr = g_try_new0(struct ofono_location_reporting, 1);
+	if (lr == NULL)
+		return NULL;
+
+	lr->atom = __ofono_modem_add_atom(modem,
+					OFONO_ATOM_TYPE_LOCATION_REPORTING,
+					location_reporting_remove, lr);
+
+	for (l = g_drivers; l; l = l->next) {
+		const struct ofono_location_reporting_driver *drv = l->data;
+
+		if (g_strcmp0(drv->name, driver) != 0)
+			continue;
+
+		if (drv->probe(lr, vendor, data) < 0)
+			continue;
+
+		lr->driver = drv;
+		break;
+	}
+
+	return lr;
+}
+
+void ofono_location_reporting_register(struct ofono_location_reporting *lr)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(lr->atom);
+	const char *path = __ofono_atom_get_path(lr->atom);
+
+	if (!g_dbus_register_interface(conn, path,
+					OFONO_LOCATION_REPORTING_INTERFACE,
+					location_reporting_methods,
+					location_reporting_signals,
+					NULL, lr, 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(lr->atom, location_reporting_unregister);
+}
+
+void ofono_location_reporting_remove(struct ofono_location_reporting *lr)
+{
+	__ofono_atom_free(lr->atom);
+}
+
+void ofono_location_reporting_set_data(struct ofono_location_reporting *lr,
+								void *data)
+{
+	lr->driver_data = data;
+}
+
+void *ofono_location_reporting_get_data(struct ofono_location_reporting *lr)
+{
+	return lr->driver_data;
+}
diff --git a/src/ofono.h b/src/ofono.h
index 14dcd18..4e298f1 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -130,6 +130,7 @@ enum ofono_atom_type {
 	OFONO_ATOM_TYPE_SIM_AUTH,
 	OFONO_ATOM_TYPE_EMULATOR_DUN,
 	OFONO_ATOM_TYPE_EMULATOR_HFP,
+	OFONO_ATOM_TYPE_LOCATION_REPORTING,
 };
 
 enum ofono_atom_watch_condition {
@@ -230,6 +231,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/location-reporting.h>
 
 #include <ofono/voicecall.h>
 
-- 
1.7.4.1

[PATCH v4 3/6] mbmmodem: add location-reporting driver implementation

[Lucas De Marchi]

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

From: Rafael Ignacio Zurita <rafael.zurita@profusion.mobi>

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

diff --git a/Makefile.am b/Makefile.am
index 48d84c8..9b6edf4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -212,7 +212,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/location-reporting.c
 
 builtin_modules += hsomodem
 builtin_sources += drivers/atmodem/atutil.h \
diff --git a/drivers/mbmmodem/location-reporting.c b/drivers/mbmmodem/location-reporting.c
new file mode 100644
index 0000000..7d3e212
--- /dev/null
+++ b/drivers/mbmmodem/location-reporting.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/location-reporting.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;
+	GAtChat *data_chat;
+};
+
+static void mbm_e2gpsctl_disable_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	struct ofono_location_reporting *lr = cbd->user;
+	ofono_location_reporting_disable_cb_t cb = cbd->cb;
+	struct gps_data *gd = ofono_location_reporting_get_data(lr);
+
+	DBG("lr=%p, ok=%d", lr, ok);
+
+	if (!ok) {
+		struct ofono_error error;
+
+		decode_at_error(&error, g_at_result_final_response(result));
+		cb(&error, cbd->data);
+
+		return;
+	}
+
+	g_at_chat_unref(gd->data_chat);
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+}
+
+static void mbm_location_reporting_disable(struct ofono_location_reporting *lr,
+				ofono_location_reporting_disable_cb_t cb,
+				void *data)
+{
+	struct gps_data *gd = ofono_location_reporting_get_data(lr);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	DBG("lr=%p", lr);
+
+	cbd->user = lr;
+
+	if (g_at_chat_send(gd->chat, "AT*E2GPSCTL=0,5,1", none_prefix,
+				mbm_e2gpsctl_disable_cb, cbd, g_free) > 0)
+		return;
+
+	CALLBACK_WITH_FAILURE(cb, data);
+	g_free(cbd);
+}
+
+static int mbm_create_data_chat(struct ofono_location_reporting *lr)
+{
+	struct gps_data *gd = ofono_location_reporting_get_data(lr);
+	struct ofono_modem *modem;
+	const char *gps_dev;
+	GAtSyntax *syntax;
+	GIOChannel *channel;
+	int fd;
+
+	modem = ofono_location_reporting_get_modem(lr);
+	gps_dev = ofono_modem_get_string(modem, "GPSDevice");
+
+	channel = g_at_tty_open(gps_dev, NULL);
+	if (channel == NULL)
+		return -1;
+
+	syntax = g_at_syntax_new_gsm_permissive();
+	gd->data_chat = g_at_chat_new(channel, syntax);
+	fd = g_io_channel_unix_get_fd(channel);
+
+	g_at_syntax_unref(syntax);
+	g_io_channel_unref(channel);
+
+	if (gd->data_chat == NULL)
+		return -1;
+
+	return fd;
+}
+
+static void mbm_e2gpsctl_enable_cb(gboolean ok, GAtResult *result,
+							 gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_location_reporting_enable_cb_t cb = cbd->cb;
+	struct ofono_location_reporting *lr = cbd->user;
+	struct gps_data *gd = ofono_location_reporting_get_data(lr);
+	struct ofono_error error;
+	int fd;
+
+	DBG("lr=%p ok=%d", lr, ok);
+
+	decode_at_error(&error, g_at_result_final_response(result));
+
+	if (!ok) {
+		cb(&error, -1, cbd->data);
+
+		return;
+	}
+
+	fd = mbm_create_data_chat(lr);
+
+	if (fd < 0)
+		goto out;
+
+	if (g_at_chat_send(gd->data_chat, "AT*E2GPSNPD", NULL, NULL, NULL,
+								NULL) > 0) {
+		cb(&error, fd, cbd->data);
+
+		return;
+	}
+
+out:
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+}
+
+static void mbm_location_reporting_enable(struct ofono_location_reporting *lr,
+					ofono_location_reporting_enable_cb_t cb,
+					void *data)
+{
+	struct gps_data *gd = ofono_location_reporting_get_data(lr);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	DBG("lr=%p", lr);
+
+	if (cbd == NULL)
+		goto out;
+
+	cbd->user = lr;
+
+	if (g_at_chat_send(gd->chat, "AT*E2GPSCTL=1,5,1", none_prefix,
+				mbm_e2gpsctl_enable_cb, cbd, g_free) > 0)
+		return;
+
+out:
+	CALLBACK_WITH_FAILURE(cb, -1, data);
+	g_free(cbd);
+}
+
+static void mbm_location_reporting_support_cb(gboolean ok, GAtResult *result,
+							gpointer user_data)
+{
+	struct ofono_location_reporting *lr = user_data;
+
+	if (!ok) {
+		ofono_location_reporting_remove(lr);
+
+		return;
+	}
+
+	ofono_location_reporting_register(lr);
+}
+
+static int mbm_location_reporting_probe(struct ofono_location_reporting *lr,
+						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_location_reporting_set_data(lr, gd);
+
+	g_at_chat_send(gd->chat, "AT*E2GPSCTL=?", e2gpsctl_prefix,
+					mbm_location_reporting_support_cb,
+					lr, NULL);
+
+	return 0;
+}
+
+static void mbm_location_reporting_remove(struct ofono_location_reporting *lr)
+{
+	struct gps_data *gd = ofono_location_reporting_get_data(lr);
+
+	ofono_location_reporting_set_data(lr, NULL);
+
+	g_at_chat_unref(gd->chat);
+	g_free(gd);
+}
+
+static struct ofono_location_reporting_driver driver = {
+	.name			= "mbmmodem",
+	.type			= OFONO_LOCATION_REPORTING_TYPE_NMEA,
+	.probe			= mbm_location_reporting_probe,
+	.remove			= mbm_location_reporting_remove,
+	.enable			= mbm_location_reporting_enable,
+	.disable		= mbm_location_reporting_disable,
+};
+
+void mbm_location_reporting_init()
+{
+	ofono_location_reporting_driver_register(&driver);
+}
+
+void mbm_location_reporting_exit()
+{
+	ofono_location_reporting_driver_unregister(&driver);
+}
diff --git a/drivers/mbmmodem/mbmmodem.c b/drivers/mbmmodem/mbmmodem.c
index 03b61b3..9938350 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_location_reporting_init();
 
 	return 0;
 }
 
 static void mbmmodem_exit(void)
 {
+	mbm_location_reporting_exit();
 	mbm_stk_exit();
 	mbm_gprs_context_exit();
 }
diff --git a/drivers/mbmmodem/mbmmodem.h b/drivers/mbmmodem/mbmmodem.h
index 4c6f263..aaa911d 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_location_reporting_init();
+extern void mbm_location_reporting_exit();
-- 
1.7.4.1

[PATCH v4 4/6] mbm: add location-reporting atom

[Lucas De Marchi]

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

From: Rafael Ignacio Zurita <rafael.zurita@profusion.mobi>

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

diff --git a/plugins/mbm.c b/plugins/mbm.c
index 2ab80b4..105843f 100644
--- a/plugins/mbm.c
+++ b/plugins/mbm.c
@@ -46,6 +46,7 @@
 #include <ofono/gprs-context.h>
 #include <ofono/radio-settings.h>
 #include <ofono/log.h>
+#include <ofono/location-reporting.h>
 
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
@@ -67,6 +68,7 @@ struct mbm_data {
 	gboolean have_sim;
 	struct ofono_gprs *gprs;
 	struct ofono_gprs_context *gc;
+	struct ofono_location_reporting *lr;
 	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->lr = ofono_location_reporting_create(modem, 0,
+					"mbmmodem", data->modem_port);
+
 	ofono_netreg_create(modem, OFONO_VENDOR_MBM,
 					"atmodem", data->modem_port);
 
-- 
1.7.4.1

[PATCH v4 5/6] udev: add location-reporting device to mbm

[Lucas De Marchi]

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

From: Rafael Ignacio Zurita <rafael.zurita@profusion.mobi>

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

diff --git a/plugins/udev.c b/plugins/udev.c
index 84478d7..aa5702e 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.4.1

[PATCH v4 6/6] location-reporting: add documentation

[Lucas De Marchi]

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

From: Rafael Ignacio Zurita <rafael.zurita@profusion.mobi>

---
 Makefile.am                    |    3 ++-
 doc/location-reporting-api.txt |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletions(-)
 create mode 100644 doc/location-reporting-api.txt

diff --git a/Makefile.am b/Makefile.am
index 9b6edf4..ca558ea 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -420,7 +420,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/location-reporting-api.txt
 
 
 test_scripts = test/backtrace \
diff --git a/doc/location-reporting-api.txt b/doc/location-reporting-api.txt
new file mode 100644
index 0000000..b8ce840
--- /dev/null
+++ b/doc/location-reporting-api.txt
@@ -0,0 +1,39 @@
+Location Reporting Hierarchy [experimental]
+=================
+
+Service		org.ofono
+Interface	org.ofono.LocationReporting
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		dict GetProperties()
+
+			Returns all LocationReporting properties. See the
+			properties section for available properties.
+
+		byte Request()
+
+			Asks to turn ON the NMEA stream and supplies the
+			gps device file descriptor. The external cliend should
+			use the file descriptor to receive the NMEA data.
+
+			Possible Errors: [service].Error.InProgress
+					 [service].Error.InUse
+					 [service].Error.Failed
+
+		void Release()
+
+			Releases the gps device file descriptor and turns
+			OFF the NMEA stream.
+
+			Possible Errors: [service].Error.InProgress
+					 [service].Error.NotAvailable
+					 [service].Error.Failed
+
+Properties	boolean Enabled [readonly]
+
+                        Boolean representing the state of the NMEA stream.
+
+		string Type [readonly]
+
+			Holds the type of the device. Currently only NMEA is
+			supported.
-- 
1.7.4.1

Re: [PATCH v4 0/6] Add location-reporting atom

[Denis Kenzior]

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

Hi Lucas,

On 02/22/2011 12:35 PM, Lucas De Marchi wrote:
> From: Lucas De Marchi <lucas.de.marchi@gmail.com>
> 
> This is the 4th version of the location-reporting patches (previous title was
> "Add GPS atom"). I worked with Rafael Ignacio Zurita to implement this last
> version, making some changes in order to fit the new design as discussed on irc.
> 
> Now when a client calls 'Request()' we give it a file
> descriptor in which the location reporting will be written. By checking the
> 'Type' property the client can decide what to do with such information. As of
> now only NMEA is supported and it's implemented on mbm modem.
> 
> If client exits or calls 'Release()', the location reporting is disabled.
> 
> Rafael Ignacio Zurita (6):
>   location-reporting: add public header
>   location-reporting: add atom implementation
>   mbmmodem: add location-reporting driver implementation
>   mbm: add location-reporting atom
>   udev: add location-reporting device to mbm
>   location-reporting: add documentation
> 
>  Makefile.am                           |   11 +-
>  doc/location-reporting-api.txt        |   39 ++++
>  drivers/mbmmodem/location-reporting.c |  247 +++++++++++++++++++++
>  drivers/mbmmodem/mbmmodem.c           |    2 +
>  drivers/mbmmodem/mbmmodem.h           |    3 +
>  include/location-reporting.h          |   81 +++++++
>  plugins/mbm.c                         |    8 +
>  plugins/udev.c                        |    5 +-
>  src/location-reporting.c              |  393 +++++++++++++++++++++++++++++++++
>  src/ofono.h                           |    2 +
>  10 files changed, 785 insertions(+), 6 deletions(-)
>  create mode 100644 doc/location-reporting-api.txt
>  create mode 100644 drivers/mbmmodem/location-reporting.c
>  create mode 100644 include/location-reporting.h
>  create mode 100644 src/location-reporting.c
> 

So I applied all of these patches, however see my comments for patch 3.

Regards,
-Denis

Re: [PATCH v4 3/6] mbmmodem: add location-reporting driver implementation

[Denis Kenzior]

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

Hi Lucas,

> +static int mbm_create_data_chat(struct ofono_location_reporting *lr)
> +{
> +	struct gps_data *gd = ofono_location_reporting_get_data(lr);
> +	struct ofono_modem *modem;
> +	const char *gps_dev;
> +	GAtSyntax *syntax;
> +	GIOChannel *channel;
> +	int fd;
> +
> +	modem = ofono_location_reporting_get_modem(lr);
> +	gps_dev = ofono_modem_get_string(modem, "GPSDevice");
> +
> +	channel = g_at_tty_open(gps_dev, NULL);
> +	if (channel == NULL)
> +		return -1;
> +
> +	syntax = g_at_syntax_new_gsm_permissive();
> +	gd->data_chat = g_at_chat_new(channel, syntax);
> +	fd = g_io_channel_unix_get_fd(channel);
> +
> +	g_at_syntax_unref(syntax);
> +	g_io_channel_unref(channel);
> +
> +	if (gd->data_chat == NULL)
> +		return -1;
> +
> +	return fd;
> +}
> +

Why do you bother creating a GAtChat for the NMEA port?  It seems that a
GIOChannel or a simple fd would be enough.

Also, what happens if the channel is hupped by the modem?  Shouldn't we
have a watch in the core for that and set Enabled accordingly?

Regards,
-Denis

Re: [PATCH v4 3/6] mbmmodem: add location-reporting driver implementation

[Lucas De Marchi]

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

Hi Denis

On Wed, Feb 23, 2011 at 9:57 PM, Denis Kenzior <denkenz@gmail.com> wrote:
> Hi Lucas,
>
>> +static int mbm_create_data_chat(struct ofono_location_reporting *lr)
>> +{
>> +     struct gps_data *gd = ofono_location_reporting_get_data(lr);
>> +     struct ofono_modem *modem;
>> +     const char *gps_dev;
>> +     GAtSyntax *syntax;
>> +     GIOChannel *channel;
>> +     int fd;
>> +
>> +     modem = ofono_location_reporting_get_modem(lr);
>> +     gps_dev = ofono_modem_get_string(modem, "GPSDevice");
>> +
>> +     channel = g_at_tty_open(gps_dev, NULL);
>> +     if (channel == NULL)
>> +             return -1;
>> +
>> +     syntax = g_at_syntax_new_gsm_permissive();
>> +     gd->data_chat = g_at_chat_new(channel, syntax);
>> +     fd = g_io_channel_unix_get_fd(channel);
>> +
>> +     g_at_syntax_unref(syntax);
>> +     g_io_channel_unref(channel);
>> +
>> +     if (gd->data_chat == NULL)
>> +             return -1;
>> +
>> +     return fd;
>> +}
>> +
>
> Why do you bother creating a GAtChat for the NMEA port?  It seems that a
> GIOChannel or a simple fd would be enough.

This is because of this line:

+       if (g_at_chat_send(gd->data_chat, "AT*E2GPSNPD", NULL, NULL, NULL,
+                                                               NULL) > 0) {

AT*E2GPSNPD is sent on the tty that will contain the nmea stream, not
the control one.

> Also, what happens if the channel is hupped by the modem?  Shouldn't we
> have a watch in the core for that and set Enabled accordingly?

Humn... maybe. Though client should have called Release(). Otherwise,
isn't the Release() method superfluous?



regards,
Lucas De Marchi

Re: [PATCH v4 3/6] mbmmodem: add location-reporting driver implementation

[Denis Kenzior]

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

Hi Lucas,

On 02/23/2011 08:26 PM, Lucas De Marchi wrote:
> Hi Denis
> 
> On Wed, Feb 23, 2011 at 9:57 PM, Denis Kenzior <denkenz@gmail.com> wrote:
>> Hi Lucas,
>>
>>> +static int mbm_create_data_chat(struct ofono_location_reporting *lr)
>>> +{
>>> +     struct gps_data *gd = ofono_location_reporting_get_data(lr);
>>> +     struct ofono_modem *modem;
>>> +     const char *gps_dev;
>>> +     GAtSyntax *syntax;
>>> +     GIOChannel *channel;
>>> +     int fd;
>>> +
>>> +     modem = ofono_location_reporting_get_modem(lr);
>>> +     gps_dev = ofono_modem_get_string(modem, "GPSDevice");
>>> +
>>> +     channel = g_at_tty_open(gps_dev, NULL);
>>> +     if (channel == NULL)
>>> +             return -1;
>>> +
>>> +     syntax = g_at_syntax_new_gsm_permissive();
>>> +     gd->data_chat = g_at_chat_new(channel, syntax);
>>> +     fd = g_io_channel_unix_get_fd(channel);
>>> +
>>> +     g_at_syntax_unref(syntax);
>>> +     g_io_channel_unref(channel);
>>> +
>>> +     if (gd->data_chat == NULL)
>>> +             return -1;
>>> +
>>> +     return fd;
>>> +}
>>> +
>>
>> Why do you bother creating a GAtChat for the NMEA port?  It seems that a
>> GIOChannel or a simple fd would be enough.
> 
> This is because of this line:
> 
> +       if (g_at_chat_send(gd->data_chat, "AT*E2GPSNPD", NULL, NULL, NULL,
> +                                                               NULL) > 0) {
> 
> AT*E2GPSNPD is sent on the tty that will contain the nmea stream, not
> the control one.
> 

Then I'm confused how this even works.  The GAtChat is installing its
own IO watches on this channel and would be reading from the stream.
Wouldn't this cause you to have a race between the location reporting
client and ofono itself?

Maybe you should be sending the E2GPSNPD, unrefing the data_chat and
then sending the fd upwards?

>> Also, what happens if the channel is hupped by the modem?  Shouldn't we
>> have a watch in the core for that and set Enabled accordingly?
> 
> Humn... maybe. Though client should have called Release(). Otherwise,
> isn't the Release() method superfluous?
> 

Assuming the client is well behaved..

Also, what are we doing with the open socket if oFono is forcefully shut
down?  Shouldn't we shut it down as well?

Regards,
-Denis