All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Add Suspended property to GPRS atom
  2010-09-08  9:16 [PATCH 0/2] Add Suspended property to GPRS Mika Liljeberg
@ 2010-09-08  9:17 ` Mika Liljeberg
  0 siblings, 0 replies; 6+ messages in thread
From: Mika Liljeberg @ 2010-09-08  9:17 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 3384 bytes --]


Signed-off-by: Mika Liljeberg <mika.liljeberg@nokia.com>
---
 doc/connman-api.txt |   19 +++++++++++++++++++
 include/gprs.h      |    1 +
 src/gprs.c          |   21 +++++++++++++++++++++
 3 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/doc/connman-api.txt b/doc/connman-api.txt
index 43d8897..b576e19 100644
--- a/doc/connman-api.txt
+++ b/doc/connman-api.txt
@@ -77,6 +77,25 @@ Properties	boolean Attached [readonly]
 			be available, e.g. receiving SMS over packet radio
 			or network initiated PDP activation.
 
+		boolean Suspended [readonly]
+
+			Contains whether the GPRS service is suspended.
+			During suspended state the modem is attached to the
+			GPRS service and all contexts remain established,
+			however, data transfer is not possible.
+
+			The suspended state may be entered if the modem is
+			temporarily out of network coverage. GPRS class B
+			modems will suspend GPRS whenever a voice call is
+			active at the same time. GPRS may also be suspended
+			if the network does not support simultaneous packet
+			data and voice. Various signalling procedures may
+			also cause GPRS to be briefly suspended.
+
+			As the suspension may be brief, clients should wait
+			for an appropriate time for GPRS service to resume
+			before taking corrective action.
+
 		boolean RoamingAllowed [readwrite]
 
 			Contains whether data roaming is allowed.  In the off
diff --git a/include/gprs.h b/include/gprs.h
index a1cbcd9..7578513 100644
--- a/include/gprs.h
+++ b/include/gprs.h
@@ -49,6 +49,7 @@ struct ofono_gprs_driver {
 
 void ofono_gprs_status_notify(struct ofono_gprs *gprs, int status);
 void ofono_gprs_detached_notify(struct ofono_gprs *gprs);
+void ofono_gprs_suspend_notify(struct ofono_gprs *gprs, ofono_bool_t status);
 
 int ofono_gprs_driver_register(const struct ofono_gprs_driver *d);
 void ofono_gprs_driver_unregister(const struct ofono_gprs_driver *d);
diff --git a/src/gprs.c b/src/gprs.c
index d57115b..288ddd5 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -64,6 +64,7 @@ struct ofono_gprs {
 	ofono_bool_t driver_attached;
 	ofono_bool_t roaming_allowed;
 	ofono_bool_t powered;
+	ofono_bool_t suspended;
 	int status;
 	int flags;
 	struct idmap *pid_map;
@@ -1052,6 +1053,9 @@ static DBusMessage *gprs_get_properties(DBusConnection *conn,
 	value = gprs->powered;
 	ofono_dbus_dict_append(&dict, "Powered", DBUS_TYPE_BOOLEAN, &value);
 
+	value = gprs->suspended;
+	ofono_dbus_dict_append(&dict, "Suspended", DBUS_TYPE_BOOLEAN, &value);
+
 	dbus_message_iter_close_container(&iter, &dict);
 
 	return reply;
@@ -1428,6 +1432,23 @@ static GDBusSignalTable manager_signals[] = {
 	{ }
 };
 
+void ofono_gprs_suspend_notify(struct ofono_gprs *gprs, ofono_bool_t status)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(gprs->atom);
+	dbus_bool_t value = status;
+
+	if (gprs->suspended == status)
+		return;
+
+	DBG("%s suspended %d", __ofono_atom_get_path(gprs->atom), status);
+
+	gprs->suspended = status;
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_CONNECTION_MANAGER_INTERFACE,
+					"Suspended", DBUS_TYPE_BOOLEAN, &value);
+}
+
 void ofono_gprs_detached_notify(struct ofono_gprs *gprs)
 {
 	DBG("%s", __ofono_atom_get_path(gprs->atom));
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Add Suspended property to GPRS (take 2)
@ 2010-09-09 12:08 Mika Liljeberg
  2010-09-09 12:08 ` [PATCH 1/2] Add Suspended property to GPRS atom Mika Liljeberg
  2010-09-09 12:08 ` [PATCH 2/2] isimodem: implement Suspended property Mika Liljeberg
  0 siblings, 2 replies; 6+ messages in thread
From: Mika Liljeberg @ 2010-09-09 12:08 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 530 bytes --]

[PATCH 1/2] Add Suspended property to GPRS atom
[PATCH 2/2] isimodem: implement Suspended property

 doc/connman-api.txt      |   19 ++++++++++++
 drivers/isimodem/debug.c |   25 ++++++++++++++++
 drivers/isimodem/debug.h |    2 +
 drivers/isimodem/gpds.h  |   17 +++++++++++
 drivers/isimodem/gprs.c  |   56 ++++++++++++++++++++++++++++++++++++-
 include/gprs.h           |   10 ++++++
 src/gprs.c               |   69 ++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 197 insertions(+), 1 deletions(-)

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] Add Suspended property to GPRS atom
  2010-09-09 12:08 Add Suspended property to GPRS (take 2) Mika Liljeberg
@ 2010-09-09 12:08 ` Mika Liljeberg
  2010-09-09 17:57   ` Denis Kenzior
  2010-09-09 12:08 ` [PATCH 2/2] isimodem: implement Suspended property Mika Liljeberg
  1 sibling, 1 reply; 6+ messages in thread
From: Mika Liljeberg @ 2010-09-09 12:08 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 5886 bytes --]


Signed-off-by: Mika Liljeberg <mika.liljeberg@nokia.com>
---
 doc/connman-api.txt |   19 ++++++++++++++
 include/gprs.h      |   10 +++++++
 src/gprs.c          |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+), 0 deletions(-)

diff --git a/doc/connman-api.txt b/doc/connman-api.txt
index 43d8897..b576e19 100644
--- a/doc/connman-api.txt
+++ b/doc/connman-api.txt
@@ -77,6 +77,25 @@ Properties	boolean Attached [readonly]
 			be available, e.g. receiving SMS over packet radio
 			or network initiated PDP activation.
 
+		boolean Suspended [readonly]
+
+			Contains whether the GPRS service is suspended.
+			During suspended state the modem is attached to the
+			GPRS service and all contexts remain established,
+			however, data transfer is not possible.
+
+			The suspended state may be entered if the modem is
+			temporarily out of network coverage. GPRS class B
+			modems will suspend GPRS whenever a voice call is
+			active at the same time. GPRS may also be suspended
+			if the network does not support simultaneous packet
+			data and voice. Various signalling procedures may
+			also cause GPRS to be briefly suspended.
+
+			As the suspension may be brief, clients should wait
+			for an appropriate time for GPRS service to resume
+			before taking corrective action.
+
 		boolean RoamingAllowed [readwrite]
 
 			Contains whether data roaming is allowed.  In the off
diff --git a/include/gprs.h b/include/gprs.h
index a1cbcd9..ad7925c 100644
--- a/include/gprs.h
+++ b/include/gprs.h
@@ -47,8 +47,18 @@ struct ofono_gprs_driver {
 					ofono_gprs_status_cb_t cb, void *data);
 };
 
+enum gprs_suspend_cause {
+	GPRS_SUSPENDED_DETACHED,
+	GPRS_SUSPENDED_SIGNALLING,
+	GPRS_SUSPENDED_CALL,
+	GPRS_SUSPENDED_NO_COVERAGE,
+	GPRS_SUSPENDED_UNKNOWN_CAUSE,
+};
+
 void ofono_gprs_status_notify(struct ofono_gprs *gprs, int status);
 void ofono_gprs_detached_notify(struct ofono_gprs *gprs);
+void ofono_gprs_suspend_notify(struct ofono_gprs *gprs, int cause);
+void ofono_gprs_resume_notify(struct ofono_gprs *gprs);
 
 int ofono_gprs_driver_register(const struct ofono_gprs_driver *d);
 void ofono_gprs_driver_unregister(const struct ofono_gprs_driver *d);
diff --git a/src/gprs.c b/src/gprs.c
index d57115b..742aec1 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -47,6 +47,7 @@
 #define SETTINGS_GROUP "Settings"
 #define MAX_CONTEXT_NAME_LENGTH 127
 #define MAX_CONTEXTS 256
+#define SUSPEND_TIMEOUT 8
 
 static GSList *g_drivers = NULL;
 static GSList *g_context_drivers = NULL;
@@ -64,8 +65,10 @@ struct ofono_gprs {
 	ofono_bool_t driver_attached;
 	ofono_bool_t roaming_allowed;
 	ofono_bool_t powered;
+	ofono_bool_t suspended;
 	int status;
 	int flags;
+	guint suspend_timeout;
 	struct idmap *pid_map;
 	unsigned int last_context_id;
 	struct idmap *cid_map;
@@ -894,6 +897,64 @@ static gboolean context_dbus_unregister(struct pri_context *ctx)
 					OFONO_CONNECTION_CONTEXT_INTERFACE);
 }
 
+static void update_suspended_property(struct ofono_gprs *gprs,
+				ofono_bool_t suspended)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	const char *path = __ofono_atom_get_path(gprs->atom);
+	dbus_bool_t value = suspended;
+
+	if (gprs->suspend_timeout) {
+		g_source_remove(gprs->suspend_timeout);
+		gprs->suspend_timeout = 0;
+	}
+
+	if (gprs->suspended == suspended)
+		return;
+
+	DBG("%s GPRS service %s", __ofono_atom_get_path(gprs->atom),
+		suspended ? "suspended" : "resumed");
+
+	gprs->suspended = suspended;
+	ofono_dbus_signal_property_changed(conn, path,
+					OFONO_CONNECTION_MANAGER_INTERFACE,
+					"Suspended", DBUS_TYPE_BOOLEAN, &value);
+}
+
+static gboolean suspend_timeout(gpointer data)
+{
+       struct ofono_gprs *gprs = data;
+
+       gprs->suspend_timeout = 0;
+       update_suspended_property(gprs, TRUE);
+       return FALSE;
+}
+
+void ofono_gprs_suspend_notify(struct ofono_gprs *gprs, int cause)
+{
+	switch (cause) {
+	case GPRS_SUSPENDED_DETACHED:
+	case GPRS_SUSPENDED_CALL:
+	case GPRS_SUSPENDED_NO_COVERAGE:
+		update_suspended_property(gprs, TRUE);
+		break;
+
+	case GPRS_SUSPENDED_SIGNALLING:
+	case GPRS_SUSPENDED_UNKNOWN_CAUSE:
+		if (gprs->suspend_timeout)
+			g_source_remove(gprs->suspend_timeout);
+		gprs->suspend_timeout = g_timeout_add_seconds(SUSPEND_TIMEOUT,
+							suspend_timeout,
+							gprs);
+		break;
+	}
+}
+
+void ofono_gprs_resume_notify(struct ofono_gprs *gprs)
+{
+	update_suspended_property(gprs, FALSE);
+}
+
 static void gprs_attached_update(struct ofono_gprs *gprs)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
@@ -909,6 +970,7 @@ static void gprs_attached_update(struct ofono_gprs *gprs)
 		return;
 
 	gprs->attached = attached;
+	update_suspended_property(gprs, !attached);
 
 	if (gprs->attached == FALSE) {
 		GSList *l;
@@ -1052,6 +1114,9 @@ static DBusMessage *gprs_get_properties(DBusConnection *conn,
 	value = gprs->powered;
 	ofono_dbus_dict_append(&dict, "Powered", DBUS_TYPE_BOOLEAN, &value);
 
+	value = gprs->suspended;
+	ofono_dbus_dict_append(&dict, "Suspended", DBUS_TYPE_BOOLEAN, &value);
+
 	dbus_message_iter_close_container(&iter, &dict);
 
 	return reply;
@@ -1697,6 +1762,9 @@ static void gprs_remove(struct ofono_atom *atom)
 	if (gprs == NULL)
 		return;
 
+	if (gprs->suspend_timeout)
+		g_source_remove(gprs->suspend_timeout);
+
 	if (gprs->pid_map) {
 		idmap_free(gprs->pid_map);
 		gprs->pid_map = NULL;
@@ -1746,6 +1814,7 @@ struct ofono_gprs *ofono_gprs_create(struct ofono_modem *modem,
 
 	gprs->status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
 	gprs->netreg_status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
+	gprs->suspended = TRUE;
 	gprs->pid_map = idmap_new(MAX_CONTEXTS);
 
 	return gprs;
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] isimodem: implement Suspended property
  2010-09-09 12:08 Add Suspended property to GPRS (take 2) Mika Liljeberg
  2010-09-09 12:08 ` [PATCH 1/2] Add Suspended property to GPRS atom Mika Liljeberg
@ 2010-09-09 12:08 ` Mika Liljeberg
  2010-09-09 17:59   ` Denis Kenzior
  1 sibling, 1 reply; 6+ messages in thread
From: Mika Liljeberg @ 2010-09-09 12:08 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 5745 bytes --]


Signed-off-by: Mika Liljeberg <mika.liljeberg@nokia.com>
---
 drivers/isimodem/debug.c |   25 ++++++++++++++++++++
 drivers/isimodem/debug.h |    2 +
 drivers/isimodem/gpds.h  |   17 ++++++++++++++
 drivers/isimodem/gprs.c  |   56 +++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 99 insertions(+), 1 deletions(-)

diff --git a/drivers/isimodem/debug.c b/drivers/isimodem/debug.c
index 86530fd..b398797 100644
--- a/drivers/isimodem/debug.c
+++ b/drivers/isimodem/debug.c
@@ -979,6 +979,31 @@ const char *gpds_isi_cause_name(enum gpds_isi_cause value)
 	return "GPDS_<UNKNOWN>";
 }
 
+const char *gpds_transfer_status_name(enum gpds_transfer_status value)
+{
+	switch (value) {
+		_(GPDS_TRANSFER_NOT_AVAIL);
+		_(GPDS_TRANSFER_AVAIL);
+	}
+	return "GPDS_<UNKNOWN>";
+}
+
+const char *gpds_transfer_cause_name(enum gpds_transfer_cause value)
+{
+	switch (value) {
+		_(GPDS_TRANSFER_CAUSE_ATTACHED);
+		_(GPDS_TRANSFER_CAUSE_DETACHED);
+		_(GPDS_TRANSFER_CAUSE_RESUMED);
+		_(GPDS_TRANSFER_CAUSE_SUSPENDED_NO_COVERAGE);
+		_(GPDS_TRANSFER_CAUSE_SUSPENDED_CALL_SMS);
+		_(GPDS_TRANSFER_CAUSE_SUSPENDED_CALL);
+		_(GPDS_TRANSFER_CAUSE_SUSPENDED_RAU);
+		_(GPDS_TRANSFER_CAUSE_SUSPENDED_LU);
+		_(GPDS_TRANSFER_CAUSE_DSAC_RESTRICTION);
+	}
+	return "GPDS_<UNKNOWN>";
+}
+
 #undef _
 
 static void hex_dump(const char *name, const uint8_t m[], size_t len)
diff --git a/drivers/isimodem/debug.h b/drivers/isimodem/debug.h
index d507991..1ad5547 100644
--- a/drivers/isimodem/debug.h
+++ b/drivers/isimodem/debug.h
@@ -70,6 +70,8 @@ const char *gpds_message_id_name(enum gpds_message_id value);
 const char *gpds_subblock_name(enum gpds_subblock value);
 const char *gpds_status_name(enum gpds_status value);
 const char *gpds_isi_cause_name(enum gpds_isi_cause value);
+const char *gpds_transfer_status_name(enum gpds_transfer_status value);
+const char *gpds_transfer_cause_name(enum gpds_transfer_cause value);
 
 void ss_debug(const void *restrict buf, size_t len, void *data);
 void mtc_debug(const void *restrict buf, size_t len, void *data);
diff --git a/drivers/isimodem/gpds.h b/drivers/isimodem/gpds.h
index 86d4d95..9bb60e6 100644
--- a/drivers/isimodem/gpds.h
+++ b/drivers/isimodem/gpds.h
@@ -202,6 +202,23 @@ enum gpds_isi_cause {
 	GPDS_CAUSE_AUT_FAILURE =		0xFF,
 };
 
+enum gpds_transfer_status {
+	GPDS_TRANSFER_NOT_AVAIL =		0x00,
+	GPDS_TRANSFER_AVAIL =			0x01,
+};
+
+enum gpds_transfer_cause {
+	GPDS_TRANSFER_CAUSE_ATTACHED =			0x02,
+	GPDS_TRANSFER_CAUSE_DETACHED =			0x03,
+	GPDS_TRANSFER_CAUSE_RESUMED =			0x04,
+	GPDS_TRANSFER_CAUSE_SUSPENDED_NO_COVERAGE =	0x05,
+	GPDS_TRANSFER_CAUSE_SUSPENDED_CALL_SMS =	0x07,
+	GPDS_TRANSFER_CAUSE_SUSPENDED_CALL =		0x08,
+	GPDS_TRANSFER_CAUSE_SUSPENDED_RAU =		0x09,
+	GPDS_TRANSFER_CAUSE_SUSPENDED_LU =		0x0A,
+	GPDS_TRANSFER_CAUSE_DSAC_RESTRICTION =		0x0B,
+};
+
 enum gpds_context_type {
 	GPDS_CONT_TYPE_NORMAL =			0x00,
 	GPDS_CONT_TYPE_NWI =			0x01,
diff --git a/drivers/isimodem/gprs.c b/drivers/isimodem/gprs.c
index 6f4f686..93786ec 100644
--- a/drivers/isimodem/gprs.c
+++ b/drivers/isimodem/gprs.c
@@ -72,6 +72,55 @@ static void detach_ind_cb(GIsiClient *client,
 	/*ofono_gprs_detached_notify(gprs);*/
 }
 
+static void suspend_notify(struct ofono_gprs *gprs, uint8_t suspend_status,
+			uint8_t suspend_cause)
+{
+	int cause;
+
+	DBG("transfer status: %s (0x%02"PRIx8") cause %s (0x%02"PRIx8")",
+		gpds_transfer_status_name(suspend_status), suspend_status,
+		gpds_transfer_cause_name(suspend_cause), suspend_cause);
+
+	if (suspend_status == GPDS_TRANSFER_AVAIL) {
+		ofono_gprs_resume_notify(gprs);
+		return;
+	}
+
+	switch (suspend_cause) {
+	case GPDS_TRANSFER_CAUSE_SUSPENDED_NO_COVERAGE:
+		cause = GPRS_SUSPENDED_NO_COVERAGE;
+		break;
+
+	case GPDS_TRANSFER_CAUSE_SUSPENDED_CALL:
+		cause = GPRS_SUSPENDED_CALL;
+		break;
+
+	case GPDS_TRANSFER_CAUSE_SUSPENDED_CALL_SMS:
+	case GPDS_TRANSFER_CAUSE_SUSPENDED_RAU:
+	case GPDS_TRANSFER_CAUSE_SUSPENDED_LU:
+		cause = GPRS_SUSPENDED_SIGNALLING;
+		break;
+
+	default:
+		return;
+	}
+
+	ofono_gprs_suspend_notify(gprs, cause);
+}
+
+static void transfer_status_ind_cb(GIsiClient *client,
+					const void *restrict data, size_t len,
+					uint16_t object, void *opaque)
+{
+	struct ofono_gprs *gprs = opaque;
+	const unsigned char *msg = data;
+
+	if (!msg || len < 3 || msg[0] != GPDS_TRANSFER_STATUS_IND)
+		return;
+
+	suspend_notify(gprs, msg[1], msg[2]);
+}
+
 static gboolean isi_gprs_register(gpointer user)
 {
 	struct ofono_gprs *gprs = user;
@@ -83,6 +132,8 @@ static gboolean isi_gprs_register(gpointer user)
 		g_isi_client_set_debug(gd->client, gpds_debug, NULL);
 
 	g_isi_subscribe(gd->client, GPDS_DETACH_IND, detach_ind_cb, gprs);
+	g_isi_subscribe(gd->client, GPDS_TRANSFER_STATUS_IND,
+			transfer_status_ind_cb, gprs);
 
 	ofono_gprs_register(user);
 
@@ -258,6 +309,7 @@ static gboolean status_resp_cb(GIsiClient *client,
 	const unsigned char *msg = data;
 	struct isi_cb_data *cbd = opaque;
 	ofono_gprs_status_cb_t cb = cbd->cb;
+	struct ofono_gprs *gprs = cbd->data;
 	int status;
 
 	if (!msg) {
@@ -265,7 +317,7 @@ static gboolean status_resp_cb(GIsiClient *client,
 		goto error;
 	}
 
-	if (len < 2 || msg[0] != GPDS_STATUS_RESP)
+	if (len < 13 || msg[0] != GPDS_STATUS_RESP)
 		return FALSE;
 
 	/* FIXME: the core still expects reg status, and not a boolean
@@ -281,6 +333,8 @@ static gboolean status_resp_cb(GIsiClient *client,
 		status = GPRS_STAT_UNKNOWN;
 	}
 
+	suspend_notify(gprs, msg[11], msg[12]);
+
 	CALLBACK_WITH_SUCCESS(cb, status, cbd->data);
 
 	return TRUE;
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Add Suspended property to GPRS atom
  2010-09-09 12:08 ` [PATCH 1/2] Add Suspended property to GPRS atom Mika Liljeberg
@ 2010-09-09 17:57   ` Denis Kenzior
  0 siblings, 0 replies; 6+ messages in thread
From: Denis Kenzior @ 2010-09-09 17:57 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 2654 bytes --]

Hi Mika,

On 09/09/2010 07:08 AM, Mika Liljeberg wrote:
> 
> Signed-off-by: Mika Liljeberg <mika.liljeberg@nokia.com>

We don't use Signed-off-by, could you please configure your git not to
include it?  The only other nitpick I have is on your commit header, it
should include gprs: prefix.  E.g.

gprs: add Suspended property...

We do this to help people quickly scan git log and figure out whether a
patch potentially affects the area they are interested in.

> ---
>  doc/connman-api.txt |   19 ++++++++++++++
>  include/gprs.h      |   10 +++++++
>  src/gprs.c          |   69 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 98 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/connman-api.txt b/doc/connman-api.txt
> index 43d8897..b576e19 100644
> --- a/doc/connman-api.txt
> +++ b/doc/connman-api.txt
> @@ -77,6 +77,25 @@ Properties	boolean Attached [readonly]
>  			be available, e.g. receiving SMS over packet radio
>  			or network initiated PDP activation.
>  
> +		boolean Suspended [readonly]

My only thought here is that we should also make Suspended property
optional.  My thought here is that if we're not 'Attached', then
reporting Suspended is not really useful.

> @@ -1052,6 +1114,9 @@ static DBusMessage *gprs_get_properties(DBusConnection *conn,
>  	value = gprs->powered;
>  	ofono_dbus_dict_append(&dict, "Powered", DBUS_TYPE_BOOLEAN, &value);
>  
> +	value = gprs->suspended;
> +	ofono_dbus_dict_append(&dict, "Suspended", DBUS_TYPE_BOOLEAN, &value);
> +
>  	dbus_message_iter_close_container(&iter, &dict);
>  
>  	return reply;

See my comment above here, perhaps we should only include this in the
dict if Attached is true.

> @@ -1697,6 +1762,9 @@ static void gprs_remove(struct ofono_atom *atom)
>  	if (gprs == NULL)
>  		return;
>  
> +	if (gprs->suspend_timeout)
> +		g_source_remove(gprs->suspend_timeout);
> +
>  	if (gprs->pid_map) {
>  		idmap_free(gprs->pid_map);
>  		gprs->pid_map = NULL;
> @@ -1746,6 +1814,7 @@ struct ofono_gprs *ofono_gprs_create(struct ofono_modem *modem,
>  
>  	gprs->status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
>  	gprs->netreg_status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
> +	gprs->suspended = TRUE;
>  	gprs->pid_map = idmap_new(MAX_CONTEXTS);
>  
>  	return gprs;

And this could be just set to FALSE (or not set since the structure is
zeroed)

Otherwise patch looks great.

Another housekeeping item: Please send a patch taking ownership of this
task on the TODO list.  See ofono/TODO, the GPRS section.  This way
others know not to interfere with your work.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] isimodem: implement Suspended property
  2010-09-09 12:08 ` [PATCH 2/2] isimodem: implement Suspended property Mika Liljeberg
@ 2010-09-09 17:59   ` Denis Kenzior
  0 siblings, 0 replies; 6+ messages in thread
From: Denis Kenzior @ 2010-09-09 17:59 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

Hi Mika,

On 09/09/2010 07:08 AM, Mika Liljeberg wrote:
> 
> Signed-off-by: Mika Liljeberg <mika.liljeberg@nokia.com>
> ---
>  drivers/isimodem/debug.c |   25 ++++++++++++++++++++
>  drivers/isimodem/debug.h |    2 +
>  drivers/isimodem/gpds.h  |   17 ++++++++++++++
>  drivers/isimodem/gprs.c  |   56 +++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 99 insertions(+), 1 deletions(-)
> 

Please resubmit this with no Signed-off-by.  Looks good otherwise.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-09-09 17:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-09 12:08 Add Suspended property to GPRS (take 2) Mika Liljeberg
2010-09-09 12:08 ` [PATCH 1/2] Add Suspended property to GPRS atom Mika Liljeberg
2010-09-09 17:57   ` Denis Kenzior
2010-09-09 12:08 ` [PATCH 2/2] isimodem: implement Suspended property Mika Liljeberg
2010-09-09 17:59   ` Denis Kenzior
  -- strict thread matches above, loose matches on Subject: below --
2010-09-08  9:16 [PATCH 0/2] Add Suspended property to GPRS Mika Liljeberg
2010-09-08  9:17 ` [PATCH 1/2] Add Suspended property to GPRS atom Mika Liljeberg

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.