[offline-atoms PATCH 0/2]

[offline-atoms PATCH 0/2]

[Pekka.Pessi]

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

Hi all,

Here are patches that make gprs and sim atom online-agnostic. The atoms
follow the network registration status and they can be added in
post_sim. 

The intention is to allow GPRS context to be configured and short
messages to be queued while in offline.

The patches has been tested with mbm and n900 modems (admittedly in
December last year).

--Pekka

[offline-atoms PATCH 1/2] Make gprs atom offline-safe.

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Allow use of GPRS atom both in online and offline (post_sim) states.

The GPRS now considers the removal of the netreg atom as implicit detach
and registration change to NOT_REGISTERED.
---
 src/gprs.c |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/gprs.c b/src/gprs.c
index 0e86bdf..35cc475 100644
--- a/src/gprs.c
+++ b/src/gprs.c
@@ -1370,10 +1370,27 @@ static void gprs_attach_callback(const struct ofono_error *error, void *data)
 	}
 }
 
+static void gprs_netreg_removed(struct ofono_gprs *gprs)
+{
+	gprs->netreg = NULL;
+
+	gprs->flags &= ~(GPRS_FLAG_RECHECK | GPRS_FLAG_ATTACHING);
+	gprs->status_watch = 0;
+	gprs->netreg_status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
+	gprs->driver_attached = FALSE;
+
+	gprs_attached_update(gprs);
+}
+
 static void gprs_netreg_update(struct ofono_gprs *gprs)
 {
 	ofono_bool_t attach;
 
+	if (gprs->netreg == NULL) {
+		gprs_netreg_removed(gprs);
+		return;
+	}
+
 	attach = gprs->netreg_status == NETWORK_REGISTRATION_STATUS_REGISTERED;
 
 	attach = attach || (gprs->roaming_allowed &&
@@ -2250,8 +2267,7 @@ static void netreg_watch(struct ofono_atom *atom,
 	struct ofono_gprs *gprs = data;
 
 	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
-		gprs->status_watch = 0;
-		gprs->netreg = NULL;
+		gprs_netreg_removed(gprs);
 		return;
 	}
 
-- 
1.7.1

[offline-atoms PATCH 2/2] Make sms atom offline-safe

[Pekka.Pessi]

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

From: Pekka Pessi <Pekka.Pessi@nokia.com>

Allow use of SMS atom both in online and offline (post_sim) states.

SMS atom now watches network registration atom and state.  Thmessages
are queued but not sent if not registered to a network.  Errors
occurring when a short message is being sent while transition from
online to offline or when network coverage is dropped are now handled
gracefully.
---
 src/sms.c |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 91 insertions(+), 6 deletions(-)

diff --git a/src/sms.c b/src/sms.c
index 9bb5c93..4d1a929 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -80,6 +80,10 @@ struct ofono_sms {
 	guint tx_source;
 	struct ofono_message_waiting *mw;
 	unsigned int mw_watch;
+	ofono_bool_t registered;
+	struct ofono_netreg *netreg;
+	unsigned int netreg_watch;
+	unsigned int status_watch;
 	struct ofono_sim *sim;
 	GKeyFile *settings;
 	char *imsi;
@@ -747,6 +751,11 @@ static void tx_finished(const struct ofono_error *error, int mr, void *data)
 	DBG("tx_finished");
 
 	if (ok == FALSE) {
+		/* Retry again when back in online mode */
+		/* Note this does not increment retry count */
+		if (sms->registered == FALSE)
+			return;
+
 		if (!(entry->flags & OFONO_SMS_SUBMIT_FLAG_RETRY))
 			goto next_q;
 
@@ -810,6 +819,9 @@ next_q:
 
 	tx_queue_entry_destroy(entry);
 
+	if (sms->registered == FALSE)
+		return;
+
 	if (g_queue_peek_head(sms->txq)) {
 		DBG("Scheduling next");
 		sms->tx_source = g_timeout_add(0, tx_next, sms);
@@ -833,6 +845,9 @@ static gboolean tx_next(gpointer user_data)
 	if (entry == NULL)
 		return FALSE;
 
+	if (sms->registered == FALSE)
+		return FALSE;
+
 	if (g_queue_get_length(sms->txq) > 1
 			|| (entry->num_pdus - entry->cur_pdu) > 1)
 		send_mms = 1;
@@ -843,6 +858,55 @@ static gboolean tx_next(gpointer user_data)
 	return FALSE;
 }
 
+static void netreg_status_watch(int status, int lac, int ci, int tech,
+					const char *mcc, const char *mnc,
+					void *data)
+{
+	struct ofono_sms *sms = data;
+
+	switch (status) {
+	case NETWORK_REGISTRATION_STATUS_REGISTERED:
+	case NETWORK_REGISTRATION_STATUS_ROAMING:
+		sms->registered = TRUE;
+		break;
+	default:
+		sms->registered = FALSE;
+		break;
+	}
+
+	if (sms->registered == FALSE)
+		return;
+
+	if (sms->tx_source > 0)
+		return;
+
+	if (g_queue_get_length(sms->txq))
+		sms->tx_source = g_timeout_add(0, tx_next, sms);
+}
+
+static void netreg_watch(struct ofono_atom *atom,
+				enum ofono_atom_watch_condition cond,
+				void *data)
+{
+	struct ofono_sms *sms = data;
+	int status;
+
+	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
+		sms->registered = FALSE;
+		sms->status_watch = 0;
+		sms->netreg = NULL;
+		return;
+	}
+
+	sms->netreg = __ofono_atom_get_data(atom);
+	sms->status_watch = __ofono_netreg_add_status_watch(sms->netreg,
+					netreg_status_watch, sms, NULL);
+
+	status = ofono_netreg_get_status(sms->netreg);
+	netreg_status_watch(status, 0, 0, 0, NULL, NULL, sms);
+}
+
+
 /**
  * Generate a UUID from an SMS PDU List
  *
@@ -1630,6 +1694,19 @@ static void sms_unregister(struct ofono_atom *atom)
 		sms->mw = NULL;
 	}
 
+	if (sms->status_watch) {
+		__ofono_netreg_remove_status_watch(sms->netreg,
+							sms->status_watch);
+		sms->status_watch = 0;
+	}
+
+	if (sms->netreg_watch) {
+		__ofono_modem_remove_atom_watch(modem, sms->netreg_watch);
+		sms->netreg_watch = 0;
+	}
+
+	sms->netreg = NULL;
+
 	if (sms->messages) {
 		GHashTableIter iter;
 		struct message *m;
@@ -1813,7 +1890,7 @@ void ofono_sms_register(struct ofono_sms *sms)
 	DBusConnection *conn = ofono_dbus_get_connection();
 	struct ofono_modem *modem = __ofono_atom_get_modem(sms->atom);
 	const char *path = __ofono_atom_get_path(sms->atom);
-	struct ofono_atom *mw_atom;
+	struct ofono_atom *atom;
 	struct ofono_atom *sim_atom;
 
 	if (!g_dbus_register_interface(conn, path,
@@ -1832,11 +1909,19 @@ void ofono_sms_register(struct ofono_sms *sms)
 					OFONO_ATOM_TYPE_MESSAGE_WAITING,
 					mw_watch, sms, NULL);
 
-	mw_atom = __ofono_modem_find_atom(modem,
-					OFONO_ATOM_TYPE_MESSAGE_WAITING);
+	atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_MESSAGE_WAITING);
+
+	if (atom && __ofono_atom_get_registered(atom))
+		mw_watch(atom, OFONO_ATOM_WATCH_CONDITION_REGISTERED, sms);
+
+	sms->netreg_watch = __ofono_modem_add_atom_watch(modem,
+					OFONO_ATOM_TYPE_NETREG,
+					netreg_watch, sms, NULL);
+
+	atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_NETREG);
 
-	if (mw_atom && __ofono_atom_get_registered(mw_atom))
-		mw_watch(mw_atom, OFONO_ATOM_WATCH_CONDITION_REGISTERED, sms);
+	if (atom && __ofono_atom_get_registered(atom))
+		netreg_watch(atom, OFONO_ATOM_WATCH_CONDITION_REGISTERED, sms);
 
 	sim_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_SIM);
 
@@ -1924,7 +2009,7 @@ int __ofono_sms_txq_submit(struct ofono_sms *sms, GSList *list,
 
 	g_queue_push_tail(sms->txq, entry);
 
-	if (g_queue_get_length(sms->txq) == 1)
+	if (sms->registered && g_queue_get_length(sms->txq) == 1)
 		sms->tx_source = g_timeout_add(0, tx_next, sms);
 
 	if (uuid)
-- 
1.7.1

Re: [offline-atoms PATCH 2/2] Make sms atom offline-safe

[Denis Kenzior]

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

Hi Pekka,

On 01/14/2011 08:40 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Allow use of SMS atom both in online and offline (post_sim) states.
> 
> SMS atom now watches network registration atom and state.  Thmessages
> are queued but not sent if not registered to a network.  Errors
> occurring when a short message is being sent while transition from
> online to offline or when network coverage is dropped are now handled
> gracefully.
> ---
>  src/sms.c |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 91 insertions(+), 6 deletions(-)
> 

Patch looked fine to me, so I went ahead and applied it.  Can we get
this one tested ASAP on various modem drivers?

Regards,
-Denis

Re: [offline-atoms PATCH 1/2] Make gprs atom offline-safe.

[Denis Kenzior]

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

Hi Pekka,

On 01/14/2011 08:40 AM, Pekka.Pessi(a)nokia.com wrote:
> From: Pekka Pessi <Pekka.Pessi@nokia.com>
> 
> Allow use of GPRS atom both in online and offline (post_sim) states.
> 
> The GPRS now considers the removal of the netreg atom as implicit detach
> and registration change to NOT_REGISTERED.
> ---
>  src/gprs.c |   20 ++++++++++++++++++--
>  1 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gprs.c b/src/gprs.c
> index 0e86bdf..35cc475 100644
> --- a/src/gprs.c
> +++ b/src/gprs.c
> @@ -1370,10 +1370,27 @@ static void gprs_attach_callback(const struct ofono_error *error, void *data)
>  	}
>  }
>  
> +static void gprs_netreg_removed(struct ofono_gprs *gprs)
> +{
> +	gprs->netreg = NULL;
> +
> +	gprs->flags &= ~(GPRS_FLAG_RECHECK | GPRS_FLAG_ATTACHING);
> +	gprs->status_watch = 0;
> +	gprs->netreg_status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
> +	gprs->driver_attached = FALSE;
> +
> +	gprs_attached_update(gprs);
> +}
> +
>  static void gprs_netreg_update(struct ofono_gprs *gprs)
>  {
>  	ofono_bool_t attach;
>  
> +	if (gprs->netreg == NULL) {
> +		gprs_netreg_removed(gprs);
> +		return;
> +	}
> +

I'm having trouble visualizing why this chunk is necessary.  We reset
netreg to NULL only when shutting down the atom or when netreg atom goes
away.  So this condition should never be triggered.

>  	attach = gprs->netreg_status == NETWORK_REGISTRATION_STATUS_REGISTERED;
>  
>  	attach = attach || (gprs->roaming_allowed &&
> @@ -2250,8 +2267,7 @@ static void netreg_watch(struct ofono_atom *atom,
>  	struct ofono_gprs *gprs = data;
>  
>  	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
> -		gprs->status_watch = 0;
> -		gprs->netreg = NULL;
> +		gprs_netreg_removed(gprs);
>  		return;
>  	}
>  

Otherwise the rest seems fine to me.

Regards,
-Denis

Re: [offline-atoms PATCH 1/2] Make gprs atom offline-safe.

[Pekka Pessi]

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

Hi Denis,

2011/1/19 Denis Kenzior <denkenz@gmail.com>:
>> +     if (gprs->netreg == NULL) {
>> +             gprs_netreg_removed(gprs);
>> +             return;
>> +     }
>> +
>
> I'm having trouble visualizing why this chunk is necessary.  We reset
> netreg to NULL only when shutting down the atom or when netreg atom goes
> away.  So this condition should never be triggered.

It is for the case where the gprs atom is inserted in post_sim but
netreg in post_online.

-- 
Pekka.Pessi mail at nokia.com

Re: [offline-atoms PATCH 1/2] Make gprs atom offline-safe.

[Denis Kenzior]

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

Hi Pekka,

On 01/19/2011 07:33 AM, Pekka Pessi wrote:
> Hi Denis,
> 
> 2011/1/19 Denis Kenzior <denkenz@gmail.com>:
>>> +     if (gprs->netreg == NULL) {
>>> +             gprs_netreg_removed(gprs);
>>> +             return;
>>> +     }
>>> +
>>
>> I'm having trouble visualizing why this chunk is necessary.  We reset
>> netreg to NULL only when shutting down the atom or when netreg atom goes
>> away.  So this condition should never be triggered.
> 
> It is for the case where the gprs atom is inserted in post_sim but
> netreg in post_online.
> 

I still don't see it, applying your patch and removing this chunk works
just fine.  I can set Powered / Roaming Allowed with no issues...

Regards,
-Denis