* [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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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; 9+ 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] 9+ 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 0 siblings, 1 reply; 9+ 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] 9+ 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 0 siblings, 1 reply; 9+ 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] 9+ 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; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2010-09-09 17:57 UTC | newest] Thread overview: 9+ 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 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
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.