* [PATCH 1/6] radio settings: add FastDormancy property
2010-10-25 13:28 [PATCH 0/6] radio settings: fast dormancy support Mika Liljeberg
@ 2010-10-25 13:28 ` Mika Liljeberg
2010-10-25 14:30 ` Marcel Holtmann
2010-10-25 15:57 ` Denis Kenzior
2010-10-25 13:28 ` [PATCH 2/6] radio settings: document " Mika Liljeberg
` (4 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Mika Liljeberg @ 2010-10-25 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 8191 bytes --]
---
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;
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);
+ }
+
+ if (rs->fast_dormancy != -1) {
+ dbus_bool_t value = (rs->fast_dormancy != 0);
+ ofono_dbus_dict_append(&dict, "FastDormancy",
+ DBUS_TYPE_BOOLEAN, &value);
+ }
dbus_message_iter_close_container(&iter, &dict);
return reply;
}
+static void radio_set_fast_dormancy(struct ofono_radio_settings *rs, int enable)
+{
+ if (rs->fast_dormancy != enable) {
+
+ 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;
rs->atom = __ofono_modem_add_atom(modem, OFONO_ATOM_TYPE_RADIO_SETTINGS,
radio_settings_remove, rs);
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] radio settings: add FastDormancy property
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:57 ` Denis Kenzior
1 sibling, 1 reply; 14+ messages in thread
From: Marcel Holtmann @ 2010-10-25 14:30 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3433 bytes --]
Hi Mika,
> 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;
> 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);
what is up with this (int) rs->mode cast here. That looks highly wrong
to me. The mode is an enum so please don't hack around it like this.
If mode can be invalid or not present then we need to extend this enum
with an initial value of OFONO_RADIO_ACCESS_MODE_UNKNOWN, but not hack
some cast magic into it.
Or you use some flags like for the cached value.
And I would propose the same for fast dormancy value. Lets store it as a
boolean and have a flag if it is present or not.
Regards
Marcel
^ permalink raw reply [flat|nested] 14+ messages in thread* RE: [PATCH 1/6] radio settings: add FastDormancy property
2010-10-25 14:30 ` Marcel Holtmann
@ 2010-10-25 15:05 ` Mika.Liljeberg
2010-10-25 15:19 ` Denis Kenzior
0 siblings, 1 reply; 14+ messages in thread
From: Mika.Liljeberg @ 2010-10-25 15:05 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]
Hi Marcel,
> > @@ -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);
>
> what is up with this (int) rs->mode cast here. That looks highly wrong
> to me. The mode is an enum so please don't hack around it like this.
>
> If mode can be invalid or not present then we need to extend this enum
> with an initial value of OFONO_RADIO_ACCESS_MODE_UNKNOWN, but not hack
> some cast magic into it.
Yes, it's fishy. Denis introduced the enum in commit 81bc8884b414e6c2d511789d2e183cdad55182f0 but left mode initialized as -1. I'm not sure what's up with that but I did not want to start fixing it. I suppose the initializer could be added to the enum, as you say, or the whole patch could be reverted. Not my call, though.
> Or you use some flags like for the cached value.
>
> And I would propose the same for fast dormancy value. Lets
> store it as a
> boolean and have a flag if it is present or not.
That's what I did in the previous patch but Denis wanted a single CACHED flag. Guys, please try to agree on this.
MikaL
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH 1/6] radio settings: add FastDormancy property
2010-10-25 15:05 ` Mika.Liljeberg
@ 2010-10-25 15:19 ` Denis Kenzior
0 siblings, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2010-10-25 15:19 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1735 bytes --]
Hi Mika,
On 10/25/2010 10:05 AM, Mika.Liljeberg(a)nokia.com wrote:
> Hi Marcel,
>
>>> @@ -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);
>>
>> what is up with this (int) rs->mode cast here. That looks highly wrong
>> to me. The mode is an enum so please don't hack around it like this.
>>
>> If mode can be invalid or not present then we need to extend this enum
>> with an initial value of OFONO_RADIO_ACCESS_MODE_UNKNOWN, but not hack
>> some cast magic into it.
>
> Yes, it's fishy. Denis introduced the enum in commit 81bc8884b414e6c2d511789d2e183cdad55182f0 but left mode initialized as -1. I'm not sure what's up with that but I did not want to start fixing it. I suppose the initializer could be added to the enum, as you say, or the whole patch could be reverted. Not my call, though.
I must have missed the -1 initialization. In general the preference is
as follows:
- If the property is queried at the network, then set to a value that
means "unknown". Otherwise set to a default sane value.
- Only set to the new value if the query succeeds
- If the query fails (a bizarre case if querying the modem), don't reset
the sane value and don't set cached. The next GetProperties will try to
re-query the setting.
- Don't show the attribute if the query_ method is not provided by the
driver.
Regards,
-Denis
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/6] radio settings: add FastDormancy property
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:57 ` Denis Kenzior
1 sibling, 0 replies; 14+ messages in thread
From: Denis Kenzior @ 2010-10-25 15:57 UTC (permalink / raw)
To: ofono
[-- 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/6] radio settings: document FastDormancy property
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 13:28 ` 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
` (3 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Mika Liljeberg @ 2010-10-25 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3062 bytes --]
---
doc/radio-settings-api.txt | 59 ++++++++++++++++++++++++++++++-------------
1 files changed, 41 insertions(+), 18 deletions(-)
diff --git a/doc/radio-settings-api.txt b/doc/radio-settings-api.txt
index f1b91ad..00eca54 100644
--- a/doc/radio-settings-api.txt
+++ b/doc/radio-settings-api.txt
@@ -44,22 +44,45 @@ Properties string TechnologyPreference [read-write]
boolean FastDormancy [read-write, optional]
- This property will enable or disable fast
- dormancy. Fast dormancy refers to UE initiated
- release of radio resources quickly after a
- burst of data transfer has ended. Normally,
+ This property will enable or disable the fast
+ dormancy feature in the modem. Fast dormancy
+ refers to a modem feature that allows the
+ modem to quickly release radio resources after
+ a burst of data transfer has ended. Normally,
radio resources are released by the network
- after a timeout configured by the network
- operator. Fast dormancy allows the modem to
- release radio resources more quickly.
- Typically, fast dormancy would be enabled if
- no data transfer is predicted to occur in the
- near future, for instance, when the end user
- is not actively using the device. This is a
- major power-saving feature for mobile devices,
- but can be ignored for USB sticks or PCI
- devices.
-
- If the modem does not support such a feature
- the property should never be exposed to the
- user.
+ after a timeout configured by the network.
+ Fast dormancy allows the modem to release the
+ radio resources more quickly.
+
+ Fast dormancy is a major power-saving feature
+ for mobile devices. Typically, fast dormancy
+ would be enabled when the device is not being
+ interactively used by a human user and only
+ networking applications with keep-alive
+ traffic are active (e.g. mail client or a
+ presence application). In this case it is
+ desirable to release radio resources quickly
+ after a keep-alive transaction has ended,
+ since typically no network traffic will occur
+ until the next keep-alive transaction. Fast
+ dormancy should not be enabled during
+ interactive use because the release and setup
+ of radio resources introduces perceivable
+ delay for the end user.
+
+ The fast dormancy implementation in the modem
+ is vendor specific. The implementation should
+ try to release radio resources more quickly,
+ when the situation allows it, but should also
+ take care not to increase the signalling load
+ on the cellular network by releasing and
+ re-establishing radio resources too often. The
+ modem should adjust its behaviour to the 3GPP
+ release supported by the network and the
+ parameters set by the operator.
+
+ Fast dormancy can be ignored for externally
+ powered modems such as USB sticks or PCI
+ devices. If the modem does not support such a
+ feature the property should never be exposed
+ to the user.
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 3/6] test: add scripts to enable and disable fast dormancy
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 13:28 ` [PATCH 2/6] radio settings: document " Mika Liljeberg
@ 2010-10-25 13:28 ` 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
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Mika Liljeberg @ 2010-10-25 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2020 bytes --]
---
Makefile.am | 4 +++-
test/disable-fast-dormancy | 20 ++++++++++++++++++++
test/enable-fast-dormancy | 20 ++++++++++++++++++++
3 files changed, 43 insertions(+), 1 deletions(-)
create mode 100755 test/disable-fast-dormancy
create mode 100755 test/enable-fast-dormancy
diff --git a/Makefile.am b/Makefile.am
index 25c18bd..cd2cf78 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -410,7 +410,9 @@ test_scripts = test/backtrace \
test/unlock-pin \
test/enable-gprs \
test/disable-gprs \
- test/get-icon
+ test/get-icon \
+ test/enable-fast-dormancy \
+ test/disable-fast-dormancy
if TEST
testdir = $(pkglibdir)/test
diff --git a/test/disable-fast-dormancy b/test/disable-fast-dormancy
new file mode 100755
index 0000000..a9d7286
--- /dev/null
+++ b/test/disable-fast-dormancy
@@ -0,0 +1,20 @@
+#!/usr/bin/python
+
+import dbus
+import sys
+
+bus = dbus.SystemBus()
+
+if len(sys.argv) == 2:
+ path = sys.argv[1]
+else:
+ manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+ 'org.ofono.Manager')
+ modems = manager.GetModems()
+ path = modems[0][0]
+
+print "Disabling fast dormancy on modem %s..." % path
+rs = dbus.Interface(bus.get_object('org.ofono', path),
+ 'org.ofono.RadioSettings')
+
+rs.SetProperty("FastDormancy", dbus.Boolean(0))
diff --git a/test/enable-fast-dormancy b/test/enable-fast-dormancy
new file mode 100755
index 0000000..518a662
--- /dev/null
+++ b/test/enable-fast-dormancy
@@ -0,0 +1,20 @@
+#!/usr/bin/python
+
+import dbus
+import sys
+
+bus = dbus.SystemBus()
+
+if len(sys.argv) == 2:
+ path = sys.argv[1]
+else:
+ manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+ 'org.ofono.Manager')
+ modems = manager.GetModems()
+ path = modems[0][0]
+
+print "Enabling fast dormancy on modem %s..." % path
+rs = dbus.Interface(bus.get_object('org.ofono', path),
+ 'org.ofono.RadioSettings')
+
+rs.SetProperty("FastDormancy", dbus.Boolean(1))
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 4/6] isimodem: add support for FastDormancy property
2010-10-25 13:28 [PATCH 0/6] radio settings: fast dormancy support Mika Liljeberg
` (2 preceding siblings ...)
2010-10-25 13:28 ` [PATCH 3/6] test: add scripts to enable and disable fast dormancy Mika Liljeberg
@ 2010-10-25 13:28 ` 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
5 siblings, 0 replies; 14+ messages in thread
From: Mika Liljeberg @ 2010-10-25 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3894 bytes --]
---
drivers/isimodem/radio-settings.c | 91 ++++++++++++++++++++++++++++++++++++-
1 files changed, 90 insertions(+), 1 deletions(-)
diff --git a/drivers/isimodem/radio-settings.c b/drivers/isimodem/radio-settings.c
index d220476..558e032 100644
--- a/drivers/isimodem/radio-settings.c
+++ b/drivers/isimodem/radio-settings.c
@@ -41,11 +41,16 @@
#include "isimodem.h"
#include "isiutil.h"
#include "debug.h"
+#include "gpds.h"
#include "gss.h"
#include "network.h"
+#define PN_WRAN 0xb4
+
struct radio_data {
GIsiClient *client;
+ uint16_t wran_object;
+ uint16_t quick_release:1;
};
static enum ofono_radio_access_mode isi_mode_to_ofono_mode(guint8 mode)
@@ -236,6 +241,65 @@ error:
g_free(cbd);
}
+static void update_fast_dormancy(struct radio_data *rd)
+{
+ struct sockaddr_pn dst = {
+ .spn_family = AF_PHONET,
+ .spn_resource = 0x3a,
+ .spn_dev = rd->wran_object >> 8,
+ .spn_obj = rd->wran_object & 0xff,
+ };
+
+ if (!rd->wran_object)
+ return;
+
+ if (rd->quick_release) {
+ const unsigned char msg[] = {
+ 0x1f, 0x00, 0x01, 0x01, 0x01, 0x00
+ };
+
+ g_isi_sendto(rd->client, &dst, msg, sizeof(msg), 0,
+ NULL, NULL, NULL);
+ } else {
+ const unsigned char msg[] = {
+ 0x1f, 0x00, 0x01, 0x01, 0x02, 0x0a
+ };
+
+ g_isi_sendto(rd->client, &dst, msg, sizeof(msg), 0,
+ NULL, NULL, NULL);
+ }
+
+ DBG("3G PS quick release %s",
+ rd->quick_release ? "enabled" : "disabled");
+}
+
+static void gpds_context_activating_ind_cb(GIsiClient *client,
+ const void *restrict data, size_t len,
+ uint16_t object, void *opaque)
+{
+ struct radio_data *rd = opaque;
+ update_fast_dormancy(rd);
+}
+
+static void isi_query_fast_dormancy(struct ofono_radio_settings *rs,
+ ofono_radio_settings_fast_dormancy_query_cb_t cb,
+ void *data)
+{
+ struct radio_data *rd = ofono_radio_settings_get_data(rs);
+ CALLBACK_WITH_SUCCESS(cb, rd->quick_release, data);
+}
+
+static void isi_set_fast_dormancy(struct ofono_radio_settings *rs,
+ int enable,
+ ofono_radio_settings_fast_dormancy_set_cb_t cb,
+ void *data)
+{
+ struct radio_data *rd = ofono_radio_settings_get_data(rs);
+ rd->quick_release = enable;
+ update_fast_dormancy(rd);
+ CALLBACK_WITH_SUCCESS(cb, data);
+}
+
static gboolean isi_radio_settings_register(gpointer user)
{
struct ofono_radio_settings *rs = user;
@@ -249,9 +313,31 @@ static gboolean isi_radio_settings_register(gpointer user)
ofono_radio_settings_register(rs);
+ g_isi_add_subscription(rd->client,
+ PN_GPDS, GPDS_CONTEXT_ACTIVATING_IND,
+ gpds_context_activating_ind_cb, rd);
+ g_isi_commit_subscriptions(rd->client);
+
return FALSE;
}
+static void wran_reachable_cb(GIsiClient *client, gboolean alive,
+ uint16_t object, void *opaque)
+{
+ struct radio_data *rd = opaque;
+
+ if (!alive) {
+ DBG("fast dormancy support disabled");
+ return;
+ }
+
+ rd->wran_object = object;
+
+ DBG("PN_WRAN reachable, object=0x%04x", object);
+
+ update_fast_dormancy(rd);
+}
+
static void reachable_cb(GIsiClient *client, gboolean alive, uint16_t object,
void *opaque)
{
@@ -289,6 +375,7 @@ static int isi_radio_settings_probe(struct ofono_radio_settings *rs,
ofono_radio_settings_set_data(rs, rd);
g_isi_verify(rd->client, reachable_cb, rs);
+ g_isi_verify_resource(rd->client, PN_WRAN, wran_reachable_cb, rd);
return 0;
}
@@ -310,7 +397,9 @@ static struct ofono_radio_settings_driver driver = {
.probe = isi_radio_settings_probe,
.remove = isi_radio_settings_remove,
.query_rat_mode = isi_query_rat_mode,
- .set_rat_mode = isi_set_rat_mode
+ .set_rat_mode = isi_set_rat_mode,
+ .query_fast_dormancy = isi_query_fast_dormancy,
+ .set_fast_dormancy = isi_set_fast_dormancy,
};
void isi_radio_settings_init()
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 5/6] TODO: mark fast dormancy as done
2010-10-25 13:28 [PATCH 0/6] radio settings: fast dormancy support Mika Liljeberg
` (3 preceding siblings ...)
2010-10-25 13:28 ` [PATCH 4/6] isimodem: add support for FastDormancy property Mika Liljeberg
@ 2010-10-25 13:28 ` Mika Liljeberg
2010-10-25 13:28 ` [PATCH 6/6] AUTHORS: add myself Mika Liljeberg
5 siblings, 0 replies; 14+ messages in thread
From: Mika Liljeberg @ 2010-10-25 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2204 bytes --]
---
TODO | 20 --------------------
doc/features.txt | 8 ++++++++
2 files changed, 8 insertions(+), 20 deletions(-)
diff --git a/TODO b/TODO
index d9a6580..d3d6687 100644
--- a/TODO
+++ b/TODO
@@ -519,26 +519,6 @@ Miscellaneous
Priority: Medium
Complexity: C2
-- Add a property for Fast Dormancy in the RadioSettings atom. This property
- will enable or disable Fast Dormancy. Fast Dormancy refers to UE initiated
- release of radio resources quickly after a burst of data transfer has ended.
- Normally, radio resources are released by the network after a timeout
- configured by the network operator. Fast Dormancy allows the modem to release
- radio resources more quickly. Typically, fast dormancy would be enabled
- if no data transfer is predicted to occur in the near future (e.g. end user
- is not actively using the device). This is a major power-saving feature for
- mobile devices, but can be ignored for USB sticks or PCI devices.
-
- If the modem does not support such a feature the property should never be
- exposed to the user.
-
- This feature is not discussed in 27.007, thus manufacturer specific commands
- are required.
-
- Priority: High
- Complexity: C1
- Owner: Mika Liljeberg <mika.liljeberg@nokia.com>
-
- TTY (hearing impaired) support. Add a new oFono atom type that will enable
the user to enable or disable the TTY support on the modem. Support for
automatic detection of TTY (signaled by the driver) is also desired.
diff --git a/doc/features.txt b/doc/features.txt
index 3524e79..deb1fea 100644
--- a/doc/features.txt
+++ b/doc/features.txt
@@ -135,3 +135,11 @@ SIM
check if FDN support is allocated and enabled in the SIM. If enabled,
oFono halts the SIM initialization procedure and the modem remains in the
PRESIM state. In this state oFono will only allow emergency calls.
+
+Radio settings
+==============
+
+- Fast dormancy support. A fast dormancy feature can be enabled in the
+ cellular modem to conserve power when the end user is not actively
+ using the device but some networking applications are online using
+ packet data.
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 6/6] AUTHORS: add myself
2010-10-25 13:28 [PATCH 0/6] radio settings: fast dormancy support Mika Liljeberg
` (4 preceding siblings ...)
2010-10-25 13:28 ` [PATCH 5/6] TODO: mark fast dormancy as done Mika Liljeberg
@ 2010-10-25 13:28 ` Mika Liljeberg
2010-10-25 14:37 ` Marcel Holtmann
5 siblings, 1 reply; 14+ messages in thread
From: Mika Liljeberg @ 2010-10-25 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 421 bytes --]
---
AUTHORS | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/AUTHORS b/AUTHORS
index 654b314..c0218c6 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -37,3 +37,4 @@ Petteri Tikander <petteri.tikander@ixonos.com>
Jeevaka Badrappan <jeevaka.badrappan@elektrobit.com>
Frank Gau <fgau@gau-net.de>
Kai Vehmanen <kai.vehmanen@nokia.com>
+Mika Liljeberg <mika.liljeberg@nokia.com>
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread