Skip to contentSkip to conversation

Long dial string support (2nd)

PATCH In review Andras Domokos 3 participants 10 messages Nov 22, 2010 – Nov 24, 2010

lore
Share & tools
lore

Andras Domokos

author
#0 lore raw

cover Long dial string support (2nd)

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(-)
author
#1 lore raw

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

---
 include/types.h |    2 +-
 src/common.c    |   38 +++++++++++++++++++++++++++++++++-----
 src/common.h    |    1 +
 3 files changed, 35 insertions(+), 6 deletions(-)
include/types.h +1 -1
index ba2481f..b3d2247 100644
@@ -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];
src/common.c +33 -5
index b5b9a6f..56a6aa8 100644
@@ -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;
 }
src/common.h +1 -0
index 8b5798a..c7bfe26 100644
@@ -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);
Signature · 1 line
1.7.0.4
author
#2 lore raw

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

In reply to Andras Domokos
---
 src/voicecall.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
src/voicecall.c +3 -3
index bd64432..4022f36 100644
@@ -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)
Signature · 1 line
1.7.0.4
author
#3 lore raw

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

In reply to Andras Domokos
---
 drivers/isimodem/voicecall.c |   83 +++++++++++++++++++++++++++++++++---------
 1 files changed, 65 insertions(+), 18 deletions(-)
drivers/isimodem/voicecall.c +65 -18
index c3365f6..b0dad72 100644
@@ -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:
Signature · 1 line
1.7.0.4
#4 lore raw
In reply to Andras Domokos

Hi Andras,

Quoted 7 lines

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.

Quoted 5 lines

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

#6 lore raw
In reply to Andras Domokos
Quoted 6 lines · Andras Domokos

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?

Quoted 2 lines

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

author
#7 lore raw
Quoted 5 lines · ext Denis Kenzior
Quoted 3 lines · Andras Domokos

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

Quoted 8 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 6 lines

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

Regards,
-Denis

Regards,
Andras

#8 lore raw

Hi Andras,

Quoted 9 lines

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

author
#9 lore raw

Hi Denis,

Quoted 27 lines · ext Denis Kenzior

Hi Andras,

Quoted 10 lines

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.

Quoted 8 lines

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.

Quoted 3 lines

Regards,
-Denis

Regards,
Andras

author
#5 lore raw
In reply to [email protected]

Hi Rajesh,

Quoted 36 lines · ext Rajesh.Nagaiah(a)elektrobit.com

Hi Andras,

Quoted 7 lines

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.

Quoted 6 lines

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.

Quoted 3 lines

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.

Quoted 7 lines

BR,
Rajesh
_______________________________________________
ofono mailing list
ofono(a)ofono.org
http://lists.ofono.org/listinfo/ofono

Regards,
Andras