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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox