[PATCH 0/3] Long dial string support (2nd)

[PATCH 0/3] Long dial string support (2nd)

[Andras Domokos]

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

Dial strings can now be as long as 120 digits (not counting '+'). This feature
is going to be needed by the FDN feature.

An FDN may consist of a phone number plus a DTMF string. In order to have the
call succeed when FDN is enabled, both parts need to be provided when making a
call, as a single dial string.

For AT modems the long dial string can be and has to be passed as is to the
modem, for ISI modems the dial string needs to be split into phone number (dest.
address sub block) and DTMF tone (post dest. address sub block) before feeding
it to the modem.

Andras Domokos (3):
  common: add long dial string support
  voicecall: add long dial string support
  isimodem/voicecall: add long dial string support

 drivers/isimodem/voicecall.c |   83 +++++++++++++++++++++++++++++++++---------
 include/types.h              |    2 +-
 src/common.c                 |   38 +++++++++++++++++---
 src/common.h                 |    1 +
 src/voicecall.c              |    6 ++--
 5 files changed, 103 insertions(+), 27 deletions(-)

[RFC PATCH 1/3] common: add long dial string support

[Andras Domokos]

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

---
 include/types.h |    2 +-
 src/common.c    |   38 +++++++++++++++++++++++++++++++++-----
 src/common.h    |    1 +
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/include/types.h b/include/types.h
index ba2481f..b3d2247 100644
--- a/include/types.h
+++ b/include/types.h
@@ -76,7 +76,7 @@ struct ofono_error {
 	int error;
 };
 
-#define OFONO_MAX_PHONE_NUMBER_LENGTH 20
+#define OFONO_MAX_PHONE_NUMBER_LENGTH 120
 
 struct ofono_phone_number {
 	char number[OFONO_MAX_PHONE_NUMBER_LENGTH + 1];
diff --git a/src/common.c b/src/common.c
index b5b9a6f..56a6aa8 100644
--- a/src/common.c
+++ b/src/common.c
@@ -262,6 +262,34 @@ gboolean valid_phone_number_format(const char *number)
 	return TRUE;
 }
 
+gboolean valid_dial_string(const char *number)
+{
+	int len = strlen(number);
+	int begin = 0;
+	int i;
+
+	if (!len)
+		return FALSE;
+
+	if (number[0] == '+')
+		begin = 1;
+
+	if ((len - begin) > OFONO_MAX_PHONE_NUMBER_LENGTH)
+		return FALSE;
+
+	for (i = begin; i < len; i++) {
+		if (number[i] >= '0' && number[i] <= '9')
+			continue;
+
+		if (number[i] == '*' || number[i] == '#' || number[i] == 'p')
+			continue;
+
+		return FALSE;
+	}
+
+	return TRUE;
+}
+
 const char *telephony_error_to_str(const struct ofono_error *error)
 {
 	struct error_entry *e;
@@ -379,18 +407,18 @@ int mmi_service_code_to_bearer_class(int code)
 
 const char *phone_number_to_string(const struct ofono_phone_number *ph)
 {
-	static char buffer[64];
+	static char buffer[(OFONO_MAX_PHONE_NUMBER_LENGTH + 1) + 1];
 
 	if (ph->type == 145 && (strlen(ph->number) > 0) &&
 			ph->number[0] != '+') {
 		buffer[0] = '+';
-		strncpy(buffer + 1, ph->number, 62);
-		buffer[63] = '\0';
+		strncpy(buffer + 1, ph->number, sizeof(buffer) - 2);
 	} else {
-		strncpy(buffer, ph->number, 63);
-		buffer[63] = '\0';
+		strncpy(buffer, ph->number, sizeof(buffer) - 1);
 	}
 
+	buffer[sizeof(buffer) - 1] = '\0';
+
 	return buffer;
 }
 
diff --git a/src/common.h b/src/common.h
index 8b5798a..c7bfe26 100644
--- a/src/common.h
+++ b/src/common.h
@@ -125,6 +125,7 @@ enum pin_type {
 const char *telephony_error_to_str(const struct ofono_error *error);
 
 gboolean valid_phone_number_format(const char *number);
+gboolean valid_dial_string(const char *number);
 const char *phone_number_to_string(const struct ofono_phone_number *ph);
 void string_to_phone_number(const char *str, struct ofono_phone_number *ph);
 
-- 
1.7.0.4

[RFC PATCH 2/3] voicecall: add long dial string support

[Andras Domokos]

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

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

diff --git a/src/voicecall.c b/src/voicecall.c
index bd64432..4022f36 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -408,7 +408,7 @@ static DBusMessage *voicecall_deflect(DBusConnection *conn,
 					DBUS_TYPE_INVALID) == FALSE)
 		return __ofono_error_invalid_args(msg);
 
-	if (!valid_phone_number_format(number))
+	if (!valid_dial_string(number))
 		return __ofono_error_invalid_format(msg);
 
 	vc->pending = dbus_message_ref(msg);
@@ -1172,7 +1172,7 @@ static DBusMessage *manager_dial(DBusConnection *conn,
 					DBUS_TYPE_INVALID) == FALSE)
 		return __ofono_error_invalid_args(msg);
 
-	if (!valid_phone_number_format(number))
+	if (!valid_dial_string(number))
 		return __ofono_error_invalid_format(msg);
 
 	if (clir_string_to_clir(clirstr, &clir) == FALSE)
@@ -2360,7 +2360,7 @@ int __ofono_voicecall_dial(struct ofono_voicecall *vc,
 {
 	struct dial_request *req;
 
-	if (!valid_phone_number_format(addr))
+	if (!valid_dial_string(addr))
 		return -EINVAL;
 
 	if (!vc->driver->dial)
-- 
1.7.0.4

[RFC PATCH 3/3] isimodem/voicecall: add long dial string support

[Andras Domokos]

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

---
 drivers/isimodem/voicecall.c |   83 +++++++++++++++++++++++++++++++++---------
 1 files changed, 65 insertions(+), 18 deletions(-)

diff --git a/drivers/isimodem/voicecall.c b/drivers/isimodem/voicecall.c
index c3365f6..b0dad72 100644
--- a/drivers/isimodem/voicecall.c
+++ b/drivers/isimodem/voicecall.c
@@ -49,7 +49,7 @@ struct isi_call {
 	uint8_t id, call_id, status, mode, mode_info, cause_type, cause;
 	uint8_t addr_type, presentation;
 	uint8_t reason;
-	char address[20], addr_pad[4];
+	char address[OFONO_MAX_PHONE_NUMBER_LENGTH + 1], addr_pad[4];
 };
 
 struct isi_voicecall {
@@ -244,6 +244,28 @@ static void isi_call_destination_address_sb_proc(struct isi_voicecall *ivc,
 		isi_call_any_address_sb_proc(ivc, call, sb);
 }
 
+static void isi_call_destination_post_address_sb_proc(struct isi_voicecall *ivc,
+						struct isi_call *call,
+						GIsiSubBlockIter const *sb)
+{
+	uint8_t addr_info, addr_len;
+	char *address;
+
+	if (!call->address[0] ||
+		strlen(call->address) >= OFONO_MAX_PHONE_NUMBER_LENGTH)
+		return;
+
+	if (!g_isi_sb_iter_get_byte(sb, &addr_info, 2) ||
+		!g_isi_sb_iter_get_byte(sb, &addr_len, 3) ||
+		!g_isi_sb_iter_get_alpha_tag(sb, &address, 2 * addr_len, 4))
+		return;
+
+	strncat(call->address, address,
+		OFONO_MAX_PHONE_NUMBER_LENGTH - strlen(call->address));
+	g_free(address);
+
+}
+
 static void isi_call_mode_sb_proc(struct isi_voicecall *ivc,
 					struct isi_call *call,
 					GIsiSubBlockIter const *sb)
@@ -371,40 +393,61 @@ static struct isi_call_req_context *
 isi_call_create_req(struct ofono_voicecall *ovc,
 			uint8_t presentation,
 			uint8_t addr_type,
-			char const address[21],
+			char const address[OFONO_MAX_PHONE_NUMBER_LENGTH + 1],
 			ofono_voicecall_cb_t cb,
 			void *data)
 {
-	size_t addr_len = strlen(address);
-	size_t sub_len = (6 + 2 * addr_len + 3) & ~3;
-	size_t i, offset = 3 + 4 + 8 + 6;
-	uint8_t req[3 + 4 + 8 + 6 + 40] = {
+	uint8_t req[3 + 4 + 8 + 10 + 2 * OFONO_MAX_PHONE_NUMBER_LENGTH + 4] = {
 		CALL_CREATE_REQ,
 		0,		/* No id */
-		3,		/* Mode, Clir, Number */
+		4,		/* Mode, Clir, Number, Postfix */
 		/* MODE SB */
 		CALL_MODE, 4, CALL_MODE_SPEECH, CALL_MODE_INFO_NONE,
 		/* ORIGIN_INFO SB */
 		CALL_ORIGIN_INFO, 8, presentation, 0, 0, 0, 0, 0,
 		/* DESTINATION_ADDRESS SB */
 		CALL_DESTINATION_ADDRESS,
-		sub_len,
+		0,		/* sub_len */
 		addr_type & 0x7F,
 		0, 0,
-		addr_len,
-		/* uint16_t addr[20] */
+		0,		/* addr_len */
+		/* uint16_t addr[OFONO_MAX_PHONE_NUMBER_LENGTH] */
 	};
-	size_t rlen = 3 + 4 + 8 + sub_len;
+	size_t i, j, offset;
 
-	if (addr_len > 20) {
-		CALLBACK_WITH_FAILURE(cb, data);
-		return NULL;
+	offset = 3 + 4 + 8;
+	for (i = 0, j = 0; address[i]; i++, j++) {
+		if (i > OFONO_MAX_PHONE_NUMBER_LENGTH)
+			goto out;
+		if (address[i] != '*')
+			break;
+		req[offset + 6 + 2 * j + 1] = address[i];
 	}
 
-	for (i = 0; i < addr_len; i++)
-		req[offset + 2 * i + 1] = address[i];
+	for (; address[i]; i++, j++) {
+		if (i > OFONO_MAX_PHONE_NUMBER_LENGTH)
+			goto out;
+		if (address[i] == '*' || address[i] == 'p' || address[i] == '#')
+			break;
+		req[offset + 6 + 2 * j + 1] = address[i];
+	}
+	req[offset + 5] = j;
+	offset += (req[offset + 1] = (6 + 2 * j + 3) & ~3);
+
+	req[offset + 0] = CALL_DESTINATION_POST_ADDRESS;
+	for (j = 0; address[i]; i++, j++) {
+		if (i > OFONO_MAX_PHONE_NUMBER_LENGTH)
+			goto out;
+		req[offset + 4 + 2 * j + 1] = address[i];
+	}
+	req[offset + 3] = j;
+	offset += (req[offset + 1] = (4 + 2 * j + 3) & ~3);
 
-	return isi_call_req(ovc, req, rlen, isi_call_create_resp, cb, data);
+	return isi_call_req(ovc, req, offset, isi_call_create_resp, cb, data);
+
+out:
+	CALLBACK_WITH_FAILURE(cb, data);
+	return NULL;
 }
 
 static gboolean isi_call_create_resp(GIsiClient *client,
@@ -477,13 +520,17 @@ static void isi_call_status_ind_cb(GIsiClient *client,
 			isi_call_destination_address_sb_proc(ivc, call, sb);
 			break;
 
+		case CALL_DESTINATION_POST_ADDRESS:
+			isi_call_destination_post_address_sb_proc(ivc,
+								call, sb);
+			break;
+
 		case CALL_ORIGIN_ADDRESS:
 			isi_call_origin_address_sb_proc(ivc, call, sb);
 			break;
 
 		case CALL_GSM_DETAILED_CAUSE:
 		case CALL_DESTINATION_PRE_ADDRESS:
-		case CALL_DESTINATION_POST_ADDRESS:
 		case CALL_DESTINATION_SUBADDRESS:
 		case CALL_GSM_EVENT_INFO:
 		case CALL_NW_CAUSE:
-- 
1.7.0.4

RE: [PATCH 0/3] Long dial string support (2nd)

[Rajesh.Nagaiah]

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

Hi Andras, 

> Dial strings can now be as long as 120 digits (not counting 
> '+'). This feature is going to be needed by the FDN feature.
> 
> An FDN may consist of a phone number plus a DTMF string. In 
> order to have the call succeed when FDN is enabled, both 
> parts need to be provided when making a call, as a single dial string.
> 
> For AT modems the long dial string can be and has to be 
> passed as is to the modem, for ISI modems the dial string 
> needs to be split into phone number (dest.
> address sub block) and DTMF tone (post dest. address sub 
> block) before feeding it to the modem.

Sending the DTMF tone in post dest. address sub block in case of 
the ISI modem or sending as part of the dial string in case of 
AT modems, will the modem take care of DTMF postfix handling by 
itself or its only going to use that for FDN check ?

If its going to be earlier case, then not all modems support 
this feature and also sending the postfix DTMF tone automatically 
by the modem without notifying to the AP, then the user neither 
wont be aware of ongoing DTMF tone sending as there will be no 
local playback of the tone nor will have any control over the 
DTMF tones sending.

So we should parse the dial string and extract the DTMF postfix 
(after the FDN check, anyways we are not suppoting FDN in oFono). 
Once the call gets connected, then the postfix DTMF tones can be 
sent out and signaled to the application about the start and stop
events of each tone, so the application can generate the 
corresponding DTMF tones using the tonegenerator for local 
playback.

Also currently the "w"/"W" stop character handling is not there, 
so I am not sure if its intentionally left out or missing ?

BR,
Rajesh

Re: [PATCH 0/3] Long dial string support (2nd)

[Andras Domokos]

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

Hi Rajesh,

On 11/22/2010 10:58 PM, ext Rajesh.Nagaiah(a)elektrobit.com wrote:
> Hi Andras,
>
>    
>> Dial strings can now be as long as 120 digits (not counting
>> '+'). This feature is going to be needed by the FDN feature.
>>
>> An FDN may consist of a phone number plus a DTMF string. In
>> order to have the call succeed when FDN is enabled, both
>> parts need to be provided when making a call, as a single dial string.
>>
>> For AT modems the long dial string can be and has to be
>> passed as is to the modem, for ISI modems the dial string
>> needs to be split into phone number (dest.
>> address sub block) and DTMF tone (post dest. address sub
>> block) before feeding it to the modem.
>>      
> Sending the DTMF tone in post dest. address sub block in case of
> the ISI modem or sending as part of the dial string in case of
> AT modems, will the modem take care of DTMF postfix handling by
> itself or its only going to use that for FDN check ?
>
> If its going to be earlier case, then not all modems support
> this feature and also sending the postfix DTMF tone automatically
> by the modem without notifying to the AP, then the user neither
> wont be aware of ongoing DTMF tone sending as there will be no
> local playback of the tone nor will have any control over the
> DTMF tones sending.
>
> So we should parse the dial string and extract the DTMF postfix
> (after the FDN check, anyways we are not suppoting FDN in oFono).
> Once the call gets connected, then the postfix DTMF tones can be
> sent out and signaled to the application about the start and stop
> events of each tone, so the application can generate the
> corresponding DTMF tones using the tonegenerator for local
> playback.
>
> Also currently the "w"/"W" stop character handling is not there,
> so I am not sure if its intentionally left out or missing ?
>    

I think the situation has been clarified based on the
discussion with Denis. I stop pushing these patches.
> BR,
> Rajesh
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> http://lists.ofono.org/listinfo/ofono
>    

Regards,
Andras

Re: [PATCH 0/3] Long dial string support (2nd)

[Denis Kenzior]

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

On 11/22/2010 10:36 AM, Andras Domokos wrote:
> Dial strings can now be as long as 120 digits (not counting '+'). This feature
> is going to be needed by the FDN feature.
> 
> An FDN may consist of a phone number plus a DTMF string. In order to have the
> call succeed when FDN is enabled, both parts need to be provided when making a
> call, as a single dial string.

So as mentioned previously, oFono does not support FDN.  So why are we
doing this again?

> 
> For AT modems the long dial string can be and has to be passed as is to the

Most AT modems have serious limitations on how many characters are
supported in the dial string.  Not to mention that the pause character
doesn't really always work.

I can understand supporting long dialing numbers, but supporting dial
strings is something else entirely...

Regards,
-Denis

Re: [PATCH 0/3] Long dial string support (2nd)

[Andras Domokos]

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

On 11/23/2010 07:41 PM, ext Denis Kenzior wrote:
> On 11/22/2010 10:36 AM, Andras Domokos wrote:
>    
>> Dial strings can now be as long as 120 digits (not counting '+'). This feature
>> is going to be needed by the FDN feature.
>>
>> An FDN may consist of a phone number plus a DTMF string. In order to have the
>> call succeed when FDN is enabled, both parts need to be provided when making a
>> call, as a single dial string.
>>      
> So as mentioned previously, oFono does not support FDN.  So why are we
> doing this again?
>
>    
Even if we don't support FDN at this point, there is still a remaining
related issue, who we are going to dial numbers like:

12345456p1234#1111

I think the the dial string could still be passed to the voicecall driver
that will take care of the modem specific details, most importantly
playing the DTMF tones and generating the tone events that would be
propagated back to voicecall manager and to D-Bus from there.
>> For AT modems the long dial string can be and has to be passed as is to the
>>      
> Most AT modems have serious limitations on how many characters are
> supported in the dial string.  Not to mention that the pause character
> doesn't really always work.
>
> I can understand supporting long dialing numbers, but supporting dial
> strings is something else entirely...
>
> Regards,
> -Denis
>    
Regards,
Andras

Re: [PATCH 0/3] Long dial string support (2nd)

[Denis Kenzior]

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

Hi Andras,

> Even if we don't support FDN at this point, there is still a remaining
> related issue, who we are going to dial numbers like:
> 
> 12345456p1234#1111
> 
> I think the the dial string could still be passed to the voicecall driver
> that will take care of the modem specific details, most importantly
> playing the DTMF tones and generating the tone events that would be
> propagated back to voicecall manager and to D-Bus from there.

So we looked into what it would take to implement dial strings with AT
modems.  The consensus was that passing the entire string to the driver
was a bad idea as most modems simply do not support pause characters and
have strict limitations on the number length.

The best idea we have came up with so far is to parse the string passed
to dial and separate the actual number from the dial string.  The dial
string then gets assigned to a separate property on the voice call
object (see Dial String task in the TODO).

We would then extend the call state logic to queue the dial string the
same as a DTMF once the call is active.  If you study the DTMF logic,
you will note that Andrew has recently made it into a tone queue.

Today we burst up to 8 (arbitrarily picked number) DTMF tones per driver
request.  What we could do is send a single tone at a time.  In theory
this would allow us to emit the Tone Started / Tone Stopped signals
(please see the provide feedback of sent DTMF tones task) as well.

It sounds like the ISI modems already work this way and are nicely
covered by this approach...

Regards,
-Denis

Re: [PATCH 0/3] Long dial string support (2nd)

[Andras Domokos]

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

Hi Denis,

On 11/24/2010 12:04 AM, ext Denis Kenzior wrote:
> Hi Andras,
>
>    
>> Even if we don't support FDN at this point, there is still a remaining
>> related issue, who we are going to dial numbers like:
>>
>> 12345456p1234#1111
>>
>> I think the the dial string could still be passed to the voicecall driver
>> that will take care of the modem specific details, most importantly
>> playing the DTMF tones and generating the tone events that would be
>> propagated back to voicecall manager and to D-Bus from there.
>>      
> So we looked into what it would take to implement dial strings with AT
> modems.  The consensus was that passing the entire string to the driver
> was a bad idea as most modems simply do not support pause characters and
> have strict limitations on the number length.
>
> The best idea we have came up with so far is to parse the string passed
> to dial and separate the actual number from the dial string.  The dial
> string then gets assigned to a separate property on the voice call
> object (see Dial String task in the TODO).
>
> We would then extend the call state logic to queue the dial string the
> same as a DTMF once the call is active.  If you study the DTMF logic,
> you will note that Andrew has recently made it into a tone queue.
>
> Today we burst up to 8 (arbitrarily picked number) DTMF tones per driver
> request.  What we could do is send a single tone at a time.  In theory
> this would allow us to emit the Tone Started / Tone Stopped signals
> (please see the provide feedback of sent DTMF tones task) as well.
>
> It sounds like the ISI modems already work this way and are nicely
> covered by this approach...
>    
Thanks for your clarification. I also had a thorough look at the
the DTMF tones handling implementation. Based on these infos
and assuming that we don't want to implement (full) FDN
support, these patches are not needed indeed.

> Regards,
> -Denis
>    

Regards,
Andras