Re: [PATCH 1/6] gprs-provision: add driver API header
Hi Jukka, 2011/1/24 Jukka Saunamaki jukka.saunam...@nokia.com: Then how about something like this: Lets make provisioning API synchronous (so that plugins do not need to care about SIM or other safety). In stead, if in gprs atom ofono_gprs_register() we notice the need for provisioning, ofono_sim_read(SPN) is called there. All issues would be localised there. Provisioning modules would be called with MCC,MNC,SPN as parameters. I think this is a better approach. It would reduce the role of the provisioning plugins to database abstraction only, which is essentially what they are. I think the provisioning atom/plugins also should not be modem specific, but just created once for all modems to use. Cheers, Aki ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/6] gprs-provision: add driver API header
Hi Denis, 2011/1/21 Denis Kenzior denk...@gmail.com: How exactly are you guaranteeing that 'nothing bad should happen'? There is no cancellation mechanism that I see. Not to mention that the current ofono_sim_read API is not even safe either. For exactly the same reasons. This is a problem with all users of ofono_sim_read() at the moment. I suppose if the atoms get removed at different times, it may happen that after the gprs atom is gone, the SIM atom is still calling its read callback. Seems like we need some sort of cancellation API to ofono_sim_read(), or use Jukka's approach of a request object, perhaps with a GDestroyNotify parameter, or simply change the way atoms are created and removed so that this could be handled inside the SIM atom. Cheers, Aki ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/6] gprs-provision: add driver API header
Hi Jukka, On 01/21/2011 01:39 AM, Jukka Saunamaki wrote: Hi On Thu, 2011-01-20 at 15:51 -0600, Denis Kenzior wrote: So I don't really see the point in an asynchronous provisioning driver. If you want to do this over D-Bus or something then you might as well provision via the ConnectionManager interface. The other problem with being async is that is nearly impossible to support multiple provisioning plugins properly. I'd rather not deal with the race conditions. If we can't make the lookup fast enough to be done synchronously, then I think we should give up. The reason for asyncronous API is still that SPN value reading from SIM. Is there any way to make sure it is available synchronously when provisioning is run? Not really. So you're introducing a boatload of extra complexity just because of the need for EFspn? Are you 100% sure that you really need this? Can some of these corner cases be covered differently? And what do you mean with it being impossible to support multiple provisioning plugins properly? Plugins are run one after another until first returns something. Race conditions I tried to address in gprs, so if gprs atom goes away while provisioning is running nothing bad should happen. But sure, there might something else, and hopefully someone could point them. How exactly are you guaranteeing that 'nothing bad should happen'? There is no cancellation mechanism that I see. Not to mention that the current ofono_sim_read API is not even safe either. For exactly the same reasons. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH 1/6] gprs-provision: add driver API header
---
Makefile.am |2 +-
include/gprs-provision.h | 74 ++
2 files changed, 75 insertions(+), 1 deletions(-)
create mode 100644 include/gprs-provision.h
diff --git a/Makefile.am b/Makefile.am
index c1c34ca..7a03f08 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -13,7 +13,7 @@ pkginclude_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_pkginclude_HEADERS = include/version.h
diff --git a/include/gprs-provision.h b/include/gprs-provision.h
new file mode 100644
index 000..cc751d2
--- /dev/null
+++ b/include/gprs-provision.h
@@ -0,0 +1,74 @@
+/*
+ *
+ * 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_provision_context {
+ struct ofono_gprs_provision_driver *driver;
+ struct ofono_modem *modem;
+ void *data;
+};
+
+struct ofono_gprs_provision_data {
+ enum ofono_gprs_context_type type;
+ enum ofono_gprs_proto proto;
+ char *name;
+ char *apn;
+ char *username;
+ char *password;
+ char *message_proxy;
+ char *message_center;
+};
+
+/*
+ * Callback from provisioning plugin.
+ */
+typedef void (*ofono_gprs_provision_cb_t)(
+ const struct ofono_gprs_provision_data settings[],
+ int count, void *user_data);
+
+struct ofono_gprs_provision_driver {
+ const char *name;
+ int priority;
+ int (*probe)(struct ofono_gprs_provision_context *context);
+ void (*remove)(struct ofono_gprs_provision_context *context);
+ void (*get_settings)(struct ofono_gprs_provision_context *context,
+ ofono_gprs_provision_cb_t cb,
+ void *user_data);
+};
+
+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);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_GPRS_PROVISION_H */
--
1.7.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/6] gprs-provision: add driver API header
Hi Jukka,
+/*
+ * Callback from provisioning plugin.
+ */
+typedef void (*ofono_gprs_provision_cb_t)(
+ const struct ofono_gprs_provision_data settings[],
+ int count, void *user_data);
+
+struct ofono_gprs_provision_driver {
+ const char *name;
+ int priority;
+ int (*probe)(struct ofono_gprs_provision_context *context);
+ void (*remove)(struct ofono_gprs_provision_context *context);
+ void (*get_settings)(struct ofono_gprs_provision_context *context,
+ ofono_gprs_provision_cb_t cb,
+ void *user_data);
+};
So I don't really see the point in an asynchronous provisioning driver.
If you want to do this over D-Bus or something then you might as well
provision via the ConnectionManager interface. The other problem with
being async is that is nearly impossible to support multiple
provisioning plugins properly. I'd rather not deal with the race
conditions.
If we can't make the lookup fast enough to be done synchronously, then I
think we should give up.
I also don't see the point of instantiating these per modem. The API
already has all the information it needs in the get_settings call. So
having the plugin allocate its data in the plugin init function and free
it there seems enough to me.
Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/6] gprs-provision: add driver API header
Hi On Thu, 2011-01-20 at 15:51 -0600, Denis Kenzior wrote: So I don't really see the point in an asynchronous provisioning driver. If you want to do this over D-Bus or something then you might as well provision via the ConnectionManager interface. The other problem with being async is that is nearly impossible to support multiple provisioning plugins properly. I'd rather not deal with the race conditions. If we can't make the lookup fast enough to be done synchronously, then I think we should give up. The reason for asyncronous API is still that SPN value reading from SIM. Is there any way to make sure it is available synchronously when provisioning is run? And what do you mean with it being impossible to support multiple provisioning plugins properly? Plugins are run one after another until first returns something. Race conditions I tried to address in gprs, so if gprs atom goes away while provisioning is running nothing bad should happen. But sure, there might something else, and hopefully someone could point them. Of cause there is always the possibility to do all this provisioning stuff outside of oFono, especially if we add SPN property to SIM API. I also don't see the point of instantiating these per modem. The API already has all the information it needs in the get_settings call. So having the plugin allocate its data in the plugin init function and free it there seems enough to me. This I agree, earlier patches did it that way. I only changed this since Marcel wished that all this kind of driver interfaces (like nettime) would look similar. Or did I misunderstand that? --Jukka ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
