Hi Mika,

On 10/25/2010 08:28 AM, Mika Liljeberg wrote:
> ---
>  include/radio-settings.h |   11 ++++
>  src/radio-settings.c     |  134 +++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 133 insertions(+), 12 deletions(-)
> 
> diff --git a/include/radio-settings.h b/include/radio-settings.h
> index d41ec0b..a6b19d0 100644
> --- a/include/radio-settings.h
> +++ b/include/radio-settings.h
> @@ -42,6 +42,10 @@ typedef void (*ofono_radio_settings_rat_mode_set_cb_t)(const struct ofono_error
>  typedef void (*ofono_radio_settings_rat_mode_query_cb_t)(const struct ofono_error *error,
>  						enum ofono_radio_access_mode mode,
>  						void *data);
> +typedef void (*ofono_radio_settings_fast_dormancy_set_cb_t)(const struct ofono_error *error,
> +							void *data);
> +typedef void (*ofono_radio_settings_fast_dormancy_query_cb_t)(const struct ofono_error *error,
> +							int enable, void *data);
>  
>  struct ofono_radio_settings_driver {
>  	const char *name;
> @@ -55,6 +59,13 @@ struct ofono_radio_settings_driver {
>  				enum ofono_radio_access_mode mode,
>  				ofono_radio_settings_rat_mode_set_cb_t cb,
>  				void *data);
> +	void (*query_fast_dormancy)(struct ofono_radio_settings *rs,
> +			ofono_radio_settings_fast_dormancy_query_cb_t cb,
> +			void *data);
> +	void (*set_fast_dormancy)(struct ofono_radio_settings *rs,
> +				int enable,
> +				ofono_radio_settings_fast_dormancy_set_cb_t,
> +				void *data);
>  };
>  
>  int ofono_radio_settings_driver_register(const struct ofono_radio_settings_driver *d);
> diff --git a/src/radio-settings.c b/src/radio-settings.c
> index 3306be6..5441481 100644
> --- a/src/radio-settings.c
> +++ b/src/radio-settings.c
> @@ -33,7 +33,7 @@
>  #include "ofono.h"
>  #include "common.h"
>  
> -#define RADIO_SETTINGS_MODE_CACHED 0x1
> +#define RADIO_SETTINGS_FLAG_CACHED 0x1
>  
>  static GSList *g_drivers = NULL;
>  
> @@ -42,6 +42,8 @@ struct ofono_radio_settings {
>  	int flags;
>  	enum ofono_radio_access_mode mode;
>  	enum ofono_radio_access_mode pending_mode;
> +	int fast_dormancy;
> +	int fast_dormancy_pending;

I suggest simply using ofono_bool_t here.

>  	const struct ofono_radio_settings_driver *driver;
>  	void *driver_data;
>  	struct ofono_atom *atom;
> @@ -91,8 +93,6 @@ static DBusMessage *radio_get_properties_reply(DBusMessage *msg,
>  	DBusMessageIter iter;
>  	DBusMessageIter dict;
>  
> -	const char *mode = radio_access_mode_to_string(rs->mode);
> -
>  	reply = dbus_message_new_method_return(msg);
>  	if (!reply)
>  		return NULL;
> @@ -103,14 +103,60 @@ static DBusMessage *radio_get_properties_reply(DBusMessage *msg,
>  					OFONO_PROPERTIES_ARRAY_SIGNATURE,
>  					&dict);
>  
> -	ofono_dbus_dict_append(&dict, "TechnologyPreference",
> +	if ((int)rs->mode != -1) {
> +		const char *mode = radio_access_mode_to_string(rs->mode);
> +		ofono_dbus_dict_append(&dict, "TechnologyPreference",
>  					DBUS_TYPE_STRING, &mode);
> +	}
> +

I suggest always showing this property, otherwise RadioSettings
interface is pretty pointless.

> +	if (rs->fast_dormancy != -1) {
> +		dbus_bool_t value = (rs->fast_dormancy != 0);
> +		ofono_dbus_dict_append(&dict, "FastDormancy",
> +					DBUS_TYPE_BOOLEAN, &value);
> +	}

This should be guarded by the query_fast_dormancy implementation
availability.

>  
>  	dbus_message_iter_close_container(&iter, &dict);
>  
>  	return reply;
>  }
>  
> +static void radio_set_fast_dormancy(struct ofono_radio_settings *rs, int enable)
> +{

I really suggest something like:

if (rs->fast_dormancy == enable)
	return;

...

> +	if (rs->fast_dormancy != enable) {
> +

In general, please don't add empty lines like this

> +		DBusConnection *conn = ofono_dbus_get_connection();
> +		const char *path = __ofono_atom_get_path(rs->atom);
> +		dbus_bool_t value = (enable != 0);
> +
> +		ofono_dbus_signal_property_changed(conn, path,
> +						OFONO_RADIO_SETTINGS_INTERFACE,
> +						"FastDormancy",
> +						DBUS_TYPE_BOOLEAN, &value);
> +	}
> +
> +	rs->fast_dormancy = enable;
> +}
> +
> +static void radio_fast_dormancy_set_callback(const struct ofono_error *error,
> +						void *data)
> +{
> +	struct ofono_radio_settings *rs = data;
> +	DBusMessage *reply;
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		DBG("Error setting fast dormancy");
> +		rs->fast_dormancy_pending = rs->fast_dormancy;
> +		reply = __ofono_error_failed(rs->pending);
> +		__ofono_dbus_pending_reply(&rs->pending, reply);
> +		return;
> +	}
> +
> +	reply = dbus_message_new_method_return(rs->pending);
> +	__ofono_dbus_pending_reply(&rs->pending, reply);
> +
> +	radio_set_fast_dormancy(rs, rs->fast_dormancy_pending);
> +}
> +
>  static void radio_set_rat_mode(struct ofono_radio_settings *rs,
>  				enum ofono_radio_access_mode mode)
>  {
> @@ -122,7 +168,6 @@ static void radio_set_rat_mode(struct ofono_radio_settings *rs,
>  		return;
>  
>  	rs->mode = mode;
> -	rs->flags |= RADIO_SETTINGS_MODE_CACHED;
>  
>  	path = __ofono_atom_get_path(rs->atom);
>  	str_mode = radio_access_mode_to_string(rs->mode);
> @@ -152,6 +197,43 @@ static void radio_mode_set_callback(const struct ofono_error *error, void *data)
>  	radio_set_rat_mode(rs, rs->pending_mode);
>  }
>  
> +static void radio_send_properties_reply(struct ofono_radio_settings *rs)
> +{
> +	DBusMessage *reply;
> +
> +	rs->flags |= RADIO_SETTINGS_FLAG_CACHED;
> +	reply = radio_get_properties_reply(rs->pending, rs);
> +	__ofono_dbus_pending_reply(&rs->pending, reply);
> +}
> +
> +static void radio_fast_dormancy_query_callback(const struct ofono_error *error,
> +						int enable, void *data)
> +{
> +	struct ofono_radio_settings *rs = data;
> +	DBusMessage *reply;
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> +		DBG("Error during fast dormancy query");
> +		reply = __ofono_error_failed(rs->pending);
> +		__ofono_dbus_pending_reply(&rs->pending, reply);
> +		return;
> +	}
> +
> +	radio_set_fast_dormancy(rs, enable);
> +	radio_send_properties_reply(rs);
> +}
> +
> +static void radio_query_fast_dormancy(struct ofono_radio_settings *rs)
> +{
> +	if (!rs->driver->query_fast_dormancy) {
> +		radio_send_properties_reply(rs);
> +		return;
> +	}
> +
> +	rs->driver->query_fast_dormancy(rs, radio_fast_dormancy_query_callback,
> +					rs);
> +}
> +
>  static void radio_rat_mode_query_callback(const struct ofono_error *error,
>  					enum ofono_radio_access_mode mode,
>  					void *data)
> @@ -167,9 +249,17 @@ static void radio_rat_mode_query_callback(const struct ofono_error *error,
>  	}
>  
>  	radio_set_rat_mode(rs, mode);
> +	radio_query_fast_dormancy(rs);
> +}
>  
> -	reply = radio_get_properties_reply(rs->pending, rs);
> -	__ofono_dbus_pending_reply(&rs->pending, reply);
> +static void radio_query_rat_mode(struct ofono_radio_settings *rs)
> +{
> +	if (!rs->driver->query_rat_mode) {
> +		radio_query_fast_dormancy(rs);
> +		return;
> +	}
> +
> +	rs->driver->query_rat_mode(rs, radio_rat_mode_query_callback, rs);
>  }
>  
>  static DBusMessage *radio_get_properties(DBusConnection *conn,
> @@ -177,17 +267,14 @@ static DBusMessage *radio_get_properties(DBusConnection *conn,
>  {
>  	struct ofono_radio_settings *rs = data;
>  
> -	if (rs->flags & RADIO_SETTINGS_MODE_CACHED)
> +	if (rs->flags & RADIO_SETTINGS_FLAG_CACHED)
>  		return radio_get_properties_reply(msg, rs);
>  
> -	if (!rs->driver->query_rat_mode)
> -		return __ofono_error_not_implemented(msg);
> -
>  	if (rs->pending)
>  		return __ofono_error_busy(msg);
>  
>  	rs->pending = dbus_message_ref(msg);
> -	rs->driver->query_rat_mode(rs, radio_rat_mode_query_callback, rs);
> +	radio_query_rat_mode(rs);
>  
>  	return NULL;
>  }
> @@ -240,6 +327,28 @@ static DBusMessage *radio_set_property(DBusConnection *conn, DBusMessage *msg,
>  		rs->driver->set_rat_mode(rs, mode, radio_mode_set_callback, rs);
>  
>  		return NULL;
> +	} else if (g_strcmp0(property, "FastDormancy") == 0) {
> +		dbus_bool_t value;
> +		int target;
> +
> +		if (!rs->driver->set_fast_dormancy)
> +			return __ofono_error_not_implemented(msg);
> +
> +		if (dbus_message_iter_get_arg_type(&var) != DBUS_TYPE_BOOLEAN)
> +			return __ofono_error_invalid_args(msg);
> +
> +		dbus_message_iter_get_basic(&var, &value);
> +		target = value;
> +
> +		if (rs->fast_dormancy_pending == target)
> +			return dbus_message_new_method_return(msg);
> +
> +		rs->pending = dbus_message_ref(msg);
> +		rs->fast_dormancy_pending = target;
> +
> +		rs->driver->set_fast_dormancy(rs, target,
> +					radio_fast_dormancy_set_callback, rs);
> +		return NULL;
>  	}
>  
>  	return __ofono_error_invalid_args(msg);
> @@ -322,6 +431,7 @@ struct ofono_radio_settings *ofono_radio_settings_create(struct ofono_modem *mod
>  		return NULL;
>  
>  	rs->mode = -1;
> +	rs->fast_dormancy = -1;

If you use ofono_bool_t for fast_dormancy, this one can be omitted.

>  
>  	rs->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_RADIO_SETTINGS,
>  						radio_settings_remove, rs);

Regards,
-Denis