From: Denis Kenzior <denkenz@gmail.com>
To: ofono@ofono.org
Subject: Re: [PATCH 1/6] radio settings: add FastDormancy property
Date: Mon, 25 Oct 2010 10:57:38 -0500 [thread overview]
Message-ID: <4CC5A8F2.6040500@gmail.com> (raw)
In-Reply-To: <1288013290-6124-2-git-send-email-mika.liljeberg@nokia.com>
[-- Attachment #1: Type: text/plain, Size: 9219 bytes --]
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
next prev parent reply other threads:[~2010-10-25 15:57 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-25 13:28 [PATCH 0/6] radio settings: fast dormancy support Mika Liljeberg
2010-10-25 13:28 ` [PATCH 1/6] radio settings: add FastDormancy property Mika Liljeberg
2010-10-25 14:30 ` Marcel Holtmann
2010-10-25 15:05 ` Mika.Liljeberg
2010-10-25 15:19 ` Denis Kenzior
2010-10-25 15:57 ` Denis Kenzior [this message]
2010-10-25 13:28 ` [PATCH 2/6] radio settings: document " Mika Liljeberg
2010-10-25 14:32 ` Marcel Holtmann
2010-10-25 13:28 ` [PATCH 3/6] test: add scripts to enable and disable fast dormancy Mika Liljeberg
2010-10-25 14:34 ` Marcel Holtmann
2010-10-25 13:28 ` [PATCH 4/6] isimodem: add support for FastDormancy property Mika Liljeberg
2010-10-25 13:28 ` [PATCH 5/6] TODO: mark fast dormancy as done Mika Liljeberg
2010-10-25 13:28 ` [PATCH 6/6] AUTHORS: add myself Mika Liljeberg
2010-10-25 14:37 ` Marcel Holtmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CC5A8F2.6040500@gmail.com \
--to=denkenz@gmail.com \
--cc=ofono@ofono.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.