Re: [PATCH_v2 2/4] emulator: add routine to request/release private network from ConnMan

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Guillaume,

On 04/28/2011 08:29 AM, Guillaume Zajac wrote:
> Hi Denis,
> 
> On 27/04/2011 05:25, Denis Kenzior wrote:
>> Hi Guillaume,
>>
>> On 04/22/2011 07:06 AM, Guillaume Zajac wrote:
>>> ---
>>>   include/emulator.h |   10 +++++++
>>>   src/emulator.c     |   75
>>> +++++++++++++++++++++++++++++++++++++++++++++++-----
>>>   2 files changed, 78 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/emulator.h b/include/emulator.h
>>> index 5cd894b..29abf55 100644
>>> --- a/include/emulator.h
>>> +++ b/include/emulator.h
>>> @@ -105,6 +105,16 @@ enum ofono_emulator_request_type
>>> ofono_emulator_request_get_type(
>>>   void ofono_emulator_set_indicator(struct ofono_emulator *em,
>>>                       const char *name, int value);
>>>
>>> +typedef void (*network_request)(struct ofono_error *, int *,
>>> +                const char **, const char **,
>>> +                const char **, const char **);
>>> +typedef void (*network_release)(void);
>>> +
>>> +void ofono_emulator_add_network_request_cb(struct ofono_emulator *em,
>>> +                            network_request cb);
>>> +void ofono_emulator_add_network_release_cb(struct ofono_emulator *em,
>>> +                            network_release cb);
>>> +
>> I really don't like this, you're re-inventing the wheel here.  We
>> already solved this pattern a while ago ;)  I suggest you look at
>> include/gprs-provision.h and examples/provision.c
> 
> In looking the example, I should create some emulator network drivers to
> get settings and release the network like:
> 
> struct emulator_network{
>     ...
> }

struct private_network_settings or something might be better

> 
> struct emulator_network_provision_driver{
>     char *name;
>     (void)(*get_settings)(struct emulator_network **out_en);

You will probably need to make this async, and allow cancellation, since
there can be multiple emulators requesting private networks
concurrently.  This probably means you need to return a unique
identifier as well.

>     (void)(*release)(void);

And you need to take the unique id as input here.

> }
> 
> and un/register drivers into emulator.c
> 
>>>   #ifdef __cplusplus
>>>   }
>>>   #endif
>> Regards,
>> -Denis
>>
> 
> Kind regards,
> Guillaume

Regards,
-Denis