Skip to contentSkip to conversation

Motorola Droid 4 voice: integrate into atmodem or completely separate?

RFC Applied Pavel Machek 2 participants 7 messages Aug 11, 2019 – Aug 19, 2019

lore
Share & tools
lore
#1 lore raw
In reply to Pavel Machek

Hi Pavel,

Quoted 7 lines · Pavel Machek

Hi!

Droid 4 has ... something similar to AT commands, but not quite. For voice calls, I modified atmodem... but I guess there are so many changes that creating separate motorolamodem/voicecall.c might be an option? (send_clcc() changes make sense as a cleanup, and were sent in separate mail.)

Since it looks like the Modem does actually provide some rudimentary unsolicited call state notifications via CIEV, you're better off just writing a custom driver. CLCC polling logic is actually quite complex as it is, so adding vendor branching to it just makes things worse. Also the driver is likely to be much simpler if you can rely on unsolicited call state reporting events.

The send_clcc change does sort of make sense separately.

Regards,
-Denis

Pavel Machek

author
#0 lore raw

RFC 1/1 Motorola Droid 4 voice: integrate into atmodem or completely separate?

Hi!

Droid 4 has ... something similar to AT commands, but not quite. For voice calls, I modified atmodem... but I guess there are so many changes that creating separate motorolamodem/voicecall.c might be an option? (send_clcc() changes make sense as a cleanup, and were sent in separate mail.)

Any ideas?

Best regards,
Pavel

drivers/atmodem/voicecall.c +103 -26
index d55cf00..e22d412 100644
@@ -1,4 +1,4 @@
-/*
+/* -*- linux-c -*-
  *
  *  oFono - Open Source Telephony
  *

@@ -264,14 +264,18 @@ poll_again:
 						poll_clcc, vc);
 }
 
+static void send_clcc(struct voicecall_data *vd, struct ofono_voicecall *vc)
+{
+	if (vd->vendor != OFONO_VENDOR_MOTMDM)
+	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix, clcc_poll_cb, vc, NULL);
+}
+
 static gboolean poll_clcc(gpointer user_data)
 {
 	struct ofono_voicecall *vc = user_data;
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-				clcc_poll_cb, vc, NULL);
-
+	send_clcc(vd, vc);
 	vd->clcc_source = 0;
 
 	return FALSE;

@@ -297,8 +301,7 @@ static void generic_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		}
 	}
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, req->vc, NULL);
+	send_clcc(vd, req->vc);
 
 	/* We have to callback after we schedule a poll if required */
 	req->cb(&error, req->data);

@@ -316,8 +319,7 @@ static void release_id_cb(gboolean ok, GAtResult *result,
 	if (ok)
 		vd->local_release = 1 << req->id;
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, req->vc, NULL);
+	send_clcc(vd, req->vc);
 
 	/* We have to callback after we schedule a poll if required */
 	req->cb(&error, req->data);

@@ -408,16 +410,17 @@ static void at_dial(struct ofono_voicecall *vc,
 
 	switch (clir) {
 	case OFONO_CLIR_OPTION_INVOCATION:
-		strcat(buf, "I");
+		strcat(buf, (vd->vendor != OFONO_VENDOR_MOTMDM) ? "I" : ",0");
 		break;
 	case OFONO_CLIR_OPTION_SUPPRESSION:
-		strcat(buf, "i");
+		strcat(buf, (vd->vendor != OFONO_VENDOR_MOTMDM) ? "i" : ",1");
 		break;
 	default:
 		break;
 	}
 
-	strcat(buf, ";");
+	if (vd->vendor != OFONO_VENDOR_MOTMDM)
+		strcat(buf, ";");
 
 	if (g_at_chat_send(vd->chat, buf, atd_prefix,
 				atd_cb, cbd, g_free) > 0)

@@ -462,8 +465,13 @@ static void at_answer(struct ofono_voicecall *vc,
 static void at_hangup(struct ofono_voicecall *vc,
 			ofono_voicecall_cb_t cb, void *data)
 {
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+
 	/* Hangup active call */
-	at_template("AT+CHUP", vc, generic_cb, 0x3f, cb, data);
+	if (vd->vendor != OFONO_VENDOR_MOTMDM)
+		at_template("AT+CHUP", vc, generic_cb, 0x3f, cb, data);
+	else
+		at_template("ATH", vc, generic_cb, 0x3f, cb, data);
 }
 
 static void clcc_cb(gboolean ok, GAtResult *result, gpointer user_data)

@@ -745,6 +753,8 @@ static void clip_notify(GAtResult *result, gpointer user_data)
 	GSList *l;
 	struct ofono_call *call;
 
+	printf("got clip, searching for incoming calls\n");
+
 	l = g_slist_find_custom(vd->calls,
 				GINT_TO_POINTER(CALL_STATUS_INCOMING),
 				at_util_call_compare_by_status);

@@ -759,7 +769,10 @@ static void clip_notify(GAtResult *result, gpointer user_data)
 
 	g_at_result_iter_init(&iter, result);
 
-	if (!g_at_result_iter_next(&iter, "+CLIP:"))
+	printf("Got clip...\n");
+
+	if (/* !g_at_result_iter_next(&iter, "+CLIP:") && */
+	    !g_at_result_iter_next(&iter, "~+CLIP="))
 		return;
 
 	if (!g_at_result_iter_next_string(&iter, &num))

@@ -962,8 +975,7 @@ static void no_carrier_notify(GAtResult *result, gpointer user_data)
 	struct ofono_voicecall *vc = user_data;
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, vc, NULL);
+	send_clcc(vd, vc);
 }
 
 static void no_answer_notify(GAtResult *result, gpointer user_data)

@@ -971,8 +983,7 @@ static void no_answer_notify(GAtResult *result, gpointer user_data)
 	struct ofono_voicecall *vc = user_data;
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, vc, NULL);
+	send_clcc(vd, vc);
 }
 
 static void busy_notify(GAtResult *result, gpointer user_data)

@@ -984,8 +995,7 @@ static void busy_notify(GAtResult *result, gpointer user_data)
 	 * or UDUB on the other side
 	 * TODO: Handle UDUB or other conditions somehow
 	 */
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, vc, NULL);
+	send_clcc(vd, vc);
 }
 
 static void cssi_notify(GAtResult *result, gpointer user_data)

@@ -1063,6 +1073,61 @@ static void vtd_query_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		vd->tone_duration = duration * 100;
 }
 
+
+static void ciev_notify(GAtResult *result, gpointer user_data)
+{
+  	struct ofono_voicecall *vc = user_data;
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	int strength, ind;
+	GAtResultIter iter;
+	struct ofono_call *call;
+	enum ofono_disconnect_reason reason;
+
+	g_at_result_iter_init(&iter, result);
+
+	printf("Got ciev...\n");
+	if (!g_at_result_iter_next(&iter, "~+CIEV="))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &ind))
+		return;
+
+	if (ind != 1)
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &strength))
+		return;
+
+	printf("Got ciev 1,%d...: \n", strength);
+
+	switch (strength) {
+	case 7: /* outgoing call starts */
+		printf("Outgoing notification, but ATD should have created it for us\n");
+		break;
+	case 4: /* call incoming ringing */
+		printf("Call ringing\n");
+		call = create_call(vc, 9, 1, CALL_STATUS_INCOMING, NULL, 128, 2);
+		if (call == NULL) {
+			ofono_error("Couldn't create call, call management is fubar!");
+			return;
+		}
+		call->type = 0;
+		vd->flags = FLAG_NEED_CLIP;
+		/* FIXME: we should really do that at +CLIP callback .. when that works */
+		//ofono_voicecall_notify(vc, call);
+		break;
+	case 0: /* call ends */
+		call = vd->calls->data;
+	  
+		reason = OFONO_DISCONNECT_REASON_REMOTE_HANGUP;
+		if (!call->type)
+			ofono_voicecall_disconnected(vc, call->id, reason, NULL);
+
+		printf("Call ends\n"); break;
+	}	   
+}
+
+
 static void at_voicecall_initialized(gboolean ok, GAtResult *result,
 					gpointer user_data)
 {

@@ -1074,6 +1139,9 @@ static void at_voicecall_initialized(gboolean ok, GAtResult *result,
 	g_at_chat_register(vd->chat, "RING", ring_notify, FALSE, vc, NULL);
 	g_at_chat_register(vd->chat, "+CRING:", cring_notify, FALSE, vc, NULL);
 	g_at_chat_register(vd->chat, "+CLIP:", clip_notify, FALSE, vc, NULL);
+	g_at_chat_register(vd->chat, "~+CLIP=", clip_notify, FALSE, vc, NULL);
+	g_at_chat_register(vd->chat, "~+CIEV=", ciev_notify, FALSE, vc, NULL);
+	
 	g_at_chat_register(vd->chat, "+CDIP:", cdip_notify, FALSE, vc, NULL);
 	g_at_chat_register(vd->chat, "+CNAP:", cnap_notify, FALSE, vc, NULL);
 	g_at_chat_register(vd->chat, "+CCWA:", ccwa_notify, FALSE, vc, NULL);

@@ -1093,7 +1161,8 @@ static void at_voicecall_initialized(gboolean ok, GAtResult *result,
 	ofono_voicecall_register(vc);
 
 	/* Populate the call list */
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix, clcc_cb, vc, NULL);
+	if (vd->vendor != OFONO_VENDOR_MOTMDM)
+		g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix, clcc_cb, vc, NULL);
 }
 
 static int at_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,

@@ -1112,12 +1181,18 @@ static int at_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
 
 	ofono_voicecall_set_data(vc, vd);
 
-	g_at_chat_send(vd->chat, "AT+CRC=1", NULL, NULL, NULL, NULL);
+	if (vd->vendor != OFONO_VENDOR_MOTMDM) {
+		g_at_chat_send(vd->chat, "AT+CRC=1", NULL, NULL, NULL, NULL);
+	}
 	g_at_chat_send(vd->chat, "AT+CLIP=1", NULL, NULL, NULL, NULL);
-	g_at_chat_send(vd->chat, "AT+CDIP=1", NULL, NULL, NULL, NULL);
-	g_at_chat_send(vd->chat, "AT+CNAP=1", NULL, NULL, NULL, NULL);
+	if (vd->vendor != OFONO_VENDOR_MOTMDM) {
+		g_at_chat_send(vd->chat, "AT+CDIP=1", NULL, NULL, NULL, NULL);
+		g_at_chat_send(vd->chat, "AT+CNAP=1", NULL, NULL, NULL, NULL);
+	}
 
 	switch (vd->vendor) {
+	case OFONO_VENDOR_MOTMDM:
+		break;
 	case OFONO_VENDOR_QUALCOMM_MSM:
 	case OFONO_VENDOR_SIMCOM:
 		g_at_chat_send(vd->chat, "AT+COLP=0", NULL, NULL, NULL, NULL);

@@ -1127,9 +1202,11 @@ static int at_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor,
 		break;
 	}
 
-	g_at_chat_send(vd->chat, "AT+CSSN=1,1", NULL, NULL, NULL, NULL);
-	g_at_chat_send(vd->chat, "AT+VTD?", NULL,
-				vtd_query_cb, vc, NULL);
+	if (vd->vendor != OFONO_VENDOR_MOTMDM) {
+		g_at_chat_send(vd->chat, "AT+CSSN=1,1", NULL, NULL, NULL, NULL);
+		g_at_chat_send(vd->chat, "AT+VTD?", NULL,
+			       vtd_query_cb, vc, NULL);
+	}
 	g_at_chat_send(vd->chat, "AT+CCWA=1", NULL,
 				at_voicecall_initialized, vc, NULL);
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#2 lore raw
In reply to Denis Kenzior

Hi!

Quoted 6 lines
Quoted 5 lines

Droid 4 has ... something similar to AT commands, but not quite. For voice calls, I modified atmodem... but I guess there are so many changes that creating separate motorolamodem/voicecall.c might be an option? (send_clcc() changes make sense as a cleanup, and were sent in separate mail.)

Quoted 6 lines

Since it looks like the Modem does actually provide some rudimentary unsolicited call state notifications via CIEV, you're better off just writing a custom driver. CLCC polling logic is actually quite complex as it is, so adding vendor branching to it just makes things worse. Also the driver is likely to be much simpler if you can rely on unsolicited call state reporting events.

Thanks for feedback. Done, but there are more cleanups possible.

Quoted 1 line

The send_clcc change does sort of make sense separately.

Yes, you should have it separate in your inbox ;-). If not, let me know and I'll resend.

Best regards,
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#3 lore raw

Hi Pavel,

Quoted 4 lines
Quoted 1 line

The send_clcc change does sort of make sense separately.

Yes, you should have it separate in your inbox ;-). If not, let me know and I'll resend.

Doesn't look like I do?

Regards,
-Denis

author
#4 lore raw

atmodem: introduce send_clcc() to reduce code duplication

Reduce code duplication by introducing send_clcc function.

---

Quoted 4 lines
Quoted 2 lines

Yes, you should have it separate in your inbox ;-). If not, let me know and I'll resend.

Doesn't look like I do?

sorry about that. Here you go...

Best regards,
Pavel

drivers/atmodem/voicecall.c +11 -13
index d55cf008..e22d412c 100644
@@ -264,14 +264,17 @@ poll_again:
 						poll_clcc, vc);
 }
 
+static void send_clcc(struct voicecall_data *vd, struct ofono_voicecall *vc)
+{
+	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix, clcc_poll_cb, vc, NULL);
+}
+
 static gboolean poll_clcc(gpointer user_data)
 {
 	struct ofono_voicecall *vc = user_data;
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-				clcc_poll_cb, vc, NULL);
-
+	send_clcc(vd, vc);
 	vd->clcc_source = 0;
 
 	return FALSE;

@@ -297,8 +301,7 @@ static void generic_cb(gboolean ok, GAtResult *result, gpointer user_data)
 		}
 	}
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, req->vc, NULL);
+	send_clcc(vd, req->vc);
 
 	/* We have to callback after we schedule a poll if required */
 	req->cb(&error, req->data);

@@ -316,8 +319,7 @@ static void release_id_cb(gboolean ok, GAtResult *result,
 	if (ok)
 		vd->local_release = 1 << req->id;
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, req->vc, NULL);
+	send_clcc(vd, req->vc);
 
 	/* We have to callback after we schedule a poll if required */
 	req->cb(&error, req->data);

@@ -962,8 +975,7 @@ static void no_carrier_notify(GAtResult *result, gpointer user_data)
 	struct ofono_voicecall *vc = user_data;
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, vc, NULL);
+	send_clcc(vd, vc);
 }
 
 static void no_answer_notify(GAtResult *result, gpointer user_data)

@@ -971,8 +983,7 @@ static void no_answer_notify(GAtResult *result, gpointer user_data)
 	struct ofono_voicecall *vc = user_data;
 	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
 
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, vc, NULL);
+	send_clcc(vd, vc);
 }
 
 static void busy_notify(GAtResult *result, gpointer user_data)

@@ -984,8 +995,7 @@ static void busy_notify(GAtResult *result, gpointer user_data)
 	 * or UDUB on the other side
 	 * TODO: Handle UDUB or other conditions somehow
 	 */
-	g_at_chat_send(vd->chat, "AT+CLCC", clcc_prefix,
-			clcc_poll_cb, vc, NULL);
+	send_clcc(vd, vc);
 }
 
 static void cssi_notify(GAtResult *result, gpointer user_data)
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#6 lore raw
In reply to Denis Kenzior

Hi!

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

The send_clcc change does sort of make sense separately.

Yes, you should have it separate in your inbox ;-). If not, let me know and I'll resend.

Doesn't look like I do?

You should have now. :-).

I pushed updated sources to
https://github.com/pavelmachek/ofono/pull/new/m-v1.29-1 .

Now I'm fighting with network registration, and there's problem I don't know how to solve nicely.

Regular AT commands behave like this:

AT+CREG?
+CREG: 0,0

OK

Why Droid4 reacts with single line (and no OK)

+CREG=0,0,0,0,0,0,0,0,0,0,,0

I'm using

g_at_chat_add_terminator(chat, "+CREG=", 6, TRUE );

so I get my callbacks called, but I can't really get that to work.

Is there a better way?

Best regards,

Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Applied
#5 lore raw

Re: atmodem: introduce send_clcc() to reduce code duplication

In reply to Pavel Machek
Quoted 3 lines · Pavel Machek

Reduce code duplication by introducing send_clcc function.

Applied, thanks.

Regards,
-Denis