* [PATCH 0/3] Add method to force re-provisioning of contexts
@ 2015-05-14 16:39 Alfonso Sanchez-Beato
2015-05-14 16:39 ` [PATCH 1/3] gprs: Add DBus method to reset contexts Alfonso Sanchez-Beato
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Alfonso Sanchez-Beato @ 2015-05-14 16:39 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 797 bytes --]
This series of patches implement a way to force re-provisioning of the
ConnectionManager contexts. Although the idea is similar to a previous
approach which was posted to the list, it is quite different as it
wipes all existing contexts and then runs the provisioning code. The
implemented method checks different conditions to make the reset safe
for clients.
Alfonso Sanchez-Beato (3):
gprs: Add DBus method to reset contexts
doc: Add description for ResetContexts method
test: Add script for resetting contexts
doc/connman-api.txt | 10 ++++
src/gprs.c | 144 +++++++++++++++++++++++++++++++++++++++++++---------
test/reset-contexts | 20 ++++++++
3 files changed, 150 insertions(+), 24 deletions(-)
create mode 100644 test/reset-contexts
--
2.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] gprs: Add DBus method to reset contexts 2015-05-14 16:39 [PATCH 0/3] Add method to force re-provisioning of contexts Alfonso Sanchez-Beato @ 2015-05-14 16:39 ` Alfonso Sanchez-Beato 2015-05-15 2:28 ` Denis Kenzior 2015-05-14 16:39 ` [PATCH 2/3] doc: Add description for ResetContexts method Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 3/3] test: Add script for resetting contexts Alfonso Sanchez-Beato 2 siblings, 1 reply; 8+ messages in thread From: Alfonso Sanchez-Beato @ 2015-05-14 16:39 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 5866 bytes --] Add DBus method that removes the current contexts and re-provisions using the APN database. --- src/gprs.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 120 insertions(+), 24 deletions(-) diff --git a/src/gprs.c b/src/gprs.c index 05ab499..de57b14 100644 --- a/src/gprs.c +++ b/src/gprs.c @@ -149,6 +149,8 @@ struct pri_context { static void gprs_netreg_update(struct ofono_gprs *gprs); static void gprs_deactivate_next(struct ofono_gprs *gprs); +static void provision_contexts(struct ofono_gprs *gprs, const char *mcc, + const char *mnc, const char *spn); static GSList *g_drivers = NULL; static GSList *g_context_drivers = NULL; @@ -1885,6 +1887,38 @@ static struct pri_context *add_context(struct ofono_gprs *gprs, return context; } +static void send_context_added_signal(struct ofono_gprs *gprs, + struct pri_context *context, + DBusConnection *conn) +{ + const char *path; + DBusMessage *signal; + + path = __ofono_atom_get_path(gprs->atom); + signal = dbus_message_new_signal(path, + OFONO_CONNECTION_MANAGER_INTERFACE, + "ContextAdded"); + + if (signal) { + DBusMessageIter iter; + DBusMessageIter dict; + + dbus_message_iter_init_append(signal, &iter); + + path = context->path; + dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, + &path); + + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, + OFONO_PROPERTIES_ARRAY_SIGNATURE, + &dict); + append_context_properties(context, &dict); + dbus_message_iter_close_container(&iter, &dict); + + g_dbus_send_message(conn, signal); + } +} + static DBusMessage *gprs_add_context(DBusConnection *conn, DBusMessage *msg, void *data) { @@ -1894,7 +1928,6 @@ static DBusMessage *gprs_add_context(DBusConnection *conn, const char *name; const char *path; enum ofono_gprs_context_type type; - DBusMessage *signal; if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &typestr, DBUS_TYPE_INVALID)) @@ -1916,29 +1949,7 @@ static DBusMessage *gprs_add_context(DBusConnection *conn, g_dbus_send_reply(conn, msg, DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_INVALID); - path = __ofono_atom_get_path(gprs->atom); - signal = dbus_message_new_signal(path, - OFONO_CONNECTION_MANAGER_INTERFACE, - "ContextAdded"); - - if (signal) { - DBusMessageIter iter; - DBusMessageIter dict; - - dbus_message_iter_init_append(signal, &iter); - - path = context->path; - dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, - &path); - - dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, - OFONO_PROPERTIES_ARRAY_SIGNATURE, - &dict); - append_context_properties(context, &dict); - dbus_message_iter_close_container(&iter, &dict); - - g_dbus_send_message(conn, signal); - } + send_context_added_signal(gprs, context, conn); return NULL; } @@ -2173,6 +2184,89 @@ static DBusMessage *gprs_get_contexts(DBusConnection *conn, return reply; } +static void remove_non_active_context(struct ofono_gprs *gprs, + struct pri_context *ctx, DBusConnection *conn) +{ + char *path; + const char *atompath; + + if (gprs->settings) { + g_key_file_remove_group(gprs->settings, ctx->key, NULL); + storage_sync(gprs->imsi, SETTINGS_STORE, gprs->settings); + } + + /* Make a backup copy of path for signal emission below */ + path = g_strdup(ctx->path); + + context_dbus_unregister(ctx); + gprs->contexts = g_slist_remove(gprs->contexts, ctx); + + atompath = __ofono_atom_get_path(gprs->atom); + g_dbus_emit_signal(conn, atompath, OFONO_CONNECTION_MANAGER_INTERFACE, + "ContextRemoved", DBUS_TYPE_OBJECT_PATH, &path, + DBUS_TYPE_INVALID); + g_free(path); +} + +static DBusMessage *gprs_reset_contexts(DBusConnection *conn, + DBusMessage *msg, void *data) +{ + struct ofono_gprs *gprs = data; + struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom); + struct ofono_sim *sim = __ofono_atom_find(OFONO_ATOM_TYPE_SIM, modem); + DBusMessage *reply; + GSList *l; + + if (gprs->pending) + return __ofono_error_busy(msg); + + for (l = gprs->contexts; l; l = l->next) { + struct pri_context *ctx = l->data; + + if (ctx->pending) + return __ofono_error_busy(msg); + } + + if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_INVALID)) + return __ofono_error_invalid_args(msg); + + if (gprs->powered) + return __ofono_error_not_allowed(msg); + + for (l = gprs->contexts; l; l = l->next) { + struct pri_context *ctx = l->data; + + if (ctx->active) + return __ofono_error_not_allowed(msg); + } + + reply = dbus_message_new_method_return(msg); + if (reply == NULL) + return NULL; + + /* Remove first the current contexts, re-provision after */ + + while (gprs->contexts != NULL) { + struct pri_context *ctx = gprs->contexts->data; + remove_non_active_context(gprs, ctx, conn); + } + + gprs->last_context_id = 0; + + provision_contexts(gprs, ofono_sim_get_mcc(sim), + ofono_sim_get_mnc(sim), ofono_sim_get_spn(sim)); + + if (gprs->contexts == NULL) /* Automatic provisioning failed */ + add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET); + + for (l = gprs->contexts; l; l = l->next) { + struct pri_context *ctx = l->data; + send_context_added_signal(gprs, ctx, conn); + } + + return reply; +} + static const GDBusMethodTable manager_methods[] = { { GDBUS_METHOD("GetProperties", NULL, GDBUS_ARGS({ "properties", "a{sv}" }), @@ -2192,6 +2286,8 @@ static const GDBusMethodTable manager_methods[] = { { GDBUS_METHOD("GetContexts", NULL, GDBUS_ARGS({ "contexts_with_properties", "a(oa{sv})" }), gprs_get_contexts) }, + { GDBUS_ASYNC_METHOD("ResetContexts", NULL, NULL, + gprs_reset_contexts) }, { } }; -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] gprs: Add DBus method to reset contexts 2015-05-14 16:39 ` [PATCH 1/3] gprs: Add DBus method to reset contexts Alfonso Sanchez-Beato @ 2015-05-15 2:28 ` Denis Kenzior 2015-05-18 6:53 ` Alfonso Sanchez-Beato 0 siblings, 1 reply; 8+ messages in thread From: Denis Kenzior @ 2015-05-15 2:28 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 6951 bytes --] Hi Alfonso, On 05/14/2015 11:39 AM, Alfonso Sanchez-Beato wrote: > Add DBus method that removes the current contexts and re-provisions > using the APN database. > --- > src/gprs.c | 144 ++++++++++++++++++++++++++++++++++++++++++++++++++----------- > 1 file changed, 120 insertions(+), 24 deletions(-) > Looks okay to me, just a couple of small nitpicks: > diff --git a/src/gprs.c b/src/gprs.c > index 05ab499..de57b14 100644 > --- a/src/gprs.c > +++ b/src/gprs.c > @@ -149,6 +149,8 @@ struct pri_context { > > static void gprs_netreg_update(struct ofono_gprs *gprs); > static void gprs_deactivate_next(struct ofono_gprs *gprs); > +static void provision_contexts(struct ofono_gprs *gprs, const char *mcc, > + const char *mnc, const char *spn); Our preference is not to have forward declarations of static functions unless absolutely necessary (e.g. circular dependencies). Can we move provision_contexts up to avoid this? Feel free to do this in a separate commit if this makes things clearer. > > static GSList *g_drivers = NULL; > static GSList *g_context_drivers = NULL; > @@ -1885,6 +1887,38 @@ static struct pri_context *add_context(struct ofono_gprs *gprs, > return context; > } > > +static void send_context_added_signal(struct ofono_gprs *gprs, > + struct pri_context *context, > + DBusConnection *conn) > +{ > + const char *path; > + DBusMessage *signal; > + > + path = __ofono_atom_get_path(gprs->atom); > + signal = dbus_message_new_signal(path, > + OFONO_CONNECTION_MANAGER_INTERFACE, > + "ContextAdded"); > + > + if (signal) { Please avoid unnecessary nesting. It is much easier to read this code if it was changed to: if (!signal) return; > + DBusMessageIter iter; > + DBusMessageIter dict; > + > + dbus_message_iter_init_append(signal, &iter); > + > + path = context->path; > + dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, > + &path); > + > + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, > + OFONO_PROPERTIES_ARRAY_SIGNATURE, > + &dict); > + append_context_properties(context, &dict); > + dbus_message_iter_close_container(&iter, &dict); > + > + g_dbus_send_message(conn, signal); > + } > +} > + > static DBusMessage *gprs_add_context(DBusConnection *conn, > DBusMessage *msg, void *data) > { > @@ -1894,7 +1928,6 @@ static DBusMessage *gprs_add_context(DBusConnection *conn, > const char *name; > const char *path; > enum ofono_gprs_context_type type; > - DBusMessage *signal; > > if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &typestr, > DBUS_TYPE_INVALID)) > @@ -1916,29 +1949,7 @@ static DBusMessage *gprs_add_context(DBusConnection *conn, > g_dbus_send_reply(conn, msg, DBUS_TYPE_OBJECT_PATH, &path, > DBUS_TYPE_INVALID); > > - path = __ofono_atom_get_path(gprs->atom); > - signal = dbus_message_new_signal(path, > - OFONO_CONNECTION_MANAGER_INTERFACE, > - "ContextAdded"); > - > - if (signal) { > - DBusMessageIter iter; > - DBusMessageIter dict; > - > - dbus_message_iter_init_append(signal, &iter); > - > - path = context->path; > - dbus_message_iter_append_basic(&iter, DBUS_TYPE_OBJECT_PATH, > - &path); > - > - dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, > - OFONO_PROPERTIES_ARRAY_SIGNATURE, > - &dict); > - append_context_properties(context, &dict); > - dbus_message_iter_close_container(&iter, &dict); > - > - g_dbus_send_message(conn, signal); > - } > + send_context_added_signal(gprs, context, conn); > > return NULL; > } > @@ -2173,6 +2184,89 @@ static DBusMessage *gprs_get_contexts(DBusConnection *conn, > return reply; > } > > +static void remove_non_active_context(struct ofono_gprs *gprs, > + struct pri_context *ctx, DBusConnection *conn) > +{ > + char *path; > + const char *atompath; > + > + if (gprs->settings) { > + g_key_file_remove_group(gprs->settings, ctx->key, NULL); > + storage_sync(gprs->imsi, SETTINGS_STORE, gprs->settings); > + } > + > + /* Make a backup copy of path for signal emission below */ > + path = g_strdup(ctx->path); > + > + context_dbus_unregister(ctx); > + gprs->contexts = g_slist_remove(gprs->contexts, ctx); > + > + atompath = __ofono_atom_get_path(gprs->atom); > + g_dbus_emit_signal(conn, atompath, OFONO_CONNECTION_MANAGER_INTERFACE, > + "ContextRemoved", DBUS_TYPE_OBJECT_PATH, &path, > + DBUS_TYPE_INVALID); > + g_free(path); > +} > + > +static DBusMessage *gprs_reset_contexts(DBusConnection *conn, > + DBusMessage *msg, void *data) > +{ > + struct ofono_gprs *gprs = data; > + struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom); > + struct ofono_sim *sim = __ofono_atom_find(OFONO_ATOM_TYPE_SIM, modem); > + DBusMessage *reply; > + GSList *l; > + > + if (gprs->pending) > + return __ofono_error_busy(msg); > + > + for (l = gprs->contexts; l; l = l->next) { > + struct pri_context *ctx = l->data; > + > + if (ctx->pending) > + return __ofono_error_busy(msg); > + } > + > + if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_INVALID)) > + return __ofono_error_invalid_args(msg); > + > + if (gprs->powered) > + return __ofono_error_not_allowed(msg); > + > + for (l = gprs->contexts; l; l = l->next) { > + struct pri_context *ctx = l->data; > + > + if (ctx->active) > + return __ofono_error_not_allowed(msg); Is there a particular reason you're running through the list of contexts again here? Why not check for ctx->active at the same time as ctx->pending? > + } > + > + reply = dbus_message_new_method_return(msg); > + if (reply == NULL) > + return NULL; > + > + /* Remove first the current contexts, re-provision after */ > + > + while (gprs->contexts != NULL) { > + struct pri_context *ctx = gprs->contexts->data; > + remove_non_active_context(gprs, ctx, conn); > + } > + > + gprs->last_context_id = 0; > + > + provision_contexts(gprs, ofono_sim_get_mcc(sim), > + ofono_sim_get_mnc(sim), ofono_sim_get_spn(sim)); > + > + if (gprs->contexts == NULL) /* Automatic provisioning failed */ > + add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET); > + > + for (l = gprs->contexts; l; l = l->next) { > + struct pri_context *ctx = l->data; > + send_context_added_signal(gprs, ctx, conn); > + } > + > + return reply; > +} > + > static const GDBusMethodTable manager_methods[] = { > { GDBUS_METHOD("GetProperties", > NULL, GDBUS_ARGS({ "properties", "a{sv}" }), > @@ -2192,6 +2286,8 @@ static const GDBusMethodTable manager_methods[] = { > { GDBUS_METHOD("GetContexts", NULL, > GDBUS_ARGS({ "contexts_with_properties", "a(oa{sv})" }), > gprs_get_contexts) }, > + { GDBUS_ASYNC_METHOD("ResetContexts", NULL, NULL, > + gprs_reset_contexts) }, > { } > }; > > Regards, -Denis ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] gprs: Add DBus method to reset contexts 2015-05-15 2:28 ` Denis Kenzior @ 2015-05-18 6:53 ` Alfonso Sanchez-Beato 2015-05-18 14:09 ` Denis Kenzior 0 siblings, 1 reply; 8+ messages in thread From: Alfonso Sanchez-Beato @ 2015-05-18 6:53 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 9393 bytes --] Hi Denis, On Fri, May 15, 2015 at 4:28 AM, Denis Kenzior <denkenz@gmail.com> wrote: > Hi Alfonso, > > On 05/14/2015 11:39 AM, Alfonso Sanchez-Beato wrote: > >> Add DBus method that removes the current contexts and re-provisions >> using the APN database. >> --- >> src/gprs.c | 144 >> ++++++++++++++++++++++++++++++++++++++++++++++++++----------- >> 1 file changed, 120 insertions(+), 24 deletions(-) >> >> > Looks okay to me, just a couple of small nitpicks: > > diff --git a/src/gprs.c b/src/gprs.c >> index 05ab499..de57b14 100644 >> --- a/src/gprs.c >> +++ b/src/gprs.c >> @@ -149,6 +149,8 @@ struct pri_context { >> >> static void gprs_netreg_update(struct ofono_gprs *gprs); >> static void gprs_deactivate_next(struct ofono_gprs *gprs); >> +static void provision_contexts(struct ofono_gprs *gprs, const char *mcc, >> + const char *mnc, const char *spn); >> > > Our preference is not to have forward declarations of static functions > unless absolutely necessary (e.g. circular dependencies). Can we move > provision_contexts up to avoid this? Feel free to do this in a separate > commit if this makes things clearer. > > >> static GSList *g_drivers = NULL; >> static GSList *g_context_drivers = NULL; >> @@ -1885,6 +1887,38 @@ static struct pri_context *add_context(struct >> ofono_gprs *gprs, >> return context; >> } >> >> +static void send_context_added_signal(struct ofono_gprs *gprs, >> + struct pri_context *context, >> + DBusConnection *conn) >> +{ >> + const char *path; >> + DBusMessage *signal; >> + >> + path = __ofono_atom_get_path(gprs->atom); >> + signal = dbus_message_new_signal(path, >> + >> OFONO_CONNECTION_MANAGER_INTERFACE, >> + "ContextAdded"); >> + >> + if (signal) { >> > > Please avoid unnecessary nesting. It is much easier to read this code if > it was changed to: > > if (!signal) > return; > > > + DBusMessageIter iter; >> + DBusMessageIter dict; >> + >> + dbus_message_iter_init_append(signal, &iter); >> + >> + path = context->path; >> + dbus_message_iter_append_basic(&iter, >> DBUS_TYPE_OBJECT_PATH, >> + &path); >> + >> + dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, >> + OFONO_PROPERTIES_ARRAY_SIGNATURE, >> + &dict); >> + append_context_properties(context, &dict); >> + dbus_message_iter_close_container(&iter, &dict); >> + >> + g_dbus_send_message(conn, signal); >> + } >> +} >> + >> static DBusMessage *gprs_add_context(DBusConnection *conn, >> DBusMessage *msg, void *data) >> { >> @@ -1894,7 +1928,6 @@ static DBusMessage *gprs_add_context(DBusConnection >> *conn, >> const char *name; >> const char *path; >> enum ofono_gprs_context_type type; >> - DBusMessage *signal; >> >> if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &typestr, >> DBUS_TYPE_INVALID)) >> @@ -1916,29 +1949,7 @@ static DBusMessage >> *gprs_add_context(DBusConnection *conn, >> g_dbus_send_reply(conn, msg, DBUS_TYPE_OBJECT_PATH, &path, >> DBUS_TYPE_INVALID); >> >> - path = __ofono_atom_get_path(gprs->atom); >> - signal = dbus_message_new_signal(path, >> - >> OFONO_CONNECTION_MANAGER_INTERFACE, >> - "ContextAdded"); >> - >> - if (signal) { >> - DBusMessageIter iter; >> - DBusMessageIter dict; >> - >> - dbus_message_iter_init_append(signal, &iter); >> - >> - path = context->path; >> - dbus_message_iter_append_basic(&iter, >> DBUS_TYPE_OBJECT_PATH, >> - &path); >> - >> - dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY, >> - OFONO_PROPERTIES_ARRAY_SIGNATURE, >> - &dict); >> - append_context_properties(context, &dict); >> - dbus_message_iter_close_container(&iter, &dict); >> - >> - g_dbus_send_message(conn, signal); >> - } >> + send_context_added_signal(gprs, context, conn); >> >> return NULL; >> } >> @@ -2173,6 +2184,89 @@ static DBusMessage >> *gprs_get_contexts(DBusConnection *conn, >> return reply; >> } >> >> +static void remove_non_active_context(struct ofono_gprs *gprs, >> + struct pri_context *ctx, DBusConnection >> *conn) >> +{ >> + char *path; >> + const char *atompath; >> + >> + if (gprs->settings) { >> + g_key_file_remove_group(gprs->settings, ctx->key, NULL); >> + storage_sync(gprs->imsi, SETTINGS_STORE, gprs->settings); >> + } >> + >> + /* Make a backup copy of path for signal emission below */ >> + path = g_strdup(ctx->path); >> + >> + context_dbus_unregister(ctx); >> + gprs->contexts = g_slist_remove(gprs->contexts, ctx); >> + >> + atompath = __ofono_atom_get_path(gprs->atom); >> + g_dbus_emit_signal(conn, atompath, >> OFONO_CONNECTION_MANAGER_INTERFACE, >> + "ContextRemoved", DBUS_TYPE_OBJECT_PATH, >> &path, >> + DBUS_TYPE_INVALID); >> + g_free(path); >> +} >> + >> +static DBusMessage *gprs_reset_contexts(DBusConnection *conn, >> + DBusMessage *msg, void *data) >> +{ >> + struct ofono_gprs *gprs = data; >> + struct ofono_modem *modem = __ofono_atom_get_modem(gprs->atom); >> + struct ofono_sim *sim = __ofono_atom_find(OFONO_ATOM_TYPE_SIM, >> modem); >> + DBusMessage *reply; >> + GSList *l; >> + >> + if (gprs->pending) >> + return __ofono_error_busy(msg); >> + >> + for (l = gprs->contexts; l; l = l->next) { >> + struct pri_context *ctx = l->data; >> + >> + if (ctx->pending) >> + return __ofono_error_busy(msg); >> + } >> + >> + if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_INVALID)) >> + return __ofono_error_invalid_args(msg); >> + >> + if (gprs->powered) >> + return __ofono_error_not_allowed(msg); >> + >> + for (l = gprs->contexts; l; l = l->next) { >> + struct pri_context *ctx = l->data; >> + >> + if (ctx->active) >> + return __ofono_error_not_allowed(msg); >> > > Is there a particular reason you're running through the list of contexts > again here? Why not check for ctx->active at the same time as ctx->pending? I wanted to make sure that __ofono_error_busy was returned before __ofono_error_not_allowed if one context was busy and another one was active, for instance. Not that I feel is really important and I can change this if the other way is preferred. > > > + } >> + >> + reply = dbus_message_new_method_return(msg); >> + if (reply == NULL) >> + return NULL; >> + >> + /* Remove first the current contexts, re-provision after */ >> + >> + while (gprs->contexts != NULL) { >> + struct pri_context *ctx = gprs->contexts->data; >> + remove_non_active_context(gprs, ctx, conn); >> + } >> + >> + gprs->last_context_id = 0; >> + >> + provision_contexts(gprs, ofono_sim_get_mcc(sim), >> + ofono_sim_get_mnc(sim), >> ofono_sim_get_spn(sim)); >> + >> + if (gprs->contexts == NULL) /* Automatic provisioning failed */ >> + add_context(gprs, NULL, OFONO_GPRS_CONTEXT_TYPE_INTERNET); >> + >> + for (l = gprs->contexts; l; l = l->next) { >> + struct pri_context *ctx = l->data; >> + send_context_added_signal(gprs, ctx, conn); >> + } >> + >> + return reply; >> +} >> + >> static const GDBusMethodTable manager_methods[] = { >> { GDBUS_METHOD("GetProperties", >> NULL, GDBUS_ARGS({ "properties", "a{sv}" }), >> @@ -2192,6 +2286,8 @@ static const GDBusMethodTable manager_methods[] = { >> { GDBUS_METHOD("GetContexts", NULL, >> GDBUS_ARGS({ "contexts_with_properties", >> "a(oa{sv})" }), >> gprs_get_contexts) }, >> + { GDBUS_ASYNC_METHOD("ResetContexts", NULL, NULL, >> + gprs_reset_contexts) }, >> { } >> }; >> >> >> > Regards, > -Denis > Thanks for the comments, I have re-submitted the patches addressing the issues pointed out above. Br, Alfonso > _______________________________________________ > ofono mailing list > ofono(a)ofono.org > https://lists.ofono.org/mailman/listinfo/ofono > [-- Attachment #2: attachment.html --] [-- Type: text/html, Size: 12631 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] gprs: Add DBus method to reset contexts 2015-05-18 6:53 ` Alfonso Sanchez-Beato @ 2015-05-18 14:09 ` Denis Kenzior 2015-05-18 17:08 ` Alfonso Sanchez-Beato 0 siblings, 1 reply; 8+ messages in thread From: Denis Kenzior @ 2015-05-18 14:09 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1546 bytes --] On 05/18/2015 01:53 AM, Alfonso Sanchez-Beato wrote: > Hi Denis, > <snip> > + > + for (l = gprs->contexts; l; l = l->next) { > + struct pri_context *ctx = l->data; > + > + if (ctx->pending) > + return __ofono_error_busy(msg); > + } > + > + if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_INVALID)) > + return __ofono_error_invalid_args(msg); > + > + if (gprs->powered) > + return __ofono_error_not_allowed(msg); > + > + for (l = gprs->contexts; l; l = l->next) { > + struct pri_context *ctx = l->data; > + > + if (ctx->active) > + return __ofono_error_not_allowed(msg); > > > Is there a particular reason you're running through the list of > contexts again here? Why not check for ctx->active at the same time > as ctx->pending? > > > I wanted to make sure that __ofono_error_busy was returned before > __ofono_error_not_allowed if one context was busy and another one was > active, for instance. Not that I feel is really important and I can > change this if the other way is preferred. > Makes sense. However, since this is not obvious, we probably should add a comment as to why this is being done. Want to submit a patch or do you want me to do this? Regards, -Denis ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] gprs: Add DBus method to reset contexts 2015-05-18 14:09 ` Denis Kenzior @ 2015-05-18 17:08 ` Alfonso Sanchez-Beato 0 siblings, 0 replies; 8+ messages in thread From: Alfonso Sanchez-Beato @ 2015-05-18 17:08 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1867 bytes --] On Mon, May 18, 2015 at 4:09 PM, Denis Kenzior <denkenz@gmail.com> wrote: > On 05/18/2015 01:53 AM, Alfonso Sanchez-Beato wrote: > >> Hi Denis, >> >> > <snip> > > > + >> + for (l = gprs->contexts; l; l = l->next) { >> + struct pri_context *ctx = l->data; >> + >> + if (ctx->pending) >> + return __ofono_error_busy(msg); >> + } >> + >> + if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_INVALID)) >> + return __ofono_error_invalid_args(msg); >> + >> + if (gprs->powered) >> + return __ofono_error_not_allowed(msg); >> + >> + for (l = gprs->contexts; l; l = l->next) { >> + struct pri_context *ctx = l->data; >> + >> + if (ctx->active) >> + return __ofono_error_not_allowed(msg); >> >> >> Is there a particular reason you're running through the list of >> contexts again here? Why not check for ctx->active at the same time >> as ctx->pending? >> >> >> I wanted to make sure that __ofono_error_busy was returned before >> __ofono_error_not_allowed if one context was busy and another one was >> active, for instance. Not that I feel is really important and I can >> change this if the other way is preferred. >> >> > Makes sense. However, since this is not obvious, we probably should add a > comment as to why this is being done. Want to submit a patch or do you > want me to do this? > Submitted. Br, Alfonso > > Regards, > -Denis > > _______________________________________________ > ofono mailing list > ofono(a)ofono.org > https://lists.ofono.org/mailman/listinfo/ofono > [-- Attachment #2: attachment.html --] [-- Type: text/html, Size: 3002 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] doc: Add description for ResetContexts method 2015-05-14 16:39 [PATCH 0/3] Add method to force re-provisioning of contexts Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 1/3] gprs: Add DBus method to reset contexts Alfonso Sanchez-Beato @ 2015-05-14 16:39 ` Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 3/3] test: Add script for resetting contexts Alfonso Sanchez-Beato 2 siblings, 0 replies; 8+ messages in thread From: Alfonso Sanchez-Beato @ 2015-05-14 16:39 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 798 bytes --] --- doc/connman-api.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/connman-api.txt b/doc/connman-api.txt index 8f72087..1ce1d61 100644 --- a/doc/connman-api.txt +++ b/doc/connman-api.txt @@ -60,6 +60,16 @@ Methods dict GetProperties() [service].Error.NotFound [service].Error.Failed + void ResetContexts() + + Removes all contexts and re-provisions from the APN + database. Contexts must all be deactivated for this + method to work, and the atom must not be powered. + + Possible Errors: [service].Error.InProgress + [service].Error.InvalidArguments + [service].Error.NotAllowed + Signals PropertyChanged(string property, variant value) This signal indicates a changed value of the given -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] test: Add script for resetting contexts 2015-05-14 16:39 [PATCH 0/3] Add method to force re-provisioning of contexts Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 1/3] gprs: Add DBus method to reset contexts Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 2/3] doc: Add description for ResetContexts method Alfonso Sanchez-Beato @ 2015-05-14 16:39 ` Alfonso Sanchez-Beato 2 siblings, 0 replies; 8+ messages in thread From: Alfonso Sanchez-Beato @ 2015-05-14 16:39 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 766 bytes --] --- test/reset-contexts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 test/reset-contexts diff --git a/test/reset-contexts b/test/reset-contexts new file mode 100644 index 0000000..1676636 --- /dev/null +++ b/test/reset-contexts @@ -0,0 +1,20 @@ +#!/usr/bin/python3 + +import dbus +import sys + +bus = dbus.SystemBus() + +if len(sys.argv) == 2: + path = sys.argv[1] +else: + manager = dbus.Interface(bus.get_object('org.ofono', '/'), + 'org.ofono.Manager') + modems = manager.GetModems() + path = modems[0][0] + +print("Resetting contexts for SIM on modem %s..." % path) +cm = dbus.Interface(bus.get_object('org.ofono', path), + 'org.ofono.ConnectionManager') + +cm.ResetContexts() -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-05-18 17:08 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-14 16:39 [PATCH 0/3] Add method to force re-provisioning of contexts Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 1/3] gprs: Add DBus method to reset contexts Alfonso Sanchez-Beato 2015-05-15 2:28 ` Denis Kenzior 2015-05-18 6:53 ` Alfonso Sanchez-Beato 2015-05-18 14:09 ` Denis Kenzior 2015-05-18 17:08 ` Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 2/3] doc: Add description for ResetContexts method Alfonso Sanchez-Beato 2015-05-14 16:39 ` [PATCH 3/3] test: Add script for resetting contexts Alfonso Sanchez-Beato
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.