[PATCH -v3 1/5] modem: only unref pending if it is a powered pending

[PATCH -v3 1/5] modem: only unref pending if it is a powered pending

[Gustavo F. Padovan]

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

---
 src/modem.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 52e647b..d6ab316 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -804,7 +804,7 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 		modem->timeout = 0;
 	}
 
-	if (modem->pending != NULL) {
+	if (modem->powered_pending != powered && modem->pending != NULL) {
 		DBusMessage *reply;
 
 		if (powered == modem->powered_pending)
-- 
1.7.3.2

[PATCH -v3 2/5] modem: add ofono_modem_reset() to modem.h

[Gustavo F. Padovan]

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

---
 include/modem.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/modem.h b/include/modem.h
index 7b13ee0..a92eb88 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -46,6 +46,8 @@ int ofono_modem_register(struct ofono_modem *modem);
 ofono_bool_t ofono_modem_is_registered(struct ofono_modem *modem);
 void ofono_modem_remove(struct ofono_modem *modem);
 
+void ofono_modem_reset(struct ofono_modem *modem);
+
 void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered);
 ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem);
 
-- 
1.7.3.2

[PATCH -v3 3/5] Add ofono_modem_reset()

[Gustavo F. Padovan]

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

Some modems can screw up everything and then we will need to do a silent
reset of the modem. This patch take the modem back to the OFFLINE state.
---
 src/modem.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index d6ab316..3126af1 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -1615,6 +1615,26 @@ void ofono_modem_remove(struct ofono_modem *modem)
 	g_free(modem);
 }
 
+void ofono_modem_reset(struct ofono_modem *modem)
+{
+	int err;
+
+	DBG("%p", modem);
+
+	if (modem->pending) {
+		DBusMessage *reply = __ofono_error_failed(modem->pending);
+		__ofono_dbus_pending_reply(&modem->pending, reply);
+	}
+
+	ofono_modem_set_powered(modem, FALSE);
+
+	err = set_powered(modem, TRUE);
+	if (err == -EINPROGRESS)
+		return;
+
+	modem_change_state(modem, MODEM_STATE_PRE_SIM);
+}
+
 int ofono_modem_driver_register(const struct ofono_modem_driver *d)
 {
 	DBG("driver: %p, name: %s", d, d->name);
-- 
1.7.3.2

[PATCH -v3 4/5] phonesim: Add modem reset trigger

[Gustavo F. Padovan]

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

---
 plugins/phonesim.c |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/plugins/phonesim.c b/plugins/phonesim.c
index 6b483ff..c7ef0d7 100644
--- a/plugins/phonesim.c
+++ b/plugins/phonesim.c
@@ -374,6 +374,30 @@ static void cfun_set_on_cb(gboolean ok, GAtResult *result, gpointer user_data)
 	ofono_modem_set_powered(modem, ok);
 }
 
+static gboolean phonesim_reset(void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+	struct phonesim_data *data = ofono_modem_get_data(modem);
+
+	g_at_chat_unref(data->chat);
+	data->chat = NULL;
+
+	if (data->mux) {
+		g_at_mux_shutdown(data->mux);
+		g_at_mux_unref(data->mux);
+		data->mux = NULL;
+	}
+
+	ofono_modem_reset(modem);
+
+	return FALSE;
+}
+
+static void crst_notify(GAtResult *result, gpointer user_data)
+{
+	g_idle_add(phonesim_reset, user_data);
+}
+
 static void phonesim_disconnected(gpointer user_data)
 {
 	struct ofono_modem *modem = user_data;
@@ -526,6 +550,9 @@ static int phonesim_enable(struct ofono_modem *modem)
 	g_at_chat_send(data->chat, "AT+CSCS=\"GSM\"", none_prefix,
 			NULL, NULL, NULL);
 
+	g_at_chat_register(data->chat, "+CRST:",
+				crst_notify, FALSE, modem, NULL);
+
 	return 0;
 }
 
-- 
1.7.3.2

[PATCH -v3 5/5] modem: add support to restore state when resetting the modem

[Gustavo F. Padovan]

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

---
 src/modem.c |   74 +++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 3126af1..638c33f 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -70,6 +70,7 @@ struct ofono_modem {
 	guint			interface_update;
 	ofono_bool_t		powered;
 	ofono_bool_t		powered_pending;
+	ofono_bool_t		get_online;
 	guint			timeout;
 	ofono_bool_t		online;
 	struct ofono_watchlist	*online_watches;
@@ -434,30 +435,6 @@ static void modem_change_state(struct ofono_modem *modem,
 	}
 }
 
-static void sim_state_watch(enum ofono_sim_state new_state, void *user)
-{
-	struct ofono_modem *modem = user;
-
-	switch (new_state) {
-	case OFONO_SIM_STATE_NOT_PRESENT:
-		modem_change_state(modem, MODEM_STATE_PRE_SIM);
-		break;
-	case OFONO_SIM_STATE_INSERTED:
-		break;
-	case OFONO_SIM_STATE_READY:
-		modem_change_state(modem, MODEM_STATE_OFFLINE);
-
-		/*
-		 * If we don't have the set_online method, also proceed
-		 * straight to the online state
-		 */
-		if (modem->driver->set_online == NULL)
-			modem_change_state(modem, MODEM_STATE_ONLINE);
-
-		break;
-	}
-}
-
 unsigned int __ofono_modem_add_online_watch(struct ofono_modem *modem,
 					ofono_modem_online_notify_func notify,
 					void *data, ofono_destroy_func destroy)
@@ -488,16 +465,22 @@ static void online_cb(const struct ofono_error *error, void *data)
 	DBusMessage *reply;
 
 	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
-			modem->modem_state == MODEM_STATE_OFFLINE)
+			modem->modem_state == MODEM_STATE_OFFLINE) {
+		modem_change_state(modem, MODEM_STATE_ONLINE);
+
+		if (!modem->pending)
+			return;
+
 		reply = dbus_message_new_method_return(modem->pending);
-	else
+	} else {
+
+		if (!modem->pending)
+			return;
+
 		reply = __ofono_error_failed(modem->pending);
+	}
 
 	__ofono_dbus_pending_reply(&modem->pending, reply);
-
-	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
-			modem->modem_state == MODEM_STATE_OFFLINE)
-		modem_change_state(modem, MODEM_STATE_ONLINE);
 }
 
 static void offline_cb(const struct ofono_error *error, void *data)
@@ -517,6 +500,34 @@ static void offline_cb(const struct ofono_error *error, void *data)
 		modem_change_state(modem, MODEM_STATE_OFFLINE);
 }
 
+static void sim_state_watch(enum ofono_sim_state new_state, void *user)
+{
+	struct ofono_modem *modem = user;
+
+	switch (new_state) {
+	case OFONO_SIM_STATE_NOT_PRESENT:
+		modem_change_state(modem, MODEM_STATE_PRE_SIM);
+		break;
+	case OFONO_SIM_STATE_INSERTED:
+		break;
+	case OFONO_SIM_STATE_READY:
+		modem_change_state(modem, MODEM_STATE_OFFLINE);
+
+		/*
+		 * If we don't have the set_online method, also proceed
+		 * straight to the online state
+		 */
+		if (modem->driver->set_online == NULL)
+			modem_change_state(modem, MODEM_STATE_ONLINE);
+		else if (modem->get_online)
+			modem->driver->set_online(modem, 1, online_cb, modem);
+
+		modem->get_online = FALSE;
+
+		break;
+	}
+}
+
 static DBusMessage *set_property_online(struct ofono_modem *modem,
 					DBusMessage *msg,
 					DBusMessageIter *var)
@@ -1626,6 +1637,9 @@ void ofono_modem_reset(struct ofono_modem *modem)
 		__ofono_dbus_pending_reply(&modem->pending, reply);
 	}
 
+	if (modem->modem_state == MODEM_STATE_ONLINE)
+		modem->get_online = TRUE;
+
 	ofono_modem_set_powered(modem, FALSE);
 
 	err = set_powered(modem, TRUE);
-- 
1.7.3.2

Re: [PATCH -v3 5/5] modem: add support to restore state when resetting the modem

[Denis Kenzior]

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

Hi Gustavo,

> @@ -488,16 +465,22 @@ static void online_cb(const struct ofono_error *error, void *data)
>  	DBusMessage *reply;
>  
>  	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
> -			modem->modem_state == MODEM_STATE_OFFLINE)
> +			modem->modem_state == MODEM_STATE_OFFLINE) {
> +		modem_change_state(modem, MODEM_STATE_ONLINE);

This actually breaks the (yet unwritten) rule to try to return from the
method call before signaling the property changed.

Can you leave this be and add something like:

if (modem->pending == NULL)
	goto out;

or something prior to this if condition?

> +
> +		if (!modem->pending)
> +			return;
> +

Then you can omit this

>  		reply = dbus_message_new_method_return(modem->pending);
> -	else
> +	} else {
> +
> +		if (!modem->pending)
> +			return;
> +

And this

>  		reply = __ofono_error_failed(modem->pending);
> +	}
>  
>  	__ofono_dbus_pending_reply(&modem->pending, reply);

And just add a label here...

> -
> -	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
> -			modem->modem_state == MODEM_STATE_OFFLINE)
> -		modem_change_state(modem, MODEM_STATE_ONLINE);
>  }
>  
>  static void offline_cb(const struct ofono_error *error, void *data)

Regards,
-Denis

[PATCH] modem: add support to restore state when resetting the modem

[Gustavo F. Padovan]

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

---
 src/modem.c |   60 +++++++++++++++++++++++++++++++++++-----------------------
 1 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 2f9387c..816443e 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -70,6 +70,7 @@ struct ofono_modem {
 	guint			interface_update;
 	ofono_bool_t		powered;
 	ofono_bool_t		powered_pending;
+	ofono_bool_t		get_online;
 	guint			timeout;
 	ofono_bool_t		online;
 	struct ofono_watchlist	*online_watches;
@@ -434,30 +435,6 @@ static void modem_change_state(struct ofono_modem *modem,
 	}
 }
 
-static void sim_state_watch(enum ofono_sim_state new_state, void *user)
-{
-	struct ofono_modem *modem = user;
-
-	switch (new_state) {
-	case OFONO_SIM_STATE_NOT_PRESENT:
-		modem_change_state(modem, MODEM_STATE_PRE_SIM);
-		break;
-	case OFONO_SIM_STATE_INSERTED:
-		break;
-	case OFONO_SIM_STATE_READY:
-		modem_change_state(modem, MODEM_STATE_OFFLINE);
-
-		/*
-		 * If we don't have the set_online method, also proceed
-		 * straight to the online state
-		 */
-		if (modem->driver->set_online == NULL)
-			modem_change_state(modem, MODEM_STATE_ONLINE);
-
-		break;
-	}
-}
-
 unsigned int __ofono_modem_add_online_watch(struct ofono_modem *modem,
 					ofono_modem_online_notify_func notify,
 					void *data, ofono_destroy_func destroy)
@@ -487,6 +464,9 @@ static void online_cb(const struct ofono_error *error, void *data)
 	struct ofono_modem *modem = data;
 	DBusMessage *reply;
 
+	if (!modem->pending)
+		goto out;
+
 	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
 			modem->modem_state == MODEM_STATE_OFFLINE)
 		reply = dbus_message_new_method_return(modem->pending);
@@ -495,6 +475,7 @@ static void online_cb(const struct ofono_error *error, void *data)
 
 	__ofono_dbus_pending_reply(&modem->pending, reply);
 
+out:
 	if (error->type == OFONO_ERROR_TYPE_NO_ERROR &&
 			modem->modem_state == MODEM_STATE_OFFLINE)
 		modem_change_state(modem, MODEM_STATE_ONLINE);
@@ -517,6 +498,34 @@ static void offline_cb(const struct ofono_error *error, void *data)
 		modem_change_state(modem, MODEM_STATE_OFFLINE);
 }
 
+static void sim_state_watch(enum ofono_sim_state new_state, void *user)
+{
+	struct ofono_modem *modem = user;
+
+	switch (new_state) {
+	case OFONO_SIM_STATE_NOT_PRESENT:
+		modem_change_state(modem, MODEM_STATE_PRE_SIM);
+		break;
+	case OFONO_SIM_STATE_INSERTED:
+		break;
+	case OFONO_SIM_STATE_READY:
+		modem_change_state(modem, MODEM_STATE_OFFLINE);
+
+		/*
+		 * If we don't have the set_online method, also proceed
+		 * straight to the online state
+		 */
+		if (modem->driver->set_online == NULL)
+			modem_change_state(modem, MODEM_STATE_ONLINE);
+		else if (modem->get_online)
+			modem->driver->set_online(modem, 1, online_cb, modem);
+
+		modem->get_online = FALSE;
+
+		break;
+	}
+}
+
 static DBusMessage *set_property_online(struct ofono_modem *modem,
 					DBusMessage *msg,
 					DBusMessageIter *var)
@@ -1627,6 +1636,9 @@ void ofono_modem_reset(struct ofono_modem *modem)
 		__ofono_dbus_pending_reply(&modem->pending, reply);
 	}
 
+	if (modem->modem_state == MODEM_STATE_ONLINE)
+		modem->get_online = TRUE;
+
 	ofono_modem_set_powered(modem, FALSE);
 
 	err = set_powered(modem, TRUE);
-- 
1.7.3.2

Re: [PATCH] modem: add support to restore state when resetting the modem

[Denis Kenzior]

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

Hi Gustavo,

On 12/07/2010 12:31 PM, Gustavo F. Padovan wrote:
> ---
>  src/modem.c |   60 +++++++++++++++++++++++++++++++++++-----------------------
>  1 files changed, 36 insertions(+), 24 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH -v3 4/5] phonesim: Add modem reset trigger

[Denis Kenzior]

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

Hi Gustavo,

On 12/03/2010 12:52 PM, Gustavo F. Padovan wrote:
> ---
>  plugins/phonesim.c |   27 +++++++++++++++++++++++++++
>  1 files changed, 27 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH -v3 3/5] Add ofono_modem_reset()

[Denis Kenzior]

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

Hi Gustavo,

On 12/03/2010 12:52 PM, Gustavo F. Padovan wrote:
> Some modems can screw up everything and then we will need to do a silent
> reset of the modem. This patch take the modem back to the OFFLINE state.
> ---
>  src/modem.c |   20 ++++++++++++++++++++
>  1 files changed, 20 insertions(+), 0 deletions(-)
> 

Looks good to me.  Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH -v3 2/5] modem: add ofono_modem_reset() to modem.h

[Denis Kenzior]

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

Hi Gustavo,

On 12/03/2010 12:52 PM, Gustavo F. Padovan wrote:
> ---
>  include/modem.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH -v3 1/5] modem: only unref pending if it is a powered pending

[Gustavo F. Padovan]

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

* Gustavo F. Padovan <padovan@profusion.mobi> [2010-12-03 16:52:53 -0200]:

> ---
>  src/modem.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/src/modem.c b/src/modem.c
> index 52e647b..d6ab316 100644
> --- a/src/modem.c
> +++ b/src/modem.c
> @@ -804,7 +804,7 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
>  		modem->timeout = 0;
>  	}
>  
> -	if (modem->pending != NULL) {
> +	if (modem->powered_pending != powered && modem->pending != NULL) {

This one is wrong, I'll resend it. It should be modem->powered instead of
powered.

-- 
Gustavo F. Padovan
http://profusion.mobi

[PATCH] modem: only unref pending if it is a powered pending

[Gustavo F. Padovan]

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

---
 src/modem.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 52e647b..ae427b3 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -804,7 +804,8 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 		modem->timeout = 0;
 	}
 
-	if (modem->pending != NULL) {
+	if (modem->powered_pending != modem->powered &&
+						modem->pending != NULL) {
 		DBusMessage *reply;
 
 		if (powered == modem->powered_pending)
-- 
1.7.3.2

Re: [PATCH] modem: only unref pending if it is a powered pending

[Denis Kenzior]

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

Hi Gustavo,

On 12/03/2010 03:40 PM, Gustavo F. Padovan wrote:
> ---
>  src/modem.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/src/modem.c b/src/modem.c
> index 52e647b..ae427b3 100644
> --- a/src/modem.c
> +++ b/src/modem.c
> @@ -804,7 +804,8 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
>  		modem->timeout = 0;
>  	}
>  
> -	if (modem->pending != NULL) {
> +	if (modem->powered_pending != modem->powered &&
> +						modem->pending != NULL) {
>  		DBusMessage *reply;
>  
>  		if (powered == modem->powered_pending)

Patch has been applied, thanks.

Regards,
-Denis