Re: [PATCH 11/11] gprs: Add API to set a 'detach'notification callback

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Philippe,

On 06/28/2011 12:16 PM, Philippe Nunes wrote:
> ---
>  src/gprs.c  |   37 +++++++++++++++++++++++++++++++++++++
>  src/ofono.h |    6 ++++++
>  2 files changed, 43 insertions(+), 0 deletions(-)
> 
> diff --git a/src/gprs.c b/src/gprs.c
> index b3e6869..0166e70 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -1495,6 +1495,28 @@ static void gprs_attached_update(struct ofono_gprs *gprs)
>  					"Active", DBUS_TYPE_BOOLEAN, &value);
>  		}
>  
> +		for (l = g_private_contexts; l; l = l->next) {
> +			struct gprs_private_context *ctx = l->data;
> +			struct context_settings *settings =
> +						ctx->context_driver->settings;
> +
> +			pri_set_ipv4_addr(settings->interface, NULL);
> +			pri_ifupdown(settings->interface, FALSE);
> +			context_settings_free(settings);
> +
> +			DBG("Release private context cid %d", ctx->cid);
> +
> +			gprs_cid_release(gprs, ctx->cid);
> +			ctx->context_driver->inuse = FALSE;
> +
> +			if (ctx->notify)
> +				((__ofono_gprs_deactivated_on_detach_cb_t)
> +						ctx->notify)(ctx->notify_data);
> +
> +			g_private_contexts = g_slist_remove(g_private_contexts,
> +								ctx);
> +		}
> +

Doing this only here might not quite be enough.  The context can also be
deactivated due to gprs_context atom destruction or network forced detach.

>  		gprs->bearer = -1;
>  	}
>  
> @@ -3223,3 +3245,18 @@ int __ofono_gprs_deactivate_context(unsigned int id,
>  				deactivate_request_callback, private_ctx);
>  	return 0;
>  }
> +
> +int __ofono_gprs_set_deactivated_on_detach_notify(unsigned int id,
> +				__ofono_gprs_deactivated_on_detach_cb_t cb,
> +				void *data)

I'd prefer ofono_gprs_private_context_set_detached_notify

> +{
> +	struct gprs_private_context *private_ctx;
> +
> +	private_ctx = gprs_private_context_by_id(id);
> +	if (private_ctx == NULL)
> +		return -EINVAL;
> +
> +	private_ctx->notify = cb;
> +	private_ctx->notify_data = data;
> +	return 0;
> +}
> diff --git a/src/ofono.h b/src/ofono.h
> index 6b72816..a1215e8 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -248,6 +248,8 @@ typedef void (*__ofono_gprs_activate_context_cb_t)(int error,
>  
>  typedef void (*__ofono_gprs_deactivate_context_cb_t)(int error, void *data);
>  
> +typedef void (*__ofono_gprs_deactivated_on_detach_cb_t)(void *data);
> +
>  int __ofono_gprs_deactivate_context(unsigned int id,
>  					__ofono_gprs_deactivate_context_cb_t cb,
>  					void *data);
> @@ -257,6 +259,10 @@ int __ofono_gprs_activate_context(struct ofono_gprs *gprs,
>  				__ofono_gprs_activate_context_cb_t cb,
>  				void *data);
>  
> +int __ofono_gprs_set_deactivated_on_detach_notify(unsigned int id,
> +				__ofono_gprs_deactivated_on_detach_cb_t cb,
> +				void *data);
> +
>  #include <ofono/radio-settings.h>
>  #include <ofono/audio-settings.h>
>  #include <ofono/ctm.h>

Do we have some way of testing at least the channel open / close parts?
 Can we add this to phonesim to enable some basic sanity testing?

Regards,
-Denis