Re: [RFC PATCH 1/4] Added GPRS context provisioning driver API sources
Hi Marcel,
+
+/*
+ * Callback from provisioning plugin.
+ * settings: list of struct ofono_gprs_provisioning_data
+ *
+ * It is responsibility of callback function to free settings-list
+ * settings-list elements must be freed with
ofono_gprs_provisioning_data_free()
+ */
+typedef void (*ofono_gprs_provision_cb_t)(GSList *settings, void *userdata);
+
+struct ofono_gprs_provision_driver {
+const char *name;
+int priority;
+void (*get_settings) (struct ofono_modem *modem,
+ofono_gprs_provision_cb_t cb,
+void *userdata);
+};
So here is something we need to talk about. The nettime plugin
infrastructure using a pseudo atom. So do we wanna do the same here or
do we wanna change the nettime atom to something simple like this.
I like to have consistency here. Aki, Denis, thoughts?
The nettime and history plugins can create their own D-Bus interfaces.
Hence the need for probe and remove. Since the provisioning driver
might want to pre-parse its database, I believe it should follow the
same approach...
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 1/4] Added GPRS context provisioning driver API sources
Hi Jukka,
+#ifndef __OFONO_GPRS_PROVISION_H
+#define __OFONO_GPRS_PROVISION_H
+
+#ifdef __cplusplus
+extern C {
+#endif
+
+#include gprs-context.h
+
+struct ofono_gprs_provisioning_data {
+ enum ofono_gprs_context_type type;
+ enum ofono_gprs_proto proto;
+ gchar *name;
+ gchar *apn;
+ gchar *username;
+ gchar *password;
+ gchar *message_proxy;
+ gchar *message_center;
+};
don't bother with gchar *. Just use char * here. The gchar is
essentially bad idea. We only use it a few rare occasions or when it got
missed in a review.
+
+/*
+ * Callback from provisioning plugin.
+ * settings: list of struct ofono_gprs_provisioning_data
+ *
+ * It is responsibility of callback function to free settings-list
+ * settings-list elements must be freed with
ofono_gprs_provisioning_data_free()
+ */
+typedef void (*ofono_gprs_provision_cb_t)(GSList *settings, void *userdata);
+
+struct ofono_gprs_provision_driver {
+ const char *name;
+ int priority;
+ void (*get_settings) (struct ofono_modem *modem,
+ ofono_gprs_provision_cb_t cb,
+ void *userdata);
+};
So here is something we need to talk about. The nettime plugin
infrastructure using a pseudo atom. So do we wanna do the same here or
do we wanna change the nettime atom to something simple like this.
I like to have consistency here. Aki, Denis, thoughts?
+struct gprs_provisioning_request {
+ GSList *current_driver;
+ struct ofono_modem *modem;
+ ofono_gprs_provision_cb_t cb;
+ void *userdata;
The sort of general style is to call this user_data.
+};
+
+static GSList *gprs_provision_drivers = NULL;
+
+
We also normally don't do double empty lines.
+void ofono_gprs_provisioning_data_free(struct ofono_gprs_provisioning_data
*data)
+{
+ if (data == NULL)
+ return;
+
+ g_free(data-name);
+ g_free(data-apn);
+ g_free(data-username);
+ g_free(data-password);
+ g_free(data-message_proxy);
+ g_free(data-message_center);
+ g_free(data);
+}
+
+
+static void settings_cb(GSList *settings, void *userdata)
+{
Same here, please use user_data.
+ GSList *d;
+ struct gprs_provisioning_request *req = userdata;
And as another general rule, the variable that have an assignment like
the user_data come first.
+
+ if (settings != NULL)
+ DBG(Provisioning plugin returned settings for %d contexts,
+ g_slist_length(settings));
+ else
+ DBG(Provisioning plugin returned no settings);
+
+ d = req-current_driver-next;
+ if (settings == NULL d != NULL) {
+ /* No success from this driver, try next */
+ const struct ofono_gprs_provision_driver *driver = d-data;
+ req-current_driver = d;
+ DBG(Calling provisioning plugin '%s', driver-name);
+ driver-get_settings(req-modem, settings_cb, req);
+ return;
+ }
+
+ req-cb(settings, req-userdata);
+ g_free(req);
+}
+
+
Same here for the double empty lines. Please fix them all.
+void ofono_gprs_provision_get_settings(struct ofono_modem *modem,
+ ofono_gprs_provision_cb_t cb,
+ void *userdata)
+{
+ struct gprs_provisioning_request *req;
+ const struct ofono_gprs_provision_driver *driver;
+
+ if (gprs_provision_drivers == 0)
This needs to be a == NULL.
+ goto error;
+
+ req = g_try_new0(struct gprs_provisioning_request, 1);
+ if (req == NULL)
+ goto error;
+
+ req-modem = modem;
+ req-cb = cb;
+ req-userdata = userdata;
+ req-current_driver = gprs_provision_drivers;
+
+ driver = gprs_provision_drivers-data;
+ DBG(Calling provisioning plugin '%s', driver-name);
Normally we put empty lines on top and below the DBG statement. Just to
make the stand out a bit.
Also in this case we might can remove some of the debug statements once
the code has been tested.
+ driver-get_settings(modem, settings_cb, req);
+ return;
+
+error:
+ cb(NULL, userdata);
+}
+
+static gint compare_priority(gconstpointer a, gconstpointer b)
+{
+ const struct ofono_gprs_provision_driver *plugin1 = a;
+ const struct ofono_gprs_provision_driver *plugin2 = b;
+
+ return plugin2-priority - plugin1-priority;
+}
+
+int ofono_gprs_provision_driver_register(const struct
ofono_gprs_provision_driver *driver)
+{
+ DBG(driver: %p name: %s, driver, driver-name);
+
+ gprs_provision_drivers = g_slist_insert_sorted(gprs_provision_drivers,
+ (void *) driver,
+ compare_priority);
+ return 0;
+}
+
+void ofono_gprs_provision_driver_unregister(const struct
[RFC PATCH 1/4] Added GPRS context provisioning driver API sources
---
Makefile.am |4 +-
include/gprs-provision.h | 73 +
src/gprs-provision.c | 133 ++
3 files changed, 208 insertions(+), 2 deletions(-)
create mode 100644 include/gprs-provision.h
create mode 100644 src/gprs-provision.c
diff --git a/Makefile.am b/Makefile.am
index 8ad01cd..0f330a7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -15,7 +15,7 @@ include_HEADERS = include/log.h include/plugin.h
include/history.h \
include/radio-settings.h include/stk.h \
include/audio-settings.h include/nettime.h \
include/ctm.h include/cdma-voicecall.h \
- include/cdma-sms.h
+ include/cdma-sms.h include/gprs-provision.h
nodist_include_HEADERS = include/version.h
@@ -331,7 +331,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources)
src/ofono.ver \
src/nettime.c src/stkagent.c src/stkagent.h \
src/simfs.c src/simfs.h src/audio-settings.c \
src/smsagent.c src/smsagent.h src/ctm.c \
- src/cdma-voicecall.c
+ src/cdma-voicecall.c src/gprs-provision.c
src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
diff --git a/include/gprs-provision.h b/include/gprs-provision.h
new file mode 100644
index 000..97e356b
--- /dev/null
+++ b/include/gprs-provision.h
@@ -0,0 +1,73 @@
+/*
+ *
+ * oFono - Open Telephony stack for Linux
+ *
+ * Copyright (C) 2011 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_GPRS_PROVISION_H
+#define __OFONO_GPRS_PROVISION_H
+
+#ifdef __cplusplus
+extern C {
+#endif
+
+#include gprs-context.h
+
+struct ofono_gprs_provisioning_data {
+ enum ofono_gprs_context_type type;
+ enum ofono_gprs_proto proto;
+ gchar *name;
+ gchar *apn;
+ gchar *username;
+ gchar *password;
+ gchar *message_proxy;
+ gchar *message_center;
+};
+
+/*
+ * Callback from provisioning plugin.
+ * settings: list of struct ofono_gprs_provisioning_data
+ *
+ * It is responsibility of callback function to free settings-list
+ * settings-list elements must be freed with
ofono_gprs_provisioning_data_free()
+ */
+typedef void (*ofono_gprs_provision_cb_t)(GSList *settings, void *userdata);
+
+struct ofono_gprs_provision_driver {
+ const char *name;
+ int priority;
+ void (*get_settings) (struct ofono_modem *modem,
+ ofono_gprs_provision_cb_t cb,
+ void *userdata);
+};
+
+/* For provisioning drivers/plugins */
+int ofono_gprs_provision_driver_register(const struct
ofono_gprs_provision_driver *driver);
+void ofono_gprs_provision_driver_unregister(const struct
ofono_gprs_provision_driver *driver);
+
+/* For gprs */
+void ofono_gprs_provision_get_settings(struct ofono_modem *modem,
+ ofono_gprs_provision_cb_t cb,
+ void *data);
+void ofono_gprs_provisioning_data_free(struct ofono_gprs_provisioning_data
*data);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_GPRS_PROVISION_H */
diff --git a/src/gprs-provision.c b/src/gprs-provision.c
new file mode 100644
index 000..c143fb6
--- /dev/null
+++ b/src/gprs-provision.c
@@ -0,0 +1,133 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2011 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 glib.h
