[PATCH 1/3] Add radio settings atom and driver API

Aki Niemi aki at protocolpolice.com
Thu Feb 4 14:01:16 PST 2010


Hi Denis,

2010/2/4 Denis Kenzior <denkenz at gmail.com>:
>> +typedef void (*ofono_radio_settings_mode_query_cb_t)(const struct
>>  ofono_error *error, +                                                enum ofono_radio_access_mode mode,
>> +                                             void *data);
>
> I still say this part is not required.
>
>> +
>> +struct ofono_radio_settings_driver {
>> +     const char *name;
>> +     int (*probe)(struct ofono_radio_settings *rs, unsigned int vendor, void
>>  *data); +    void (*remove)(struct ofono_radio_settings *rs);
>> +     void (*query_mode)(struct ofono_radio_settings *rs,
>> +                             ofono_radio_settings_mode_query_cb_t cb, void *data);
>
> Neither is query_mode.  This is a local modem setting, not a network setting.
> There's simply no reason to query it when oFono can store the settings.

Even with storage, it's needed the very first time a new SIM card is
inserted, no?

> Lets just add ability to read the mode setting from storage and set it at
> interface startup.

I can agreee to storing SIM data, but for other settings that are
anyway saved on the modem's non-volatile memory, this seems wrong.
oFono is aimed to be usable also on the desktop using tethering, which
means most of the time these settings are touched on the phone.

(As an aside, I have right now two N900s connected to my laptop via
USB, and was just making calls between them using oFono and d-feet.)

>> +     void (*set_mode)(struct ofono_radio_settings *rs,
>> +                             enum ofono_radio_access_mode mode,
>> +                             ofono_radio_settings_mode_set_cb_t cb, void *data);
>
> Should this be called something else? E.g. set_technology or set_rat_mode.

Makes sense.

>> +#define RADIO_SETTINGS_INTERFACE "org.ofono.RadioSettings"
>> +#define RADIO_SETTINGS_INTERFACE "org.ofono.RadioSettings"
>> +
>
> Double define?

Too much coffee.

> Otherwise it looks good to me.

Cool. I will make the changes and push in a bit.

Cheers,
Aki


More information about the ofono mailing list