Hi Denis,
On 06/01/2011 08:18 AM, Denis Kenzior wrote:
Hi Philippe,
On 05/20/2011 11:26 AM, Philippe Nunes wrote:
---
src/gprs.c | 239 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
src/ofono.h | 24 ++++++
2 files changed, 263 insertions(+), 0 deletions(-)
diff --git a/src/gprs.c b/src/gprs.c
index 9657a3e..86d95bc 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -147,6 +147,8 @@ struct pri_context {
struct ofono_gprs_primary_context context;
struct ofono_gprs_context *context_driver;
struct ofono_gprs *gprs;
+ void *notify;
+ void *notify_data;
};
Modifying pri_context for this does not seem like a good idea, I suggest
you make your own structure since pretty much all the members of this
structure are not relevant to what you're trying to accomplish.
As a general philosophy we prefer clearer code (even if there's more of
it) than trying to complicate the logic of code designed for a
completely different purpose.
Ok, I can remove this callback pointer since this callback is only
needed for the private primary context created by the STK atom.
static void gprs_netreg_update(struct ofono_gprs *gprs);
@@ -356,6 +358,35 @@ static struct pri_context *gprs_context_by_path(struct
ofono_gprs *gprs,
return NULL;
}
+static struct pri_context *gprs_context_by_id(struct ofono_gprs *gprs,
+ unsigned int id)
+{
+ GSList *l;
+
+ for (l = gprs->contexts; l; l = l->next) {
+ struct pri_context *ctx = l->data;
+
+ if (ctx->id == id)
+ return ctx;
+ }
+
+ return NULL;
+}
+
+static struct pri_context *gprs_get_default_context(struct ofono_gprs *gprs)
+{
+ GSList *l;
+
+ for (l = gprs->contexts; l; l = l->next) {
+ struct pri_context *ctx = l->data;
+
+ if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_INTERNET)
+ return ctx;
+ }
+
+ return NULL;
+}
+
static void context_settings_free(struct context_settings *settings)
{
if (settings->ipv4) {
@@ -537,6 +568,9 @@ static void signal_settings(struct pri_context *ctx, const
char *prop,
DBusMessageIter iter;
struct context_settings *settings;
+ if (path == NULL)
+ return;
+
signal = dbus_message_new_signal(path,
OFONO_CONNECTION_CONTEXT_INTERFACE,
"PropertyChanged");
@@ -2968,3 +3002,208 @@ void *ofono_gprs_get_data(struct ofono_gprs *gprs)
{
return gprs->driver_data;
}
+
+unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
+ enum ofono_gprs_context_type context_type,
+ enum ofono_gprs_proto proto,
+ const char *apn,
+ const char *username,
+ const char *password,
+ const char *host)
What is the purpose of the host parameter? Doesn't STK use IP addresses
for the destination?
Here, the purpose of the host parameter is to set the interface with a
special host route (id the IP stream with this host/destination address
has to go through this interface).
Indeed, I need to bind the UDP/TCP socket to the interface created
precisely for STK purpose.
But, I can remove this parameter since you suggested to make
pri_setproxy (gprs.c) into a utility function and call it from stk.c
+{
Returning int here would be better:
+ unsigned int id;
+ struct pri_context *context;
+ struct pri_context *default_ctx = NULL;
+ const char *name;
+
+ /* Sanity check */
+ if (apn&& strlen(apn)> OFONO_GPRS_MAX_APN_LENGTH)
+ return 0;
Then you can return meaningful error here, e.g. -EINVAL
+
+ if (username&& strlen(username)> OFONO_GPRS_MAX_USERNAME_LENGTH)
+ return 0;
+
+ if (password&& strlen(password)> OFONO_GPRS_MAX_PASSWORD_LENGTH)
+ return 0;
+
+ if (apn == NULL || (username == NULL&& password == NULL)) {
+ /* take the default primary internet context */
+ default_ctx = gprs_get_default_context(gprs);
+
+ if (default_ctx == NULL&& apn == NULL)
+ return 0;
Maybe ENOENT here
+ }
+
+ if (apn&& is_valid_apn(apn) == FALSE)
+ return 0;
+
etc
This will allow you to return meaningful terminal responses in case
things fail.
+ name = gprs_context_default_name(context_type);
+ if (name == NULL)
+ return 0;
+
+ if (gprs->last_context_id)
+ id = idmap_alloc_next(gprs->pid_map, gprs->last_context_id);
+ else
+ id = idmap_alloc(gprs->pid_map);
+
+ if (id> idmap_get_max(gprs->pid_map))
+ return 0;
+
+ context = pri_context_create(gprs, name, context_type);
+ if (context == NULL) {
+ idmap_put(gprs->pid_map, id);
+ return 0;
+ }
+
As mentioned previously, I don't see the point of creating pri_context
structure for this. So you can rip much of this stuff out.
I don't understand why you think we don't need to create a pri_context.
Could you clarify please?
Even if no parameters are provided by the UICC (id use default
parameters), I still need to setup a dedicated primary context with
defaults parameters since we can't double-activate an existing primary
context.
+ context->id = id;
+
+ if (username != NULL)
+ strcpy(context->context.username, username);
+
+ if (password != NULL)
+ strcpy(context->context.password, password);
+
+ if (username == NULL&& password == NULL&& default_ctx) {
+ if (default_ctx->context.username)
+ strcpy(context->context.username,
+ default_ctx->context.username);
+ if (default_ctx->context.password)
+ strcpy(context->context.password,
+ default_ctx->context.password);
+ }
+
+ if (apn)
+ strcpy(context->context.apn, apn);
+ else if (default_ctx) {
+ strcpy(context->context.apn, default_ctx->context.apn);
+ if (default_ctx->context.username)
+ strcpy(context->context.username,
+ default_ctx->context.username);
+ if (default_ctx->context.password)
+ strcpy(context->context.password,
+ default_ctx->context.password);
+ }
+
+ context->context.proto = proto;
+
+ gprs->last_context_id = id;
+ gprs->contexts = g_slist_append(gprs->contexts, context);
+
+ return id;
+}
+
+static void activate_request_callback(const struct ofono_error *error,
+ void *data)
+{
+ struct pri_context *ctx = data;
+ struct ofono_gprs_context *gc = ctx->context_driver;
+
+ DBG("%p", ctx);
+
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Activating context failed with error: %s",
+ telephony_error_to_str(error));
+ context_settings_free(ctx->context_driver->settings);
+ release_context(ctx);
+
+ if (ctx->notify)
+ ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
+ (-ENOSYS, NULL, NULL, ctx->notify_data);
+ return;
+ }
+
+ ctx->active = TRUE;
+
+ if (gc->settings->interface != NULL) {
+ pri_ifupdown(gc->settings->interface, TRUE);
+
+ if (ctx->type == OFONO_GPRS_CONTEXT_TYPE_STK&&
+ gc->settings->ipv4) {
+ pri_set_ipv4_addr(gc->settings->interface,
+ gc->settings->ipv4->ip);
+ }
+ }
+
+ if (ctx->notify&& gc->settings->ipv4)
+ ((__ofono_gprs_add_pdp_context_cb_t) ctx->notify)
+ (0, gc->settings->interface,
+ gc->settings->ipv4->ip,
+ ctx->notify_data);
+}
+
+int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_add_pdp_context_cb_t cb,
+ void *data)
+{
+ struct pri_context *ctx;
+ struct ofono_gprs_context *gc;
+
+ ctx = gprs_context_by_id(gprs, id);
+ if (ctx == NULL)
+ return -EINVAL;
+
+ if (ctx->active == TRUE)
+ return 0;
+
+ if (assign_context(ctx) == FALSE)
+ return -ENOSYS;
+
+ gc = ctx->context_driver;
+
+ ctx->notify = cb;
+ ctx->notify_data = data;
+
+ gc->driver->activate_primary(gc,&ctx->context,
+ activate_request_callback, ctx);
+ return 0;
+}
+
+static void remove_request_callback(const struct ofono_error *error,
+ void *data)
+{
+ struct pri_context *ctx = data;
+ struct ofono_gprs *gprs = ctx->gprs;
+ int err = 0;
+
+ if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+ DBG("Removing context failed with error: %s",
+ telephony_error_to_str(error));
+ err = -ENOSYS;
+ }
+
+ pri_reset_context_settings(ctx);
+ release_context(ctx);
+ idmap_put(gprs->pid_map, ctx->id);
+ gprs->contexts = g_slist_remove(gprs->contexts, ctx);
+
+ if (ctx->notify)
+ ((__ofono_gprs_remove_pdp_context_cb_t) ctx->notify)
+ (err, ctx->notify_data);
+}
+
+int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_remove_pdp_context_cb_t cb,
+ void *data)
+{
+ struct pri_context *ctx;
+
+ ctx = gprs_context_by_id(gprs, id);
+ if (ctx == NULL)
+ return -EINVAL;
+
+ if (ctx->active) {
+ struct ofono_gprs_context *gc = ctx->context_driver;
+
+ ctx->notify = cb;
+ ctx->notify_data = data;
+
+ gc->driver->deactivate_primary(gc, ctx->context.cid,
+ remove_request_callback, ctx);
+ return -EINPROGRESS;
+ }
+
+ idmap_put(gprs->pid_map, ctx->id);
+ gprs->contexts = g_slist_remove(gprs->contexts, ctx);
+
+ return 0;
+}
diff --git a/src/ofono.h b/src/ofono.h
index 82d7e34..ee5864d 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -239,6 +239,30 @@ gboolean __ofono_call_settings_is_busy(struct
ofono_call_settings *cs);
#include<ofono/phonebook.h>
#include<ofono/gprs.h>
#include<ofono/gprs-context.h>
+
+typedef void (*__ofono_gprs_add_pdp_context_cb_t)(int error,
+ const char *interface,
+ const char *ip,
+ void *data);
+
+typedef void (*__ofono_gprs_remove_pdp_context_cb_t)(int error, void *data);
+
+unsigned int __ofono_gprs_add_pdp_context(struct ofono_gprs *gprs,
+ enum ofono_gprs_context_type type,
+ enum ofono_gprs_proto proto,
+ const char *apn,
+ const char *username,
+ const char *password,
+ const char *host);
+
I don't think this is needed at all. All STK needs to do is just say
'please activate a context with these params'. You can pretty much get
away with __ofono_gprs_activate_context and __ofono_gprs_deactivate_context.
When "on link demand" mode is requested, we just need to setup the PDP
context and return the PDP identifier. The PDP context activation is
done only once we receive a proactive command SEND DATA with the
qualifier "Send data immediately".
So, I don't see how to differentiate this mode without the API
"__ofono_gprs_add_pdp_context".
+int __ofono_gprs_remove_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_remove_pdp_context_cb_t cb,
+ void *data);
+
Are you sure you really need the callback function here? The spec is a
little fuzzy on when the terminal response is actually sent.
At first sight, yes, that was also my opinion but then I saw the
examples in annex I (TS 102 223).
For Close channel, we can see the following sequence:
UICC Terminal SGSN
|---------------------> | |
| Close Channel | |
| | --------------------------> |
| | Deactivate PDP context request|
| | <-----------------------------|
| | Deactivate PDP context accept |
|<----------------------
Terminal response
That's why, I introduced the callback to trigger the Terminal response.
Regards,
Philippe.
+int __ofono_gprs_activate_pdp_context(struct ofono_gprs *gprs, unsigned int id,
+ __ofono_gprs_add_pdp_context_cb_t cb,
+ void *data);
+
#include<ofono/radio-settings.h>
#include<ofono/audio-settings.h>
#include<ofono/ctm.h>
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
