Hi Denis, 2010/2/4 Denis Kenzior <[email protected]>: >> +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 _______________________________________________ ofono mailing list [email protected] http://lists.ofono.org/listinfo/ofono
