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

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

Hi All,

The following patches add the Suspended property to the
GPRS atom and implement it in the isimodem driver. The
property is opt-in for drivers that have the relevant
information. The value is read-only and defaults to false.

GPRS class B modems are not able to support simultaneous
GPRS data transfer with other services, such as voice call
or SMS. GPRS services may also be suspended if the network
does not support DTM, if the modem has poor network coverage,
or because of signalling procedures such as location or
routing area updates. While GPRS services are suspended,
the modem remains attached to the cellular network and all
PDP contexts remain established. Data transfer, however,
is not possible.

Some discussion:

The isimodem driver will currently suppress changes in the
property value for short-lived suspended states caused by SMS
transmission or other signalling. It would also be possible
to do the suppression in the oFono core in a generic way.
Alternatively, all the state changes and their reasons could
also be exposed to oFono clients. However, my feeling is
that the information might not be available in a sufficiently
coherent way from the different modems. Anyway, I'm open to
suggestions.

Br,

	MikaL


[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  |   74 +++++++++++++++++++++++++++++++++++++++++++++-
 include/gprs.h           |    1 +
 src/gprs.c               |   21 +++++++++++++
 7 files changed, 158 insertions(+), 1 deletions(-)

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

* [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
  2010-09-08  9:17 ` [PATCH 2/2] isimodem: Implement Suspended property Mika Liljeberg
  2010-09-08 13:52 ` [PATCH 0/2] Add Suspended property to GPRS Marcel Holtmann
  2 siblings, 0 replies; 7+ 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] 7+ messages in thread

* [PATCH 2/2] isimodem: Implement Suspended property
  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
@ 2010-09-08  9:17 ` Mika Liljeberg
  2010-09-08 13:52 ` [PATCH 0/2] Add Suspended property to GPRS Marcel Holtmann
  2 siblings, 0 replies; 7+ messages in thread
From: Mika Liljeberg @ 2010-09-08  9:17 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 6514 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  |   74 +++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 117 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..573dbb5 100644
--- a/drivers/isimodem/gprs.c
+++ b/drivers/isimodem/gprs.c
@@ -41,6 +41,8 @@
 #include "gpds.h"
 #include "debug.h"
 
+#define SUSPEND_TIMEOUT		8 /* s */
+
 /* 27.007 Section 10.1.20 <stat> */
 enum network_registration_status {
 	GPRS_STAT_NOT_REGISTERED = 0,
@@ -53,6 +55,7 @@ enum network_registration_status {
 
 struct gprs_data {
 	GIsiClient *client;
+	guint timeout;
 };
 
 static void detach_ind_cb(GIsiClient *client,
@@ -72,6 +75,70 @@ static void detach_ind_cb(GIsiClient *client,
 	/*ofono_gprs_detached_notify(gprs);*/
 }
 
+static gboolean suspend_timeout(gpointer data)
+{
+	struct ofono_gprs *gprs = data;
+	struct gprs_data *gd = ofono_gprs_get_data(gprs);
+
+	ofono_gprs_suspend_notify(gprs, TRUE);
+	gd->timeout = 0;
+	return FALSE;
+}
+
+static void suspend_notify(struct ofono_gprs *gprs,
+				uint8_t suspend_status,
+				uint8_t suspend_cause)
+{
+	struct gprs_data *gd = ofono_gprs_get_data(gprs);
+	ofono_bool_t status = FALSE;
+
+	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_NOT_AVAIL) {
+		switch (suspend_cause) {
+		case GPDS_TRANSFER_CAUSE_SUSPENDED_NO_COVERAGE:
+		case GPDS_TRANSFER_CAUSE_SUSPENDED_CALL:
+			status = TRUE;
+			break;
+
+		case GPDS_TRANSFER_CAUSE_SUSPENDED_CALL_SMS:
+		case GPDS_TRANSFER_CAUSE_SUSPENDED_RAU:
+		case GPDS_TRANSFER_CAUSE_SUSPENDED_LU:
+			if (gd->timeout)
+				return;
+			gd->timeout = g_timeout_add_seconds(SUSPEND_TIMEOUT,
+								suspend_timeout,
+								gprs);
+			return;
+
+		default:
+			return;
+		}
+	}
+
+	if (gd->timeout) {
+		g_source_remove(gd->timeout);
+		gd->timeout = 0;
+	}
+
+	ofono_gprs_suspend_notify(gprs, status);
+}
+
+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 +150,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 +327,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 +335,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 +351,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] 7+ messages in thread

* Re: [PATCH 0/2] Add Suspended property to GPRS
  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
  2010-09-08  9:17 ` [PATCH 2/2] isimodem: Implement Suspended property Mika Liljeberg
@ 2010-09-08 13:52 ` Marcel Holtmann
  2010-09-08 14:39   ` Mika.Liljeberg
  2 siblings, 1 reply; 7+ messages in thread
From: Marcel Holtmann @ 2010-09-08 13:52 UTC (permalink / raw)
  To: ofono

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

Hi Mika,

> The following patches add the Suspended property to the
> GPRS atom and implement it in the isimodem driver. The
> property is opt-in for drivers that have the relevant
> information. The value is read-only and defaults to false.
> 
> GPRS class B modems are not able to support simultaneous
> GPRS data transfer with other services, such as voice call
> or SMS. GPRS services may also be suspended if the network
> does not support DTM, if the modem has poor network coverage,
> or because of signalling procedures such as location or
> routing area updates. While GPRS services are suspended,
> the modem remains attached to the cellular network and all
> PDP contexts remain established. Data transfer, however,
> is not possible.
> 
> Some discussion:
> 
> The isimodem driver will currently suppress changes in the
> property value for short-lived suspended states caused by SMS
> transmission or other signalling. It would also be possible
> to do the suppression in the oFono core in a generic way.
> Alternatively, all the state changes and their reasons could
> also be exposed to oFono clients. However, my feeling is
> that the information might not be available in a sufficiently
> coherent way from the different modems. Anyway, I'm open to
> suggestions.

we had a brief discussion about this some time ago. And I don't think
that we can do anything than give the clients (UI and ConnMan) and
indication that the suspend has happened right now. In general it is too
late anyway. Since making a voice call and sending a SMS are decisions
by the end user. And he/she wants to execute them right now. So it is
expected that GPRS gets suspended.

I do agree that we should not blindly tell the clients suspended if we
now that it is a short lived situation like SMS. My personal preference
here would be that this is done inside oFono core and the hardware just
signals suspend (if it actually supports such notifications).

So how much work would it take to do the delay in the core? I know, that
the ISI modem is more sophisticated protocol, but we do have AT modems
that have a bit harder time gathering all the information.

Regards

Marcel



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

* RE: [PATCH 0/2] Add Suspended property to GPRS
  2010-09-08 13:52 ` [PATCH 0/2] Add Suspended property to GPRS Marcel Holtmann
@ 2010-09-08 14:39   ` Mika.Liljeberg
  2010-09-08 14:58     ` Marcel Holtmann
  0 siblings, 1 reply; 7+ messages in thread
From: Mika.Liljeberg @ 2010-09-08 14:39 UTC (permalink / raw)
  To: ofono

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

Hi Marcel, 

> From: ofono-bounces(a)ofono.org 
> [mailto:ofono-bounces(a)ofono.org] On Behalf Of ext Marcel Holtmann

> > The isimodem driver will currently suppress changes in the
> > property value for short-lived suspended states caused by SMS
> > transmission or other signalling. It would also be possible
> > to do the suppression in the oFono core in a generic way.
> > Alternatively, all the state changes and their reasons could
> > also be exposed to oFono clients. However, my feeling is
> > that the information might not be available in a sufficiently
> > coherent way from the different modems. Anyway, I'm open to
> > suggestions.
> 
> we had a brief discussion about this some time ago. And I don't think
> that we can do anything than give the clients (UI and ConnMan) and
> indication that the suspend has happened right now. In 
> general it is too
> late anyway. Since making a voice call and sending a SMS are decisions
> by the end user. And he/she wants to execute them right now. So it is
> expected that GPRS gets suspended.

Yes, I agree, there's no sane way to provide pre-warning. A signal that
comes after the fact still has some potential uses, though:

- a hint for internet connection management to start looking for an
  alternative access, in case GPRS is not back shortly
- a signal for mobile-aware applications to adjust their behavior
- UI notification for the end user

> I do agree that we should not blindly tell the clients suspended if we
> now that it is a short lived situation like SMS. My personal 
> preference
> here would be that this is done inside oFono core and the 
> hardware just
> signals suspend (if it actually supports such notifications).
> 
> So how much work would it take to do the delay in the core?

Not that hard, I guess. We could define a suspend cause code that gets
notified to the core, based on which the core would either modify the
property immediately or start a guard timer. Perhaps something along
the lines of:

enum gprs_suspend_status {
	GPRS_SUSPENDED_DETACHED,
	GPRS_SUSPENDED_SIGNALLING,
	GPRS_SUSPENDED_CALL,
	GPRS_SUSPENDED_NO_COVERAGE,
	GPRS_SUSPENDED_UKNOWN_CAUSE,
	GPRS_AVAILABLE,
};

src/gprs.c:
void ofono_gprs_suspend_notify(struct ofono_gprs *gprs,
				enum gprs_suspend_status status)
{
	...
}

The cause code could also be exposed in the DBus interface.

> I know, that
> the ISI modem is more sophisticated protocol, but we do have AT modems
> that have a bit harder time gathering all the information.

AT modem drivers could use any subset of the above causes.
To allow opt-in implementation in the drivers, the core could
automatically change the status between GPRS_SUSPENDED_DETACHED
and GPRS_AVAILABLE based on attach/detach notifications.

I can redraft the patches to do this. I don't really have a clue
what kind of vendor extensions are available for this in AT modems,
though. Any examples would be helpful.

Br,

	MikaL

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

* RE: [PATCH 0/2] Add Suspended property to GPRS
  2010-09-08 14:39   ` Mika.Liljeberg
@ 2010-09-08 14:58     ` Marcel Holtmann
  2010-09-09  7:27       ` Mika.Liljeberg
  0 siblings, 1 reply; 7+ messages in thread
From: Marcel Holtmann @ 2010-09-08 14:58 UTC (permalink / raw)
  To: ofono

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

Hi Mika,

> > > The isimodem driver will currently suppress changes in the
> > > property value for short-lived suspended states caused by SMS
> > > transmission or other signalling. It would also be possible
> > > to do the suppression in the oFono core in a generic way.
> > > Alternatively, all the state changes and their reasons could
> > > also be exposed to oFono clients. However, my feeling is
> > > that the information might not be available in a sufficiently
> > > coherent way from the different modems. Anyway, I'm open to
> > > suggestions.
> > 
> > we had a brief discussion about this some time ago. And I don't think
> > that we can do anything than give the clients (UI and ConnMan) and
> > indication that the suspend has happened right now. In 
> > general it is too
> > late anyway. Since making a voice call and sending a SMS are decisions
> > by the end user. And he/she wants to execute them right now. So it is
> > expected that GPRS gets suspended.
> 
> Yes, I agree, there's no sane way to provide pre-warning. A signal that
> comes after the fact still has some potential uses, though:
> 
> - a hint for internet connection management to start looking for an
>   alternative access, in case GPRS is not back shortly
> - a signal for mobile-aware applications to adjust their behavior
> - UI notification for the end user

That is basically what we had in mind. If the UI wants to show a warning
or change it icon. It can do this with the property changed signal. And
it only does this when there is an interaction that takes longer.

And ConnMan can decide to fallback to some other connection or needs to
tell the application that their connection is stalled now.

We have something similar with MMS where the hardware or network only
supports one PDP context. In that case it is even worse actually since
we take the proper IP connection away and it won't come back as it was.

In the end similar enough since if you wait long enough your TCP
connections will time out.

> > I do agree that we should not blindly tell the clients suspended if we
> > now that it is a short lived situation like SMS. My personal 
> > preference
> > here would be that this is done inside oFono core and the 
> > hardware just
> > signals suspend (if it actually supports such notifications).
> > 
> > So how much work would it take to do the delay in the core?
> 
> Not that hard, I guess. We could define a suspend cause code that gets
> notified to the core, based on which the core would either modify the
> property immediately or start a guard timer. Perhaps something along
> the lines of:
> 
> enum gprs_suspend_status {
> 	GPRS_SUSPENDED_DETACHED,
> 	GPRS_SUSPENDED_SIGNALLING,
> 	GPRS_SUSPENDED_CALL,
> 	GPRS_SUSPENDED_NO_COVERAGE,
> 	GPRS_SUSPENDED_UKNOWN_CAUSE,
> 	GPRS_AVAILABLE,
> };
> 
> src/gprs.c:
> void ofono_gprs_suspend_notify(struct ofono_gprs *gprs,
> 				enum gprs_suspend_status status)
> {
> 	...
> }
> 
> The cause code could also be exposed in the DBus interface.

I wouldn't go there as exposing it over D-Bus. However telling the core
a bit more details about this would be a good idea. Then it can decide
to either delay the notification or send it right away.

I prefer to start simple first. Making it too complicated in the
beginning is a bad idea. And giving application too much information is
a bad idea anyway. They should only get what they really need. And right
now I don't see a need to tell them any more details, than the GPRS
connection is suspended. And that means we don't know when we get it
back. So deal with this situation.

Corner cases where a phone call gets hung up right away is nasty, but
that happens, but then again, the user was making a phone call in the
first place. So a user initiated choice we are trying to cope with.

However "suspend" with flag "available" sounds like a bad API. Either we
call this gprs_state or something similar or we have suspend/resume
pairs. Just to be a bit more symmetric here.

> > I know, that
> > the ISI modem is more sophisticated protocol, but we do have AT modems
> > that have a bit harder time gathering all the information.
> 
> AT modem drivers could use any subset of the above causes.
> To allow opt-in implementation in the drivers, the core could
> automatically change the status between GPRS_SUSPENDED_DETACHED
> and GPRS_AVAILABLE based on attach/detach notifications.
> 
> I can redraft the patches to do this. I don't really have a clue
> what kind of vendor extensions are available for this in AT modems,
> though. Any examples would be helpful.

I have no idea either. Denis might know.

Regards

Marcel



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

* RE: [PATCH 0/2] Add Suspended property to GPRS
  2010-09-08 14:58     ` Marcel Holtmann
@ 2010-09-09  7:27       ` Mika.Liljeberg
  0 siblings, 0 replies; 7+ messages in thread
From: Mika.Liljeberg @ 2010-09-09  7:27 UTC (permalink / raw)
  To: ofono

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

Hi Marcel, 

> We have something similar with MMS where the hardware or network only
> supports one PDP context. In that case it is even worse actually since
> we take the proper IP connection away and it won't come back 
> as it was.

AFAIK, there is no reliable way to detect the case where an operator
only supports a single context per mobile. The second context activation
fails but there is no standard error code that would indicate why.
I suppose the number of contexts could be configurable somehow but
determining the correct setting for a particular operator is probably
a problem for the higher layers.

> In the end similar enough since if you wait long enough your TCP
> connections will time out.

TCP can take several minutes to time out, though. Mobile applications
will probably try to be a bit smarter if they have the means to 
do so.

> > The cause code could also be exposed in the DBus interface.
> 
> I wouldn't go there as exposing it over D-Bus. However 
> telling the core
> a bit more details about this would be a good idea. Then it can decide
> to either delay the notification or send it right away.
> 
> I prefer to start simple first. Making it too complicated in the
> beginning is a bad idea. And giving application too much 
> information is
> a bad idea anyway. They should only get what they really 
> need. And right
> now I don't see a need to tell them any more details, than the GPRS
> connection is suspended. And that means we don't know when we get it
> back. So deal with this situation.

Agreed.

> Corner cases where a phone call gets hung up right away is nasty, but
> that happens, but then again, the user was making a phone call in the
> first place. So a user initiated choice we are trying to cope with.

Well, the user has no control over incoming calls, aside from rejecting
them or going to offline mode. Not much can be done about that, though.

> However "suspend" with flag "available" sounds like a bad 
> API. Either we
> call this gprs_state or something similar or we have suspend/resume
> pairs. Just to be a bit more symmetric here.

A suspend/resume pair sounds good to me. I'll do the patches.

	MikaL

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

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

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2010-09-08  9:17 ` [PATCH 2/2] isimodem: Implement Suspended property Mika Liljeberg
2010-09-08 13:52 ` [PATCH 0/2] Add Suspended property to GPRS Marcel Holtmann
2010-09-08 14:39   ` Mika.Liljeberg
2010-09-08 14:58     ` Marcel Holtmann
2010-09-09  7:27       ` 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.