[PATCH 00/12] basic E911 support

[PATCH 00/12] basic E911 support

[Jarko Poutiainen]

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

Hi,

This is second attempt to provide implementation for basic E911 support.

Br,
Jarko

Jarko Poutiainen (12):
  dbus: add gnss interface definition
  include: add gnss.h file
  src: add atom type for gnss
  src: add gnss atom and agent implementation
  gatchat:introduce send for +CPOS
  gatchat:new hint to handle +CPOS
  gatchat: implementation for +CPOS send
  gatchat: fix gatsyntax to support +CPOS
  atmodem: add gnss driver
  ste: add support for gnss
  ofono.conf: add positioning agent interface
  test: add test-gnss

 Makefile.am               |   11 +-
 drivers/atmodem/atmodem.c |    2 +
 drivers/atmodem/atmodem.h |    3 +
 drivers/atmodem/gnss.c    |  279 +++++++++++++++++++++++++++++++++++
 gatchat/gatchat.c         |   13 ++
 gatchat/gatchat.h         |    8 +
 gatchat/gatsyntax.c       |   19 +++
 gatchat/gatsyntax.h       |    3 +-
 include/dbus.h            |    2 +
 include/gnss.h            |   76 ++++++++++
 plugins/ste.c             |    8 +-
 src/gnss.c                |  357 +++++++++++++++++++++++++++++++++++++++++++++
 src/gnssagent.c           |  152 +++++++++++++++++++
 src/gnssagent.h           |   40 +++++
 src/ofono.conf            |    1 +
 src/ofono.h               |    2 +
 test/test-gnss            |   91 ++++++++++++
 17 files changed, 1060 insertions(+), 7 deletions(-)
 create mode 100644 drivers/atmodem/gnss.c
 create mode 100644 include/gnss.h
 create mode 100644 src/gnss.c
 create mode 100644 src/gnssagent.c
 create mode 100644 src/gnssagent.h
 create mode 100755 test/test-gnss

[PATCH 01/12] dbus: add gnss interface definition

[Jarko Poutiainen]

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

---
 include/dbus.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/dbus.h b/include/dbus.h
index 19a138a..bf3e8a4 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -56,6 +56,8 @@ extern "C" {
 #define OFONO_STK_INTERFACE OFONO_SERVICE ".SimToolkit"
 #define OFONO_SIM_APP_INTERFACE OFONO_SERVICE ".SimToolkitAgent"
 #define OFONO_LOCATION_REPORTING_INTERFACE OFONO_SERVICE ".LocationReporting"
+#define OFONO_AS_NAVIGATION_INTERFACE "org.ofono.AssistedSatelliteNavigation"
+#define OFONO_POSITIONING_REQUEST_INTERFACE "org.ofono.PositioningRequestAgent"
 
 /* CDMA Interfaces */
 #define OFONO_CDMA_VOICECALL_MANAGER_INTERFACE "org.ofono.cdma.VoiceCallManager"
-- 
1.7.0.4

[PATCH 02/12] include: add gnss.h file

[Jarko Poutiainen]

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

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

diff --git a/Makefile.am b/Makefile.am
index 3f20717..7d6acce 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -15,7 +15,7 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/ctm.h include/cdma-voicecall.h \
 			include/cdma-sms.h include/sim-auth.h \
 			include/gprs-provision.h include/emulator.h \
-			include/location-reporting.h
+			include/location-reporting.h include/gnss.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
diff --git a/include/gnss.h b/include/gnss.h
new file mode 100644
index 0000000..d10ab11
--- /dev/null
+++ b/include/gnss.h
@@ -0,0 +1,76 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2011 ST-Ericsson AB.
+ *
+ *  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_GNSS_H
+#define __OFONO_GNSS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <ofono/types.h>
+
+struct ofono_gnss;
+
+typedef void (*ofono_gnss_position_report_cb_t)(const struct ofono_error *error,
+							void *data);
+
+typedef void (*ofono_gnss_send_element_cb_t)(const struct ofono_error *error,
+						void *data);
+
+typedef void (*ofono_gnss_report_reset_cb_t)(const struct ofono_error *error,
+						void *data);
+
+struct ofono_gnss_driver {
+	const char *name;
+	int (*probe)(struct ofono_gnss *gnss, unsigned int vendor, void *data);
+	void (*remove)(struct ofono_gnss *gnss);
+	void (*send_element)(struct ofono_gnss *gnss,
+				const char *xml,
+				ofono_gnss_send_element_cb_t cb, void *data);
+	void (*position_reporting)(struct ofono_gnss *gnss,
+					ofono_bool_t enable,
+					ofono_gnss_position_report_cb_t cb,
+					void *data);
+};
+
+void ofono_gnss_receive_request(struct ofono_gnss *gnss, const char *xml);
+void ofono_gnss_receive_reset(struct ofono_gnss *gnss);
+int ofono_gnss_driver_register(const struct ofono_gnss_driver *d);
+void ofono_gnss_driver_unregister(const struct ofono_gnss_driver *d);
+
+struct ofono_gnss *ofono_gnss_create(struct ofono_modem *modem,
+					unsigned int vendor,
+					const char *driver, void *data);
+
+void ofono_gnss_register(struct ofono_gnss *gnss);
+void ofono_gnss_remove(struct ofono_gnss *gnss);
+
+void ofono_gnss_set_data(struct ofono_gnss *gnss, void *data);
+void *ofono_gnss_get_data(struct ofono_gnss *gnss);
+
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_GNSS_H */
-- 
1.7.0.4

[PATCH 03/12] src: add atom type for gnss

[Jarko Poutiainen]

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

---
 src/ofono.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index 995a1a5..2c6899e 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -131,6 +131,7 @@ enum ofono_atom_type {
 	OFONO_ATOM_TYPE_EMULATOR_DUN,
 	OFONO_ATOM_TYPE_EMULATOR_HFP,
 	OFONO_ATOM_TYPE_LOCATION_REPORTING,
+	OFONO_ATOM_TYPE_GNSS,
 };
 
 enum ofono_atom_watch_condition {
@@ -466,3 +467,4 @@ void __ofono_gprs_provision_free_settings(
 				int count);
 
 #include <ofono/emulator.h>
+#include <ofono/gnss.h>
-- 
1.7.0.4

[PATCH 04/12] src: add gnss atom and agent implementation

[Jarko Poutiainen]

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

---
 Makefile.am     |    3 +-
 src/gnss.c      |  357 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/gnssagent.c |  152 +++++++++++++++++++++++
 src/gnssagent.h |   40 ++++++
 4 files changed, 551 insertions(+), 1 deletions(-)
 create mode 100644 src/gnss.c
 create mode 100644 src/gnssagent.c
 create mode 100644 src/gnssagent.h

diff --git a/Makefile.am b/Makefile.am
index 7d6acce..24742bb 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -383,7 +383,8 @@ 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/location-reporting.c
+			src/emulator.c src/location-reporting.c \
+			src/gnss.c src/gnssagent.c src/gnssagent.h
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
 
diff --git a/src/gnss.c b/src/gnss.c
new file mode 100644
index 0000000..02f9057
--- /dev/null
+++ b/src/gnss.c
@@ -0,0 +1,357 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2011 ST-Ericsson AB.
+ *
+ *  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 <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+
+#include <glib.h>
+#include <gdbus.h>
+#include <errno.h>
+
+#include "ofono.h"
+
+#include "common.h"
+#include "gnssagent.h"
+
+static GSList *g_drivers = NULL;
+
+struct ofono_gnss {
+	const struct ofono_gnss_driver *driver;
+	void *driver_data;
+	struct ofono_atom *atom;
+	DBusMessage *pending;
+	struct gnss_agent *default_agent;
+};
+
+static void gnss_register_agent_cb(const struct ofono_error *error,
+					void *data)
+{
+	DBusMessage *reply;
+	struct ofono_gnss *gnss = data;
+
+	DBG("");
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Enabling Location Reporting Failed");
+		reply = __ofono_error_failed(gnss->pending);
+		gnss_agent_free(gnss->default_agent);
+		__ofono_dbus_pending_reply(&gnss->pending, reply);
+		return;
+	}
+
+	reply = dbus_message_new_method_return(gnss->pending);
+	__ofono_dbus_pending_reply(&gnss->pending, reply);
+}
+
+static void default_agent_notify(gpointer user_data)
+{
+	struct ofono_gnss *gnss = user_data;
+
+	gnss->default_agent = NULL;
+}
+
+static DBusMessage *gnss_register_agent(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_gnss *gnss = data;
+	const char *agent_path;
+
+	if (gnss->pending)
+		return __ofono_error_busy(msg);
+
+	if (gnss->default_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);
+
+	gnss->driver->position_reporting(gnss, 1, gnss_register_agent_cb, gnss);
+
+	gnss->default_agent = gnss_agent_new(agent_path,
+					dbus_message_get_sender(msg),
+					FALSE);
+
+	if (gnss->default_agent == NULL)
+		return __ofono_error_failed(msg);
+
+	gnss_agent_set_removed_notify(gnss->default_agent,
+					default_agent_notify, gnss);
+
+	gnss->pending = dbus_message_ref(msg);
+
+	return NULL;
+}
+
+static void gnss_unregister_agent_cb(const struct ofono_error *error,
+					void *data)
+{
+	DBusMessage *reply;
+	struct ofono_gnss *gnss = data;
+
+	DBG("");
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Disabling Location Reporting Failed");
+		reply = __ofono_error_failed(gnss->pending);
+		goto out;
+	}
+
+	gnss_agent_free(gnss->default_agent);
+
+	if (gnss->default_agent) {
+		ofono_error("Releasing agent failed");
+		reply = __ofono_error_failed(gnss->pending);
+		goto out;
+	}
+
+	reply = dbus_message_new_method_return(gnss->pending);
+
+out:
+	__ofono_dbus_pending_reply(&gnss->pending, reply);
+}
+
+static DBusMessage *gnss_unregister_agent(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct ofono_gnss *gnss = data;
+	const char *agent_path;
+	const char *agent_bus = dbus_message_get_sender(msg);
+
+	if (gnss->pending)
+		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 (gnss->default_agent == NULL)
+		return __ofono_error_failed(msg);
+
+	if (!gnss_agent_matches(gnss->default_agent, agent_path, agent_bus))
+		return __ofono_error_failed(msg);
+
+	gnss->pending = dbus_message_ref(msg);
+
+	gnss->driver->position_reporting(gnss, 0, gnss_unregister_agent_cb,
+						gnss);
+
+	return NULL;
+}
+
+static void gnss_send_element_cb(const struct ofono_error *error,
+				void *data)
+{
+	DBusMessage *reply;
+	struct ofono_gnss *gnss = data;
+
+	DBG("");
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_error("Sending Positioning Element failed");
+
+		reply = __ofono_error_failed(gnss->pending);
+
+		goto out;
+	}
+
+	reply = dbus_message_new_method_return(gnss->pending);
+
+out:
+	__ofono_dbus_pending_reply(&gnss->pending, reply);
+}
+
+static DBusMessage *gnss_send_element(DBusConnection *conn,
+					DBusMessage *msg, void *data)
+{
+	struct ofono_gnss *gnss = data;
+	const char *xml;
+
+	DBG("");
+
+	if (gnss->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &xml,
+					DBUS_TYPE_INVALID))
+		return __ofono_error_invalid_args(msg);
+
+	gnss->pending = dbus_message_ref(msg);
+
+	gnss->driver->send_element(gnss, xml, gnss_send_element_cb, gnss);
+
+	return NULL;
+}
+
+static GDBusMethodTable gnss_methods[] = {
+	{ "SendPositioningElement",		"s",	"",
+				gnss_send_element, G_DBUS_METHOD_FLAG_ASYNC },
+	{ "RegisterPositioningRequestAgent",	"o",	"",
+				gnss_register_agent,
+				G_DBUS_METHOD_FLAG_ASYNC },
+	{ "UnregisterPositioningRequestAgent",	"o",	"",
+				gnss_unregister_agent,
+				G_DBUS_METHOD_FLAG_ASYNC },
+	{ }
+};
+
+static void gnss_unregister(struct ofono_atom *atom)
+{
+	struct ofono_gnss *gnss = __ofono_atom_get_data(atom);
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
+	const char *path = __ofono_atom_get_path(atom);
+
+	if (gnss->default_agent)
+		gnss_agent_free(gnss->default_agent);
+
+	ofono_modem_remove_interface(modem, OFONO_AS_NAVIGATION_INTERFACE);
+	g_dbus_unregister_interface(conn, path, OFONO_AS_NAVIGATION_INTERFACE);
+}
+
+static void gnss_remove(struct ofono_atom *atom)
+{
+	struct ofono_gnss *gnss = __ofono_atom_get_data(atom);
+
+	DBG("atom: %p", atom);
+
+	if (gnss == NULL)
+		return;
+
+	if (gnss->driver && gnss->driver->remove)
+		gnss->driver->remove(gnss);
+
+	g_free(gnss);
+}
+
+void ofono_gnss_register(struct ofono_gnss *gnss)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(gnss->atom);
+	const char *path = __ofono_atom_get_path(gnss->atom);
+
+	if (!g_dbus_register_interface(conn, path,
+					OFONO_AS_NAVIGATION_INTERFACE,
+					gnss_methods, NULL, NULL,
+					gnss, NULL)) {
+		ofono_error("Could not create %s interface",
+				OFONO_AS_NAVIGATION_INTERFACE);
+
+		return;
+	}
+
+	ofono_modem_add_interface(modem, OFONO_AS_NAVIGATION_INTERFACE);
+
+	__ofono_atom_register(gnss->atom, gnss_unregister);
+}
+
+int ofono_gnss_driver_register(const struct ofono_gnss_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (d->probe == NULL)
+		return -EINVAL;
+
+	g_drivers = g_slist_prepend(g_drivers, (void *) d);
+
+	return 0;
+}
+
+void ofono_gnss_driver_unregister(const struct ofono_gnss_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	g_drivers = g_slist_remove(g_drivers, (void *) d);
+}
+
+struct ofono_gnss *ofono_gnss_create(struct ofono_modem *modem,
+					unsigned int vendor,
+					const char *driver,
+					void *data)
+{
+	struct ofono_gnss *gnss;
+	GSList *l;
+
+	if (driver == NULL)
+		return NULL;
+
+	gnss = g_try_new0(struct ofono_gnss, 1);
+
+	if (gnss == NULL)
+		return NULL;
+
+	gnss->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_GNSS,
+						gnss_remove, gnss);
+
+	for (l = g_drivers; l; l = l->next) {
+		const struct ofono_gnss_driver *drv = l->data;
+
+		if (g_strcmp0(drv->name, driver))
+			continue;
+
+		if (drv->probe(gnss, vendor, data) < 0)
+			continue;
+
+		gnss->driver = drv;
+		break;
+	}
+
+	return gnss;
+}
+
+void ofono_gnss_receive_request(struct ofono_gnss *gnss, const char *xml)
+{
+	if (gnss->default_agent)
+		gnss_agent_receive_request(gnss->default_agent, xml);
+}
+
+void ofono_gnss_receive_reset(struct ofono_gnss *gnss)
+{
+	if (gnss->default_agent)
+		gnss_agent_receive_reset(gnss->default_agent);
+}
+
+void ofono_gnss_remove(struct ofono_gnss *gnss)
+{
+	__ofono_atom_free(gnss->atom);
+}
+
+void ofono_gnss_set_data(struct ofono_gnss *gnss, void *data)
+{
+	gnss->driver_data = data;
+}
+
+void *ofono_gnss_get_data(struct ofono_gnss *gnss)
+{
+	return gnss->driver_data;
+}
diff --git a/src/gnssagent.c b/src/gnssagent.c
new file mode 100644
index 0000000..28714e2
--- /dev/null
+++ b/src/gnssagent.c
@@ -0,0 +1,152 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2011 ST-Ericsson AB.
+ *
+ *  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.h"
+#include "gnssagent.h"
+
+struct gnss_agent {
+	char *path;
+	char *bus;
+	guint disconnect_watch;
+	ofono_bool_t remove_on_terminate;
+	ofono_destroy_func removed_cb;
+	void *removed_data;
+};
+
+void gnss_agent_receive_request(struct gnss_agent *agent, const char *xml)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *message;
+
+	message = dbus_message_new_method_call(agent->bus, agent->path,
+					OFONO_POSITIONING_REQUEST_INTERFACE,
+						"Request");
+
+	dbus_message_append_args(message,
+					DBUS_TYPE_STRING, &xml,
+					DBUS_TYPE_INVALID);
+
+	dbus_message_set_no_reply(message, TRUE);
+
+	g_dbus_send_message(conn, message);
+}
+
+static void gnss_agent_send_noreply(struct gnss_agent *agent,
+					const char *method)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	DBusMessage *message;
+
+	message = dbus_message_new_method_call(agent->bus, agent->path,
+					OFONO_POSITIONING_REQUEST_INTERFACE,
+						method);
+
+	dbus_message_set_no_reply(message, TRUE);
+
+	g_dbus_send_message(conn, message);
+}
+
+static inline void gnss_agent_send_release(struct gnss_agent *agent)
+{
+	gnss_agent_send_noreply(agent, "Release");
+}
+
+void gnss_agent_receive_reset(struct gnss_agent *agent)
+{
+	gnss_agent_send_noreply(agent, "ResetAssistanceData");
+}
+
+ofono_bool_t gnss_agent_matches(struct gnss_agent *agent,
+				const char *path, const char *sender)
+{
+	return !strcmp(agent->path, path) && !strcmp(agent->bus, sender);
+}
+
+void gnss_agent_set_removed_notify(struct gnss_agent *agent,
+					ofono_destroy_func destroy,
+					void *user_data)
+{
+	agent->removed_cb = destroy;
+	agent->removed_data = user_data;
+}
+
+void gnss_agent_free(struct gnss_agent *agent)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+
+	if (agent->disconnect_watch) {
+		gnss_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->bus);
+	g_free(agent);
+}
+
+static void gnss_agent_disconnect_cb(DBusConnection *conn, void *user_data)
+{
+	struct gnss_agent *agent = user_data;
+
+	ofono_debug("Agent exited without calling Unregister");
+
+	agent->disconnect_watch = 0;
+
+	gnss_agent_free(agent);
+}
+
+struct gnss_agent *gnss_agent_new(const char *path, const char *sender,
+				ofono_bool_t remove_on_terminate)
+{
+	struct gnss_agent *agent = g_try_new0(struct gnss_agent, 1);
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	if (agent == NULL)
+		return NULL;
+
+	agent->path = g_strdup(path);
+	agent->bus = g_strdup(sender);
+	agent->remove_on_terminate = remove_on_terminate;
+
+	agent->disconnect_watch = g_dbus_add_disconnect_watch(conn, sender,
+						gnss_agent_disconnect_cb,
+						agent, NULL);
+
+	return agent;
+}
diff --git a/src/gnssagent.h b/src/gnssagent.h
new file mode 100644
index 0000000..03c6053
--- /dev/null
+++ b/src/gnssagent.h
@@ -0,0 +1,40 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2011 ST-Ericsson AB.
+ *
+ *  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 gnss_agent;
+
+
+struct gnss_agent *gnss_agent_new(const char *path, const char *sender,
+					ofono_bool_t remove_on_terminate);
+
+void gnss_agent_free(struct gnss_agent *agent);
+
+void gnss_agent_receive_request(struct gnss_agent *agent, const char *xml);
+
+void gnss_agent_receive_reset(struct gnss_agent *agent);
+
+void gnss_agent_set_removed_notify(struct gnss_agent *agent,
+					ofono_destroy_func removed_cb,
+					void *user_data);
+
+ofono_bool_t gnss_agent_matches(struct gnss_agent *agent,
+				const char *path, const char *sender);
-- 
1.7.0.4

[PATCH 05/12] gatchat:introduce send for +CPOS

[Jarko Poutiainen]

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

---
 gatchat/gatchat.h |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatchat.h b/gatchat/gatchat.h
index 8cbb559..eb82daa 100644
--- a/gatchat/gatchat.h
+++ b/gatchat/gatchat.h
@@ -124,6 +124,14 @@ guint g_at_chat_send_pdu_listing(GAtChat *chat, const char *cmd,
 				GAtNotifyFunc listing, GAtResultFunc func,
 				gpointer user_data, GDestroyNotify notify);
 
+/*!
+ * Same as g_at_chat_send except parser will know to expect short prompt syntax
+ * used with +CPOS.
+ */
+guint g_at_chat_send_and_expect_short_prompt(GAtChat *chat, const char *cmd,
+				const char **valid_resp, GAtResultFunc func,
+				gpointer user_data, GDestroyNotify notify);
+
 gboolean g_at_chat_cancel(GAtChat *chat, guint id);
 gboolean g_at_chat_cancel_all(GAtChat *chat);
 
-- 
1.7.0.4

[PATCH 06/12] gatchat:new hint to handle +CPOS

[Jarko Poutiainen]

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

---
 gatchat/gatsyntax.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatsyntax.h b/gatchat/gatsyntax.h
index afc0bd3..2580ec6 100644
--- a/gatchat/gatsyntax.h
+++ b/gatchat/gatsyntax.h
@@ -29,7 +29,8 @@ extern "C" {
 enum _GAtSyntaxExpectHint {
 	G_AT_SYNTAX_EXPECT_PDU,
 	G_AT_SYNTAX_EXPECT_MULTILINE,
-	G_AT_SYNTAX_EXPECT_PROMPT
+	G_AT_SYNTAX_EXPECT_PROMPT,
+	G_AT_SYNTAX_EXPECT_SHORT_PROMPT
 };
 
 typedef enum _GAtSyntaxExpectHint GAtSyntaxExpectHint;
-- 
1.7.0.4

[PATCH 07/12] gatchat: implementation for +CPOS send

[Jarko Poutiainen]

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

---
 gatchat/gatchat.c |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatchat.c b/gatchat/gatchat.c
index 3fd564d..64f131d 100644
--- a/gatchat/gatchat.c
+++ b/gatchat/gatchat.c
@@ -1468,6 +1468,19 @@ guint g_at_chat_send_pdu_listing(GAtChat *chat, const char *cmd,
 					listing, func, user_data, notify);
 }
 
+guint g_at_chat_send_and_expect_short_prompt(GAtChat *chat, const char *cmd,
+			const char **prefix_list, GAtResultFunc func,
+			gpointer user_data, GDestroyNotify notify)
+{
+	if(chat != NULL)
+		chat->parent->syntax->set_hint(chat->parent->syntax,
+					G_AT_SYNTAX_EXPECT_SHORT_PROMPT);
+
+	return at_chat_send_common(chat->parent, chat->group,
+					cmd, prefix_list, FALSE, NULL,
+					func, user_data, notify);
+}
+
 gboolean g_at_chat_cancel(GAtChat *chat, guint id)
 {
 	/* We use id 0 for wakeup commands */
-- 
1.7.0.4

[PATCH 08/12] gatchat: fix gatsyntax to support +CPOS

[Jarko Poutiainen]

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

---
 gatchat/gatsyntax.c |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/gatchat/gatsyntax.c b/gatchat/gatsyntax.c
index 2fc70b8..f1d896b 100644
--- a/gatchat/gatsyntax.c
+++ b/gatchat/gatsyntax.c
@@ -64,6 +64,9 @@ static void gsmv1_hint(GAtSyntax *syntax, GAtSyntaxExpectHint hint)
 	case G_AT_SYNTAX_EXPECT_MULTILINE:
 		syntax->state = GSMV1_STATE_GUESS_MULTILINE_RESPONSE;
 		break;
+	case G_AT_SYNTAX_EXPECT_SHORT_PROMPT:
+		syntax->state = GSMV1_STATE_PROMPT;
+		break;
 	default:
 		break;
 	};
@@ -195,6 +198,13 @@ static GAtSyntaxResult gsmv1_feed(GAtSyntax *syntax,
 				i += 1;
 				res = G_AT_SYNTAX_RESULT_PROMPT;
 				goto out;
+			} else if (byte == '\r')
+				break;
+			else if (byte == '\n') {
+				syntax->state = GSMV1_STATE_IDLE;
+				i += 1;
+				res = G_AT_SYNTAX_RESULT_PROMPT;
+				goto out;
 			}
 
 			syntax->state = GSMV1_STATE_RESPONSE;
@@ -239,6 +249,8 @@ static void gsm_permissive_hint(GAtSyntax *syntax, GAtSyntaxExpectHint hint)
 {
 	if (hint == G_AT_SYNTAX_EXPECT_PDU)
 		syntax->state = GSM_PERMISSIVE_STATE_GUESS_PDU;
+	else if (hint == G_AT_SYNTAX_EXPECT_SHORT_PROMPT)
+		syntax->state = GSM_PERMISSIVE_STATE_PROMPT;
 }
 
 static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
@@ -298,6 +310,13 @@ static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
 				i += 1;
 				res = G_AT_SYNTAX_RESULT_PROMPT;
 				goto out;
+			} else if (byte == '\r')
+				break;
+			else if (byte == '\n') {
+				syntax->state = GSM_PERMISSIVE_STATE_IDLE;
+				i += 1;
+				res = G_AT_SYNTAX_RESULT_PROMPT;
+				goto out;
 			}
 
 			syntax->state = GSM_PERMISSIVE_STATE_RESPONSE;
-- 
1.7.0.4

[PATCH 09/12] atmodem: add gnss driver

[Jarko Poutiainen]

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

---
 Makefile.am               |    3 +-
 drivers/atmodem/atmodem.c |    2 +
 drivers/atmodem/atmodem.h |    3 +
 drivers/atmodem/gnss.c    |  279 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 286 insertions(+), 1 deletions(-)
 create mode 100644 drivers/atmodem/gnss.c

diff --git a/Makefile.am b/Makefile.am
index 24742bb..3dae7f4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -180,7 +180,8 @@ builtin_sources += $(gatchat_sources) \
 				drivers/atmodem/atutil.c \
 				drivers/atmodem/gprs.c \
 				drivers/atmodem/gprs-context.c \
-				drivers/atmodem/sim-auth.c
+				drivers/atmodem/sim-auth.c \
+				drivers/atmodem/gnss.c
 
 builtin_modules += nwmodem
 builtin_sources += drivers/atmodem/atutil.h \
diff --git a/drivers/atmodem/atmodem.c b/drivers/atmodem/atmodem.c
index e140281..3093c23 100644
--- a/drivers/atmodem/atmodem.c
+++ b/drivers/atmodem/atmodem.c
@@ -52,6 +52,7 @@ static int atmodem_init(void)
 	at_gprs_init();
 	at_gprs_context_init();
 	at_sim_auth_init();
+	at_gnss_init();
 
 	return 0;
 }
@@ -76,6 +77,7 @@ static void atmodem_exit(void)
 	at_call_volume_exit();
 	at_gprs_exit();
 	at_gprs_context_exit();
+	at_gnss_exit();
 }
 
 OFONO_PLUGIN_DEFINE(atmodem, "AT modem driver", VERSION,
diff --git a/drivers/atmodem/atmodem.h b/drivers/atmodem/atmodem.h
index 1b7cf67..1a73b84 100644
--- a/drivers/atmodem/atmodem.h
+++ b/drivers/atmodem/atmodem.h
@@ -74,3 +74,6 @@ extern void at_gprs_context_exit(void);
 
 extern void at_sim_auth_init(void);
 extern void at_sim_auth_exit(void);
+
+extern void at_gnss_init(void);
+extern void at_gnss_exit(void);
diff --git a/drivers/atmodem/gnss.c b/drivers/atmodem/gnss.c
new file mode 100644
index 0000000..c1b1de3
--- /dev/null
+++ b/drivers/atmodem/gnss.c
@@ -0,0 +1,279 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2011 ST-Ericsson AB.
+ *
+ *  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 <glib.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/gnss.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "atmodem.h"
+#include "vendor.h"
+
+struct gnss_data {
+	GAtChat *chat;
+	unsigned int vendor;
+};
+
+static const char *none_prefix[] = { NULL };
+static const char *cpos_prefix[] = { "+CPOS:", NULL };
+static const char *cposr_prefix[] = { "+CPOSR:", NULL };
+
+static void gnsspr_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_gnss_position_report_cb_t cb = cbd->cb;
+
+	DBG("");
+
+	if (ok)
+		CALLBACK_WITH_SUCCESS(cb, cbd->data);
+	else
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+}
+
+static void at_gnss_position_reporting(struct ofono_gnss *gnss,
+					ofono_bool_t enable,
+					ofono_gnss_position_report_cb_t cb,
+					void *data)
+{
+	struct gnss_data *ad = ofono_gnss_get_data(gnss);
+	struct cb_data *cbd = cb_data_new(cb, data);
+
+	DBG("");
+
+	if (enable) {
+		g_at_chat_send(ad->chat, "AT+CPOSR=1",
+				cposr_prefix, gnsspr_cb, cbd, g_free);
+
+		if (ad->vendor == OFONO_VENDOR_STE)
+			g_at_chat_send(ad->chat, "AT*EPOSADRR=1",
+					NULL, NULL, NULL, NULL);
+	} else {
+		g_at_chat_send(ad->chat, "AT+CPOSR=0",
+				cposr_prefix, gnsspr_cb, cbd, g_free);
+
+		if (ad->vendor == OFONO_VENDOR_STE)
+			g_at_chat_send(ad->chat, "AT*EPOSADRR=0",
+					NULL, NULL, NULL, NULL);
+	}
+}
+
+static void gnssse_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_gnss_send_element_cb_t cb = cbd->cb;
+
+	DBG("");
+
+	if (ok)
+		CALLBACK_WITH_SUCCESS(cb, cbd->data);
+	else
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+}
+
+static void at_gnss_send_element(struct ofono_gnss *gnss,
+				const char *xml,
+				ofono_gnss_send_element_cb_t cb, void *data)
+{
+	struct gnss_data *ad = ofono_gnss_get_data(gnss);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	char *buf = g_try_new(char, strlen(xml) + 10);
+	int len;
+
+	DBG("");
+
+	if (buf == NULL)
+		goto error;
+
+	len = sprintf(buf, "AT+CPOS\r");
+	len += sprintf(buf + len, "%s", xml);
+
+	if (g_at_chat_send_and_expect_short_prompt(ad->chat, buf, cpos_prefix,
+							gnssse_cb, cbd,
+							g_free) > 0) {
+		g_free(buf);
+		return;
+	}
+error:
+	g_free(buf);
+	g_free(cbd);
+
+	CALLBACK_WITH_FAILURE(cb, data);
+}
+
+static gboolean gnss_parse_report(GAtResult *result, const char *prefix,
+					const char **xml)
+{
+	GAtResultIter iter;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, prefix))
+		return FALSE;
+
+	if (!g_at_result_iter_next_unquoted_string(&iter, xml))
+		return FALSE;
+
+	return TRUE;
+}
+
+static void gnss_report(GAtResult *result, gpointer user_data)
+{
+	struct ofono_gnss *gnss = user_data;
+	const char *xml;
+
+	DBG("");
+
+	xml = NULL;
+	if (!gnss_parse_report(result, "+CPOSR:", &xml)) {
+		ofono_error("Unable to parse CPOSR notification");
+		return;
+	}
+
+	if (xml == NULL) {
+		ofono_error("Unable to parse CPOSR notification");
+		return;
+	}
+
+	ofono_gnss_receive_request(gnss, xml);
+}
+
+static void at_gnssreset_cb(GAtResult *result, gpointer user_data)
+{
+	struct ofono_gnss *gnss = user_data;
+
+	DBG("");
+
+	ofono_gnss_receive_reset(gnss);
+}
+
+static void at_gnss_not_supported(struct ofono_gnss *gnss)
+{
+	ofono_error("gnss not supported by this modem.");
+
+	ofono_gnss_remove(gnss);
+}
+
+static void at_gnsscposr_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_gnss *gnss = user_data;
+	struct gnss_data *ad = ofono_gnss_get_data(gnss);
+
+	DBG("");
+
+	if (!ok) {
+		at_gnss_not_supported(gnss);
+		return;
+	}
+
+	g_at_chat_register(ad->chat, "+CPOSR:", gnss_report,
+				FALSE, gnss, NULL);
+
+	if (ad->vendor == OFONO_VENDOR_STE)
+		g_at_chat_register(ad->chat, "*EPOSADRR:", at_gnssreset_cb,
+					FALSE, gnss, NULL);
+
+	ofono_gnss_register(gnss);
+}
+
+static void at_gnsscpos_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_gnss *gnss = user_data;
+	struct gnss_data *ad = ofono_gnss_get_data(gnss);
+
+	DBG("");
+
+	if (!ok) {
+		at_gnss_not_supported(gnss);
+		return;
+	}
+
+	g_at_chat_send(ad->chat, "AT+CPOSR=?",
+			none_prefix, at_gnsscposr_cb, gnss, NULL);
+}
+
+static int at_gnss_probe(struct ofono_gnss *gnss, unsigned int vendor,
+				void *user)
+{
+	GAtChat *chat = user;
+	struct gnss_data *gd;
+
+	DBG("");
+
+	gd = g_try_new0(struct gnss_data, 1);
+	if (gd == NULL)
+		return -ENOMEM;
+
+	gd->chat = g_at_chat_clone(chat);
+	gd->vendor = vendor;
+
+	ofono_gnss_set_data(gnss, gd);
+
+	g_at_chat_send(gd->chat, "AT+CPOS=?",
+			none_prefix, at_gnsscpos_cb, gnss, NULL);
+
+	return 0;
+}
+
+static void at_gnss_remove(struct ofono_gnss *gnss)
+{
+	struct gnss_data *gd = ofono_gnss_get_data(gnss);
+
+	DBG("");
+
+	ofono_gnss_set_data(gnss, NULL);
+
+	g_at_chat_unref(gd->chat);
+	g_free(gd);
+}
+
+static struct ofono_gnss_driver driver = {
+	.name			= "atmodem",
+	.probe			= at_gnss_probe,
+	.remove			= at_gnss_remove,
+	.send_element		= at_gnss_send_element,
+	.position_reporting	= at_gnss_position_reporting,
+};
+
+void at_gnss_init(void)
+{
+	ofono_gnss_driver_register(&driver);
+}
+
+void at_gnss_exit(void)
+{
+	ofono_gnss_driver_unregister(&driver);
+}
-- 
1.7.0.4

[PATCH 10/12] ste: add support for gnss

[Jarko Poutiainen]

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

---
 plugins/ste.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/plugins/ste.c b/plugins/ste.c
index efae573..749f673 100644
--- a/plugins/ste.c
+++ b/plugins/ste.c
@@ -58,6 +58,7 @@
 #include <ofono/gprs-context.h>
 #include <ofono/radio-settings.h>
 #include <ofono/stk.h>
+#include <ofono/gnss.h>
 
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>
@@ -65,17 +66,18 @@
 #include <drivers/stemodem/caif_socket.h>
 #include <drivers/stemodem/if_caif.h>
 
-#define NUM_CHAT	5
+#define NUM_CHAT	6
 #define AT_DEFAULT	0
 #define AT_NET		1
 #define AT_VOICE	2
 #define AT_GPRS	3
 #define AT_SIM		4
+#define AT_GNSS	5
 
 #define MAX_PDP_CONTEXTS	4
 
 static char *chat_prefixes[NUM_CHAT] = { "Default: ", "Net: ", "Voice: ",
-					"GPRS: ", "SIM: " };
+					 "GPRS: ", "SIM: ", "GNSS:" };
 
 struct ste_data {
 	GAtChat *chat[NUM_CHAT];
@@ -473,6 +475,8 @@ static void ste_post_online(struct ofono_modem *modem)
 	ofono_ssn_create(modem, 0, "atmodem", data->chat[AT_DEFAULT]);
 	ofono_call_volume_create(modem, 0, "atmodem", data->chat[AT_DEFAULT]);
 	ofono_cbs_create(modem, 0, "atmodem", data->chat[AT_DEFAULT]);
+	ofono_gnss_create(modem, OFONO_VENDOR_STE, "atmodem",
+				data->chat[AT_GNSS]);
 
 	gprs = ofono_gprs_create(modem, OFONO_VENDOR_MBM,
 					"atmodem", data->chat[AT_GPRS]);
-- 
1.7.0.4

[PATCH 11/12] ofono.conf: add positioning agent interface

[Jarko Poutiainen]

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

---
 src/ofono.conf |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/ofono.conf b/src/ofono.conf
index 0dfa038..8a83cd0 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.PositioningRequestAgent"/>
   </policy>
 
   <policy at_console="true">
-- 
1.7.0.4

[PATCH 12/12] test: add test-gnss

[Jarko Poutiainen]

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

---
 Makefile.am    |    3 +-
 test/test-gnss |   91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 1 deletions(-)
 create mode 100755 test/test-gnss

diff --git a/Makefile.am b/Makefile.am
index 3dae7f4..daa5dd0 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -503,7 +503,8 @@ test_scripts = test/backtrace \
 		test/cdma-hangup \
 		test/disable-call-forwarding \
 		test/list-messages \
-		test/test-sms
+		test/test-sms \
+		test/test-gnss
 
 if TEST
 testdir = $(pkglibdir)/test
diff --git a/test/test-gnss b/test/test-gnss
new file mode 100755
index 0000000..d7d9027
--- /dev/null
+++ b/test/test-gnss
@@ -0,0 +1,91 @@
+#!/usr/bin/python
+
+import gobject
+import sys
+import os
+
+import dbus
+import dbus.service
+import dbus.mainloop.glib
+
+class PositioningAgent(dbus.service.Object):
+	@dbus.service.method("org.ofono.PositioningRequestAgent",
+					in_signature="", out_signature="")
+	def Release(self):
+		print "Release"
+		mainloop.quit()
+
+	@dbus.service.method("org.ofono.PositioningRequestAgent",
+					in_signature="s", out_signature="")
+	def Request(self, xml):
+		print "positioning data: %s" % (xml)
+
+	@dbus.service.method("org.ofono.PositioningRequestAgent",
+					in_signature="", out_signature="")
+	def ResetAssistanceData(self):
+		print "Reset Assistance Data request received"
+
+def print_menu():
+	print "Select test case"
+	print "-----------------------------------------------------------"
+	print "[0] SendPositioningElement"
+	print "[1] RegisterPositioningRequestAgent"
+	print "[2] UnregisterPositioningRequestAgent"
+	print "[x] Exit"
+	print "-----------------------------------------------------------"
+
+def stdin_handler(fd, condition, positioning, path):
+
+	in_key = os.read(fd.fileno(), 8).rstrip()
+	if in_key == '0':
+		xml = raw_input('type the element and press enter: ')
+		try:
+			positioning.SendPositioningElement(dbus.String(xml))
+			print "ok"
+		except dbus.DBusException, e:
+			print "Unable to send positioning element"
+
+	elif in_key == '1':
+		try:
+			positioning.RegisterPositioningRequestAgent("/test/posagent")
+			print "ok"
+		except dbus.DBusException, e:
+			print "Unable to register positioning agent"
+
+	elif in_key == '2':
+		try:
+			positioning.UnregisterPositioningRequestAgent(path)
+			print "ok"
+		except dbus.DBusException, e:
+			print "Unable to unregister positioning agent"
+	elif in_key == 'x':
+		sys.exit(1)
+
+	return True
+
+if __name__ == "__main__":
+
+	if len(sys.argv) < 1:
+		sys.exit(1)
+
+	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.AssistedSatelliteNavigation" not in properties["Interfaces"]:
+			continue
+
+		positioning = dbus.Interface(bus.get_object('org.ofono', path),
+					'org.ofono.AssistedSatelliteNavigation')
+
+	path = "/test/posagent"
+	agent = PositioningAgent(bus, path)
+
+	print_menu()
+
+	gobject.io_add_watch(sys.stdin, gobject.IO_IN, stdin_handler, positioning, path)
+	mainloop = gobject.MainLoop()
+	mainloop.run()
-- 
1.7.0.4

Re: [PATCH 01/12] dbus: add gnss interface definition

[Denis Kenzior]

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

Hi Jarko,

On 03/11/2011 06:22 AM, Jarko Poutiainen wrote:
> ---
>  include/dbus.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/dbus.h b/include/dbus.h
> index 19a138a..bf3e8a4 100644
> --- a/include/dbus.h
> +++ b/include/dbus.h
> @@ -56,6 +56,8 @@ extern "C" {
>  #define OFONO_STK_INTERFACE OFONO_SERVICE ".SimToolkit"
>  #define OFONO_SIM_APP_INTERFACE OFONO_SERVICE ".SimToolkitAgent"
>  #define OFONO_LOCATION_REPORTING_INTERFACE OFONO_SERVICE ".LocationReporting"
> +#define OFONO_AS_NAVIGATION_INTERFACE "org.ofono.AssistedSatelliteNavigation"

Please name this OFONO_GNSS_INTERFACE

> +#define OFONO_POSITIONING_REQUEST_INTERFACE "org.ofono.PositioningRequestAgent"

Please name this OFONO_GNSS_POSR_INTERFACE or
OFONO_GNSS_POSR_AGENT_INTERFACE

Or feel free to suggest a better name, but the current names are getting
too big

>  
>  /* CDMA Interfaces */
>  #define OFONO_CDMA_VOICECALL_MANAGER_INTERFACE "org.ofono.cdma.VoiceCallManager"

Regards,
-Denis

Re: [PATCH 02/12] include: add gnss.h file

[Denis Kenzior]

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

Hi Jarko,

On 03/11/2011 06:22 AM, Jarko Poutiainen wrote:
> ---
>  Makefile.am    |    2 +-
>  include/gnss.h |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 77 insertions(+), 1 deletions(-)
>  create mode 100644 include/gnss.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index 3f20717..7d6acce 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -15,7 +15,7 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
>  			include/ctm.h include/cdma-voicecall.h \
>  			include/cdma-sms.h include/sim-auth.h \
>  			include/gprs-provision.h include/emulator.h \
> -			include/location-reporting.h
> +			include/location-reporting.h include/gnss.h
>  
>  nodist_pkginclude_HEADERS = include/version.h
>  
> diff --git a/include/gnss.h b/include/gnss.h
> new file mode 100644
> index 0000000..d10ab11
> --- /dev/null
> +++ b/include/gnss.h
> @@ -0,0 +1,76 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2011 ST-Ericsson AB.
> + *
> + *  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_GNSS_H
> +#define __OFONO_GNSS_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <ofono/types.h>
> +
> +struct ofono_gnss;
> +
> +typedef void (*ofono_gnss_position_report_cb_t)(const struct ofono_error *error,
> +							void *data);
> +
> +typedef void (*ofono_gnss_send_element_cb_t)(const struct ofono_error *error,
> +						void *data);
> +
> +typedef void (*ofono_gnss_report_reset_cb_t)(const struct ofono_error *error,
> +						void *data);

Please just combine these into a single callback, they have the same
signature.  E.g. something like ofono_gnss_cb_t

> +
> +struct ofono_gnss_driver {
> +	const char *name;
> +	int (*probe)(struct ofono_gnss *gnss, unsigned int vendor, void *data);
> +	void (*remove)(struct ofono_gnss *gnss);
> +	void (*send_element)(struct ofono_gnss *gnss,
> +				const char *xml,
> +				ofono_gnss_send_element_cb_t cb, void *data);
> +	void (*position_reporting)(struct ofono_gnss *gnss,
> +					ofono_bool_t enable,
> +					ofono_gnss_position_report_cb_t cb,
> +					void *data);

Please name this set_position_reporting or set_posr

> +};
> +
> +void ofono_gnss_receive_request(struct ofono_gnss *gnss, const char *xml);
> +void ofono_gnss_receive_reset(struct ofono_gnss *gnss);

Please use notify instead of receive.  E.g. ofono_gnss_notify_posr_reset
and ofono_gnss_notify_posr_request

> +int ofono_gnss_driver_register(const struct ofono_gnss_driver *d);
> +void ofono_gnss_driver_unregister(const struct ofono_gnss_driver *d);
> +
> +struct ofono_gnss *ofono_gnss_create(struct ofono_modem *modem,
> +					unsigned int vendor,
> +					const char *driver, void *data);
> +
> +void ofono_gnss_register(struct ofono_gnss *gnss);
> +void ofono_gnss_remove(struct ofono_gnss *gnss);
> +
> +void ofono_gnss_set_data(struct ofono_gnss *gnss, void *data);
> +void *ofono_gnss_get_data(struct ofono_gnss *gnss);
> +
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __OFONO_GNSS_H */

Regards,
-Denis

Re: [PATCH 04/12] src: add gnss atom and agent implementation

[Denis Kenzior]

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

Hi Jarko,

On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> ---
>  Makefile.am     |    3 +-
>  src/gnss.c      |  357 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/gnssagent.c |  152 +++++++++++++++++++++++
>  src/gnssagent.h |   40 ++++++
>  4 files changed, 551 insertions(+), 1 deletions(-)
>  create mode 100644 src/gnss.c
>  create mode 100644 src/gnssagent.c
>  create mode 100644 src/gnssagent.h
> 
> diff --git a/Makefile.am b/Makefile.am
> index 7d6acce..24742bb 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -383,7 +383,8 @@ 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/location-reporting.c
> +			src/emulator.c src/location-reporting.c \
> +			src/gnss.c src/gnssagent.c src/gnssagent.h
>  
>  src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
>  
> diff --git a/src/gnss.c b/src/gnss.c
> new file mode 100644
> index 0000000..02f9057
> --- /dev/null
> +++ b/src/gnss.c
> @@ -0,0 +1,357 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2011 ST-Ericsson AB.
> + *
> + *  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 <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +
> +#include <glib.h>
> +#include <gdbus.h>
> +#include <errno.h>
> +
> +#include "ofono.h"
> +
> +#include "common.h"
> +#include "gnssagent.h"
> +
> +static GSList *g_drivers = NULL;
> +
> +struct ofono_gnss {
> +	const struct ofono_gnss_driver *driver;
> +	void *driver_data;
> +	struct ofono_atom *atom;
> +	DBusMessage *pending;
> +	struct gnss_agent *default_agent;

Why is this called default_agent? posr_agent might be a better name

> +};
> +
> +static void gnss_register_agent_cb(const struct ofono_error *error,
> +					void *data)
> +{
> +	DBusMessage *reply;
> +	struct ofono_gnss *gnss = data;
> +
> +	DBG("");
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		ofono_error("Enabling Location Reporting Failed");
> +		reply = __ofono_error_failed(gnss->pending);
> +		gnss_agent_free(gnss->default_agent);
> +		__ofono_dbus_pending_reply(&gnss->pending, reply);
> +		return;
> +	}
> +
> +	reply = dbus_message_new_method_return(gnss->pending);
> +	__ofono_dbus_pending_reply(&gnss->pending, reply);
> +}
> +
> +static void default_agent_notify(gpointer user_data)

Why is this called default_agent_notify?  Can you have multiple agents?
If not, then agent_notify is sufficient.

> +{
> +	struct ofono_gnss *gnss = user_data;
> +
> +	gnss->default_agent = NULL;

Since you're enabling CPOSR when an agent is registered, you have to
also guard against the case of the application exiting.  Note that the
agent can even exit while your CPOSR enable is in progress.  Have a peek
at location-reporting.c for ideas on how Lucas solved this.

> +}
> +
> +static DBusMessage *gnss_register_agent(DBusConnection *conn,
> +					DBusMessage *msg, void *data)
> +{
> +	struct ofono_gnss *gnss = data;
> +	const char *agent_path;
> +
> +	if (gnss->pending)
> +		return __ofono_error_busy(msg);
> +
> +	if (gnss->default_agent)
> +		return __ofono_error_busy(msg);
> +
> +	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH,
> +				  &agent_path, DBUS_TYPE_INVALID) == FALSE)

Please don't mix spaces and tabs for indentation, always use tabs

> +		return __ofono_error_invalid_args(msg);
> +
> +	if (!__ofono_dbus_valid_object_path(agent_path))
> +		return __ofono_error_invalid_format(msg);
> +
> +	gnss->driver->position_reporting(gnss, 1, gnss_register_agent_cb, gnss);

Please use TRUE instead of 1 here

> +
> +	gnss->default_agent = gnss_agent_new(agent_path,
> +					dbus_message_get_sender(msg),
> +					FALSE);
> +
> +	if (gnss->default_agent == NULL)
> +		return __ofono_error_failed(msg);
> +
> +	gnss_agent_set_removed_notify(gnss->default_agent,
> +					default_agent_notify, gnss);
> +
> +	gnss->pending = dbus_message_ref(msg);
> +
> +	return NULL;
> +}
> +
> +static void gnss_unregister_agent_cb(const struct ofono_error *error,
> +					void *data)
> +{
> +	DBusMessage *reply;
> +	struct ofono_gnss *gnss = data;
> +
> +	DBG("");
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		ofono_error("Disabling Location Reporting Failed");
> +		reply = __ofono_error_failed(gnss->pending);
> +		goto out;
> +	}
> +
> +	gnss_agent_free(gnss->default_agent);
> +
> +	if (gnss->default_agent) {
> +		ofono_error("Releasing agent failed");
> +		reply = __ofono_error_failed(gnss->pending);
> +		goto out;
> +	}
> +
> +	reply = dbus_message_new_method_return(gnss->pending);
> +
> +out:
> +	__ofono_dbus_pending_reply(&gnss->pending, reply);
> +}
> +
> +static DBusMessage *gnss_unregister_agent(DBusConnection *conn,
> +						DBusMessage *msg, void *data)
> +{
> +	struct ofono_gnss *gnss = data;
> +	const char *agent_path;
> +	const char *agent_bus = dbus_message_get_sender(msg);
> +
> +	if (gnss->pending)
> +		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 (gnss->default_agent == NULL)
> +		return __ofono_error_failed(msg);
> +
> +	if (!gnss_agent_matches(gnss->default_agent, agent_path, agent_bus))
> +		return __ofono_error_failed(msg);
> +
> +	gnss->pending = dbus_message_ref(msg);
> +
> +	gnss->driver->position_reporting(gnss, 0, gnss_unregister_agent_cb,

Please use FALSE here

> +						gnss);
> +
> +	return NULL;
> +}
> +
> +static void gnss_send_element_cb(const struct ofono_error *error,
> +				void *data)
> +{
> +	DBusMessage *reply;
> +	struct ofono_gnss *gnss = data;
> +
> +	DBG("");
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		ofono_error("Sending Positioning Element failed");
> +
> +		reply = __ofono_error_failed(gnss->pending);
> +
> +		goto out;
> +	}
> +
> +	reply = dbus_message_new_method_return(gnss->pending);
> +
> +out:
> +	__ofono_dbus_pending_reply(&gnss->pending, reply);
> +}
> +
> +static DBusMessage *gnss_send_element(DBusConnection *conn,
> +					DBusMessage *msg, void *data)
> +{
> +	struct ofono_gnss *gnss = data;
> +	const char *xml;
> +
> +	DBG("");
> +
> +	if (gnss->pending)
> +		return __ofono_error_busy(msg);
> +
> +	if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &xml,
> +					DBUS_TYPE_INVALID))
> +		return __ofono_error_invalid_args(msg);
> +
> +	gnss->pending = dbus_message_ref(msg);
> +

Please make sure that only the application which has registered the
agent can use this method and return AccessDenied otherwise.

> +	gnss->driver->send_element(gnss, xml, gnss_send_element_cb, gnss);
> +
> +	return NULL;
> +}
> +
> +static GDBusMethodTable gnss_methods[] = {
> +	{ "SendPositioningElement",		"s",	"",
> +				gnss_send_element, G_DBUS_METHOD_FLAG_ASYNC },
> +	{ "RegisterPositioningRequestAgent",	"o",	"",
> +				gnss_register_agent,
> +				G_DBUS_METHOD_FLAG_ASYNC },
> +	{ "UnregisterPositioningRequestAgent",	"o",	"",
> +				gnss_unregister_agent,
> +				G_DBUS_METHOD_FLAG_ASYNC },
> +	{ }
> +};
> +
> +static void gnss_unregister(struct ofono_atom *atom)
> +{
> +	struct ofono_gnss *gnss = __ofono_atom_get_data(atom);
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
> +	const char *path = __ofono_atom_get_path(atom);
> +
> +	if (gnss->default_agent)
> +		gnss_agent_free(gnss->default_agent);
> +
> +	ofono_modem_remove_interface(modem, OFONO_AS_NAVIGATION_INTERFACE);
> +	g_dbus_unregister_interface(conn, path, OFONO_AS_NAVIGATION_INTERFACE);
> +}
> +
> +static void gnss_remove(struct ofono_atom *atom)
> +{
> +	struct ofono_gnss *gnss = __ofono_atom_get_data(atom);
> +
> +	DBG("atom: %p", atom);
> +
> +	if (gnss == NULL)
> +		return;
> +
> +	if (gnss->driver && gnss->driver->remove)
> +		gnss->driver->remove(gnss);
> +
> +	g_free(gnss);
> +}
> +
> +void ofono_gnss_register(struct ofono_gnss *gnss)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	struct ofono_modem *modem = __ofono_atom_get_modem(gnss->atom);
> +	const char *path = __ofono_atom_get_path(gnss->atom);
> +
> +	if (!g_dbus_register_interface(conn, path,
> +					OFONO_AS_NAVIGATION_INTERFACE,
> +					gnss_methods, NULL, NULL,
> +					gnss, NULL)) {
> +		ofono_error("Could not create %s interface",
> +				OFONO_AS_NAVIGATION_INTERFACE);
> +
> +		return;
> +	}
> +
> +	ofono_modem_add_interface(modem, OFONO_AS_NAVIGATION_INTERFACE);
> +
> +	__ofono_atom_register(gnss->atom, gnss_unregister);
> +}
> +
> +int ofono_gnss_driver_register(const struct ofono_gnss_driver *d)
> +{
> +	DBG("driver: %p, name: %s", d, d->name);
> +
> +	if (d->probe == NULL)
> +		return -EINVAL;
> +
> +	g_drivers = g_slist_prepend(g_drivers, (void *) d);
> +
> +	return 0;
> +}
> +
> +void ofono_gnss_driver_unregister(const struct ofono_gnss_driver *d)
> +{
> +	DBG("driver: %p, name: %s", d, d->name);
> +
> +	g_drivers = g_slist_remove(g_drivers, (void *) d);
> +}
> +
> +struct ofono_gnss *ofono_gnss_create(struct ofono_modem *modem,
> +					unsigned int vendor,
> +					const char *driver,
> +					void *data)
> +{
> +	struct ofono_gnss *gnss;
> +	GSList *l;
> +
> +	if (driver == NULL)
> +		return NULL;
> +
> +	gnss = g_try_new0(struct ofono_gnss, 1);
> +
> +	if (gnss == NULL)
> +		return NULL;
> +
> +	gnss->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_GNSS,
> +						gnss_remove, gnss);
> +
> +	for (l = g_drivers; l; l = l->next) {
> +		const struct ofono_gnss_driver *drv = l->data;
> +
> +		if (g_strcmp0(drv->name, driver))
> +			continue;
> +
> +		if (drv->probe(gnss, vendor, data) < 0)
> +			continue;
> +
> +		gnss->driver = drv;
> +		break;
> +	}
> +
> +	return gnss;
> +}
> +
> +void ofono_gnss_receive_request(struct ofono_gnss *gnss, const char *xml)
> +{
> +	if (gnss->default_agent)
> +		gnss_agent_receive_request(gnss->default_agent, xml);
> +}
> +
> +void ofono_gnss_receive_reset(struct ofono_gnss *gnss)
> +{
> +	if (gnss->default_agent)
> +		gnss_agent_receive_reset(gnss->default_agent);
> +}
> +
> +void ofono_gnss_remove(struct ofono_gnss *gnss)
> +{
> +	__ofono_atom_free(gnss->atom);
> +}
> +
> +void ofono_gnss_set_data(struct ofono_gnss *gnss, void *data)
> +{
> +	gnss->driver_data = data;
> +}
> +
> +void *ofono_gnss_get_data(struct ofono_gnss *gnss)
> +{
> +	return gnss->driver_data;
> +}

<snip>

Regards,
-Denis

Re: [PATCH 07/12] gatchat: implementation for +CPOS send

[Denis Kenzior]

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

Hi Jarko,

On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> ---
>  gatchat/gatchat.c |   13 +++++++++++++
>  1 files changed, 13 insertions(+), 0 deletions(-)
> 
> diff --git a/gatchat/gatchat.c b/gatchat/gatchat.c
> index 3fd564d..64f131d 100644
> --- a/gatchat/gatchat.c
> +++ b/gatchat/gatchat.c
> @@ -1468,6 +1468,19 @@ guint g_at_chat_send_pdu_listing(GAtChat *chat, const char *cmd,
>  					listing, func, user_data, notify);
>  }
>  
> +guint g_at_chat_send_and_expect_short_prompt(GAtChat *chat, const char *cmd,
> +			const char **prefix_list, GAtResultFunc func,
> +			gpointer user_data, GDestroyNotify notify)
> +{
> +	if(chat != NULL)
> +		chat->parent->syntax->set_hint(chat->parent->syntax,
> +					G_AT_SYNTAX_EXPECT_SHORT_PROMPT);

So unfortunately you can't do this.  GAtChat is a command queue and we
might be in the process of sending / receiving responses for multiple
commands that are ahead of this one.  So you can't really manipulate the
syntax directly here.  You have to do this only once the CPOSR has been
sent on the wire.

> +
> +	return at_chat_send_common(chat->parent, chat->group,
> +					cmd, prefix_list, FALSE, NULL,
> +					func, user_data, notify);
> +}
> +
>  gboolean g_at_chat_cancel(GAtChat *chat, guint id)
>  {
>  	/* We use id 0 for wakeup commands */

Regards,
-Denis

Re: [PATCH 08/12] gatchat: fix gatsyntax to support +CPOS

[Denis Kenzior]

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

Hi Jarko,

On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> ---
>  gatchat/gatsyntax.c |   19 +++++++++++++++++++
>  1 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/gatchat/gatsyntax.c b/gatchat/gatsyntax.c
> index 2fc70b8..f1d896b 100644
> --- a/gatchat/gatsyntax.c
> +++ b/gatchat/gatsyntax.c
> @@ -64,6 +64,9 @@ static void gsmv1_hint(GAtSyntax *syntax, GAtSyntaxExpectHint hint)
>  	case G_AT_SYNTAX_EXPECT_MULTILINE:
>  		syntax->state = GSMV1_STATE_GUESS_MULTILINE_RESPONSE;
>  		break;
> +	case G_AT_SYNTAX_EXPECT_SHORT_PROMPT:
> +		syntax->state = GSMV1_STATE_PROMPT;
> +		break;
>  	default:
>  		break;
>  	};
> @@ -195,6 +198,13 @@ static GAtSyntaxResult gsmv1_feed(GAtSyntax *syntax,
>  				i += 1;
>  				res = G_AT_SYNTAX_RESULT_PROMPT;
>  				goto out;
> +			} else if (byte == '\r')
> +				break;
> +			else if (byte == '\n') {
> +				syntax->state = GSMV1_STATE_IDLE;
> +				i += 1;
> +				res = G_AT_SYNTAX_RESULT_PROMPT;
> +				goto out;

Please just create a dedicated state for this one and don't hack the
PROMPT state.

>  			}
>  
>  			syntax->state = GSMV1_STATE_RESPONSE;
> @@ -239,6 +249,8 @@ static void gsm_permissive_hint(GAtSyntax *syntax, GAtSyntaxExpectHint hint)
>  {
>  	if (hint == G_AT_SYNTAX_EXPECT_PDU)
>  		syntax->state = GSM_PERMISSIVE_STATE_GUESS_PDU;
> +	else if (hint == G_AT_SYNTAX_EXPECT_SHORT_PROMPT)
> +		syntax->state = GSM_PERMISSIVE_STATE_PROMPT;
>  }
>  
>  static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
> @@ -298,6 +310,13 @@ static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
>  				i += 1;
>  				res = G_AT_SYNTAX_RESULT_PROMPT;
>  				goto out;
> +			} else if (byte == '\r')
> +				break;
> +			else if (byte == '\n') {
> +				syntax->state = GSM_PERMISSIVE_STATE_IDLE;
> +				i += 1;
> +				res = G_AT_SYNTAX_RESULT_PROMPT;
> +				goto out;

Same comment here, and man you better pray that your modem doesn't
insert unsolicited notifications between you sending a CPOSR and you
receiving a prompt ;)

>  			}
>  
>  			syntax->state = GSM_PERMISSIVE_STATE_RESPONSE;

Regards,
-Denis

Re: [PATCH 09/12] atmodem: add gnss driver

[Denis Kenzior]

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

On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> ---
>  Makefile.am               |    3 +-
>  drivers/atmodem/atmodem.c |    2 +
>  drivers/atmodem/atmodem.h |    3 +
>  drivers/atmodem/gnss.c    |  279 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 286 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/atmodem/gnss.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 24742bb..3dae7f4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -180,7 +180,8 @@ builtin_sources += $(gatchat_sources) \
>  				drivers/atmodem/atutil.c \
>  				drivers/atmodem/gprs.c \
>  				drivers/atmodem/gprs-context.c \
> -				drivers/atmodem/sim-auth.c
> +				drivers/atmodem/sim-auth.c \
> +				drivers/atmodem/gnss.c
>  
>  builtin_modules += nwmodem
>  builtin_sources += drivers/atmodem/atutil.h \
> diff --git a/drivers/atmodem/atmodem.c b/drivers/atmodem/atmodem.c
> index e140281..3093c23 100644
> --- a/drivers/atmodem/atmodem.c
> +++ b/drivers/atmodem/atmodem.c
> @@ -52,6 +52,7 @@ static int atmodem_init(void)
>  	at_gprs_init();
>  	at_gprs_context_init();
>  	at_sim_auth_init();
> +	at_gnss_init();
>  
>  	return 0;
>  }
> @@ -76,6 +77,7 @@ static void atmodem_exit(void)
>  	at_call_volume_exit();
>  	at_gprs_exit();
>  	at_gprs_context_exit();
> +	at_gnss_exit();
>  }
>  
>  OFONO_PLUGIN_DEFINE(atmodem, "AT modem driver", VERSION,
> diff --git a/drivers/atmodem/atmodem.h b/drivers/atmodem/atmodem.h
> index 1b7cf67..1a73b84 100644
> --- a/drivers/atmodem/atmodem.h
> +++ b/drivers/atmodem/atmodem.h
> @@ -74,3 +74,6 @@ extern void at_gprs_context_exit(void);
>  
>  extern void at_sim_auth_init(void);
>  extern void at_sim_auth_exit(void);
> +
> +extern void at_gnss_init(void);
> +extern void at_gnss_exit(void);
> diff --git a/drivers/atmodem/gnss.c b/drivers/atmodem/gnss.c
> new file mode 100644
> index 0000000..c1b1de3
> --- /dev/null
> +++ b/drivers/atmodem/gnss.c
> @@ -0,0 +1,279 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2011 ST-Ericsson AB.
> + *
> + *  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 <glib.h>
> +
> +#include <ofono/log.h>
> +#include <ofono/modem.h>
> +#include <ofono/gnss.h>
> +
> +#include "gatchat.h"
> +#include "gatresult.h"
> +
> +#include "atmodem.h"
> +#include "vendor.h"
> +
> +struct gnss_data {
> +	GAtChat *chat;
> +	unsigned int vendor;
> +};
> +
> +static const char *none_prefix[] = { NULL };
> +static const char *cpos_prefix[] = { "+CPOS:", NULL };
> +static const char *cposr_prefix[] = { "+CPOSR:", NULL };
> +
> +static void gnsspr_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_gnss_position_report_cb_t cb = cbd->cb;
> +
> +	DBG("");
> +
> +	if (ok)
> +		CALLBACK_WITH_SUCCESS(cb, cbd->data);
> +	else
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);

As a general habit, please use at_util_decode_error instead of using
these macros.  This leads to simpler code as well

> +}
> +
> +static void at_gnss_position_reporting(struct ofono_gnss *gnss,
> +					ofono_bool_t enable,
> +					ofono_gnss_position_report_cb_t cb,
> +					void *data)
> +{
> +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> +	struct cb_data *cbd = cb_data_new(cb, data);
> +
> +	DBG("");
> +
> +	if (enable) {
> +		g_at_chat_send(ad->chat, "AT+CPOSR=1",
> +				cposr_prefix, gnsspr_cb, cbd, g_free);
> +
> +		if (ad->vendor == OFONO_VENDOR_STE)
> +			g_at_chat_send(ad->chat, "AT*EPOSADRR=1",
> +					NULL, NULL, NULL, NULL);
> +	} else {
> +		g_at_chat_send(ad->chat, "AT+CPOSR=0",
> +				cposr_prefix, gnsspr_cb, cbd, g_free);
> +
> +		if (ad->vendor == OFONO_VENDOR_STE)
> +			g_at_chat_send(ad->chat, "AT*EPOSADRR=0",
> +					NULL, NULL, NULL, NULL);
> +	}
> +}
> +
> +static void gnssse_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_gnss_send_element_cb_t cb = cbd->cb;
> +
> +	DBG("");
> +
> +	if (ok)
> +		CALLBACK_WITH_SUCCESS(cb, cbd->data);
> +	else
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);

Again, please use at_util_decode_error here

> +}
> +
> +static void at_gnss_send_element(struct ofono_gnss *gnss,
> +				const char *xml,
> +				ofono_gnss_send_element_cb_t cb, void *data)
> +{
> +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> +	struct cb_data *cbd = cb_data_new(cb, data);
> +	char *buf = g_try_new(char, strlen(xml) + 10);
> +	int len;
> +
> +	DBG("");
> +
> +	if (buf == NULL)
> +		goto error;
> +
> +	len = sprintf(buf, "AT+CPOS\r");
> +	len += sprintf(buf + len, "%s", xml);
> +
> +	if (g_at_chat_send_and_expect_short_prompt(ad->chat, buf, cpos_prefix,
> +							gnssse_cb, cbd,
> +							g_free) > 0) {
> +		g_free(buf);
> +		return;
> +	}

Please add an empty line here

> +error:
> +	g_free(buf);
> +	g_free(cbd);
> +
> +	CALLBACK_WITH_FAILURE(cb, data);
> +}
> +
> +static gboolean gnss_parse_report(GAtResult *result, const char *prefix,
> +					const char **xml)
> +{
> +	GAtResultIter iter;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, prefix))
> +		return FALSE;
> +
> +	if (!g_at_result_iter_next_unquoted_string(&iter, xml))
> +		return FALSE;
> +
> +	return TRUE;

Do you really need a separate function?  This might belong in gnss_report

> +}
> +
> +static void gnss_report(GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_gnss *gnss = user_data;
> +	const char *xml;
> +
> +	DBG("");
> +
> +	xml = NULL;
> +	if (!gnss_parse_report(result, "+CPOSR:", &xml)) {
> +		ofono_error("Unable to parse CPOSR notification");
> +		return;
> +	}
> +
> +	if (xml == NULL) {
> +		ofono_error("Unable to parse CPOSR notification");
> +		return;
> +	}
> +
> +	ofono_gnss_receive_request(gnss, xml);
> +}
> +
> +static void at_gnssreset_cb(GAtResult *result, gpointer user_data)

Since this is a notification, please call this gnssreset_notify

> +{
> +	struct ofono_gnss *gnss = user_data;
> +
> +	DBG("");
> +
> +	ofono_gnss_receive_reset(gnss);
> +}
> +
> +static void at_gnss_not_supported(struct ofono_gnss *gnss)
> +{
> +	ofono_error("gnss not supported by this modem.");
> +
> +	ofono_gnss_remove(gnss);
> +}
> +
> +static void at_gnsscposr_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_gnss *gnss = user_data;
> +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> +
> +	DBG("");
> +
> +	if (!ok) {
> +		at_gnss_not_supported(gnss);
> +		return;
> +	}
> +
> +	g_at_chat_register(ad->chat, "+CPOSR:", gnss_report,
> +				FALSE, gnss, NULL);
> +
> +	if (ad->vendor == OFONO_VENDOR_STE)
> +		g_at_chat_register(ad->chat, "*EPOSADRR:", at_gnssreset_cb,
> +					FALSE, gnss, NULL);
> +
> +	ofono_gnss_register(gnss);
> +}
> +
> +static void at_gnsscpos_cb(gboolean ok, GAtResult *result, gpointer user_data)

This might be better called at_gnss_cpos_support_cb

> +{
> +	struct ofono_gnss *gnss = user_data;
> +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> +
> +	DBG("");
> +
> +	if (!ok) {
> +		at_gnss_not_supported(gnss);
> +		return;
> +	}
> +
> +	g_at_chat_send(ad->chat, "AT+CPOSR=?",
> +			none_prefix, at_gnsscposr_cb, gnss, NULL);

And this one at_gnss_cposr_support_cb

> +}
> +
> +static int at_gnss_probe(struct ofono_gnss *gnss, unsigned int vendor,
> +				void *user)
> +{
> +	GAtChat *chat = user;
> +	struct gnss_data *gd;
> +
> +	DBG("");
> +
> +	gd = g_try_new0(struct gnss_data, 1);
> +	if (gd == NULL)
> +		return -ENOMEM;
> +
> +	gd->chat = g_at_chat_clone(chat);
> +	gd->vendor = vendor;
> +
> +	ofono_gnss_set_data(gnss, gd);
> +
> +	g_at_chat_send(gd->chat, "AT+CPOS=?",
> +			none_prefix, at_gnsscpos_cb, gnss, NULL);
> +
> +	return 0;
> +}
> +
> +static void at_gnss_remove(struct ofono_gnss *gnss)
> +{
> +	struct gnss_data *gd = ofono_gnss_get_data(gnss);
> +
> +	DBG("");
> +
> +	ofono_gnss_set_data(gnss, NULL);
> +
> +	g_at_chat_unref(gd->chat);
> +	g_free(gd);
> +}
> +
> +static struct ofono_gnss_driver driver = {
> +	.name			= "atmodem",
> +	.probe			= at_gnss_probe,
> +	.remove			= at_gnss_remove,
> +	.send_element		= at_gnss_send_element,
> +	.position_reporting	= at_gnss_position_reporting,
> +};
> +
> +void at_gnss_init(void)
> +{
> +	ofono_gnss_driver_register(&driver);
> +}
> +
> +void at_gnss_exit(void)
> +{
> +	ofono_gnss_driver_unregister(&driver);
> +}

Regards,
-Denis

Re: [PATCH 01/12] dbus: add gnss interface definition

[Jarko Poutiainen]

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

Hi Denis,

On Fri, 2011-03-18 at 06:26 +0200, Denis Kenzior wrote:
> Hi Jarko,
> 
> On 03/11/2011 06:22 AM, Jarko Poutiainen wrote:
> > ---
> >  include/dbus.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/dbus.h b/include/dbus.h
> > index 19a138a..bf3e8a4 100644
> > --- a/include/dbus.h
> > +++ b/include/dbus.h
> > @@ -56,6 +56,8 @@ extern "C" {
> >  #define OFONO_STK_INTERFACE OFONO_SERVICE ".SimToolkit"
> >  #define OFONO_SIM_APP_INTERFACE OFONO_SERVICE ".SimToolkitAgent"
> >  #define OFONO_LOCATION_REPORTING_INTERFACE OFONO_SERVICE ".LocationReporting"
> > +#define OFONO_AS_NAVIGATION_INTERFACE "org.ofono.AssistedSatelliteNavigation"
> 
> Please name this OFONO_GNSS_INTERFACE
> 
Ok

> > +#define OFONO_POSITIONING_REQUEST_INTERFACE "org.ofono.PositioningRequestAgent"
> 
> Please name this OFONO_GNSS_POSR_INTERFACE or
> OFONO_GNSS_POSR_AGENT_INTERFACE
> 
> Or feel free to suggest a better name, but the current names are getting
> too big
> 
Ok I'll use OFONO_GNSS_POSR_AGENT_INTERFACE
> >  
> >  /* CDMA Interfaces */
> >  #define OFONO_CDMA_VOICECALL_MANAGER_INTERFACE "org.ofono.cdma.VoiceCallManager"
> 
> Regards,
> -Denis

Br,
Jarko

Re: [PATCH 02/12] include: add gnss.h file

[Jarko Poutiainen]

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

Hi Denis,

On Fri, 2011-03-18 at 06:29 +0200, Denis Kenzior wrote:
> Hi Jarko,
> 
> On 03/11/2011 06:22 AM, Jarko Poutiainen wrote:
> > ---
> >  Makefile.am    |    2 +-
> >  include/gnss.h |   76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 77 insertions(+), 1 deletions(-)
> >  create mode 100644 include/gnss.h
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 3f20717..7d6acce 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -15,7 +15,7 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
> >  			include/ctm.h include/cdma-voicecall.h \
> >  			include/cdma-sms.h include/sim-auth.h \
> >  			include/gprs-provision.h include/emulator.h \
> > -			include/location-reporting.h
> > +			include/location-reporting.h include/gnss.h
> >  
> >  nodist_pkginclude_HEADERS = include/version.h
> >  
> > diff --git a/include/gnss.h b/include/gnss.h
> > new file mode 100644
> > index 0000000..d10ab11
> > --- /dev/null
> > +++ b/include/gnss.h
> > @@ -0,0 +1,76 @@
> > +/*
> > + *
> > + *  oFono - Open Source Telephony
> > + *
> > + *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
> > + *  Copyright (C) 2011 ST-Ericsson AB.
> > + *
> > + *  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_GNSS_H
> > +#define __OFONO_GNSS_H
> > +
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#include <ofono/types.h>
> > +
> > +struct ofono_gnss;
> > +
> > +typedef void (*ofono_gnss_position_report_cb_t)(const struct ofono_error *error,
> > +							void *data);
> > +
> > +typedef void (*ofono_gnss_send_element_cb_t)(const struct ofono_error *error,
> > +						void *data);
> > +
> > +typedef void (*ofono_gnss_report_reset_cb_t)(const struct ofono_error *error,
> > +						void *data);
> 
> Please just combine these into a single callback, they have the same
> signature.  E.g. something like ofono_gnss_cb_t
> 
That's a good point, thank you.

> > +
> > +struct ofono_gnss_driver {
> > +	const char *name;
> > +	int (*probe)(struct ofono_gnss *gnss, unsigned int vendor, void *data);
> > +	void (*remove)(struct ofono_gnss *gnss);
> > +	void (*send_element)(struct ofono_gnss *gnss,
> > +				const char *xml,
> > +				ofono_gnss_send_element_cb_t cb, void *data);
> > +	void (*position_reporting)(struct ofono_gnss *gnss,
> > +					ofono_bool_t enable,
> > +					ofono_gnss_position_report_cb_t cb,
> > +					void *data);
> 
> Please name this set_position_reporting or set_posr
> 
Ok I'll name it set_position_reporting.

> > +};
> > +
> > +void ofono_gnss_receive_request(struct ofono_gnss *gnss, const char *xml);
> > +void ofono_gnss_receive_reset(struct ofono_gnss *gnss);
> 
> Please use notify instead of receive.  E.g. ofono_gnss_notify_posr_reset
> and ofono_gnss_notify_posr_request
> 
Ok I'll do that.

> > +int ofono_gnss_driver_register(const struct ofono_gnss_driver *d);
> > +void ofono_gnss_driver_unregister(const struct ofono_gnss_driver *d);
> > +
> > +struct ofono_gnss *ofono_gnss_create(struct ofono_modem *modem,
> > +					unsigned int vendor,
> > +					const char *driver, void *data);
> > +
> > +void ofono_gnss_register(struct ofono_gnss *gnss);
> > +void ofono_gnss_remove(struct ofono_gnss *gnss);
> > +
> > +void ofono_gnss_set_data(struct ofono_gnss *gnss, void *data);
> > +void *ofono_gnss_get_data(struct ofono_gnss *gnss);
> > +
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* __OFONO_GNSS_H */
> 
> Regards,
> -Denis
Br,
Jarko

Re: [PATCH 04/12] src: add gnss atom and agent implementation

[Jarko Poutiainen]

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

Hi Denis,

On Fri, 2011-03-18 at 06:39 +0200, Denis Kenzior wrote:
> Hi Jarko,
> 
> On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> > ---
> >  Makefile.am     |    3 +-
> >  src/gnss.c      |  357 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/gnssagent.c |  152 +++++++++++++++++++++++
> >  src/gnssagent.h |   40 ++++++
> >  4 files changed, 551 insertions(+), 1 deletions(-)
> >  create mode 100644 src/gnss.c
> >  create mode 100644 src/gnssagent.c
> >  create mode 100644 src/gnssagent.h
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 7d6acce..24742bb 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -383,7 +383,8 @@ 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/location-reporting.c
> > +                     src/emulator.c src/location-reporting.c \
> > +                     src/gnss.c src/gnssagent.c src/gnssagent.h
> >
> >  src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
> >
> > diff --git a/src/gnss.c b/src/gnss.c
> > new file mode 100644
> > index 0000000..02f9057
> > --- /dev/null
> > +++ b/src/gnss.c
> > @@ -0,0 +1,357 @@
> > +/*
> > + *
> > + *  oFono - Open Source Telephony
> > + *
> > + *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
> > + *  Copyright (C) 2011 ST-Ericsson AB.
> > + *
> > + *  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 <stdio.h>
> > +#include <stdlib.h>
> > +#include <stdint.h>
> > +
> > +#include <glib.h>
> > +#include <gdbus.h>
> > +#include <errno.h>
> > +
> > +#include "ofono.h"
> > +
> > +#include "common.h"
> > +#include "gnssagent.h"
> > +
> > +static GSList *g_drivers = NULL;
> > +
> > +struct ofono_gnss {
> > +     const struct ofono_gnss_driver *driver;
> > +     void *driver_data;
> > +     struct ofono_atom *atom;
> > +     DBusMessage *pending;
> > +     struct gnss_agent *default_agent;
> 
> Why is this called default_agent? posr_agent might be a better name
> 
Good point. I will change that. Wasn't sure how I should implement this
at first and later just forgot to change the name 

> > +};
> > +
> > +static void gnss_register_agent_cb(const struct ofono_error *error,
> > +                                     void *data)
> > +{
> > +     DBusMessage *reply;
> > +     struct ofono_gnss *gnss = data;
> > +
> > +     DBG("");
> > +
> > +     if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> > +             ofono_error("Enabling Location Reporting Failed");
> > +             reply = __ofono_error_failed(gnss->pending);
> > +             gnss_agent_free(gnss->default_agent);
> > +             __ofono_dbus_pending_reply(&gnss->pending, reply);
> > +             return;
> > +     }
> > +
> > +     reply = dbus_message_new_method_return(gnss->pending);
> > +     __ofono_dbus_pending_reply(&gnss->pending, reply);
> > +}
> > +
> > +static void default_agent_notify(gpointer user_data)
> 
> Why is this called default_agent_notify?  Can you have multiple agents?
> If not, then agent_notify is sufficient.
> 
No. At least I don't see a need.

> > +{
> > +     struct ofono_gnss *gnss = user_data;
> > +
> > +     gnss->default_agent = NULL;
> 
> Since you're enabling CPOSR when an agent is registered, you have to
> also guard against the case of the application exiting.  Note that the
> agent can even exit while your CPOSR enable is in progress.  Have a peek
> at location-reporting.c for ideas on how Lucas solved this.
> 
Not sure if I understood correctly what you're trying to say and
checking location-reporting.c didn't help but I can change the order in
which things are done i.e. I can create agent first and then enable
CPOSR. That way if the client dies we will know and can destroy the
agent.

> > +}
> > +
> > +static DBusMessage *gnss_register_agent(DBusConnection *conn,
> > +                                     DBusMessage *msg, void *data)
> > +{
> > +     struct ofono_gnss *gnss = data;
> > +     const char *agent_path;
> > +
> > +     if (gnss->pending)
> > +             return __ofono_error_busy(msg);
> > +
> > +     if (gnss->default_agent)
> > +             return __ofono_error_busy(msg);
> > +
> > +     if (dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH,
> > +                               &agent_path, DBUS_TYPE_INVALID) == FALSE)
> 
> Please don't mix spaces and tabs for indentation, always use tabs
> 
Sorry

> > +             return __ofono_error_invalid_args(msg);
> > +
> > +     if (!__ofono_dbus_valid_object_path(agent_path))
> > +             return __ofono_error_invalid_format(msg);
> > +
> > +     gnss->driver->position_reporting(gnss, 1, gnss_register_agent_cb, gnss);
> 
> Please use TRUE instead of 1 here
> 
Thanks, I missed that one.

> > +
> > +     gnss->default_agent = gnss_agent_new(agent_path,
> > +                                     dbus_message_get_sender(msg),
> > +                                     FALSE);
> > +
> > +     if (gnss->default_agent == NULL)
> > +             return __ofono_error_failed(msg);
> > +
> > +     gnss_agent_set_removed_notify(gnss->default_agent,
> > +                                     default_agent_notify, gnss);
> > +
> > +     gnss->pending = dbus_message_ref(msg);
> > +
> > +     return NULL;
> > +}
> > +
> > +static void gnss_unregister_agent_cb(const struct ofono_error *error,
> > +                                     void *data)
> > +{
> > +     DBusMessage *reply;
> > +     struct ofono_gnss *gnss = data;
> > +
> > +     DBG("");
> > +
> > +     if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> > +             ofono_error("Disabling Location Reporting Failed");
> > +             reply = __ofono_error_failed(gnss->pending);
> > +             goto out;
> > +     }
> > +
> > +     gnss_agent_free(gnss->default_agent);
> > +
> > +     if (gnss->default_agent) {
> > +             ofono_error("Releasing agent failed");
> > +             reply = __ofono_error_failed(gnss->pending);
> > +             goto out;
> > +     }
> > +
> > +     reply = dbus_message_new_method_return(gnss->pending);
> > +
> > +out:
> > +     __ofono_dbus_pending_reply(&gnss->pending, reply);
> > +}
> > +
> > +static DBusMessage *gnss_unregister_agent(DBusConnection *conn,
> > +                                             DBusMessage *msg, void *data)
> > +{
> > +     struct ofono_gnss *gnss = data;
> > +     const char *agent_path;
> > +     const char *agent_bus = dbus_message_get_sender(msg);
> > +
> > +     if (gnss->pending)
> > +             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 (gnss->default_agent == NULL)
> > +             return __ofono_error_failed(msg);
> > +
> > +     if (!gnss_agent_matches(gnss->default_agent, agent_path, agent_bus))
> > +             return __ofono_error_failed(msg);
> > +
> > +     gnss->pending = dbus_message_ref(msg);
> > +
> > +     gnss->driver->position_reporting(gnss, 0, gnss_unregister_agent_cb,
> 
> Please use FALSE here
> 
Sorry about that.

> > +                                             gnss);
> > +
> > +     return NULL;
> > +}
> > +
> > +static void gnss_send_element_cb(const struct ofono_error *error,
> > +                             void *data)
> > +{
> > +     DBusMessage *reply;
> > +     struct ofono_gnss *gnss = data;
> > +
> > +     DBG("");
> > +
> > +     if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> > +             ofono_error("Sending Positioning Element failed");
> > +
> > +             reply = __ofono_error_failed(gnss->pending);
> > +
> > +             goto out;
> > +     }
> > +
> > +     reply = dbus_message_new_method_return(gnss->pending);
> > +
> > +out:
> > +     __ofono_dbus_pending_reply(&gnss->pending, reply);
> > +}
> > +
> > +static DBusMessage *gnss_send_element(DBusConnection *conn,
> > +                                     DBusMessage *msg, void *data)
> > +{
> > +     struct ofono_gnss *gnss = data;
> > +     const char *xml;
> > +
> > +     DBG("");
> > +
> > +     if (gnss->pending)
> > +             return __ofono_error_busy(msg);
> > +
> > +     if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &xml,
> > +                                     DBUS_TYPE_INVALID))
> > +             return __ofono_error_invalid_args(msg);
> > +
> > +     gnss->pending = dbus_message_ref(msg);
> > +
> 
> Please make sure that only the application which has registered the
> agent can use this method and return AccessDenied otherwise.
> 
Thanks that's a good point.

> > +     gnss->driver->send_element(gnss, xml, gnss_send_element_cb, gnss);
> > +
> > +     return NULL;
> > +}
> > +
> > +static GDBusMethodTable gnss_methods[] = {
> > +     { "SendPositioningElement",             "s",    "",
> > +                             gnss_send_element, G_DBUS_METHOD_FLAG_ASYNC },
> > +     { "RegisterPositioningRequestAgent",    "o",    "",
> > +                             gnss_register_agent,
> > +                             G_DBUS_METHOD_FLAG_ASYNC },
> > +     { "UnregisterPositioningRequestAgent",  "o",    "",
> > +                             gnss_unregister_agent,
> > +                             G_DBUS_METHOD_FLAG_ASYNC },
> > +     { }
> > +};
> > +
> > +static void gnss_unregister(struct ofono_atom *atom)
> > +{
> > +     struct ofono_gnss *gnss = __ofono_atom_get_data(atom);
> > +     DBusConnection *conn = ofono_dbus_get_connection();
> > +     struct ofono_modem *modem = __ofono_atom_get_modem(atom);
> > +     const char *path = __ofono_atom_get_path(atom);
> > +
> > +     if (gnss->default_agent)
> > +             gnss_agent_free(gnss->default_agent);
> > +
> > +     ofono_modem_remove_interface(modem, OFONO_AS_NAVIGATION_INTERFACE);
> > +     g_dbus_unregister_interface(conn, path, OFONO_AS_NAVIGATION_INTERFACE);
> > +}
> > +
> > +static void gnss_remove(struct ofono_atom *atom)
> > +{
> > +     struct ofono_gnss *gnss = __ofono_atom_get_data(atom);
> > +
> > +     DBG("atom: %p", atom);
> > +
> > +     if (gnss == NULL)
> > +             return;
> > +
> > +     if (gnss->driver && gnss->driver->remove)
> > +             gnss->driver->remove(gnss);
> > +
> > +     g_free(gnss);
> > +}
> > +
> > +void ofono_gnss_register(struct ofono_gnss *gnss)
> > +{
> > +     DBusConnection *conn = ofono_dbus_get_connection();
> > +     struct ofono_modem *modem = __ofono_atom_get_modem(gnss->atom);
> > +     const char *path = __ofono_atom_get_path(gnss->atom);
> > +
> > +     if (!g_dbus_register_interface(conn, path,
> > +                                     OFONO_AS_NAVIGATION_INTERFACE,
> > +                                     gnss_methods, NULL, NULL,
> > +                                     gnss, NULL)) {
> > +             ofono_error("Could not create %s interface",
> > +                             OFONO_AS_NAVIGATION_INTERFACE);
> > +
> > +             return;
> > +     }
> > +
> > +     ofono_modem_add_interface(modem, OFONO_AS_NAVIGATION_INTERFACE);
> > +
> > +     __ofono_atom_register(gnss->atom, gnss_unregister);
> > +}
> > +
> > +int ofono_gnss_driver_register(const struct ofono_gnss_driver *d)
> > +{
> > +     DBG("driver: %p, name: %s", d, d->name);
> > +
> > +     if (d->probe == NULL)
> > +             return -EINVAL;
> > +
> > +     g_drivers = g_slist_prepend(g_drivers, (void *) d);
> > +
> > +     return 0;
> > +}
> > +
> > +void ofono_gnss_driver_unregister(const struct ofono_gnss_driver *d)
> > +{
> > +     DBG("driver: %p, name: %s", d, d->name);
> > +
> > +     g_drivers = g_slist_remove(g_drivers, (void *) d);
> > +}
> > +
> > +struct ofono_gnss *ofono_gnss_create(struct ofono_modem *modem,
> > +                                     unsigned int vendor,
> > +                                     const char *driver,
> > +                                     void *data)
> > +{
> > +     struct ofono_gnss *gnss;
> > +     GSList *l;
> > +
> > +     if (driver == NULL)
> > +             return NULL;
> > +
> > +     gnss = g_try_new0(struct ofono_gnss, 1);
> > +
> > +     if (gnss == NULL)
> > +             return NULL;
> > +
> > +     gnss->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_GNSS,
> > +                                             gnss_remove, gnss);
> > +
> > +     for (l = g_drivers; l; l = l->next) {
> > +             const struct ofono_gnss_driver *drv = l->data;
> > +
> > +             if (g_strcmp0(drv->name, driver))
> > +                     continue;
> > +
> > +             if (drv->probe(gnss, vendor, data) < 0)
> > +                     continue;
> > +
> > +             gnss->driver = drv;
> > +             break;
> > +     }
> > +
> > +     return gnss;
> > +}
> > +
> > +void ofono_gnss_receive_request(struct ofono_gnss *gnss, const char *xml)
> > +{
> > +     if (gnss->default_agent)
> > +             gnss_agent_receive_request(gnss->default_agent, xml);
> > +}
> > +
> > +void ofono_gnss_receive_reset(struct ofono_gnss *gnss)
> > +{
> > +     if (gnss->default_agent)
> > +             gnss_agent_receive_reset(gnss->default_agent);
> > +}
> > +
> > +void ofono_gnss_remove(struct ofono_gnss *gnss)
> > +{
> > +     __ofono_atom_free(gnss->atom);
> > +}
> > +
> > +void ofono_gnss_set_data(struct ofono_gnss *gnss, void *data)
> > +{
> > +     gnss->driver_data = data;
> > +}
> > +
> > +void *ofono_gnss_get_data(struct ofono_gnss *gnss)
> > +{
> > +     return gnss->driver_data;
> > +}
> 
> <snip>
> 
> Regards,
> -Denis
Br,
Jarko

Re: [PATCH 09/12] atmodem: add gnss driver

[Jarko Poutiainen]

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

Hi Denis,

On Fri, 2011-03-18 at 07:03 +0200, Denis Kenzior wrote:
> On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> > ---
> >  Makefile.am               |    3 +-
> >  drivers/atmodem/atmodem.c |    2 +
> >  drivers/atmodem/atmodem.h |    3 +
> >  drivers/atmodem/gnss.c    |  279 +++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 286 insertions(+), 1 deletions(-)
> >  create mode 100644 drivers/atmodem/gnss.c
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index 24742bb..3dae7f4 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -180,7 +180,8 @@ builtin_sources += $(gatchat_sources) \
> >  				drivers/atmodem/atutil.c \
> >  				drivers/atmodem/gprs.c \
> >  				drivers/atmodem/gprs-context.c \
> > -				drivers/atmodem/sim-auth.c
> > +				drivers/atmodem/sim-auth.c \
> > +				drivers/atmodem/gnss.c
> >  
> >  builtin_modules += nwmodem
> >  builtin_sources += drivers/atmodem/atutil.h \
> > diff --git a/drivers/atmodem/atmodem.c b/drivers/atmodem/atmodem.c
> > index e140281..3093c23 100644
> > --- a/drivers/atmodem/atmodem.c
> > +++ b/drivers/atmodem/atmodem.c
> > @@ -52,6 +52,7 @@ static int atmodem_init(void)
> >  	at_gprs_init();
> >  	at_gprs_context_init();
> >  	at_sim_auth_init();
> > +	at_gnss_init();
> >  
> >  	return 0;
> >  }
> > @@ -76,6 +77,7 @@ static void atmodem_exit(void)
> >  	at_call_volume_exit();
> >  	at_gprs_exit();
> >  	at_gprs_context_exit();
> > +	at_gnss_exit();
> >  }
> >  
> >  OFONO_PLUGIN_DEFINE(atmodem, "AT modem driver", VERSION,
> > diff --git a/drivers/atmodem/atmodem.h b/drivers/atmodem/atmodem.h
> > index 1b7cf67..1a73b84 100644
> > --- a/drivers/atmodem/atmodem.h
> > +++ b/drivers/atmodem/atmodem.h
> > @@ -74,3 +74,6 @@ extern void at_gprs_context_exit(void);
> >  
> >  extern void at_sim_auth_init(void);
> >  extern void at_sim_auth_exit(void);
> > +
> > +extern void at_gnss_init(void);
> > +extern void at_gnss_exit(void);
> > diff --git a/drivers/atmodem/gnss.c b/drivers/atmodem/gnss.c
> > new file mode 100644
> > index 0000000..c1b1de3
> > --- /dev/null
> > +++ b/drivers/atmodem/gnss.c
> > @@ -0,0 +1,279 @@
> > +/*
> > + *
> > + *  oFono - Open Source Telephony
> > + *
> > + *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
> > + *  Copyright (C) 2011 ST-Ericsson AB.
> > + *
> > + *  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 <glib.h>
> > +
> > +#include <ofono/log.h>
> > +#include <ofono/modem.h>
> > +#include <ofono/gnss.h>
> > +
> > +#include "gatchat.h"
> > +#include "gatresult.h"
> > +
> > +#include "atmodem.h"
> > +#include "vendor.h"
> > +
> > +struct gnss_data {
> > +	GAtChat *chat;
> > +	unsigned int vendor;
> > +};
> > +
> > +static const char *none_prefix[] = { NULL };
> > +static const char *cpos_prefix[] = { "+CPOS:", NULL };
> > +static const char *cposr_prefix[] = { "+CPOSR:", NULL };
> > +
> > +static void gnsspr_cb(gboolean ok, GAtResult *result, gpointer user_data)
> > +{
> > +	struct cb_data *cbd = user_data;
> > +	ofono_gnss_position_report_cb_t cb = cbd->cb;
> > +
> > +	DBG("");
> > +
> > +	if (ok)
> > +		CALLBACK_WITH_SUCCESS(cb, cbd->data);
> > +	else
> > +		CALLBACK_WITH_FAILURE(cb, cbd->data);
> 
> As a general habit, please use at_util_decode_error instead of using
> these macros.  This leads to simpler code as well
> 
Thanks I will do that

> > +}
> > +
> > +static void at_gnss_position_reporting(struct ofono_gnss *gnss,
> > +					ofono_bool_t enable,
> > +					ofono_gnss_position_report_cb_t cb,
> > +					void *data)
> > +{
> > +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> > +	struct cb_data *cbd = cb_data_new(cb, data);
> > +
> > +	DBG("");
> > +
> > +	if (enable) {
> > +		g_at_chat_send(ad->chat, "AT+CPOSR=1",
> > +				cposr_prefix, gnsspr_cb, cbd, g_free);
> > +
> > +		if (ad->vendor == OFONO_VENDOR_STE)
> > +			g_at_chat_send(ad->chat, "AT*EPOSADRR=1",
> > +					NULL, NULL, NULL, NULL);
> > +	} else {
> > +		g_at_chat_send(ad->chat, "AT+CPOSR=0",
> > +				cposr_prefix, gnsspr_cb, cbd, g_free);
> > +
> > +		if (ad->vendor == OFONO_VENDOR_STE)
> > +			g_at_chat_send(ad->chat, "AT*EPOSADRR=0",
> > +					NULL, NULL, NULL, NULL);
> > +	}
> > +}
> > +
> > +static void gnssse_cb(gboolean ok, GAtResult *result, gpointer user_data)
> > +{
> > +	struct cb_data *cbd = user_data;
> > +	ofono_gnss_send_element_cb_t cb = cbd->cb;
> > +
> > +	DBG("");
> > +
> > +	if (ok)
> > +		CALLBACK_WITH_SUCCESS(cb, cbd->data);
> > +	else
> > +		CALLBACK_WITH_FAILURE(cb, cbd->data);
> 
> Again, please use at_util_decode_error here
> 
Ok

> > +}
> > +
> > +static void at_gnss_send_element(struct ofono_gnss *gnss,
> > +				const char *xml,
> > +				ofono_gnss_send_element_cb_t cb, void *data)
> > +{
> > +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> > +	struct cb_data *cbd = cb_data_new(cb, data);
> > +	char *buf = g_try_new(char, strlen(xml) + 10);
> > +	int len;
> > +
> > +	DBG("");
> > +
> > +	if (buf == NULL)
> > +		goto error;
> > +
> > +	len = sprintf(buf, "AT+CPOS\r");
> > +	len += sprintf(buf + len, "%s", xml);
> > +
> > +	if (g_at_chat_send_and_expect_short_prompt(ad->chat, buf, cpos_prefix,
> > +							gnssse_cb, cbd,
> > +							g_free) > 0) {
> > +		g_free(buf);
> > +		return;
> > +	}
> 
> Please add an empty line here
> 
Sorry about that.

> > +error:
> > +	g_free(buf);
> > +	g_free(cbd);
> > +
> > +	CALLBACK_WITH_FAILURE(cb, data);
> > +}
> > +
> > +static gboolean gnss_parse_report(GAtResult *result, const char *prefix,
> > +					const char **xml)
> > +{
> > +	GAtResultIter iter;
> > +
> > +	g_at_result_iter_init(&iter, result);
> > +
> > +	if (!g_at_result_iter_next(&iter, prefix))
> > +		return FALSE;
> > +
> > +	if (!g_at_result_iter_next_unquoted_string(&iter, xml))
> > +		return FALSE;
> > +
> > +	return TRUE;
> 
> Do you really need a separate function?  This might belong in gnss_report
> 
I just thought that it would be more logical to have separate function
to parse the result rather than putting it all in to same function. Is
that a big deal or do you want me to change that?

> > +}
> > +
> > +static void gnss_report(GAtResult *result, gpointer user_data)
> > +{
> > +	struct ofono_gnss *gnss = user_data;
> > +	const char *xml;
> > +
> > +	DBG("");
> > +
> > +	xml = NULL;
> > +	if (!gnss_parse_report(result, "+CPOSR:", &xml)) {
> > +		ofono_error("Unable to parse CPOSR notification");
> > +		return;
> > +	}
> > +
> > +	if (xml == NULL) {
> > +		ofono_error("Unable to parse CPOSR notification");
> > +		return;
> > +	}
> > +
> > +	ofono_gnss_receive_request(gnss, xml);
> > +}
> > +
> > +static void at_gnssreset_cb(GAtResult *result, gpointer user_data)
> 
> Since this is a notification, please call this gnssreset_notify
> 
Ok, good point and in fact I'll change it to gnss_reset_notify to be
consistent with naming.

> > +{
> > +	struct ofono_gnss *gnss = user_data;
> > +
> > +	DBG("");
> > +
> > +	ofono_gnss_receive_reset(gnss);
> > +}
> > +
> > +static void at_gnss_not_supported(struct ofono_gnss *gnss)
> > +{
> > +	ofono_error("gnss not supported by this modem.");
> > +
> > +	ofono_gnss_remove(gnss);
> > +}
> > +
> > +static void at_gnsscposr_cb(gboolean ok, GAtResult *result, gpointer user_data)
> > +{
> > +	struct ofono_gnss *gnss = user_data;
> > +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> > +
> > +	DBG("");
> > +
> > +	if (!ok) {
> > +		at_gnss_not_supported(gnss);
> > +		return;
> > +	}
> > +
> > +	g_at_chat_register(ad->chat, "+CPOSR:", gnss_report,
> > +				FALSE, gnss, NULL);
> > +
> > +	if (ad->vendor == OFONO_VENDOR_STE)
> > +		g_at_chat_register(ad->chat, "*EPOSADRR:", at_gnssreset_cb,
> > +					FALSE, gnss, NULL);
> > +
> > +	ofono_gnss_register(gnss);
> > +}
> > +
> > +static void at_gnsscpos_cb(gboolean ok, GAtResult *result, gpointer user_data)
> 
> This might be better called at_gnss_cpos_support_cb
> 
Ok I'll change that.

> > +{
> > +	struct ofono_gnss *gnss = user_data;
> > +	struct gnss_data *ad = ofono_gnss_get_data(gnss);
> > +
> > +	DBG("");
> > +
> > +	if (!ok) {
> > +		at_gnss_not_supported(gnss);
> > +		return;
> > +	}
> > +
> > +	g_at_chat_send(ad->chat, "AT+CPOSR=?",
> > +			none_prefix, at_gnsscposr_cb, gnss, NULL);
> 
> And this one at_gnss_cposr_support_cb
> 
Ok.

> > +}
> > +
> > +static int at_gnss_probe(struct ofono_gnss *gnss, unsigned int vendor,
> > +				void *user)
> > +{
> > +	GAtChat *chat = user;
> > +	struct gnss_data *gd;
> > +
> > +	DBG("");
> > +
> > +	gd = g_try_new0(struct gnss_data, 1);
> > +	if (gd == NULL)
> > +		return -ENOMEM;
> > +
> > +	gd->chat = g_at_chat_clone(chat);
> > +	gd->vendor = vendor;
> > +
> > +	ofono_gnss_set_data(gnss, gd);
> > +
> > +	g_at_chat_send(gd->chat, "AT+CPOS=?",
> > +			none_prefix, at_gnsscpos_cb, gnss, NULL);
> > +
> > +	return 0;
> > +}
> > +
> > +static void at_gnss_remove(struct ofono_gnss *gnss)
> > +{
> > +	struct gnss_data *gd = ofono_gnss_get_data(gnss);
> > +
> > +	DBG("");
> > +
> > +	ofono_gnss_set_data(gnss, NULL);
> > +
> > +	g_at_chat_unref(gd->chat);
> > +	g_free(gd);
> > +}
> > +
> > +static struct ofono_gnss_driver driver = {
> > +	.name			= "atmodem",
> > +	.probe			= at_gnss_probe,
> > +	.remove			= at_gnss_remove,
> > +	.send_element		= at_gnss_send_element,
> > +	.position_reporting	= at_gnss_position_reporting,
> > +};
> > +
> > +void at_gnss_init(void)
> > +{
> > +	ofono_gnss_driver_register(&driver);
> > +}
> > +
> > +void at_gnss_exit(void)
> > +{
> > +	ofono_gnss_driver_unregister(&driver);
> > +}
> 
> Regards,
> -Denis
Br,
Jarko

Re: [PATCH 07/12] gatchat: implementation for +CPOS send

[Jarko Poutiainen]

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

Hi Denis,

On Fri, 2011-03-18 at 06:42 +0200, Denis Kenzior wrote:
> Hi Jarko,
> 
> On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> > ---
> >  gatchat/gatchat.c |   13 +++++++++++++
> >  1 files changed, 13 insertions(+), 0 deletions(-)
> > 
> > diff --git a/gatchat/gatchat.c b/gatchat/gatchat.c
> > index 3fd564d..64f131d 100644
> > --- a/gatchat/gatchat.c
> > +++ b/gatchat/gatchat.c
> > @@ -1468,6 +1468,19 @@ guint g_at_chat_send_pdu_listing(GAtChat *chat, const char *cmd,
> >  					listing, func, user_data, notify);
> >  }
> >  
> > +guint g_at_chat_send_and_expect_short_prompt(GAtChat *chat, const char *cmd,
> > +			const char **prefix_list, GAtResultFunc func,
> > +			gpointer user_data, GDestroyNotify notify)
> > +{
> > +	if(chat != NULL)
> > +		chat->parent->syntax->set_hint(chat->parent->syntax,
> > +					G_AT_SYNTAX_EXPECT_SHORT_PROMPT);
> 
> So unfortunately you can't do this.  GAtChat is a command queue and we
> might be in the process of sending / receiving responses for multiple
> commands that are ahead of this one.  So you can't really manipulate the
> syntax directly here.  You have to do this only once the CPOSR has been
> sent on the wire.
> 
Ok, thanks. Still having trouble to understand GAtChat. 
Not just sure where I should set the hint then but I get your point.
Now I'm also a little confused because previously you suggested that I'd
make a new g_at_chat_send variant that would set the hint so what did
you mean by it then if not this?

> > +
> > +	return at_chat_send_common(chat->parent, chat->group,
> > +					cmd, prefix_list, FALSE, NULL,
> > +					func, user_data, notify);
> > +}
> > +
> >  gboolean g_at_chat_cancel(GAtChat *chat, guint id)
> >  {
> >  	/* We use id 0 for wakeup commands */
> 
> Regards,
> -Denis
Br,
Jarko

Re: [PATCH 08/12] gatchat: fix gatsyntax to support +CPOS

[Jarko Poutiainen]

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

Hi Denis,

On Fri, 2011-03-18 at 06:53 +0200, Denis Kenzior wrote:
> Hi Jarko,
> 
> On 03/11/2011 06:23 AM, Jarko Poutiainen wrote:
> > ---
> >  gatchat/gatsyntax.c |   19 +++++++++++++++++++
> >  1 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/gatchat/gatsyntax.c b/gatchat/gatsyntax.c
> > index 2fc70b8..f1d896b 100644
> > --- a/gatchat/gatsyntax.c
> > +++ b/gatchat/gatsyntax.c
> > @@ -64,6 +64,9 @@ static void gsmv1_hint(GAtSyntax *syntax, GAtSyntaxExpectHint hint)
> >  	case G_AT_SYNTAX_EXPECT_MULTILINE:
> >  		syntax->state = GSMV1_STATE_GUESS_MULTILINE_RESPONSE;
> >  		break;
> > +	case G_AT_SYNTAX_EXPECT_SHORT_PROMPT:
> > +		syntax->state = GSMV1_STATE_PROMPT;
> > +		break;
> >  	default:
> >  		break;
> >  	};
> > @@ -195,6 +198,13 @@ static GAtSyntaxResult gsmv1_feed(GAtSyntax *syntax,
> >  				i += 1;
> >  				res = G_AT_SYNTAX_RESULT_PROMPT;
> >  				goto out;
> > +			} else if (byte == '\r')
> > +				break;
> > +			else if (byte == '\n') {
> > +				syntax->state = GSMV1_STATE_IDLE;
> > +				i += 1;
> > +				res = G_AT_SYNTAX_RESULT_PROMPT;
> > +				goto out;
> 
> Please just create a dedicated state for this one and don't hack the
> PROMPT state.
> 
Ok, good point.

> >  			}
> >  
> >  			syntax->state = GSMV1_STATE_RESPONSE;
> > @@ -239,6 +249,8 @@ static void gsm_permissive_hint(GAtSyntax *syntax, GAtSyntaxExpectHint hint)
> >  {
> >  	if (hint == G_AT_SYNTAX_EXPECT_PDU)
> >  		syntax->state = GSM_PERMISSIVE_STATE_GUESS_PDU;
> > +	else if (hint == G_AT_SYNTAX_EXPECT_SHORT_PROMPT)
> > +		syntax->state = GSM_PERMISSIVE_STATE_PROMPT;
> >  }
> >  
> >  static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
> > @@ -298,6 +310,13 @@ static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
> >  				i += 1;
> >  				res = G_AT_SYNTAX_RESULT_PROMPT;
> >  				goto out;
> > +			} else if (byte == '\r')
> > +				break;
> > +			else if (byte == '\n') {
> > +				syntax->state = GSM_PERMISSIVE_STATE_IDLE;
> > +				i += 1;
> > +				res = G_AT_SYNTAX_RESULT_PROMPT;
> > +				goto out;
> 
> Same comment here, and man you better pray that your modem doesn't
> insert unsolicited notifications between you sending a CPOSR and you
> receiving a prompt ;)
> 
Yep. Btw now that I think about it, shouldn't the patch that I proposed
to ste.c help? I know it's ste specific solution but still..

> >  			}
> >  
> >  			syntax->state = GSM_PERMISSIVE_STATE_RESPONSE;
> 
> Regards,
> -Denis
Br,
Jarko

Re: [PATCH 04/12] src: add gnss atom and agent implementation

[Denis Kenzior]

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

Hi Jarko,

>>> +{
>>> +     struct ofono_gnss *gnss = user_data;
>>> +
>>> +     gnss->default_agent = NULL;
>>
>> Since you're enabling CPOSR when an agent is registered, you have to
>> also guard against the case of the application exiting.  Note that the
>> agent can even exit while your CPOSR enable is in progress.  Have a peek
>> at location-reporting.c for ideas on how Lucas solved this.
>>
> Not sure if I understood correctly what you're trying to say and
> checking location-reporting.c didn't help but I can change the order in
> which things are done i.e. I can create agent first and then enable
> CPOSR. That way if the client dies we will know and can destroy the
> agent.
> 

Basically what I'm seeing is that when the application calls:

RegisterPositioningReportingAgent

you trigger CPOSR=1 to the modem and return a reply to the application
in the CPOSR callback.

When the application unregisters the agent, you trigger CPOSR=0 to the
modem and remove the agent in the callback.

Now consider what happens if the application that registered the agent
crashes or exits without unregistering the agent.  Your CPOSR reporting
is still turned on, without anyone around to handle the request.

You can handle this in a couple of ways:

- Always turn on CPOSR in the driver and get rid of the
set_position_reporting function.  Any requests going to the core with no
agent registered go to /dev/null (since I don't see a CPOS 'abort' style
functionality.

- Make sure that CPOSR is reset back to zero in both cases described above.

Regards,
-Denis

Re: [PATCH 09/12] atmodem: add gnss driver

[Denis Kenzior]

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

Hi Jarko,

>>> +static gboolean gnss_parse_report(GAtResult *result, const char *prefix,
>>> +					const char **xml)
>>> +{
>>> +	GAtResultIter iter;
>>> +
>>> +	g_at_result_iter_init(&iter, result);
>>> +
>>> +	if (!g_at_result_iter_next(&iter, prefix))
>>> +		return FALSE;
>>> +
>>> +	if (!g_at_result_iter_next_unquoted_string(&iter, xml))
>>> +		return FALSE;
>>> +
>>> +	return TRUE;
>>
>> Do you really need a separate function?  This might belong in gnss_report
>>
> I just thought that it would be more logical to have separate function
> to parse the result rather than putting it all in to same function. Is
> that a big deal or do you want me to change that?

This is not really wrong, was just wondering.  Leave it as is for now.

Regards,
-Denis

Re: [PATCH 08/12] gatchat: fix gatsyntax to support +CPOS

[Denis Kenzior]

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

Hi Jarko,

>>> @@ -298,6 +310,13 @@ static GAtSyntaxResult gsm_permissive_feed(GAtSyntax *syntax,
>>>  				i += 1;
>>>  				res = G_AT_SYNTAX_RESULT_PROMPT;
>>>  				goto out;
>>> +			} else if (byte == '\r')
>>> +				break;
>>> +			else if (byte == '\n') {
>>> +				syntax->state = GSM_PERMISSIVE_STATE_IDLE;
>>> +				i += 1;
>>> +				res = G_AT_SYNTAX_RESULT_PROMPT;
>>> +				goto out;
>>
>> Same comment here, and man you better pray that your modem doesn't
>> insert unsolicited notifications between you sending a CPOSR and you
>> receiving a prompt ;)
>>
> Yep. Btw now that I think about it, shouldn't the patch that I proposed
> to ste.c help? I know it's ste specific solution but still..

If you mean running GNSS on a dedicated mux channel, then yes it will
help ;)

But really, using a proper prompt would be way better.

Regards,
-Denis

Re: [PATCH 07/12] gatchat: implementation for +CPOS send

[Denis Kenzior]

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

Hi Jarko,

>>> +guint g_at_chat_send_and_expect_short_prompt(GAtChat *chat, const char *cmd,
>>> +			const char **prefix_list, GAtResultFunc func,
>>> +			gpointer user_data, GDestroyNotify notify)
>>> +{
>>> +	if(chat != NULL)
>>> +		chat->parent->syntax->set_hint(chat->parent->syntax,
>>> +					G_AT_SYNTAX_EXPECT_SHORT_PROMPT);
>>
>> So unfortunately you can't do this.  GAtChat is a command queue and we
>> might be in the process of sending / receiving responses for multiple
>> commands that are ahead of this one.  So you can't really manipulate the
>> syntax directly here.  You have to do this only once the CPOSR has been
>> sent on the wire.
>>
> Ok, thanks. Still having trouble to understand GAtChat. 
> Not just sure where I should set the hint then but I get your point.
> Now I'm also a little confused because previously you suggested that I'd
> make a new g_at_chat_send variant that would set the hint so what did
> you mean by it then if not this?

Look at how e.g. expect_pdu boolean is handled.  You need to do
something very similar but after sending the first chunk followed by \r.
(e.g. when AT+CPOS\r was sent).  You probably need to add another
flag/boolean to the command data structure for this.

Regards,
-Denis

[PATCH 02/12] include: add gnss.h file

[Jarko Poutiainen]

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

---
 Makefile.am    |    3 +-
 include/gnss.h |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 1 deletions(-)
 create mode 100644 include/gnss.h

diff --git a/Makefile.am b/Makefile.am
index b0cbcd7..bff2374 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,7 +16,8 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/cdma-sms.h include/sim-auth.h \
 			include/gprs-provision.h include/emulator.h \
 			include/location-reporting.h \
-			include/cdma-connman.h
+			include/cdma-connman.h \
+			include/gnss.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
diff --git a/include/gnss.h b/include/gnss.h
new file mode 100644
index 0000000..c8ff310
--- /dev/null
+++ b/include/gnss.h
@@ -0,0 +1,69 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2010  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2011 ST-Ericsson AB.
+ *
+ *  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_GNSS_H
+#define __OFONO_GNSS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <ofono/types.h>
+
+struct ofono_gnss;
+
+typedef void (*ofono_gnss_cb_t)(const struct ofono_error *error, void *data);
+
+struct ofono_gnss_driver {
+	const char *name;
+	int (*probe)(struct ofono_gnss *gnss, unsigned int vendor, void *data);
+	void (*remove)(struct ofono_gnss *gnss);
+	void (*send_element)(struct ofono_gnss *gnss,
+				const char *xml,
+				ofono_gnss_cb_t cb, void *data);
+	void (*set_position_reporting)(struct ofono_gnss *gnss,
+					ofono_bool_t enable,
+					ofono_gnss_cb_t cb,
+					void *data);
+};
+
+void ofono_gnss_notify_posr_request(struct ofono_gnss *gnss, const char *xml);
+void ofono_gnss_notify_posr_reset(struct ofono_gnss *gnss);
+int ofono_gnss_driver_register(const struct ofono_gnss_driver *d);
+void ofono_gnss_driver_unregister(const struct ofono_gnss_driver *d);
+
+struct ofono_gnss *ofono_gnss_create(struct ofono_modem *modem,
+					unsigned int vendor,
+					const char *driver, void *data);
+
+void ofono_gnss_register(struct ofono_gnss *gnss);
+void ofono_gnss_remove(struct ofono_gnss *gnss);
+
+void ofono_gnss_set_data(struct ofono_gnss *gnss, void *data);
+void *ofono_gnss_get_data(struct ofono_gnss *gnss);
+
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_GNSS_H */
-- 
1.7.0.4