[PATCH 1/3] Add radio access atom and driver API

[PATCH 1/3] Add radio access atom and driver API

[Aki Niemi]

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

This atom provides access to the modem's radio access properties. It
currently includes a single rw property, namely the radio access
selection mode setting.

This allows the user to query and select the used radio access
technology preference. In dual mode, either 2G or 3G is used depending
on reception. 2G only mode or 3G only mode force selection of the
respective access only.

In the future, this atom could be extended, if necessary, to handle
other radio access related modem features such as CELL_DCH status,
UTRAN channel, or frequency band.
---
 Makefile.am            |    6 +-
 include/radio-access.h |   72 ++++++++++
 src/ofono.h            |    2 +
 src/radio-access.c     |  354 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 432 insertions(+), 2 deletions(-)
 create mode 100644 include/radio-access.h
 create mode 100644 src/radio-access.c

diff --git a/Makefile.am b/Makefile.am
index 33339df..a18e4b9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -11,7 +11,8 @@ include_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/sms.h include/sim.h include/message-waiting.h \
 			include/netreg.h include/voicecall.h include/devinfo.h \
 			include/cbs.h include/call-volume.h \
-			include/gprs.h include/gprs-context.h
+			include/gprs.h include/gprs-context.h \
+			include/radio-access.h
 
 nodist_include_HEADERS = include/version.h
 
@@ -224,7 +225,8 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/phonebook.c src/history.c src/message-waiting.c \
 			src/simutil.h src/simutil.c src/storage.h \
 			src/storage.c src/cbs.c src/watch.c src/call-volume.c \
-			src/gprs.c src/idmap.h src/idmap.c
+			src/gprs.c src/idmap.h src/idmap.c \
+			src/radio-access.c
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ -ldl
 
diff --git a/include/radio-access.h b/include/radio-access.h
new file mode 100644
index 0000000..6677573
--- /dev/null
+++ b/include/radio-access.h
@@ -0,0 +1,72 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  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_RADIO_ACCESS_H
+#define __OFONO_RADIO_ACCESS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <ofono/types.h>
+
+struct ofono_radio_access;
+
+typedef void (*ofono_radio_access_mode_set_cb_t)(const struct ofono_error *error,
+							void *data);
+typedef void (*ofono_radio_access_mode_query_cb_t)(const struct ofono_error *error,
+							int mode, void *data);
+
+struct ofono_radio_access_driver {
+	const char *name;
+	int (*probe)(struct ofono_radio_access *radio_access,
+			unsigned int vendor,
+			void *data);
+	void (*remove)(struct ofono_radio_access *radio_access);
+	void (*query_mode)(struct ofono_radio_access *radio_access,
+				ofono_radio_access_mode_query_cb_t cb, void *data);
+	void (*set_mode)(struct ofono_radio_access *radio_access, int mode,
+				ofono_radio_access_mode_set_cb_t cb, void *data);
+};
+
+void ofono_radio_access_mode_notify(struct ofono_radio_access *radio_access,
+					int mode);
+
+int ofono_radio_access_driver_register(const struct ofono_radio_access_driver *d);
+void ofono_radio_access_driver_unregister(const struct ofono_radio_access_driver *d);
+
+struct ofono_radio_access *ofono_radio_access_create(struct ofono_modem *modem,
+						unsigned int vendor,
+						const char *driver,
+						void *data);
+
+void ofono_radio_access_register(struct ofono_radio_access *radio_access);
+void ofono_radio_access_remove(struct ofono_radio_access *radio_access);
+
+void ofono_radio_access_set_data(struct ofono_radio_access *radio_access,
+					void *data);
+void *ofono_radio_access_get_data(struct ofono_radio_access *radio_access);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_RADIO_ACCESS_H */
diff --git a/src/ofono.h b/src/ofono.h
index 379f413..c519eef 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -113,6 +113,7 @@ enum ofono_atom_type {
 	OFONO_ATOM_TYPES_CALL_VOLUME = 15,
 	OFONO_ATOM_TYPE_GPRS = 16,
 	OFONO_ATOM_TYPE_GPRS_CONTEXT = 17,
+	OFONO_ATOM_TYPE_RADIO_ACCESS = 18,
 };
 
 enum ofono_atom_watch_condition {
@@ -168,6 +169,7 @@ void __ofono_atom_free(struct ofono_atom *atom);
 #include <ofono/voicecall.h>
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
+#include <ofono/radio-access.h>
 
 #include <ofono/sim.h>
 
diff --git a/src/radio-access.c b/src/radio-access.c
new file mode 100644
index 0000000..ba34c71
--- /dev/null
+++ b/src/radio-access.c
@@ -0,0 +1,354 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <gdbus.h>
+
+#include "ofono.h"
+#include "common.h"
+
+#define RADIO_ACCESS_INTERFACE "org.ofono.RadioAccess"
+#define RADIO_ACCESS_INTERFACE "org.ofono.RadioAccess"
+
+static GSList *g_drivers = NULL;
+
+enum radio_access_mode {
+	RADIO_ACCESS_MODE_DUAL = 0,
+	RADIO_ACCESS_MODE_2G = 1,
+	RADIO_ACCESS_MODE_3G = 2
+};
+
+struct ofono_radio_access {
+	DBusMessage *pending;
+	int mode;
+	int pending_mode;
+	const struct ofono_radio_access_driver *driver;
+	void *driver_data;
+	struct ofono_atom *atom;
+};
+
+static const char *radio_access_mode_to_string(int mode)
+{
+	switch (mode) {
+	case RADIO_ACCESS_MODE_DUAL:
+		return "2g+3g";
+
+	case RADIO_ACCESS_MODE_2G:
+		return "2g";
+
+	case RADIO_ACCESS_MODE_3G:
+		return "3g";
+
+	default:
+		return "unknown";
+	}
+}
+
+static int string_to_radio_access_mode(const char *mode)
+{
+	if (g_strcmp0(mode, "2g+3g") == 0)
+		return RADIO_ACCESS_MODE_DUAL;
+
+	if (g_strcmp0(mode, "2g") == 0)
+		return RADIO_ACCESS_MODE_2G;
+
+	if (g_strcmp0(mode, "3g") == 0)
+		return RADIO_ACCESS_MODE_3G;
+
+	return -1;
+}
+
+static void ra_mode_query_callback(const struct ofono_error *error, int mode,
+					void *data)
+{
+	struct ofono_radio_access *ra = data;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_debug("Error during radio access mode query");
+		return;
+	}
+
+	ofono_radio_access_mode_notify(ra, mode);
+}
+
+static void ra_mode_set_callback(const struct ofono_error *error, void *data)
+{
+	struct ofono_radio_access *ra = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_debug("Error setting radio access mode");
+		ra->pending_mode = ra->mode;
+		reply = __ofono_error_failed(ra->pending);
+		__ofono_dbus_pending_reply(&ra->pending, reply);
+		return;
+	}
+
+	reply = dbus_message_new_method_return(ra->pending);
+	__ofono_dbus_pending_reply(&ra->pending, reply);
+
+	ofono_radio_access_mode_notify(ra, ra->pending_mode);
+}
+
+static DBusMessage *ra_get_properties(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_radio_access *ra = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+
+	const char *mode = radio_access_mode_to_string(ra->mode);
+
+	reply = dbus_message_new_method_return(msg);
+	if (!reply)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					OFONO_PROPERTIES_ARRAY_SIGNATURE,
+					&dict);
+
+	ofono_dbus_dict_append(&dict, "Mode", DBUS_TYPE_STRING, &mode);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
+static DBusMessage *ra_set_property(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_radio_access *ra = data;
+	DBusMessageIter iter;
+	DBusMessageIter var;
+	const char *property;
+
+	if (ra->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &property);
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&iter, &var);
+
+	if (g_strcmp0(property, "Mode") == 0) {
+		const char *value;
+		int mode = -1;
+
+		if (!ra->driver->set_mode)
+			return __ofono_error_not_implemented(msg);
+
+		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+			return __ofono_error_invalid_args(msg);
+
+		dbus_message_iter_get_basic(&var, &value);
+		mode = string_to_radio_access_mode(value);
+
+		if (ra->mode == mode)
+			return dbus_message_new_method_return(msg);
+
+		ra->pending = dbus_message_ref(msg);
+		ra->pending_mode = mode;
+
+		ra->driver->set_mode(ra, mode, ra_mode_set_callback, ra);
+
+		return NULL;
+	}
+
+	return __ofono_error_invalid_args(msg);
+}
+
+static GDBusMethodTable ra_methods[] = {
+	{ "GetProperties",	"",	"a{sv}",	ra_get_properties,
+							G_DBUS_METHOD_FLAG_ASYNC },
+
+	{ "SetProperty",	"sv",	"",		ra_set_property,
+							G_DBUS_METHOD_FLAG_ASYNC },
+	{ }
+};
+
+static GDBusSignalTable ra_signals[] = {
+	{ "PropertyChanged",	"sv" },
+	{ }
+};
+
+void ofono_radio_access_mode_notify(struct ofono_radio_access *ra, int mode)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	if (ra->mode == mode)
+		return;
+
+	ra->mode = mode;
+
+	if (mode != -1) {
+		const char *path = __ofono_atom_get_path(ra->atom);
+		const char *str_mode = radio_access_mode_to_string(mode);
+
+		ofono_dbus_signal_property_changed(conn, path,
+						RADIO_ACCESS_INTERFACE,
+						"Mode", DBUS_TYPE_STRING,
+						&str_mode);
+	}
+}
+
+int ofono_radio_access_driver_register(const struct ofono_radio_access_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (!d || !d->probe)
+		return -EINVAL;
+
+	g_drivers = g_slist_prepend(g_drivers, (void *)d);
+
+	return 0;
+}
+
+void ofono_radio_access_driver_unregister(const struct ofono_radio_access_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (!d)
+		return;
+
+	g_drivers = g_slist_remove(g_drivers, (void *)d);
+}
+
+static void radio_access_unregister(struct ofono_atom *atom)
+{
+	struct ofono_radio_access *ra = __ofono_atom_get_data(atom);
+	const char *path = __ofono_atom_get_path(ra->atom);
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(ra->atom);
+
+	ofono_modem_remove_interface(modem, RADIO_ACCESS_INTERFACE);
+	g_dbus_unregister_interface(conn, path, RADIO_ACCESS_INTERFACE);
+}
+
+static void radio_access_remove(struct ofono_atom *atom)
+{
+	struct ofono_radio_access *ra = __ofono_atom_get_data(atom);
+
+	DBG("atom: %p", atom);
+
+	if (!ra)
+		return;
+
+	if (ra->driver && ra->driver->remove)
+		ra->driver->remove(ra);
+
+	g_free(ra);
+}
+
+struct ofono_radio_access *ofono_radio_access_create(struct ofono_modem *modem,
+							unsigned int vendor,
+							const char *driver,
+							void *data)
+{
+	struct ofono_radio_access *ra;
+	GSList *l;
+
+	if (!driver)
+		return NULL;
+
+	ra = g_try_new0(struct ofono_radio_access, 1);
+	if (!ra)
+		return NULL;
+
+	ra->mode = -1;
+
+	ra->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_RADIO_ACCESS,
+						radio_access_remove, ra);
+
+	for (l = g_drivers; l; l = l->next) {
+		const struct ofono_radio_access_driver *drv = l->data;
+
+		if (g_strcmp0(drv->name, driver) != 0)
+			continue;
+
+		if (drv->probe(ra, vendor, data) < 0)
+			continue;
+
+		ra->driver = drv;
+		break;
+	}
+
+	return ra;
+}
+
+void ofono_radio_access_register(struct ofono_radio_access *ra)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(ra->atom);
+	const char *path = __ofono_atom_get_path(ra->atom);
+
+	if (!g_dbus_register_interface(conn, path,
+					RADIO_ACCESS_INTERFACE,
+					ra_methods, ra_signals, NULL, ra,
+					NULL)) {
+		ofono_error("Could not create %s interface",
+				RADIO_ACCESS_INTERFACE);
+
+		return;
+	}
+
+	ofono_modem_add_interface(modem, RADIO_ACCESS_INTERFACE);
+
+	if (ra->driver->query_mode)
+		ra->driver->query_mode(ra, ra_mode_query_callback, ra);
+
+	__ofono_atom_register(ra->atom, radio_access_unregister);
+}
+
+void ofono_radio_access_remove(struct ofono_radio_access *ra)
+{
+	__ofono_atom_free(ra->atom);
+}
+
+void ofono_radio_access_set_data(struct ofono_radio_access *ra,
+					void *data)
+{
+	ra->driver_data = data;
+}
+
+void *ofono_radio_access_get_data(struct ofono_radio_access *ra)
+{
+	return ra->driver_data;
+}
-- 
1.6.3.3

[PATCH 2/3] Add PN_GSS code points and debugging

[Aki Niemi]

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

---
 Makefile.am              |    3 +-
 drivers/isimodem/debug.c |   25 +++++++++++++++++++
 drivers/isimodem/debug.h |    5 ++++
 drivers/isimodem/gss.h   |   58 ++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 1 deletions(-)
 create mode 100644 drivers/isimodem/gss.h

diff --git a/Makefile.am b/Makefile.am
index a18e4b9..5f67878 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -102,7 +102,8 @@ builtin_sources += $(gisi_sources) \
 				drivers/isimodem/call-settings.c \
 				drivers/isimodem/call-barring.c \
 				drivers/isimodem/call-meter.c \
-				drivers/isimodem/ss.h
+				drivers/isimodem/ss.h \
+				drivers/isimodem/gss.h
 endif
 
 if ATMODEM
diff --git a/drivers/isimodem/debug.c b/drivers/isimodem/debug.c
index 2f8f1f4..401de9d 100644
--- a/drivers/isimodem/debug.c
+++ b/drivers/isimodem/debug.c
@@ -746,6 +746,25 @@ const char *net_subblock_name(enum net_subblock value)
 	return "NET_<UNKNOWN>";
 }
 
+const char *gss_message_id_name(enum gss_message_id value)
+{
+	switch (value) {
+		_(GSS_CS_SERVICE_REQ);
+		_(GSS_CS_SERVICE_RESP);
+		_(GSS_CS_SERVICE_FAIL_RESP);
+		_(GSS_SELECTED_RAT_IND);
+	}
+	return "GSS_<UNKNOWN>";
+}
+
+const char *gss_subblock_name(enum gss_subblock value)
+{
+	switch (value) {
+		_(GSS_RAT_INFO);
+	}
+	return "GSS_<UNKNOWN>";
+}
+
 #undef _
 
 static void hex_dump(const char *name, const uint8_t m[], size_t len)
@@ -816,3 +835,9 @@ void net_debug(const void *restrict buf, size_t len, void *data)
 	const uint8_t *m = buf;
 	hex_dump(net_message_id_name(m[0]), m, len);
 }
+
+void gss_debug(const void *restrict buf, size_t len, void *data)
+{
+	const uint8_t *m = buf;
+	hex_dump(gss_message_id_name(m[0]), m, len);
+}
diff --git a/drivers/isimodem/debug.h b/drivers/isimodem/debug.h
index b9ee39a..fa316ba 100644
--- a/drivers/isimodem/debug.h
+++ b/drivers/isimodem/debug.h
@@ -29,6 +29,7 @@
 #include "info.h"
 #include "call.h"
 #include "network.h"
+#include "gss.h"
 
 const char *ss_message_id_name(enum ss_message_id value);
 const char *ss_subblock_name(enum ss_subblock value);
@@ -61,6 +62,9 @@ const char *net_status_name(enum net_reg_status value);
 const char *net_message_id_name(enum net_message_id value);
 const char *net_subblock_name(enum net_subblock value);
 
+const char *gss_message_id_name(enum gss_message_id value);
+const char *gss_subblock_name(enum gss_subblock value);
+
 void ss_debug(const void *restrict buf, size_t len, void *data);
 void mtc_debug(const void *restrict buf, size_t len, void *data);
 void sms_debug(const void *restrict buf, size_t len, void *data);
@@ -68,6 +72,7 @@ void sim_debug(const void *restrict buf, size_t len, void *data);
 void info_debug(const void *restrict buf, size_t len, void *data);
 void call_debug(const void *restrict buf, size_t len, void *data);
 void net_debug(const void *restrict buf, size_t len, void *data);
+void gss_debug(const void *restrict buf, size_t len, void *data);
 
 const char *pn_resource_name(int value);
 
diff --git a/drivers/isimodem/gss.h b/drivers/isimodem/gss.h
new file mode 100644
index 0000000..a61c0b0
--- /dev/null
+++ b/drivers/isimodem/gss.h
@@ -0,0 +1,58 @@
+/*
+ * This file is part of oFono - Open Source Telephony
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * 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 __ISIMODEM_GSS_H
+#define __ISIMODEM_GSS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define PN_GSS			0x32
+#define GSS_TIMEOUT		5
+
+enum gss_message_id {
+	GSS_CS_SERVICE_REQ = 0x00,
+	GSS_CS_SERVICE_RESP = 0x01,
+	GSS_CS_SERVICE_FAIL_RESP = 0x02,
+	GSS_SELECTED_RAT_IND = 0x14
+};
+
+enum gss_subblock {
+	GSS_RAT_INFO = 0x0B
+};
+
+enum gss_selection_mode {
+	GSS_DUAL_RAT = 0x00,
+	GSS_GSM_RAT = 0x01,
+	GSS_UMTS_RAT = 0x02
+};
+
+enum gss_operation {
+	GSS_SELECTED_RAT_WRITE = 0x0E,
+	GSS_SELECTED_RAT_READ = 0x9C
+};
+
+#ifdef __cplusplus
+};
+#endif
+
+#endif /* !__ISIMODEM_GSS_H */
-- 
1.6.3.3

[PATCH 3/3] Add isimodem radio access driver

[Aki Niemi]

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

---
 Makefile.am                     |    1 +
 drivers/isimodem/isimodem.c     |    4 +
 drivers/isimodem/isimodem.h     |    3 +
 drivers/isimodem/radio-access.c |  299 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 307 insertions(+), 0 deletions(-)
 create mode 100644 drivers/isimodem/radio-access.c

diff --git a/Makefile.am b/Makefile.am
index 5f67878..235aa86 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -103,6 +103,7 @@ builtin_sources += $(gisi_sources) \
 				drivers/isimodem/call-barring.c \
 				drivers/isimodem/call-meter.c \
 				drivers/isimodem/ss.h \
+				drivers/isimodem/radio-access.c \
 				drivers/isimodem/gss.h
 endif
 
diff --git a/drivers/isimodem/isimodem.c b/drivers/isimodem/isimodem.c
index 48f41c3..4b65d21 100644
--- a/drivers/isimodem/isimodem.c
+++ b/drivers/isimodem/isimodem.c
@@ -49,6 +49,7 @@
 #include <ofono/call-settings.h>
 #include <ofono/call-barring.h>
 #include <ofono/call-meter.h>
+#include <ofono/radio-access.h>
 
 #include "isimodem.h"
 #include "isiutil.h"
@@ -310,6 +311,7 @@ static void isi_modem_post_sim(struct ofono_modem *modem)
 	ofono_call_settings_create(isi->modem, 0, "isimodem", isi->idx);
 	ofono_call_barring_create(isi->modem, 0, "isimodem", isi->idx);
 	ofono_call_meter_create(isi->modem, 0, "isimodem", isi->idx);
+	ofono_radio_access_create(isi->modem, 0, "isimodem", isi->idx);
 }
 
 static struct ofono_modem_driver driver = {
@@ -339,6 +341,7 @@ static int isimodem_init(void)
 	isi_call_settings_init();
 	isi_call_barring_init();
 	isi_call_meter_init();
+	isi_radio_access_init();
 
 	ofono_modem_driver_register(&driver);
 
@@ -379,6 +382,7 @@ static void isimodem_exit(void)
 	isi_call_settings_exit();
 	isi_call_barring_exit();
 	isi_call_meter_exit();
+	isi_radio_access_exit();
 }
 
 OFONO_PLUGIN_DEFINE(isimodem, "PhoNet / ISI modem driver", VERSION,
diff --git a/drivers/isimodem/isimodem.h b/drivers/isimodem/isimodem.h
index 3eb2fbe..2b3d756 100644
--- a/drivers/isimodem/isimodem.h
+++ b/drivers/isimodem/isimodem.h
@@ -57,3 +57,6 @@ extern void isi_call_barring_exit();
 
 extern void isi_call_meter_init();
 extern void isi_call_meter_exit();
+
+extern void isi_radio_access_init();
+extern void isi_radio_access_exit();
diff --git a/drivers/isimodem/radio-access.c b/drivers/isimodem/radio-access.c
new file mode 100644
index 0000000..8c9e0b5
--- /dev/null
+++ b/drivers/isimodem/radio-access.c
@@ -0,0 +1,299 @@
+/*
+ * This file is part of oFono - Open Source Telephony
+ *
+ * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ * 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 <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+
+#include <glib.h>
+
+#include <gisi/client.h>
+#include <gisi/iter.h>
+
+#include <ofono/log.h>
+#include <ofono/modem.h>
+#include <ofono/radio-access.h>
+
+#include "isimodem.h"
+#include "isiutil.h"
+#include "debug.h"
+#include "gss.h"
+
+struct access_data {
+	GIsiClient *client;
+};
+
+static bool rat_mode_read_resp_cb(GIsiClient *client, const void *restrict data,
+					size_t len, uint16_t object, void *opaque)
+{
+	const unsigned char *msg = data;
+	struct isi_cb_data *cbd = opaque;
+	ofono_radio_access_mode_query_cb_t cb = cbd->cb;
+	int mode = -1;
+
+	if (!msg) {
+		DBG("ISI client error: %d", g_isi_client_error(client));
+		goto error;
+	}
+
+	if (len < 3) {
+		DBG("truncated message");
+		return false;
+	}
+
+	if (msg[0] == GSS_CS_SERVICE_FAIL_RESP)
+		goto error;
+
+	if (msg[0] == GSS_CS_SERVICE_RESP) {
+		GIsiSubBlockIter iter;
+
+		for (g_isi_sb_iter_init(&iter, msg, len, 3);
+			g_isi_sb_iter_is_valid(&iter);
+			g_isi_sb_iter_next(&iter)) {
+
+			switch (g_isi_sb_iter_get_id(&iter)) {
+
+			case GSS_RAT_INFO: {
+
+				guint8 byte;
+
+				if (!g_isi_sb_iter_get_byte(&iter, &byte, 2))
+					goto error;
+
+				mode = (int)byte;
+
+				break;
+			}
+			default:
+				DBG("Skipping sub-block: %s (%zu bytes)",
+					gss_subblock_name(g_isi_sb_iter_get_id(&iter)),
+					g_isi_sb_iter_get_len(&iter));
+				break;
+			}
+		}
+
+		CALLBACK_WITH_SUCCESS(cb, mode, cbd->data);
+		goto out;
+	}
+
+	return false;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, -1, cbd->data);
+
+out:
+	g_free(cbd);
+	return true;
+}
+
+static void isi_query_mode(struct ofono_radio_access *ra,
+				ofono_radio_access_mode_query_cb_t cb,
+				void *data)
+{
+	struct access_data *access = ofono_radio_access_get_data(ra);
+	struct isi_cb_data *cbd = isi_cb_data_new(access, cb, data);
+
+	const unsigned char msg[] = {
+		GSS_CS_SERVICE_REQ,
+		GSS_SELECTED_RAT_READ,
+		0x00 /* subblock count */
+	};
+
+	if (!cbd)
+		goto error;
+
+	if (g_isi_request_make(access->client, msg, sizeof(msg),
+				GSS_TIMEOUT, rat_mode_read_resp_cb, cbd))
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, -1, data);
+	g_free(cbd);
+}
+
+static bool rat_mode_write_resp_cb(GIsiClient *client, const void *restrict data,
+					size_t len, uint16_t object, void *opaque)
+{
+	const unsigned char *msg = data;
+	struct isi_cb_data *cbd = opaque;
+	ofono_radio_access_mode_set_cb_t cb = cbd->cb;
+
+	if (!msg) {
+		DBG("ISI client error: %d", g_isi_client_error(client));
+		goto error;
+	}
+
+	if (len < 3) {
+		DBG("truncated message");
+		return false;
+	}
+
+	if (msg[0] == GSS_CS_SERVICE_FAIL_RESP)
+		goto error;
+
+	if (msg[0] == GSS_CS_SERVICE_RESP) {
+		CALLBACK_WITH_SUCCESS(cb, cbd->data);
+		goto out;
+	}
+
+	return false;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+out:
+	g_free(cbd);
+	return true;
+}
+
+static void isi_set_mode(struct ofono_radio_access *ra, int mode,
+				ofono_radio_access_mode_set_cb_t cb,
+				void *data)
+{
+	struct access_data *access = ofono_radio_access_get_data(ra);
+	struct isi_cb_data *cbd = isi_cb_data_new(access, cb, data);
+
+	const unsigned char msg[] = {
+		GSS_CS_SERVICE_REQ,
+		GSS_SELECTED_RAT_WRITE,
+		0x01, /* subblock count */
+		GSS_RAT_INFO,
+		0x04, /* subblock length */
+		mode,
+		0x00 /* filler */
+	};
+
+	if (!cbd)
+		goto error;
+
+	if (g_isi_request_make(access->client, msg, sizeof(msg),
+				GSS_TIMEOUT, rat_mode_write_resp_cb, cbd))
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
+	g_free(cbd);
+}
+
+static void rat_mode_ind_cb(GIsiClient *client, const void *restrict data,
+				size_t len, uint16_t object, void *opaque)
+{
+	const unsigned char *msg = data;
+	struct ofono_radio_access *ra = opaque;
+
+	if (!msg || len < 3 || msg[0] != GSS_SELECTED_RAT_IND)
+		return;
+
+	ofono_radio_access_mode_notify(ra, msg[1]);
+}
+
+static gboolean isi_radio_access_register(gpointer user)
+{
+	struct ofono_radio_access *ra = user;
+	struct access_data *access = ofono_radio_access_get_data(ra);
+
+	const char *debug = getenv("OFONO_ISI_DEBUG");
+
+	if (debug && (g_strcmp0(debug, "all") == 0
+		|| g_strcmp0(debug, "gss") == 0))
+		g_isi_client_set_debug(access->client, gss_debug, NULL);
+
+	g_isi_subscribe(access->client, GSS_SELECTED_RAT_IND,
+			rat_mode_ind_cb, ra);
+
+	ofono_radio_access_register(ra);
+
+	return FALSE;
+}
+
+static void reachable_cb(GIsiClient *client, bool alive, uint16_t object,
+				void *opaque)
+{
+	struct ofono_radio_access *ra = opaque;
+
+	if (!alive) {
+		DBG("devinfo driver bootstrap failed");
+		return;
+	}
+
+	DBG("%s (v%03d.%03d) reachable",
+		pn_resource_name(g_isi_client_resource(client)),
+		g_isi_version_major(client),
+		g_isi_version_minor(client));
+
+	g_idle_add(isi_radio_access_register, ra);
+}
+
+static int isi_radio_access_probe(struct ofono_radio_access *ra, unsigned int vendor,
+					void *user)
+{
+	GIsiModem *idx = user;
+	struct access_data *data = g_try_new0(struct access_data, 1);
+
+	if (!data)
+		return -ENOMEM;
+
+	data->client = g_isi_client_create(idx, PN_GSS);
+	if (!data->client) {
+		g_free(data);
+		return -ENOMEM;
+	}
+
+	ofono_radio_access_set_data(ra, data);
+
+	g_isi_verify(data->client, reachable_cb, ra);
+
+	return 0;
+}
+
+static void isi_radio_access_remove(struct ofono_radio_access *ra)
+{
+	struct access_data *data = ofono_radio_access_get_data(ra);
+
+	if (data) {
+		g_isi_client_destroy(data->client);
+		g_free(data);
+	}
+}
+
+static struct ofono_radio_access_driver driver = {
+	.name			= "isimodem",
+	.probe			= isi_radio_access_probe,
+	.remove			= isi_radio_access_remove,
+	.query_mode		= isi_query_mode,
+	.set_mode		= isi_set_mode
+};
+
+void isi_radio_access_init()
+{
+	ofono_radio_access_driver_register(&driver);
+}
+
+void isi_radio_access_exit()
+{
+	ofono_radio_access_driver_unregister(&driver);
+}
-- 
1.6.3.3

Re: [PATCH 1/3] Add radio access atom and driver API

[Denis Kenzior]

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

Hi Aki,

> This atom provides access to the modem's radio access properties. It
> currently includes a single rw property, namely the radio access
> selection mode setting.

It might be helpful to include the API documentation too.  One thing I don't 
like is the name RadioAccess.  The connotation is a bit too low-level.  
Perhaps BandSelection? RadioBandSelection?

> 
> This allows the user to query and select the used radio access
> technology preference. In dual mode, either 2G or 3G is used depending
> on reception. 2G only mode or 3G only mode force selection of the
> respective access only.
> 
> In the future, this atom could be extended, if necessary, to handle
> other radio access related modem features such as CELL_DCH status,
> UTRAN channel, or frequency band.
> ---
>  Makefile.am            |    6 +-
>  include/radio-access.h |   72 ++++++++++
>  src/ofono.h            |    2 +
>  src/radio-access.c     |  354
>  ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 432
>  insertions(+), 2 deletions(-)
>  create mode 100644 include/radio-access.h
>  create mode 100644 src/radio-access.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 33339df..a18e4b9 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -11,7 +11,8 @@ include_HEADERS = include/log.h include/plugin.h
>  include/history.h \ include/sms.h include/sim.h include/message-waiting.h
>  \
>  			include/netreg.h include/voicecall.h include/devinfo.h \
>  			include/cbs.h include/call-volume.h \
> -			include/gprs.h include/gprs-context.h
> +			include/gprs.h include/gprs-context.h \
> +			include/radio-access.h
> 
>  nodist_include_HEADERS = include/version.h
> 
> @@ -224,7 +225,8 @@ src_ofonod_SOURCES = $(gdbus_sources)
>  $(builtin_sources) \ src/phonebook.c src/history.c src/message-waiting.c \
>  			src/simutil.h src/simutil.c src/storage.h \
>  			src/storage.c src/cbs.c src/watch.c src/call-volume.c \
> -			src/gprs.c src/idmap.h src/idmap.c
> +			src/gprs.c src/idmap.h src/idmap.c \
> +			src/radio-access.c
> 
>  src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ -ldl
> 
> diff --git a/include/radio-access.h b/include/radio-access.h
> new file mode 100644
> index 0000000..6677573
> --- /dev/null
> +++ b/include/radio-access.h
> @@ -0,0 +1,72 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
> + *
> + *  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_RADIO_ACCESS_H
> +#define __OFONO_RADIO_ACCESS_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <ofono/types.h>
> +
> +struct ofono_radio_access;
> +
> +typedef void (*ofono_radio_access_mode_set_cb_t)(const struct ofono_error
>  *error, +							void *data);
> +typedef void (*ofono_radio_access_mode_query_cb_t)(const struct
>  ofono_error *error, +							int mode, void *data);

I really prefer mode to be an enum here.  There's no standard to reference 
here...

> +
> +struct ofono_radio_access_driver {
> +	const char *name;
> +	int (*probe)(struct ofono_radio_access *radio_access,
> +			unsigned int vendor,
> +			void *data);
> +	void (*remove)(struct ofono_radio_access *radio_access);
> +	void (*query_mode)(struct ofono_radio_access *radio_access,
> +				ofono_radio_access_mode_query_cb_t cb, void *data);
> +	void (*set_mode)(struct ofono_radio_access *radio_access, int mode,
> +				ofono_radio_access_mode_set_cb_t cb, void *data);

Same here, mode should be an enum.

> +};
> +
> +void ofono_radio_access_mode_notify(struct ofono_radio_access
>  *radio_access, +					int mode);

Why do we have this function, can the radio mode be changed outside oFono's 
control?

> +
> +int ofono_radio_access_driver_register(const struct
>  ofono_radio_access_driver *d); +void
>  ofono_radio_access_driver_unregister(const struct
>  ofono_radio_access_driver *d); +
> +struct ofono_radio_access *ofono_radio_access_create(struct ofono_modem
>  *modem, +						unsigned int vendor,
> +						const char *driver,
> +						void *data);
> +
> +void ofono_radio_access_register(struct ofono_radio_access *radio_access);
> +void ofono_radio_access_remove(struct ofono_radio_access *radio_access);
> +
> +void ofono_radio_access_set_data(struct ofono_radio_access *radio_access,
> +					void *data);
> +void *ofono_radio_access_get_data(struct ofono_radio_access
>  *radio_access); +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __OFONO_RADIO_ACCESS_H */
> diff --git a/src/ofono.h b/src/ofono.h
> index 379f413..c519eef 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -113,6 +113,7 @@ enum ofono_atom_type {
>  	OFONO_ATOM_TYPES_CALL_VOLUME = 15,
>  	OFONO_ATOM_TYPE_GPRS = 16,
>  	OFONO_ATOM_TYPE_GPRS_CONTEXT = 17,
> +	OFONO_ATOM_TYPE_RADIO_ACCESS = 18,
>  };
> 
>  enum ofono_atom_watch_condition {
> @@ -168,6 +169,7 @@ void __ofono_atom_free(struct ofono_atom *atom);
>  #include <ofono/voicecall.h>
>  #include <ofono/gprs.h>
>  #include <ofono/gprs-context.h>
> +#include <ofono/radio-access.h>
> 
>  #include <ofono/sim.h>
> 
> diff --git a/src/radio-access.c b/src/radio-access.c
> new file mode 100644
> index 0000000..ba34c71
> --- /dev/null
> +++ b/src/radio-access.c
> @@ -0,0 +1,354 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 
>  USA + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <string.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +#include <gdbus.h>
> +
> +#include "ofono.h"
> +#include "common.h"
> +
> +#define RADIO_ACCESS_INTERFACE "org.ofono.RadioAccess"
> +#define RADIO_ACCESS_INTERFACE "org.ofono.RadioAccess"
> +
> +static GSList *g_drivers = NULL;
> +
> +enum radio_access_mode {
> +	RADIO_ACCESS_MODE_DUAL = 0,
> +	RADIO_ACCESS_MODE_2G = 1,
> +	RADIO_ACCESS_MODE_3G = 2
> +};
> +
> +struct ofono_radio_access {
> +	DBusMessage *pending;
> +	int mode;
> +	int pending_mode;
> +	const struct ofono_radio_access_driver *driver;
> +	void *driver_data;
> +	struct ofono_atom *atom;
> +};
> +
> +static const char *radio_access_mode_to_string(int mode)
> +{
> +	switch (mode) {
> +	case RADIO_ACCESS_MODE_DUAL:
> +		return "2g+3g";
> +
> +	case RADIO_ACCESS_MODE_2G:
> +		return "2g";
> +
> +	case RADIO_ACCESS_MODE_3G:
> +		return "3g";
> +
> +	default:
> +		return "unknown";
> +	}
> +}
> +
> +static int string_to_radio_access_mode(const char *mode)
> +{
> +	if (g_strcmp0(mode, "2g+3g") == 0)
> +		return RADIO_ACCESS_MODE_DUAL;
> +
> +	if (g_strcmp0(mode, "2g") == 0)
> +		return RADIO_ACCESS_MODE_2G;
> +
> +	if (g_strcmp0(mode, "3g") == 0)
> +		return RADIO_ACCESS_MODE_3G;
> +
> +	return -1;
> +}
> +
> +static void ra_mode_query_callback(const struct ofono_error *error, int
>  mode, +					void *data)
> +{
> +	struct ofono_radio_access *ra = data;
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		ofono_debug("Error during radio access mode query");
> +		return;
> +	}
> +
> +	ofono_radio_access_mode_notify(ra, mode);
> +}
> +
> +static void ra_mode_set_callback(const struct ofono_error *error, void
>  *data) +{
> +	struct ofono_radio_access *ra = data;
> +	DBusMessage *reply;
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		ofono_debug("Error setting radio access mode");
> +		ra->pending_mode = ra->mode;
> +		reply = __ofono_error_failed(ra->pending);
> +		__ofono_dbus_pending_reply(&ra->pending, reply);
> +		return;
> +	}
> +
> +	reply = dbus_message_new_method_return(ra->pending);
> +	__ofono_dbus_pending_reply(&ra->pending, reply);
> +
> +	ofono_radio_access_mode_notify(ra, ra->pending_mode);
> +}
> +
> +static DBusMessage *ra_get_properties(DBusConnection *conn, DBusMessage
>  *msg, +					void *data)
> +{
> +	struct ofono_radio_access *ra = data;
> +	DBusMessage *reply;
> +	DBusMessageIter iter;
> +	DBusMessageIter dict;
> +
> +	const char *mode = radio_access_mode_to_string(ra->mode);
> +
> +	reply = dbus_message_new_method_return(msg);
> +	if (!reply)
> +		return NULL;
> +
> +	dbus_message_iter_init_append(reply, &iter);
> +
> +	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> +					OFONO_PROPERTIES_ARRAY_SIGNATURE,
> +					&dict);
> +
> +	ofono_dbus_dict_append(&dict, "Mode", DBUS_TYPE_STRING, &mode);
> +
> +	dbus_message_iter_close_container(&iter, &dict);
> +
> +	return reply;

The pattern used in the past was to query the mode from get_properties if it 
was not cached.  Once the mode was cached, we set a flag and always returned 
the cached value.  I would do it the same way in this case to minimize flood of 
queries at modem startup.

Is querying the mode even a good idea?  Perhaps this should be yet another 
setting, stored by IMSI.

> +}
> +
> +static DBusMessage *ra_set_property(DBusConnection *conn, DBusMessage
>  *msg, +					void *data)
> +{
> +	struct ofono_radio_access *ra = data;
> +	DBusMessageIter iter;
> +	DBusMessageIter var;
> +	const char *property;
> +
> +	if (ra->pending)
> +		return __ofono_error_busy(msg);
> +
> +	if (!dbus_message_iter_init(msg, &iter))
> +		return __ofono_error_invalid_args(msg);
> +
> +	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> +		return __ofono_error_invalid_args(msg);
> +
> +	dbus_message_iter_get_basic(&iter, &property);
> +	dbus_message_iter_next(&iter);
> +
> +	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> +		return __ofono_error_invalid_args(msg);
> +
> +	dbus_message_iter_recurse(&iter, &var);
> +
> +	if (g_strcmp0(property, "Mode") == 0) {
> +		const char *value;
> +		int mode = -1;
> +
> +		if (!ra->driver->set_mode)
> +			return __ofono_error_not_implemented(msg);
> +
> +		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
> +			return __ofono_error_invalid_args(msg);
> +
> +		dbus_message_iter_get_basic(&var, &value);
> +		mode = string_to_radio_access_mode(value);
> +
> +		if (ra->mode == mode)
> +			return dbus_message_new_method_return(msg);
> +
> +		ra->pending = dbus_message_ref(msg);
> +		ra->pending_mode = mode;
> +
> +		ra->driver->set_mode(ra, mode, ra_mode_set_callback, ra);
> +
> +		return NULL;
> +	}
> +
> +	return __ofono_error_invalid_args(msg);
> +}
> +
> +static GDBusMethodTable ra_methods[] = {
> +	{ "GetProperties",	"",	"a{sv}",	ra_get_properties,
> +							G_DBUS_METHOD_FLAG_ASYNC },

Why is this set to ASYNC?  The current implementation does not need it.

> +
> +	{ "SetProperty",	"sv",	"",		ra_set_property,
> +							G_DBUS_METHOD_FLAG_ASYNC },
> +	{ }
> +};
> +
> +static GDBusSignalTable ra_signals[] = {
> +	{ "PropertyChanged",	"sv" },
> +	{ }
> +};
> +
> +void ofono_radio_access_mode_notify(struct ofono_radio_access *ra, int
>  mode) +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +
> +	if (ra->mode == mode)
> +		return;
> +
> +	ra->mode = mode;
> +
> +	if (mode != -1) {
> +		const char *path = __ofono_atom_get_path(ra->atom);
> +		const char *str_mode = radio_access_mode_to_string(mode);
> +
> +		ofono_dbus_signal_property_changed(conn, path,
> +						RADIO_ACCESS_INTERFACE,
> +						"Mode", DBUS_TYPE_STRING,
> +						&str_mode);
> +	}
> +}
> +
> +int ofono_radio_access_driver_register(const struct
>  ofono_radio_access_driver *d) +{
> +	DBG("driver: %p, name: %s", d, d->name);
> +
> +	if (!d || !d->probe)
> +		return -EINVAL;
> +
> +	g_drivers = g_slist_prepend(g_drivers, (void *)d);
> +
> +	return 0;
> +}
> +
> +void ofono_radio_access_driver_unregister(const struct
>  ofono_radio_access_driver *d) +{
> +	DBG("driver: %p, name: %s", d, d->name);
> +
> +	if (!d)
> +		return;
> +
> +	g_drivers = g_slist_remove(g_drivers, (void *)d);
> +}
> +
> +static void radio_access_unregister(struct ofono_atom *atom)
> +{
> +	struct ofono_radio_access *ra = __ofono_atom_get_data(atom);
> +	const char *path = __ofono_atom_get_path(ra->atom);
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	struct ofono_modem *modem = __ofono_atom_get_modem(ra->atom);
> +
> +	ofono_modem_remove_interface(modem, RADIO_ACCESS_INTERFACE);
> +	g_dbus_unregister_interface(conn, path, RADIO_ACCESS_INTERFACE);
> +}
> +
> +static void radio_access_remove(struct ofono_atom *atom)
> +{
> +	struct ofono_radio_access *ra = __ofono_atom_get_data(atom);
> +
> +	DBG("atom: %p", atom);
> +
> +	if (!ra)
> +		return;
> +
> +	if (ra->driver && ra->driver->remove)
> +		ra->driver->remove(ra);
> +
> +	g_free(ra);
> +}
> +
> +struct ofono_radio_access *ofono_radio_access_create(struct ofono_modem
>  *modem, +							unsigned int vendor,
> +							const char *driver,
> +							void *data)
> +{
> +	struct ofono_radio_access *ra;
> +	GSList *l;
> +
> +	if (!driver)
> +		return NULL;
> +
> +	ra = g_try_new0(struct ofono_radio_access, 1);
> +	if (!ra)
> +		return NULL;
> +
> +	ra->mode = -1;
> +
> +	ra->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_RADIO_ACCESS,
> +						radio_access_remove, ra);
> +
> +	for (l = g_drivers; l; l = l->next) {
> +		const struct ofono_radio_access_driver *drv = l->data;
> +
> +		if (g_strcmp0(drv->name, driver) != 0)
> +			continue;
> +
> +		if (drv->probe(ra, vendor, data) < 0)
> +			continue;
> +
> +		ra->driver = drv;
> +		break;
> +	}
> +
> +	return ra;
> +}
> +
> +void ofono_radio_access_register(struct ofono_radio_access *ra)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	struct ofono_modem *modem = __ofono_atom_get_modem(ra->atom);
> +	const char *path = __ofono_atom_get_path(ra->atom);
> +
> +	if (!g_dbus_register_interface(conn, path,
> +					RADIO_ACCESS_INTERFACE,
> +					ra_methods, ra_signals, NULL, ra,
> +					NULL)) {
> +		ofono_error("Could not create %s interface",
> +				RADIO_ACCESS_INTERFACE);
> +
> +		return;
> +	}
> +
> +	ofono_modem_add_interface(modem, RADIO_ACCESS_INTERFACE);
> +
> +	if (ra->driver->query_mode)
> +		ra->driver->query_mode(ra, ra_mode_query_callback, ra);
> +
> +	__ofono_atom_register(ra->atom, radio_access_unregister);
> +}
> +
> +void ofono_radio_access_remove(struct ofono_radio_access *ra)
> +{
> +	__ofono_atom_free(ra->atom);
> +}
> +
> +void ofono_radio_access_set_data(struct ofono_radio_access *ra,
> +					void *data)
> +{
> +	ra->driver_data = data;
> +}
> +
> +void *ofono_radio_access_get_data(struct ofono_radio_access *ra)
> +{
> +	return ra->driver_data;
> +}
> 

RE: [PATCH 1/3] Add radio access atom and driver API

[Bastian, Waldo]

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

> > This atom provides access to the modem's radio access properties. It
> > currently includes a single rw property, namely the radio access
> > selection mode setting.
> 
> It might be helpful to include the API documentation too.  One thing I
> don't like is the name RadioAccess.  The connotation is a bit too
> low-level. Perhaps BandSelection? RadioBandSelection?

Radio access is a pretty broad high level concept. It's one of those black boxes you draw somewhere near the bottom of a phone architecture diagram, so in that sense it is low-level but that comes with the territory. BandSelection in my mind implies frequency band selection, which is a narrow aspect of radio access. The 2G/3G selection covered with this patch is about radio access _technology_, which is distinct from the actual frequency bands used by those technologies. I think RadioAccess is a fairly good name for an interface that has the potential to cover various radio related settings.

Use case for 2G/3G selection:
http://androidcommunity.com/forums/archive/index.php/t-11857.html

Use case for band selection:
http://support.t-mobile.com/doc/tm53294.xml?docid=5222&referring%20topicid=67&A2L.SERVICE=&Referring%20TopicID/DocID%20List%20Index=y&navtypeid=2&pagetypeid=26&prevPageIndex=3

Cheers,
Waldo

Re: [PATCH 1/3] Add radio access atom and driver API

[Denis Kenzior]

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

Hi Waldo,

> > > This atom provides access to the modem's radio access properties. It
> > > currently includes a single rw property, namely the radio access
> > > selection mode setting.
> >
> > It might be helpful to include the API documentation too.  One thing I
> > don't like is the name RadioAccess.  The connotation is a bit too
> > low-level. Perhaps BandSelection? RadioBandSelection?
> 
> Radio access is a pretty broad high level concept. It's one of those black
>  boxes you draw somewhere near the bottom of a phone architecture diagram,
>  so in that sense it is low-level but that comes with the territory.
>  BandSelection in my mind implies frequency band selection, which is a
>  narrow aspect of radio access. The 2G/3G selection covered with this patch
>  is about radio access _technology_, which is distinct from the actual
>  frequency bands used by those technologies. I think RadioAccess is a
>  fairly good name for an interface that has the potential to cover various
>  radio related settings.

People on this list keep forgetting two things:
1. We're not designing a kitchen sink API here.  Most of the 'radio related 
settings' will simply never be exposed, nobody really cares what UMTS channel 
he/she is currently on.
2. We're designing the API to be easy to use for everyone, not just GSM geeks.

In your AM/FM Tuner the 'Band' button switches between AM and FM.  You 
immediately know what the button does.  The interface name in oFono should 
ideally give the user an immediate idea of what it is for.  In my view 
"RadioAccess" does not fit this ideal.  Perhaps "BandSelection" is the wrong 
name too, lets come up with a better one.

Regards,
-Denis

RE: [PATCH 1/3] Add radio access atom and driver API

[Bastian, Waldo]

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

> People on this list keep forgetting two things:
> 1. We're not designing a kitchen sink API here.  Most of the 'radio
> related
> settings' will simply never be exposed, nobody really cares what UMTS
> channel
> he/she is currently on.
> 2. We're designing the API to be easy to use for everyone, not just GSM
> geeks.
> 
> In your AM/FM Tuner the 'Band' button switches between AM and FM.  You
> immediately know what the button does.  The interface name in oFono should
> ideally give the user an immediate idea of what it is for.  In my view
> "RadioAccess" does not fit this ideal.  Perhaps "BandSelection" is the
> wrong name too, lets come up with a better one.

StuffThatCustomerServiceTellsYouToUseWhenThingsDontQuiteWork

Cheers,
Waldo

Re: [PATCH 1/3] Add radio access atom and driver API

[Aki Niemi]

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

ke, 2010-02-03 kello 01:04 +0100, ext Denis Kenzior kirjoitti:
> It might be helpful to include the API documentation too.  One thing I don't
> like is the name RadioAccess.  The connotation is a bit too low-level.
> Perhaps BandSelection? RadioBandSelection?

I thought about this quite a bit, but this was the best name I could
come up with. RadioAccessSelection, maybe, but then that'd be too
specific if we wanted to add a property.

And I think we definitely need to add at least one property here, a
boolean "Active" for when the radio link is active. This is used by
applications as a power saving measure to time their non-urgent data
traffic to only when the radio link is in CELL_DCH state already.

In fact, perhaps I should add that now while at it.

> I really prefer mode to be an enum here.  There's no standard to reference
> here...

Sure, should that go to include/types.h or src/common.h?

> > +void ofono_radio_access_mode_notify(struct ofono_radio_access
> >  *radio_access, +                                     int mode);
>
> Why do we have this function, can the radio mode be changed outside oFono's
> control?

This can happen if you are using the phone as a modem on the desktop,
and you change the setting on the phone.

> > +static DBusMessage *ra_get_properties(DBusConnection *conn, DBusMessage
> >  *msg, +                                      void *data)
> > +{
> > +     struct ofono_radio_access *ra = data;
> > +     DBusMessage *reply;
> > +     DBusMessageIter iter;
> > +     DBusMessageIter dict;
> > +
> > +     const char *mode = radio_access_mode_to_string(ra->mode);
> > +
> > +     reply = dbus_message_new_method_return(msg);
> > +     if (!reply)
> > +             return NULL;
> > +
> > +     dbus_message_iter_init_append(reply, &iter);
> > +
> > +     dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> > +                                     OFONO_PROPERTIES_ARRAY_SIGNATURE,
> > +                                     &dict);
> > +
> > +     ofono_dbus_dict_append(&dict, "Mode", DBUS_TYPE_STRING, &mode);
> > +
> > +     dbus_message_iter_close_container(&iter, &dict);
> > +
> > +     return reply;
> 
> The pattern used in the past was to query the mode from get_properties if it
> was not cached.  Once the mode was cached, we set a flag and always returned
> the cached value.  I would do it the same way in this case to minimize flood of
> queries at modem startup.

I noticed call-forwarding was using it. However, when you have more than
one property to cache, of which some are optional, that initial chain of
queries to fill the cache gets a little nasty.

I've not seen problems in practice in filling the cache at modem
startup. (That's something CSD does.)

> Is querying the mode even a good idea?  Perhaps this should be yet another
> setting, stored by IMSI.

AFAIK, this is not an IMSI specific setting; it persists over a SIM
swap.

I'll create an updated set shortly.

Cheers,
Aki

Re: [PATCH 1/3] Add radio access atom and driver API

[Aki Niemi]

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

ke, 2010-02-03 kello 05:02 +0100, ext Denis Kenzior kirjoitti:
> People on this list keep forgetting two things:
> 1. We're not designing a kitchen sink API here.  Most of the 'radio related 
> settings' will simply never be exposed, nobody really cares what UMTS channel 
> he/she is currently on.

There's the link status I mentioned earlier that needs to be exposed. I
now regret not writing it there in the first place. ;)

> 2. We're designing the API to be easy to use for everyone, not just GSM geeks.

Currently, we're not doing a very good job at it, though:

From 366e4dac44734e20ad44194d8c0ab808eac082d4 Mon Sep 17 00:00:00 2001
From: Aki Niemi <aki.niemi@nokia.com>
Date: Wed, 3 Feb 2010 11:25:36 +0200
Subject: [PATCH] Technology names to use layman's terms

---
 src/common.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/src/common.c b/src/common.c
index db3e38b..a445616 100644
--- a/src/common.c
+++ b/src/common.c
@@ -622,21 +622,18 @@ const char *registration_tech_to_string(int tech)
 {
 	switch (tech) {
 	case ACCESS_TECHNOLOGY_GSM:
-		return "GSM";
 	case ACCESS_TECHNOLOGY_GSM_COMPACT:
-		return "GSMCompact";
+		return "2G";
 	case ACCESS_TECHNOLOGY_UTRAN:
-		return "UTRAN";
+		return "3G";
 	case ACCESS_TECHNOLOGY_GSM_EGPRS:
-		return "GSM+EGPRS";
+		return "2.5G";
 	case ACCESS_TECHNOLOGY_UTRAN_HSDPA:
-		return "UTRAN+HSDPA";
 	case ACCESS_TECHNOLOGY_UTRAN_HSUPA:
-		return "UTRAN+HSUPA";
 	case ACCESS_TECHNOLOGY_UTRAN_HSDPA_HSUPA:
-		return "UTRAN+HSDPA+HSUPA";
+		return "3.5G";
 	case ACCESS_TECHNOLOGY_EUTRAN:
-		return "EUTRAN";
+		return "4G";
 	default:
 		return "";
 	}
-- 
1.6.3.3

Cheers,
Aki

[PATCH 1/3] Add radio access atom and driver API

[Aki Niemi]

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

This atom provides access to the modem's radio access properties. It
currently includes an rw property for radio access selection mode, and
a readonly property for radio access link state.

This interface allows the user to query and select the preferred radio
access mode. In dual mode, either 2G or 3G is used depending on
reception. 2G only mode or 3G only mode force selection to the
respective access only.

The status property allows applications to time their non-urgent
network access to when the radio access link is already active. This
is an important enabler for saving battery.
---
 Makefile.am              |    6 +-
 doc/radio-access-api.txt |   58 +++++++
 include/radio-access.h   |   85 ++++++++++
 src/ofono.h              |    2 +
 src/radio-access.c       |  384 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 533 insertions(+), 2 deletions(-)
 create mode 100644 doc/radio-access-api.txt
 create mode 100644 include/radio-access.h
 create mode 100644 src/radio-access.c

diff --git a/Makefile.am b/Makefile.am
index 33339df..a18e4b9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -11,7 +11,8 @@ include_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/sms.h include/sim.h include/message-waiting.h \
 			include/netreg.h include/voicecall.h include/devinfo.h \
 			include/cbs.h include/call-volume.h \
-			include/gprs.h include/gprs-context.h
+			include/gprs.h include/gprs-context.h \
+			include/radio-access.h
 
 nodist_include_HEADERS = include/version.h
 
@@ -224,7 +225,8 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) \
 			src/phonebook.c src/history.c src/message-waiting.c \
 			src/simutil.h src/simutil.c src/storage.h \
 			src/storage.c src/cbs.c src/watch.c src/call-volume.c \
-			src/gprs.c src/idmap.h src/idmap.c
+			src/gprs.c src/idmap.h src/idmap.c \
+			src/radio-access.c
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ -ldl
 
diff --git a/doc/radio-access-api.txt b/doc/radio-access-api.txt
new file mode 100644
index 0000000..018c5e7
--- /dev/null
+++ b/doc/radio-access-api.txt
@@ -0,0 +1,58 @@
+Radio Access hierarchy
+======================
+
+Service		org.ofono
+Interface	org.ofono.RadioAccess
+Object path	[variable prefix]/{modem0,modem1,...}
+
+Methods		dict GetProperties()
+
+			Returns all radio access properties. See the
+			properties section for available properties.
+
+			Possible Errors: [service].Error.InvalidArguments
+
+		void SetProperty(string name, variant value)
+
+			Changes the value of the specified property. Only
+			properties that are listed as read-write are
+			changeable. On success a PropertyChanged signal
+			will be emitted.
+
+			Possible Errors: [service].Error.InvalidArguments
+					 [service].Error.DoesNotExist
+					 [service].Error.InProgress
+
+Signals		PropertyChanged(string property, variant value)
+
+			This signal indicates a changed value of the given
+			property.
+
+Properties	string Mode [read-write]
+
+			The current radio access selection mode, also known
+			as network preference.
+
+			The possible values are:
+				"2g+3g"    Radio access selection is done
+					   automatically, with internal
+					   preference given to GSM.
+				"3g+2g"    Radio access selection is done
+					   automatically, with internal
+					   preference given to UTRAN.
+				"2g"       Radio access selection is
+					   limited to GSM only.
+				"3g"       Radio access selection is
+					   limited to UTRAN only.
+
+		string State [readonly]
+
+			The current radio access link state.
+
+			The possible values are:
+				"active"   Radio access link is currently
+					   active, e.g., in CELL_DCH state.
+				"idle"     Radio access link is currently
+					   idle.
+				"unknown"  Radio access link state is
+					   currently unavailable.
diff --git a/include/radio-access.h b/include/radio-access.h
new file mode 100644
index 0000000..9afab3d
--- /dev/null
+++ b/include/radio-access.h
@@ -0,0 +1,85 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  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_RADIO_ACCESS_H
+#define __OFONO_RADIO_ACCESS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <ofono/types.h>
+
+enum ofono_radio_access_mode {
+	OFONO_RADIO_ACCESS_MODE_2G_3G = 0,
+	OFONO_RADIO_ACCESS_MODE_3G_2G = 1,
+	OFONO_RADIO_ACCESS_MODE_2G = 2,
+	OFONO_RADIO_ACCESS_MODE_3G = 3,
+};
+
+enum ofono_radio_access_state {
+	OFONO_RADIO_ACCESS_STATE_IDLE = 0,
+	OFONO_RADIO_ACCESS_STATE_ACTIVE = 1,
+};
+
+struct ofono_radio_access;
+
+typedef void (*ofono_radio_access_mode_set_cb_t)(const struct ofono_error *error,
+							void *data);
+typedef void (*ofono_radio_access_mode_query_cb_t)(const struct ofono_error *error,
+						enum ofono_radio_access_mode mode,
+						void *data);
+
+struct ofono_radio_access_driver {
+	const char *name;
+	int (*probe)(struct ofono_radio_access *ra, unsigned int vendor, void *data);
+	void (*remove)(struct ofono_radio_access *ra);
+	void (*query_mode)(struct ofono_radio_access *ra,
+				ofono_radio_access_mode_query_cb_t cb, void *data);
+	void (*set_mode)(struct ofono_radio_access *ra,
+				enum ofono_radio_access_mode mode,
+				ofono_radio_access_mode_set_cb_t cb, void *data);
+};
+
+void ofono_radio_access_mode_notify(struct ofono_radio_access *ra,
+					enum ofono_radio_access_mode mode);
+void ofono_radio_access_state_notify(struct ofono_radio_access *ra,
+					enum ofono_radio_access_state state);
+
+int ofono_radio_access_driver_register(const struct ofono_radio_access_driver *d);
+void ofono_radio_access_driver_unregister(const struct ofono_radio_access_driver *d);
+
+struct ofono_radio_access *ofono_radio_access_create(struct ofono_modem *modem,
+						unsigned int vendor,
+						const char *driver,
+						void *data);
+
+void ofono_radio_access_register(struct ofono_radio_access *ra);
+void ofono_radio_access_remove(struct ofono_radio_access *ra);
+
+void ofono_radio_access_set_data(struct ofono_radio_access *ra,	void *data);
+void *ofono_radio_access_get_data(struct ofono_radio_access *ra);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_RADIO_ACCESS_H */
diff --git a/src/ofono.h b/src/ofono.h
index 379f413..c519eef 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -113,6 +113,7 @@ enum ofono_atom_type {
 	OFONO_ATOM_TYPES_CALL_VOLUME = 15,
 	OFONO_ATOM_TYPE_GPRS = 16,
 	OFONO_ATOM_TYPE_GPRS_CONTEXT = 17,
+	OFONO_ATOM_TYPE_RADIO_ACCESS = 18,
 };
 
 enum ofono_atom_watch_condition {
@@ -168,6 +169,7 @@ void __ofono_atom_free(struct ofono_atom *atom);
 #include <ofono/voicecall.h>
 #include <ofono/gprs.h>
 #include <ofono/gprs-context.h>
+#include <ofono/radio-access.h>
 
 #include <ofono/sim.h>
 
diff --git a/src/radio-access.c b/src/radio-access.c
new file mode 100644
index 0000000..80d9cb7
--- /dev/null
+++ b/src/radio-access.c
@@ -0,0 +1,384 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies).
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <gdbus.h>
+
+#include "ofono.h"
+#include "common.h"
+
+#define RADIO_ACCESS_INTERFACE "org.ofono.RadioAccess"
+#define RADIO_ACCESS_INTERFACE "org.ofono.RadioAccess"
+
+static GSList *g_drivers = NULL;
+
+struct ofono_radio_access {
+	DBusMessage *pending;
+	enum ofono_radio_access_mode mode;
+	enum ofono_radio_access_mode pending_mode;
+	enum ofono_radio_access_state state;
+	const struct ofono_radio_access_driver *driver;
+	void *driver_data;
+	struct ofono_atom *atom;
+};
+
+static const char *radio_access_mode_to_string(enum ofono_radio_access_mode mode)
+{
+	switch (mode) {
+	case OFONO_RADIO_ACCESS_MODE_2G_3G:
+		return "2g+3g";
+	case OFONO_RADIO_ACCESS_MODE_3G_2G:
+		return "3g+2g";
+	case OFONO_RADIO_ACCESS_MODE_2G:
+		return "2g";
+	case OFONO_RADIO_ACCESS_MODE_3G:
+		return "3g";
+	default:
+		return "unknown";
+	}
+}
+
+static const char *radio_access_state_to_string(enum ofono_radio_access_state state)
+{
+	switch (state) {
+	case OFONO_RADIO_ACCESS_STATE_ACTIVE:
+		return "active";
+	case OFONO_RADIO_ACCESS_STATE_IDLE:
+		return "idle";
+	default:
+		return "unknown";
+	}
+}
+
+static enum ofono_radio_access_mode string_to_radio_access_mode(const char *mode)
+
+{
+	if (g_strcmp0(mode, "2g+3g") == 0)
+		return OFONO_RADIO_ACCESS_MODE_2G_3G;
+	if (g_strcmp0(mode, "3g+2g") == 0)
+		return OFONO_RADIO_ACCESS_MODE_3G_2G;
+	if (g_strcmp0(mode, "2g") == 0)
+		return OFONO_RADIO_ACCESS_MODE_2G;
+	if (g_strcmp0(mode, "3g") == 0)
+		return OFONO_RADIO_ACCESS_MODE_3G;
+	return -1;
+}
+
+static void ra_mode_query_callback(const struct ofono_error *error,
+					enum ofono_radio_access_mode mode,
+					void *data)
+{
+	struct ofono_radio_access *ra = data;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_debug("Error during radio access mode query");
+		return;
+	}
+
+	ofono_radio_access_mode_notify(ra, mode);
+}
+
+static void ra_mode_set_callback(const struct ofono_error *error, void *data)
+{
+	struct ofono_radio_access *ra = data;
+	DBusMessage *reply;
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		ofono_debug("Error setting radio access mode");
+		ra->pending_mode = ra->mode;
+		reply = __ofono_error_failed(ra->pending);
+		__ofono_dbus_pending_reply(&ra->pending, reply);
+		return;
+	}
+
+	reply = dbus_message_new_method_return(ra->pending);
+	__ofono_dbus_pending_reply(&ra->pending, reply);
+
+	ofono_radio_access_mode_notify(ra, ra->pending_mode);
+}
+
+static DBusMessage *ra_get_properties(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_radio_access *ra = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+
+	const char *mode = radio_access_mode_to_string(ra->mode);
+	const char *state = radio_access_state_to_string(ra->state);
+
+	reply = dbus_message_new_method_return(msg);
+	if (!reply)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					OFONO_PROPERTIES_ARRAY_SIGNATURE,
+					&dict);
+
+	ofono_dbus_dict_append(&dict, "Mode", DBUS_TYPE_STRING, &mode);
+	ofono_dbus_dict_append(&dict, "State", DBUS_TYPE_STRING, &state);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
+static DBusMessage *ra_set_property(DBusConnection *conn, DBusMessage *msg,
+					void *data)
+{
+	struct ofono_radio_access *ra = data;
+	DBusMessageIter iter;
+	DBusMessageIter var;
+	const char *property;
+
+	if (ra->pending)
+		return __ofono_error_busy(msg);
+
+	if (!dbus_message_iter_init(msg, &iter))
+		return __ofono_error_invalid_args(msg);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(&iter, &property);
+	dbus_message_iter_next(&iter);
+
+	if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_recurse(&iter, &var);
+
+	if (g_strcmp0(property, "Mode") == 0) {
+		const char *value;
+		enum ofono_radio_access_mode mode = -1;
+
+		if (!ra->driver->set_mode)
+			return __ofono_error_not_implemented(msg);
+
+		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_STRING)
+			return __ofono_error_invalid_args(msg);
+
+		dbus_message_iter_get_basic(&var, &value);
+		mode = string_to_radio_access_mode(value);
+
+		if (ra->mode == mode)
+			return dbus_message_new_method_return(msg);
+
+		ra->pending = dbus_message_ref(msg);
+		ra->pending_mode = mode;
+
+		ra->driver->set_mode(ra, mode, ra_mode_set_callback, ra);
+
+		return NULL;
+	}
+
+	return __ofono_error_invalid_args(msg);
+}
+
+static GDBusMethodTable ra_methods[] = {
+	{ "GetProperties",	"",	"a{sv}",	ra_get_properties },
+	{ "SetProperty",	"sv",	"",		ra_set_property,
+							G_DBUS_METHOD_FLAG_ASYNC },
+	{ }
+};
+
+static GDBusSignalTable ra_signals[] = {
+	{ "PropertyChanged",	"sv" },
+	{ }
+};
+
+void ofono_radio_access_mode_notify(struct ofono_radio_access *ra,
+					 enum ofono_radio_access_mode mode)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path;
+	const char *str_mode;
+
+	if (ra->mode == mode)
+		return;
+
+	ra->mode = mode;
+
+	path = __ofono_atom_get_path(ra->atom);
+	str_mode = radio_access_mode_to_string(mode);
+
+	ofono_dbus_signal_property_changed(conn, path,
+						RADIO_ACCESS_INTERFACE,
+						"Mode", DBUS_TYPE_STRING,
+						&str_mode);
+}
+
+void ofono_radio_access_state_notify(struct ofono_radio_access *ra,
+					enum ofono_radio_access_state state)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path;
+	const char *str_state;
+
+	if (ra->state == state)
+		return;
+
+	ra->state = state;
+
+	path = __ofono_atom_get_path(ra->atom);
+	str_state = radio_access_state_to_string(state);
+
+	ofono_dbus_signal_property_changed(conn, path,
+						RADIO_ACCESS_INTERFACE,
+						"State", DBUS_TYPE_STRING,
+						&str_state);
+}
+
+int ofono_radio_access_driver_register(const struct ofono_radio_access_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (!d || !d->probe)
+		return -EINVAL;
+
+	g_drivers = g_slist_prepend(g_drivers, (void *)d);
+
+	return 0;
+}
+
+void ofono_radio_access_driver_unregister(const struct ofono_radio_access_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (!d)
+		return;
+
+	g_drivers = g_slist_remove(g_drivers, (void *)d);
+}
+
+static void radio_access_unregister(struct ofono_atom *atom)
+{
+	struct ofono_radio_access *ra = __ofono_atom_get_data(atom);
+	const char *path = __ofono_atom_get_path(ra->atom);
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(ra->atom);
+
+	ofono_modem_remove_interface(modem, RADIO_ACCESS_INTERFACE);
+	g_dbus_unregister_interface(conn, path, RADIO_ACCESS_INTERFACE);
+}
+
+static void radio_access_remove(struct ofono_atom *atom)
+{
+	struct ofono_radio_access *ra = __ofono_atom_get_data(atom);
+
+	DBG("atom: %p", atom);
+
+	if (!ra)
+		return;
+
+	if (ra->driver && ra->driver->remove)
+		ra->driver->remove(ra);
+
+	g_free(ra);
+}
+
+struct ofono_radio_access *ofono_radio_access_create(struct ofono_modem *modem,
+							unsigned int vendor,
+							const char *driver,
+							void *data)
+{
+	struct ofono_radio_access *ra;
+	GSList *l;
+
+	if (!driver)
+		return NULL;
+
+	ra = g_try_new0(struct ofono_radio_access, 1);
+	if (!ra)
+		return NULL;
+
+	ra->mode = -1;
+	ra->state = -1;
+
+	ra->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_RADIO_ACCESS,
+						radio_access_remove, ra);
+
+	for (l = g_drivers; l; l = l->next) {
+		const struct ofono_radio_access_driver *drv = l->data;
+
+		if (g_strcmp0(drv->name, driver) != 0)
+			continue;
+
+		if (drv->probe(ra, vendor, data) < 0)
+			continue;
+
+		ra->driver = drv;
+		break;
+	}
+
+	return ra;
+}
+
+void ofono_radio_access_register(struct ofono_radio_access *ra)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(ra->atom);
+	const char *path = __ofono_atom_get_path(ra->atom);
+
+	if (!g_dbus_register_interface(conn, path,
+					RADIO_ACCESS_INTERFACE,
+					ra_methods, ra_signals, NULL, ra,
+					NULL)) {
+		ofono_error("Could not create %s interface",
+				RADIO_ACCESS_INTERFACE);
+
+		return;
+	}
+
+	ofono_modem_add_interface(modem, RADIO_ACCESS_INTERFACE);
+
+	if (ra->driver->query_mode)
+		ra->driver->query_mode(ra, ra_mode_query_callback, ra);
+
+	__ofono_atom_register(ra->atom, radio_access_unregister);
+}
+
+void ofono_radio_access_remove(struct ofono_radio_access *ra)
+{
+	__ofono_atom_free(ra->atom);
+}
+
+void ofono_radio_access_set_data(struct ofono_radio_access *ra,
+					void *data)
+{
+	ra->driver_data = data;
+}
+
+void *ofono_radio_access_get_data(struct ofono_radio_access *ra)
+{
+	return ra->driver_data;
+}
-- 
1.6.3.3

Re: [PATCH 1/3] Add radio access atom and driver API

[Denis Kenzior]

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

Hi Aki,

On Wed, Feb 3, 2010 at 3:21 AM, Aki Niemi <aki.niemi@nokia.com> wrote:
> ke, 2010-02-03 kello 01:04 +0100, ext Denis Kenzior kirjoitti:
>> It might be helpful to include the API documentation too.  One thing I don't
>> like is the name RadioAccess.  The connotation is a bit too low-level.
>> Perhaps BandSelection? RadioBandSelection?
>
> I thought about this quite a bit, but this was the best name I could
> come up with. RadioAccessSelection, maybe, but then that'd be too
> specific if we wanted to add a property.
>
> And I think we definitely need to add at least one property here, a
> boolean "Active" for when the radio link is active. This is used by
> applications as a power saving measure to time their non-urgent data
> traffic to only when the radio link is in CELL_DCH state already.

How about we keep this simple and not try to overstuff this interface?
 Name it something like RadioPreferences or RadioSettings or
BandSettings, and put two things on here:
- Selection of technology 2G/3G/Dual
- Perhaps selection of band

The DCH state can go to some other interface and discussed in detail there.

>> > +void ofono_radio_access_mode_notify(struct ofono_radio_access
>> >  *radio_access, +                                     int mode);
>>
>> Why do we have this function, can the radio mode be changed outside oFono's
>> control?
>
> This can happen if you are using the phone as a modem on the desktop,
> and you change the setting on the phone.

Then you're opening up a huge can of worms here.  Most interfaces will
be utterly and completely broken if you allow modem access outside
oFono's control.

In fact this whole approach seems wrong to me.  Why should we bother
querying the setting at all?  Simply read it from storage (default it
to something sane, like 2G+3G if no setting exists) and always send
the set_mode at startup.  Then get rid of the notify function, this
has to be taken care of by the ModemEmulator.

>
>> > +static DBusMessage *ra_get_properties(DBusConnection *conn, DBusMessage
>> >  *msg, +                                      void *data)
>> > +{
>> > +     struct ofono_radio_access *ra = data;
>> > +     DBusMessage *reply;
>> > +     DBusMessageIter iter;
>> > +     DBusMessageIter dict;
>> > +
>> > +     const char *mode = radio_access_mode_to_string(ra->mode);
>> > +
>> > +     reply = dbus_message_new_method_return(msg);
>> > +     if (!reply)
>> > +             return NULL;
>> > +
>> > +     dbus_message_iter_init_append(reply, &iter);
>> > +
>> > +     dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
>> > +                                     OFONO_PROPERTIES_ARRAY_SIGNATURE,
>> > +                                     &dict);
>> > +
>> > +     ofono_dbus_dict_append(&dict, "Mode", DBUS_TYPE_STRING, &mode);
>> > +
>> > +     dbus_message_iter_close_container(&iter, &dict);
>> > +
>> > +     return reply;
>>
>> The pattern used in the past was to query the mode from get_properties if it
>> was not cached.  Once the mode was cached, we set a flag and always returned
>> the cached value.  I would do it the same way in this case to minimize flood of
>> queries at modem startup.
>
> I noticed call-forwarding was using it. However, when you have more than
> one property to cache, of which some are optional, that initial chain of
> queries to fill the cache gets a little nasty.

Again, I maintain this is a symptom of trying to overstuff the
interface.  Please keep it simple.

>
> I've not seen problems in practice in filling the cache at modem
> startup. (That's something CSD does.)
>

First of all you're dealing with sane modems.  Most modems are not as
nice and shiny as the Nokia ones.  Secondly, the goal for oFono is
performance, particularly at startup.  If something should not need to
be queries at startup, then don't.

>> Is querying the mode even a good idea?  Perhaps this should be yet another
>> setting, stored by IMSI.
>
> AFAIK, this is not an IMSI specific setting; it persists over a SIM
> swap.

Unless there's a very specific use case for why it should be a
per-modem setting, I suggest to make it per-IMSI.  GPRS settings are
also not stored on a per-SIM basis on most mobile phones.  Quite
frankly that makes no sense.  Lets not step on a rake yet again.

Regards,
-Denis