Re: [PATCH 2/5] speedup: Check for supported modem capabilities first

[Guillaume Zajac <guillaume.zajac@linux.intel.com>]

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

Hi Denis,

>>>>>>    struct speedup_data {
>>>>>>        GAtChat *modem;
>>>>>>        GAtChat *aux;
>>>>>>        gboolean have_sim;
>>>>>>        struct at_util_sim_state_query *sim_state_query;
>>>>>> +    gboolean have_gsm;
>>>>>> +    gboolean have_cdma;
>>>>> It might be a good idea to use a single gboolean, or better yet an enum
>>>>> here instead.  There's no point to waste 8 bytes when a single byte
>>>>> can do.
>>>> I did this on purpose for the Huawei driver. We can unify this later on,
>>>> but at this moment I rather see what is actually happening.
>>>>
>>>> Since we keep parsing all capabilities, I wanna avoid that a later one
>>>> overwrites a previous one. That said, just using some flags would be
>>>> better anyway. Especially since we also always have have_sim as well.
>>>>
>>>> However that can be done as a further optimization in the Speedup and
>>>> Huawei drivers.
>>>>
>>> A flagged enum is indeed what I had in mind when I wrote this.  But fair
>>> enough on your reasoning
>> As we are starting thinking about tweaking SIM atom to make it work with
>> CDMA modem,
>> maybe having quickly the flagged enum would be helpful to distinguish if
>> it is a CDMA or GSM modem that is asking for the SIM atom creation.
>>
>> Then where should this enum modem_type take place?
>> Should we tweak the already existing one into modem.h in replacing:
>>
>> OFONO_MODEM_TYPE_HARDWARE = 0,
>> OFONO_MODEM_TYPE_HFP,
>> OFONO_MODEM_TYPE_SAP,
>>
>> by
>>
>> OFONO_MODEM_TYPE_UNKNOWN_HARDWARE = 0,
>> OFONO_MODEM_TYPE_GSM_HARDWARE = 1,
>> OFONO_MODEM_TYPE_CDMA_HARDWARE = 2,
>> OFONO_MODEM_TYPE_HFP = 4,
>> OFONO_MODEM_TYPE_SAP = 8,
>>
>> Then we have just to specify the modem_type in creating the SIM atom to
>> use its limited functionnalities when we have CDMA modem
>> e.g. manage only PIN protection.
> No, the type information is a kludge that will hamper us in the end, so
> I am against any such changes.  Even the current modem_type enum took
> some convincing and it is meant to be used as a hint for external
> applications, not used internally by oFono core in any way

We can have a private enum modem type into plugin, but we won't be able 
to use it into any atoms.
Maybe by deduction we can add an enum sim_type into sim.h store the type 
into the plugin data and pass it at SIM atom creation.
What do you think?

Kind regards,
Guillaume