[PATCH v2] call-forwarding: class applied is the class given by SS code

[PATCH v2] call-forwarding: class applied is the class given by SS code

[Philippe Nunes]

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

GCF test cases 31.2.1.6.1/2 are asking to make a query according a specific
class.
The default class is applied when no class is specified in the SS code.
---
 src/call-forwarding.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 91e34c6..d73bcc0 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -1012,7 +1012,12 @@ static void ss_set_query_cf_callback(const struct ofono_error *error, int total,
 
 static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf)
 {
-	cf->driver->query(cf, cf->query_next, BEARER_CLASS_DEFAULT,
+	int cls;
+
+	cls = (cf->ss_req->cls == BEARER_CLASS_SS_DEFAULT) ?
+			BEARER_CLASS_DEFAULT : cf->ss_req->cls;
+
+	cf->driver->query(cf, cf->query_next, cls,
 			ss_set_query_cf_callback, cf);
 }
 
-- 
1.7.9.5

[PATCH v2] ussd: Recover idle state in case of response sending failure

[Philippe Nunes]

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

GCF test case 31.8.1.2.3 is rejecting the user response.
Any subsequent USSD notification are not handled because USSD is always in
state user-action.
---
 src/ussd.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/src/ussd.c b/src/ussd.c
index 74888b2..21e0164 100644
--- a/src/ussd.c
+++ b/src/ussd.c
@@ -414,6 +414,18 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, int dcs,
 		return;
 	}
 
+	if (status == OFONO_USSD_STATUS_TERMINATED &&
+			(ussd->state == USSD_STATE_IDLE ||
+			ussd->state == USSD_STATE_USER_ACTION)) {
+		ussd_change_state(ussd, USSD_STATE_IDLE);
+
+		if (ussd->pending == NULL)
+			return;
+
+		reply = __ofono_error_not_supported(ussd->pending);
+		goto out;
+	}
+
 	if (status == OFONO_USSD_STATUS_NOT_SUPPORTED) {
 		ussd_change_state(ussd, USSD_STATE_IDLE);
 
-- 
1.7.9.5

Re: [PATCH v2] ussd: Recover idle state in case of response sending failure

[Denis Kenzior]

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

Hi Philippe,

On 08/23/2012 11:27 AM, Philippe Nunes wrote:
> GCF test case 31.8.1.2.3 is rejecting the user response.
> Any subsequent USSD notification are not handled because USSD is always in
> state user-action.
> ---
>   src/ussd.c |   12 ++++++++++++
>   1 file changed, 12 insertions(+)
>
> diff --git a/src/ussd.c b/src/ussd.c
> index 74888b2..21e0164 100644
> --- a/src/ussd.c
> +++ b/src/ussd.c
> @@ -414,6 +414,18 @@ void ofono_ussd_notify(struct ofono_ussd *ussd, int status, int dcs,
>   		return;
>   	}
>
> +	if (status == OFONO_USSD_STATUS_TERMINATED&&
> +			(ussd->state == USSD_STATE_IDLE ||
> +			ussd->state == USSD_STATE_USER_ACTION)) {

This check makes no sense, why bother checking for ussd->state == 
USSD_STATE_IDLE?

> +		ussd_change_state(ussd, USSD_STATE_IDLE);
> +
> +		if (ussd->pending == NULL)
> +			return;
> +
> +		reply = __ofono_error_not_supported(ussd->pending);
> +		goto out;
> +	}
> +

I would handle it just like the 'Not Supported' case but with a new 
__ofono_error_network_terminated error reply.  Or perhaps re-use 
__ofono_error_canceled.

>   	if (status == OFONO_USSD_STATUS_NOT_SUPPORTED) {
>   		ussd_change_state(ussd, USSD_STATE_IDLE);
>

Regards,
-Denis

Re: [PATCH v2] ussd: Recover idle state in case of response sending failure

[Philippe Nunes]

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

On 08/23/2012 08:03 PM, Denis Kenzior wrote:
> Hi Philippe,
>
> On 08/23/2012 11:27 AM, Philippe Nunes wrote:
>> GCF test case 31.8.1.2.3 is rejecting the user response.
>> Any subsequent USSD notification are not handled because USSD is
>> always in
>> state user-action.
>> ---
>> src/ussd.c | 12 ++++++++++++
>> 1 file changed, 12 insertions(+)
>>
>> diff --git a/src/ussd.c b/src/ussd.c
>> index 74888b2..21e0164 100644
>> --- a/src/ussd.c
>> +++ b/src/ussd.c
>> @@ -414,6 +414,18 @@ void ofono_ussd_notify(struct ofono_ussd *ussd,
>> int status, int dcs,
>> return;
>> }
>>
>> + if (status == OFONO_USSD_STATUS_TERMINATED&&
>> + (ussd->state == USSD_STATE_IDLE ||
>> + ussd->state == USSD_STATE_USER_ACTION)) {
>
> This check makes no sense, why bother checking for ussd->state ==
> USSD_STATE_IDLE?

The issue has been unveiled after a network initiated USSD request:

\r\n+CUSD: 1,"Type *70*635*562# and send",15\r\n
src/ussd.c:ofono_ussd_notify() status: 1 ACTION_REQUIRED, state: 0 IDLE

then we try to  give an answer:

AT+CUSD=1,"*70*635*562#",15\r
\r\n+CUSD: 2\r\n
\r\n+CME ERROR: 100\r\n


I presume that the same error could occur after a user USSD request. In 
this case, the state is still 'idle' and the "Terminated" notification 
results in a "NotificationReceived" signal with a NULL string. To avoid 
this, I need to check also for ussd->state == USSD_STATE_IDLE. That's 
why the first version was for me more appropriate.


>
>> + ussd_change_state(ussd, USSD_STATE_IDLE);
>> +
>> + if (ussd->pending == NULL)
>> + return;
>> +
>> + reply = __ofono_error_not_supported(ussd->pending);
>> + goto out;
>> + }
>> +
>
> I would handle it just like the 'Not Supported' case but with a new
> __ofono_error_network_terminated error reply. Or perhaps re-use
> __ofono_error_canceled.

In practice, I don't ever expect to send a reply since the 
ussd_response_callback/ ussd_callback is invoked before the TERMINATED 
notification. The error returned in the callback is __ofono_error_failed.
So, what about changing the state to 'idle' if not already set and 
simply return?

Regards,

Philippe.