[PATCH 0/4] DTMF in dialing number

[PATCH 0/4] DTMF in dialing number

[Philippe Nunes]

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

GCF test cases unveiled the following requirements not currently
supported by oFono:

- Dialing number provided by Setup Call can include a pause character
followed by a DTMF string.

I noticed that DTMFs concatenated to the dialing number are likely to be
handled directly by the modem. So, I suggest to include the pause character
as an acceptable digit in the phone number.

- Dialing number provided by Setup Call can be greater than 20 digits.

- AID shall not be displayed in case of self explanatory icon
As I'm not sure if it is intended to not take into account this icon qualifier
I just implemented a fix in stk.c
If you agree with this requirement, then we would need to check the icon
qualifier in each Stkagent method.

Philippe.

Philippe Nunes (4):
  stk: convert to phone number format
  common: Accept pause character in the dialing number
  voicecall: Accept long phone number format for STK
  stk: AID shall not be displayed in case of self explanatory icon

 src/common.c    |    2 +-
 src/stk.c       |   55 ++++++++++++++++++++++++++++++++++++++++---------------
 src/voicecall.c |    2 +-
 3 files changed, 42 insertions(+), 17 deletions(-)

-- 
1.7.9.5

[PATCH 1/4] stk: convert to phone number format

[Philippe Nunes]

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

The dialing number provided by the Setup Call proactive command
may contain pause and DTMF characters.
---
 src/stk.c |   50 +++++++++++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 15 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 19cb0eb..5c79183 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -104,6 +104,27 @@ static void timers_update(struct ofono_stk *stk);
 		result.additional_len = sizeof(addn_info);	\
 		result.additional = addn_info;			\
 
+static gboolean convert_to_phone_number_format(const char *input_str,
+							char *output_str)
+{
+	char *digit;
+	char *digit_from = "01234567890abcABC";
+	char *digit_to = "01234567890*#p*#p";
+	int pos;
+
+	for (pos = 0; input_str[pos] != '\0'; pos++) {
+		digit = strchr(digit_from, input_str[pos]);
+		if (digit == NULL)
+			return FALSE;
+
+		output_str[pos] = digit_to[digit - digit_from];
+	}
+
+	output_str[pos] = '\0';
+
+	return TRUE;
+}
+
 static int stk_respond(struct ofono_stk *stk, struct stk_response *rsp,
 			ofono_stk_generic_cb_t cb)
 {
@@ -1715,6 +1736,7 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
 	char *alpha_id = NULL;
 	struct ofono_voicecall *vc;
 	struct stk_response rsp;
+	char number[256];
 	int err;
 
 	switch (result) {
@@ -1752,7 +1774,13 @@ static void confirm_call_cb(enum stk_agent_result result, gboolean confirm,
 		}
 	}
 
-	err = __ofono_voicecall_dial(vc, sc->addr.number, sc->addr.ton_npi,
+	/* Convert the setup call number to phone number format */
+	if (convert_to_phone_number_format(sc->addr.number, number) == FALSE) {
+		send_simple_response(stk, STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD);
+		return;
+	}
+
+	err = __ofono_voicecall_dial(vc, number, sc->addr.ton_npi,
 					alpha_id, sc->icon_id_call_setup.id,
 					qualifier >> 1, call_setup_connected,
 					stk);
@@ -2353,10 +2381,8 @@ static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
 {
 	static unsigned char not_in_speech_call_result[] = { 0x07 };
 	struct ofono_voicecall *vc = NULL;
-	char dtmf[256], *digit;
-	char *dtmf_from = "01234567890abcABC";
-	char *dtmf_to = "01234567890*#p*#p";
-	int err, pos;
+	char dtmf[256];
+	int err;
 
 	vc = __ofono_atom_find(OFONO_ATOM_TYPE_VOICECALL,
 				__ofono_atom_get_modem(stk->atom));
@@ -2366,18 +2392,12 @@ static gboolean handle_command_send_dtmf(const struct stk_command *cmd,
 	}
 
 	/* Convert the DTMF string to phone number format */
-	for (pos = 0; cmd->send_dtmf.dtmf[pos] != '\0'; pos++) {
-		digit = strchr(dtmf_from, cmd->send_dtmf.dtmf[pos]);
-		if (digit == NULL) {
-			rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
-			return TRUE;
-		}
-
-		dtmf[pos] = dtmf_to[digit - dtmf_from];
+	if (convert_to_phone_number_format(cmd->send_dtmf.dtmf, dtmf) ==
+						FALSE) {
+		rsp->result.type = STK_RESULT_TYPE_DATA_NOT_UNDERSTOOD;
+		return TRUE;
 	}
 
-	dtmf[pos] = '\0';
-
 	err = __ofono_voicecall_tone_send(vc, dtmf, dtmf_sent_cb, stk);
 
 	if (err == -ENOSYS) {
-- 
1.7.9.5

[PATCH 2/4] common: Accept pause character in the dialing number

[Philippe Nunes]

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

---
 src/common.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/common.c b/src/common.c
index 94d70dd..de1ad26 100644
--- a/src/common.c
+++ b/src/common.c
@@ -257,7 +257,7 @@ gboolean valid_number_format(const char *number, int length)
 		if (number[i] >= '0' && number[i] <= '9')
 			continue;
 
-		if (number[i] == '*' || number[i] == '#')
+		if (number[i] == '*' || number[i] == '#' || number[i] == 'p')
 			continue;
 
 		return FALSE;
-- 
1.7.9.5

Re: [PATCH 2/4] common: Accept pause character in the dialing number

[Denis Kenzior]

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

Hi Philippe,

On 09/05/2012 12:19 PM, Philippe Nunes wrote:
> ---
>   src/common.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/common.c b/src/common.c
> index 94d70dd..de1ad26 100644
> --- a/src/common.c
> +++ b/src/common.c
> @@ -257,7 +257,7 @@ gboolean valid_number_format(const char *number, int length)
>   		if (number[i]>= '0'&&  number[i]<= '9')
>   			continue;
>
> -		if (number[i] == '*' || number[i] == '#')
> +		if (number[i] == '*' || number[i] == '#' || number[i] == 'p')
>   			continue;
>
>   		return FALSE;

You can't really mess directly with this function since it is used in 
places where pause characters are not appropriate, e.g. SMS, Call 
Forwarding, Deflection, numbers stored on the SIM, etc.

Also, we can't simply accept the pause character in the voicecall atom 
today.  That would require us to implement proper dial string support as 
sending pause characters directly is not really supported on many modems.

Regards,
-Denis

[PATCH 3/4] voicecall: Accept long phone number format for STK

[Philippe Nunes]

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

---
 src/voicecall.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 12e5d4e..45dfac2 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -3675,7 +3675,7 @@ int __ofono_voicecall_dial(struct ofono_voicecall *vc,
 {
 	struct dial_request *req;
 
-	if (!valid_phone_number_format(addr))
+	if (!valid_long_phone_number_format(addr))
 		return -EINVAL;
 
 	if (vc->driver->dial == NULL)
-- 
1.7.9.5

Re: [PATCH 3/4] voicecall: Accept long phone number format for STK

[Denis Kenzior]

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

Hi Philippe,

On 09/05/2012 12:19 PM, Philippe Nunes wrote:
> ---
>   src/voicecall.c |    2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>

Patch has been applied, thanks.

Regards,
-Denis

[PATCH 4/4] stk: AID shall not be displayed in case of self explanatory icon

[Philippe Nunes]

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

---
 src/stk.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/stk.c b/src/stk.c
index 5c79183..e242bf5 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -539,6 +539,11 @@ static gboolean stk_alpha_id_set(struct ofono_stk *stk,
 	if (stk->current_agent == NULL)
 		return FALSE;
 
+	/* AID shall not be displayed in case of self explanatory icon */
+	if (icon->id != 0 && icon->qualifier ==
+			STK_ICON_QUALIFIER_TYPE_SELF_EXPLANATORY)
+		alpha[0]='\0';
+
 	if (stk->respond_on_exit)
 		stk_agent_display_action(stk->current_agent, alpha, icon,
 						user_termination_cb, stk, NULL);
-- 
1.7.9.5

Re: [PATCH 4/4] stk: AID shall not be displayed in case of self explanatory icon

[Denis Kenzior]

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

Hi Philippe,

On 09/05/2012 12:19 PM, Philippe Nunes wrote:
> ---
>   src/stk.c |    5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/src/stk.c b/src/stk.c
> index 5c79183..e242bf5 100644
> --- a/src/stk.c
> +++ b/src/stk.c
> @@ -539,6 +539,11 @@ static gboolean stk_alpha_id_set(struct ofono_stk *stk,
>   	if (stk->current_agent == NULL)
>   		return FALSE;
>
> +	/* AID shall not be displayed in case of self explanatory icon */

Can you quote the relevant sentence exactly? I assume this is TS 102.223 
Section 8.31?

> +	if (icon->id != 0&&  icon->qualifier ==
> +			STK_ICON_QUALIFIER_TYPE_SELF_EXPLANATORY)
> +		alpha[0]='\0';
> +
>   	if (stk->respond_on_exit)
>   		stk_agent_display_action(stk->current_agent, alpha, icon,
>   						user_termination_cb, stk, NULL);

Regards,
-Denis