[PATCH 0/3] Mobile broadband provider info plugin

[PATCH 0/3] Mobile broadband provider info plugin

[Oleg Zhurakivskyy]

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

Hello,

Please find the mobile broadband provider info plugin ("Internet Access Provider database" TODO item).

If enabled, the plugin reads mobile-broadband-provider-info database entries (PROVIDER_DATABASE) and returns GRPS context settings to oFono provisioning module.

Regards,
Oleg

Oleg Zhurakivskyy (3):
  Mobile broadband provider info plugin
  Mobile broadband provider info plugin autoconf support
  Mobile broadband provider info plugin makefile changes

 Makefile.am                              |    8 +-
 configure.ac                             |   19 ++-
 plugins/mobile-broadband-provider-info.c |  293 ++++++++++++++++++++++++++++++
 3 files changed, 311 insertions(+), 9 deletions(-)
 create mode 100644 plugins/mobile-broadband-provider-info.c

[PATCH 1/3] Mobile broadband provider info plugin

[Oleg Zhurakivskyy]

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

---
 plugins/mobile-broadband-provider-info.c |  293 ++++++++++++++++++++++++++++++
 1 files changed, 293 insertions(+), 0 deletions(-)
 create mode 100644 plugins/mobile-broadband-provider-info.c

diff --git a/plugins/mobile-broadband-provider-info.c b/plugins/mobile-broadband-provider-info.c
new file mode 100644
index 0000000..770e4d8
--- /dev/null
+++ b/plugins/mobile-broadband-provider-info.c
@@ -0,0 +1,293 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *
+ *  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 <sys/stat.h>
+#include <sys/mman.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+
+#include <glib.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/types.h>
+#include <ofono/log.h>
+#include <ofono/plugin.h>
+#include <ofono/modem.h>
+#include <ofono/gprs-provision.h>
+
+#define MAX_SETTINGS 2
+
+struct parser_data {
+	struct ofono_gprs_provision_data **settings;
+	struct ofono_gprs_provision_data *current;
+	int count;
+	const char *match_mcc;
+	const char *match_mnc;
+	const char *name;
+	const char *current_element;
+	gboolean match_found;
+};
+
+static void parse_element_start(GMarkupParseContext *context,
+				const gchar *element_name,
+				const gchar **attribute_names,
+				const gchar **attribute_values,
+				gpointer user_data, GError **error)
+{
+	struct parser_data *data = user_data;
+
+	if (g_str_equal(element_name, "name") == TRUE) {
+		data->current_element = element_name;
+		return;
+	}
+
+	if (g_str_equal(element_name, "network-id") == TRUE) {
+		const char *mcc = NULL, *mnc = NULL;
+		int i;
+
+		for (i = 0; attribute_names[i]; i++) {
+			if (g_str_equal(attribute_names[i], "mcc") == TRUE)
+				mcc = attribute_values[i];
+			if (g_str_equal(attribute_names[i], "mnc") == TRUE)
+				mnc = attribute_values[i];
+		}
+
+		if (g_strcmp0(mcc, data->match_mcc) == 0 &&
+			g_strcmp0(mnc, data->match_mnc) == 0)
+			data->match_found = TRUE;
+	}
+
+	if (data->match_found == FALSE)
+		return;
+
+	data->current_element = element_name;
+
+	if (g_str_equal(element_name, "apn") == TRUE) {
+		const char *apn = NULL;
+		int i;
+
+		for (i = 0; attribute_names[i]; i++) {
+			if (g_str_equal(attribute_names[i], "value") == TRUE)
+				apn = attribute_values[i];
+		}
+
+		if (data->count < MAX_SETTINGS) {
+			data->current = *data->settings + data->count++;
+		} else {
+			data->match_found = FALSE;
+			DBG("APN %s is ignored, max settings reached.", apn);
+			return;
+		}
+
+		if (data->current->apn == NULL)
+			data->current->apn = g_strdup(apn);
+
+		if (data->current->name == NULL)
+			data->current->name = g_strdup(data->name);
+	}
+}
+
+static void parse_element_end(GMarkupParseContext *context,
+				const gchar *element_name,
+				gpointer user_data, GError **error)
+{
+	struct parser_data *data = user_data;
+
+	if (g_str_equal(element_name, "gsm") == TRUE ||
+		g_str_equal(element_name, "cdma") == TRUE) {
+		data->match_found = FALSE;
+		g_free((gpointer)data->name);
+		data->name = NULL;
+	}
+}
+
+static void parse_text(GMarkupParseContext *context,
+			const gchar *text, gsize text_len,
+			gpointer user_data, GError **error)
+{
+	struct parser_data *data = user_data;
+
+	if (g_strcmp0(data->current_element, "name") == 0 &&
+		data->name == NULL) {
+		data->name = g_strndup(text, text_len);
+		return;
+	}
+
+	if (data->match_found == FALSE || data->current == NULL ||
+		data->current->apn == NULL)
+		return;
+
+	if (g_strcmp0(data->current_element, "username") == 0)
+		data->current->username = g_strndup(text, text_len);
+	else if (g_strcmp0(data->current_element, "password") == 0)
+		data->current->password = g_strndup(text, text_len);
+}
+
+static void parser_error(GMarkupParseContext *context,
+				GError *error, gpointer user_data)
+{
+	ofono_error("Error parsing %s: %s", PROVIDER_DATABASE, error->message);
+}
+
+static const GMarkupParser parser = {
+	parse_element_start,
+	parse_element_end,
+	parse_text,
+	NULL,
+	parser_error,
+};
+
+static void parse_database(const char *data, ssize_t size,
+	struct parser_data *parser_data)
+{
+	GMarkupParseContext *context;
+	gboolean result;
+
+	parser_data->match_found = FALSE;
+
+	context = g_markup_parse_context_new(&parser,
+						G_MARKUP_TREAT_CDATA_AS_TEXT,
+						parser_data, NULL);
+
+	result = g_markup_parse_context_parse(context, data, size, NULL);
+	if (result == TRUE)
+		result = g_markup_parse_context_end_parse(context, NULL);
+
+	g_markup_parse_context_free(context);
+}
+
+static int lookup_apn(const char *mcc, const char *mnc, const char *spn,
+			struct parser_data *data)
+{
+	struct stat st;
+	char *map;
+	int fd;
+
+	if (mcc == NULL || mnc == NULL)
+		return -1;
+
+	fd = open(PROVIDER_DATABASE, O_RDONLY);
+	if (fd < 0)
+		return -1;
+
+	if (fstat(fd, &st) < 0) {
+		close(fd);
+		return -1;
+	}
+
+	map = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
+	if (map == NULL || map == MAP_FAILED) {
+		close(fd);
+		return -1;
+	}
+
+	data->match_mcc = mcc;
+	data->match_mnc = mnc;
+
+	parse_database(map, st.st_size, data);
+
+	munmap(map, st.st_size);
+
+	close(fd);
+
+	return data->count;
+}
+
+static int mobile_broadband_provider_info_get(const char *mcc,
+				const char *mnc, const char *spn,
+				struct ofono_gprs_provision_data **settings,
+				int *count)
+{
+	struct parser_data *data;
+	int i;
+
+	DBG("Provisioning for MCC %s, MNC %s, SPN '%s'", mcc, mnc, spn);
+
+	*settings = NULL;
+	*count = 0;
+
+	data = g_try_new0(struct parser_data, 1);
+	if (data == NULL)
+		return -ENOMEM;
+
+	data->settings = settings;
+
+	*data->settings = g_try_new0(struct ofono_gprs_provision_data,
+					MAX_SETTINGS);
+	if (*data->settings == NULL) {
+		g_free(data);
+		return -ENOMEM;
+	}
+
+	*count = lookup_apn(mcc, mnc, NULL, data);
+	if (*count <= 0) {
+		g_free(*data->settings);
+		g_free(data);
+		return -ENOENT;
+	}
+
+	DBG("settings: %p, count: %d", *settings, *count);
+
+	for (i = 0; i < *count; i++) {
+		struct ofono_gprs_provision_data *pd = *settings + i;
+
+		pd->proto = OFONO_GPRS_PROTO_IP;
+		pd->type = OFONO_GPRS_CONTEXT_TYPE_INTERNET;
+
+		DBG("Name: %s", pd->name);
+		DBG("APN: %s", pd->apn);
+		DBG("Username: %s", pd->username);
+		DBG("Password: %s", pd->password);
+	}
+
+	g_free(data);
+
+	return 0;
+}
+
+static struct ofono_gprs_provision_driver provider_info = {
+	.name		= "Mobile Broadband Provider Info",
+	.get_settings	= mobile_broadband_provider_info_get
+};
+
+static int mobile_broadband_provider_info_init(void)
+{
+	return ofono_gprs_provision_driver_register(&provider_info);
+}
+
+static void mobile_broadband_provider_info_exit(void)
+{
+	ofono_gprs_provision_driver_unregister(&provider_info);
+}
+
+OFONO_PLUGIN_DEFINE(mobile_broadband_provider_info,
+			"Mobile Broadband Provider Info Plugin",
+			VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
+			mobile_broadband_provider_info_init,
+			mobile_broadband_provider_info_exit)
-- 
1.7.1

[PATCH 2/3] Mobile broadband provider info plugin autoconf support

[Oleg Zhurakivskyy]

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

---
 configure.ac |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index a6b4094..966237c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -197,14 +197,21 @@ AC_SUBST(BLUEZ_CFLAGS)
 AC_SUBST(BLUEZ_LIBS)
 AM_CONDITIONAL(BLUETOOTH, test "${enable_bluetooth}" != "no")
 
-AC_MSG_CHECKING([for mobile-broadband-provider-info])
-PKG_CHECK_EXISTS(mobile-broadband-provider-info,
-	_PKG_CONFIG(PROVIDER_DATABASE, [variable=database],
+AC_ARG_ENABLE(provider-info, AC_HELP_STRING([--enable-provider-info],
+				[enable mobile provider database support]),
+					[enable_provider_info=${enableval}])
+if (test "${enable_provider_info}" == "yes"); then
+	AC_MSG_CHECKING([for mobile-broadband-provider-info])
+	PKG_CHECK_EXISTS(mobile-broadband-provider-info,
+			_PKG_CONFIG(PROVIDER_DATABASE, [variable=database],
 					[mobile-broadband-provider-info])
-	AC_DEFINE_UNQUOTED(PROVIDER_DATABASE, "$pkg_cv_PROVIDER_DATABASE",
+		AC_DEFINE_UNQUOTED(PROVIDER_DATABASE,
+					"$pkg_cv_PROVIDER_DATABASE",
 					[Mobile provider database])
-	AC_MSG_RESULT([yes]),
-	AC_MSG_RESULT([no]))
+		AC_MSG_RESULT([yes]),
+		AC_MSG_ERROR(mobile-broadband-provider-info package is required))
+fi
+AM_CONDITIONAL(PROVIDER_INFO, test "${enable_provider_info}" == "yes")
 
 AC_ARG_ENABLE(datafiles, AC_HELP_STRING([--disable-datafiles],
 			[don't install configuration and data files]),
-- 
1.7.1

[PATCH 3/3] Mobile broadband provider info plugin makefile changes

[Oleg Zhurakivskyy]

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

---
 Makefile.am |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 6f5d921..f5ac225 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -358,6 +358,11 @@ builtin_libadd += @BLUEZ_LIBS@
 endif
 endif
 
+if PROVIDER_INFO
+builtin_modules += mobile_broadband_provider_info
+builtin_sources += plugins/mobile-broadband-provider-info.c
+endif
+
 if MAINTAINER_MODE
 builtin_modules += example_history
 builtin_sources += examples/history.c
@@ -365,9 +370,6 @@ builtin_sources += examples/history.c
 builtin_modules += example_nettime
 builtin_sources += examples/nettime.c
 
-builtin_modules += example_provision
-builtin_sources += examples/provision.c
-
 builtin_modules += example_emulator
 builtin_sources += examples/emulator.c
 
-- 
1.7.1

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Denis Kenzior]

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

Hi Oleg,

On 07/15/2011 10:12 AM, Oleg Zhurakivskyy wrote:
> ---
>  plugins/mobile-broadband-provider-info.c |  293 ++++++++++++++++++++++++++++++
>  1 files changed, 293 insertions(+), 0 deletions(-)
>  create mode 100644 plugins/mobile-broadband-provider-info.c
> 
> diff --git a/plugins/mobile-broadband-provider-info.c b/plugins/mobile-broadband-provider-info.c
> new file mode 100644
> index 0000000..770e4d8
> --- /dev/null
> +++ b/plugins/mobile-broadband-provider-info.c
> @@ -0,0 +1,293 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *
> + *  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 <sys/stat.h>
> +#include <sys/mman.h>
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#include <glib.h>
> +
> +#define OFONO_API_SUBJECT_TO_CHANGE
> +#include <ofono/types.h>
> +#include <ofono/log.h>
> +#include <ofono/plugin.h>
> +#include <ofono/modem.h>
> +#include <ofono/gprs-provision.h>
> +
> +#define MAX_SETTINGS 2
> +

Does mobile-broadband-provider actually have multiple settings yet? If
not, then this might need to be set to 1 until it does.

> +struct parser_data {
> +	struct ofono_gprs_provision_data **settings;
> +	struct ofono_gprs_provision_data *current;
> +	int count;
> +	const char *match_mcc;
> +	const char *match_mnc;
> +	const char *name;

Why is this const?  You're performing some nasty casting later because
it is.  If you're assigning the result of g_strdup to this variable,
then it shouldn't be const in the first place ;)

> +	const char *current_element;

My concern is that this is not valgrind safe, but more importantly, why
don't we use an enum here?  That would save us some string comparisons.

> +	gboolean match_found;
> +};
> +
> +static void parse_element_start(GMarkupParseContext *context,
> +				const gchar *element_name,
> +				const gchar **attribute_names,
> +				const gchar **attribute_values,
> +				gpointer user_data, GError **error)
> +{
> +	struct parser_data *data = user_data;
> +
> +	if (g_str_equal(element_name, "name") == TRUE) {
> +		data->current_element = element_name;
> +		return;
> +	}
> +
> +	if (g_str_equal(element_name, "network-id") == TRUE) {
> +		const char *mcc = NULL, *mnc = NULL;
> +		int i;
> +
> +		for (i = 0; attribute_names[i]; i++) {
> +			if (g_str_equal(attribute_names[i], "mcc") == TRUE)
> +				mcc = attribute_values[i];
> +			if (g_str_equal(attribute_names[i], "mnc") == TRUE)
> +				mnc = attribute_values[i];
> +		}
> +
> +		if (g_strcmp0(mcc, data->match_mcc) == 0 &&
> +			g_strcmp0(mnc, data->match_mnc) == 0)

Please see doc/coding-style.txt item M4, in particular bullet 2)

> +			data->match_found = TRUE;
> +	}
> +
> +	if (data->match_found == FALSE)
> +		return;
> +
> +	data->current_element = element_name;
> +
> +	if (g_str_equal(element_name, "apn") == TRUE) {
> +		const char *apn = NULL;
> +		int i;
> +
> +		for (i = 0; attribute_names[i]; i++) {
> +			if (g_str_equal(attribute_names[i], "value") == TRUE)
> +				apn = attribute_values[i];
> +		}
> +
> +		if (data->count < MAX_SETTINGS) {
> +			data->current = *data->settings + data->count++;
> +		} else {
> +			data->match_found = FALSE;
> +			DBG("APN %s is ignored, max settings reached.", apn);
> +			return;
> +		}
> +
> +		if (data->current->apn == NULL)
> +			data->current->apn = g_strdup(apn);
> +
> +		if (data->current->name == NULL)
> +			data->current->name = g_strdup(data->name);
> +	}
> +}
> +
> +static void parse_element_end(GMarkupParseContext *context,
> +				const gchar *element_name,
> +				gpointer user_data, GError **error)
> +{
> +	struct parser_data *data = user_data;
> +
> +	if (g_str_equal(element_name, "gsm") == TRUE ||
> +		g_str_equal(element_name, "cdma") == TRUE) {

Same comment about doc/coding-style.txt M4 here

> +		data->match_found = FALSE;
> +		g_free((gpointer)data->name);

Here is that nasty cast ;)

> +		data->name = NULL;
> +	}
> +}
> +
> +static void parse_text(GMarkupParseContext *context,
> +			const gchar *text, gsize text_len,
> +			gpointer user_data, GError **error)
> +{
> +	struct parser_data *data = user_data;
> +
> +	if (g_strcmp0(data->current_element, "name") == 0 &&
> +		data->name == NULL) {

Another item M4 violation

> +		data->name = g_strndup(text, text_len);
> +		return;
> +	}
> +
> +	if (data->match_found == FALSE || data->current == NULL ||
> +		data->current->apn == NULL)

Another item M4 violation

> +		return;
> +
> +	if (g_strcmp0(data->current_element, "username") == 0)
> +		data->current->username = g_strndup(text, text_len);
> +	else if (g_strcmp0(data->current_element, "password") == 0)
> +		data->current->password = g_strndup(text, text_len);
> +}
> +

I'm a little unclear on how we handle multiple matches of the same
mcc/mnc.  To my understanding these are different plans within the same
provider and some user intervention is required to select the right
plan.  Or it could be that the operator is actually an MVNO, which is
why the SPN provided by oFono in order to to distinguish between them.

So it sounds like that if we encounter entries where multiple matches
are possible, we should not actually provision the context.

> +static void parser_error(GMarkupParseContext *context,
> +				GError *error, gpointer user_data)
> +{
> +	ofono_error("Error parsing %s: %s", PROVIDER_DATABASE, error->message);
> +}
> +
> +static const GMarkupParser parser = {
> +	parse_element_start,
> +	parse_element_end,
> +	parse_text,
> +	NULL,
> +	parser_error,
> +};
> +
> +static void parse_database(const char *data, ssize_t size,
> +	struct parser_data *parser_data)
> +{
> +	GMarkupParseContext *context;
> +	gboolean result;
> +
> +	parser_data->match_found = FALSE;
> +
> +	context = g_markup_parse_context_new(&parser,
> +						G_MARKUP_TREAT_CDATA_AS_TEXT,
> +						parser_data, NULL);
> +
> +	result = g_markup_parse_context_parse(context, data, size, NULL);
> +	if (result == TRUE)
> +		result = g_markup_parse_context_end_parse(context, NULL);
> +
> +	g_markup_parse_context_free(context);
> +}
> +
> +static int lookup_apn(const char *mcc, const char *mnc, const char *spn,
> +			struct parser_data *data)
> +{
> +	struct stat st;
> +	char *map;
> +	int fd;
> +
> +	if (mcc == NULL || mnc == NULL)
> +		return -1;
> +
> +	fd = open(PROVIDER_DATABASE, O_RDONLY);
> +	if (fd < 0)
> +		return -1;
> +
> +	if (fstat(fd, &st) < 0) {
> +		close(fd);
> +		return -1;
> +	}
> +
> +	map = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, fd, 0);
> +	if (map == NULL || map == MAP_FAILED) {
> +		close(fd);
> +		return -1;
> +	}
> +
> +	data->match_mcc = mcc;
> +	data->match_mnc = mnc;
> +
> +	parse_database(map, st.st_size, data);
> +
> +	munmap(map, st.st_size);
> +
> +	close(fd);
> +
> +	return data->count;
> +}
> +
> +static int mobile_broadband_provider_info_get(const char *mcc,
> +				const char *mnc, const char *spn,
> +				struct ofono_gprs_provision_data **settings,
> +				int *count)
> +{
> +	struct parser_data *data;
> +	int i;
> +
> +	DBG("Provisioning for MCC %s, MNC %s, SPN '%s'", mcc, mnc, spn);
> +
> +	*settings = NULL;
> +	*count = 0;
> +
> +	data = g_try_new0(struct parser_data, 1);
> +	if (data == NULL)
> +		return -ENOMEM;
> +
> +	data->settings = settings;
> +
> +	*data->settings = g_try_new0(struct ofono_gprs_provision_data,
> +					MAX_SETTINGS);
> +	if (*data->settings == NULL) {
> +		g_free(data);
> +		return -ENOMEM;
> +	}
> +
> +	*count = lookup_apn(mcc, mnc, NULL, data);
> +	if (*count <= 0) {
> +		g_free(*data->settings);
> +		g_free(data);
> +		return -ENOENT;
> +	}
> +
> +	DBG("settings: %p, count: %d", *settings, *count);
> +
> +	for (i = 0; i < *count; i++) {
> +		struct ofono_gprs_provision_data *pd = *settings + i;
> +
> +		pd->proto = OFONO_GPRS_PROTO_IP;
> +		pd->type = OFONO_GPRS_CONTEXT_TYPE_INTERNET;
> +
> +		DBG("Name: %s", pd->name);
> +		DBG("APN: %s", pd->apn);
> +		DBG("Username: %s", pd->username);
> +		DBG("Password: %s", pd->password);
> +	}
> +
> +	g_free(data);
> +
> +	return 0;
> +}
> +
> +static struct ofono_gprs_provision_driver provider_info = {
> +	.name		= "Mobile Broadband Provider Info",
> +	.get_settings	= mobile_broadband_provider_info_get
> +};
> +
> +static int mobile_broadband_provider_info_init(void)
> +{
> +	return ofono_gprs_provision_driver_register(&provider_info);
> +}
> +
> +static void mobile_broadband_provider_info_exit(void)
> +{
> +	ofono_gprs_provision_driver_unregister(&provider_info);
> +}
> +
> +OFONO_PLUGIN_DEFINE(mobile_broadband_provider_info,
> +			"Mobile Broadband Provider Info Plugin",
> +			VERSION, OFONO_PLUGIN_PRIORITY_DEFAULT,
> +			mobile_broadband_provider_info_init,
> +			mobile_broadband_provider_info_exit)

Regards,
-Denis

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Denis Kenzior]

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

Hi Oleg,

On 07/18/2011 08:32 AM, Oleg Zhurakivskyy wrote:
> 
> Hello Denis,
> 
> On 07/15/2011 09:18 PM, Denis Kenzior wrote:
>> Does mobile-broadband-provider actually have multiple settings yet? If
>> not, then this might need to be set to 1 until it does.
> 
> Actually, it does.
> 

Are you sure? Briefly looking through serviceproviders.xml I only
noticed different plans within the same provider.  You might be right of
course, but even if you are, we have no way of distinguishing between a
different plan and a different APN type.

<snip>

>> I'm a little unclear on how we handle multiple matches of the same
>> mcc/mnc.  To my understanding these are different plans within the same
>> provider and some user intervention is required to select the right
>> plan.  Or it could be that the operator is actually an MVNO, which is
>> why the SPN provided by oFono in order to to distinguish between them.
>>
>> So it sounds like that if we encounter entries where multiple matches
>> are possible, we should not actually provision the context.
> 
> To my understanding, multiple matches of the same mcc/mnc might be
> because of:
> 
>  - Different kind of settings (internet/mms/wap).
>  - Different plans for the same kind of setting (prepaid/postpaid).
>  - MVNO.
> 
> A few possible solutions in order to avoid the ambiguity would be:
> 
> 1. One could try guess the type of settings out of the access point
> name. This should work with a few exceptions, which could be handled
> case by case.
> 
> 2. Same as item 1, except when the kind of settings can't be guessed,
> just not to provision the context and let the user possibility to choose.
> 
> 3. To introduce additional tags (internet, mms, wap, prepaid, postpaid).

Option 2 sounds like the most likely candidate right now.  Option 3 is
the direction where we need mobile-broadband-provider-info to go in.  In
particular adding the internet/mms/wap tags and SPN entries to its database.

> 
> Any thoughts?
> 
> And yet a question regarding not provisioning the context. How should
> this be achieved by plugin:
> 
> - Passing no settings to oFono?
> - Passing all found settings to oFono, but indicating that the user
> intervention is required?
> 

Passing a NULL settings structure if multiple matches (or no matches)
were found.

> Anyway, thanks for the comments and ideas. I will prepare another patch.
> 
> Regards,
> Oleg
> 

Regards,
-Denis

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Oleg Zhurakivskyy]

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


Hello Denis,

On 07/15/2011 09:18 PM, Denis Kenzior wrote:
> Does mobile-broadband-provider actually have multiple settings yet? If
> not, then this might need to be set to 1 until it does.

Actually, it does.

> Why is this const?  You're performing some nasty casting later because
> it is.  If you're assigning the result of g_strdup to this variable,
> then it shouldn't be const in the first place ;)
> My concern is that this is not valgrind safe, but more importantly, why
> don't we use an enum here?  That would save us some string comparisons.

Sure, this const isn't necessary at all. Enum is also a good idea here, I will 
update.

> I'm a little unclear on how we handle multiple matches of the same
> mcc/mnc.  To my understanding these are different plans within the same
> provider and some user intervention is required to select the right
> plan.  Or it could be that the operator is actually an MVNO, which is
> why the SPN provided by oFono in order to to distinguish between them.
>
> So it sounds like that if we encounter entries where multiple matches
> are possible, we should not actually provision the context.

To my understanding, multiple matches of the same mcc/mnc might be because of:

  - Different kind of settings (internet/mms/wap).
  - Different plans for the same kind of setting (prepaid/postpaid).
  - MVNO.

A few possible solutions in order to avoid the ambiguity would be:

1. One could try guess the type of settings out of the access point name. This 
should work with a few exceptions, which could be handled case by case.

2. Same as item 1, except when the kind of settings can't be guessed, just not 
to provision the context and let the user possibility to choose.

3. To introduce additional tags (internet, mms, wap, prepaid, postpaid).

Any thoughts?

And yet a question regarding not provisioning the context. How should this be 
achieved by plugin:

- Passing no settings to oFono?
- Passing all found settings to oFono, but indicating that the user intervention 
is required?

Anyway, thanks for the comments and ideas. I will prepare another patch.

Regards,
Oleg

-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Marcel Holtmann]

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

Hi Denis,

> > On 07/15/2011 09:18 PM, Denis Kenzior wrote:
> >> Does mobile-broadband-provider actually have multiple settings yet? If
> >> not, then this might need to be set to 1 until it does.
> > 
> > Actually, it does.
> > 
> 
> Are you sure? Briefly looking through serviceproviders.xml I only
> noticed different plans within the same provider.  You might be right of
> course, but even if you are, we have no way of distinguishing between a
> different plan and a different APN type.

I think you are right. It currently has multiple entries, but it misses
any kind of type declaration.

> >> I'm a little unclear on how we handle multiple matches of the same
> >> mcc/mnc.  To my understanding these are different plans within the same
> >> provider and some user intervention is required to select the right
> >> plan.  Or it could be that the operator is actually an MVNO, which is
> >> why the SPN provided by oFono in order to to distinguish between them.
> >>
> >> So it sounds like that if we encounter entries where multiple matches
> >> are possible, we should not actually provision the context.
> > 
> > To my understanding, multiple matches of the same mcc/mnc might be
> > because of:
> > 
> >  - Different kind of settings (internet/mms/wap).
> >  - Different plans for the same kind of setting (prepaid/postpaid).
> >  - MVNO.
> > 
> > A few possible solutions in order to avoid the ambiguity would be:
> > 
> > 1. One could try guess the type of settings out of the access point
> > name. This should work with a few exceptions, which could be handled
> > case by case.
> > 
> > 2. Same as item 1, except when the kind of settings can't be guessed,
> > just not to provision the context and let the user possibility to choose.
> > 
> > 3. To introduce additional tags (internet, mms, wap, prepaid, postpaid).
> 
> Option 2 sounds like the most likely candidate right now.  Option 3 is
> the direction where we need mobile-broadband-provider-info to go in.  In
> particular adding the internet/mms/wap tags and SPN entries to its database.

Oleg, please work with the NetworkManager/ModemManager guys to get the
XML extended and get these tags and additional information merged into
to the upstream database.

Regards

Marcel

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Oleg Zhurakivskyy]

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


Hello Denis,

On 07/18/2011 03:46 PM, Denis Kenzior wrote:
> Are you sure? Briefly looking through serviceproviders.xml I only
> noticed different plans within the same provider.  You might be right of
> course, but even if you are, we have no way of distinguishing between a
> different plan and a different APN type.

Hmm, now I am not sure. In a plugin the settings is the access point settings? 
If so, yes, there are multiple access points within the same provider in 
serviceproviders.xml. Or what kind of setting is meant?

> Option 2 sounds like the most likely candidate right now.  Option 3 is
> the direction where we need mobile-broadband-provider-info to go in.  In
> particular adding the internet/mms/wap tags and SPN entries to its database.

OK, let's do option 2 for now and extend the mobile-broadband-provider-info. 
Could you please explain regarding the SPN entries, I am not sure if I 
understand this correct.

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Oleg Zhurakivskyy]

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


Hello Marcel,

On 07/18/2011 06:17 PM, Marcel Holtmann wrote:
> Oleg, please work with the NetworkManager/ModemManager guys to get the
> XML extended and get these tags and additional information merged into
> to the upstream database.

Sure, I already contacted the network manager guys, will let you to know.

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Denis Kenzior]

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

Hi Oleg,

On 07/19/2011 08:19 AM, Oleg Zhurakivskyy wrote:
> 
> Hello Denis,
> 
> On 07/18/2011 03:46 PM, Denis Kenzior wrote:
>> Are you sure? Briefly looking through serviceproviders.xml I only
>> noticed different plans within the same provider.  You might be right of
>> course, but even if you are, we have no way of distinguishing between a
>> different plan and a different APN type.
> 
> Hmm, now I am not sure. In a plugin the settings is the access point
> settings? If so, yes, there are multiple access points within the same
> provider in serviceproviders.xml. Or what kind of setting is meant?
> 

So oFono supports 3 context types:

- internet
- mms
- wap

Some (most?) providers use different APNs for these.  However, the
providers also have different service plans (e.g. pre-paid, gprs-only,
wap-only, etc)

So your database ends up needing to have settings for each provider,
each service plan for that provider and different context type settings
for each service plan.

To my understanding, with mobile-broadband-provider-info the best we can
do is provision the internet context only.

>> Option 2 sounds like the most likely candidate right now.  Option 3 is
>> the direction where we need mobile-broadband-provider-info to go in.  In
>> particular adding the internet/mms/wap tags and SPN entries to its
>> database.
> 
> OK, let's do option 2 for now and extend the
> mobile-broadband-provider-info. Could you please explain regarding the
> SPN entries, I am not sure if I understand this correct.
> 

This is there to handle MVNOs.  Basically you might have a provider (A)
that owns the network and a provider (B) that buys access to A's
network.  Both can administer different service plans (and have
different settings for APNs, etc).  In theory the MNC of provider B
should be different from that of A, however some countries / networks
are weird and have re-used provider's A MCC/MNC for subscribers of
provider B.

This is where the SPN comes in, since the MVNO is interested in
displaying their name to the customer, the SPN is nearly always provided
on the SIM.  In the case of the same MCC/MNC, the SPN can thus be used
to distinguish between provider A's settings and provider B's settings.

Regards,
-Denis

Re: [PATCH 1/3] Mobile broadband provider info plugin

[Oleg Zhurakivskyy]

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


Hello Denis,

On 07/19/2011 04:36 PM, Denis Kenzior wrote:
> To my understanding, with mobile-broadband-provider-info the best we can
> do is provision the internet context only.

Thanks for the clarification. I thought that for those few cases where mms or 
wap settings are already present in the database, we could provision them as well.

> This is there to handle MVNOs.  Basically you might have a provider (A)
> that owns the network and a provider (B) that buys access to A's
> network.  Both can administer different service plans (and have
> different settings for APNs, etc).  In theory the MNC of provider B
> should be different from that of A, however some countries / networks
> are weird and have re-used provider's A MCC/MNC for subscribers of
> provider B.
>
> This is where the SPN comes in, since the MVNO is interested in
> displaying their name to the customer, the SPN is nearly always provided
> on the SIM.  In the case of the same MCC/MNC, the SPN can thus be used
> to distinguish between provider A's settings and provider B's settings.

Thanks a lot for the help, it's clear now.

Regards,
Oleg
-- 
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki