[PATCH 1/4] qmimodem: voicecall: Implement call dialing

[PATCH 1/4] qmimodem: voicecall: Implement call dialing

[Adam Pigg]

Add voicecall dialling to the qmimodem driver
Includes required infrastructure and setup of the QMI services

Call State Handling
===================
On initialisation, register the all_call_status_ind callback to be
called for QMI_VOICE_IND_ALL_STATUS.  This will handle notification of
call status to the rest of the system

Dial Handling
=============
The dial function sets up the parameters for the QMI_VOICE_DIAL_CALL
service.  The parameters are the number to be called and the call type,
which is currently hard coded to be QMI_VOICE_CALL_TYPE_VOICE.  The
dial_cb callback will then be called and will receive the call-id.

---
Changes in V4
-merged qmi_voice_call_status and all_call_status_ind
-several minor structure/formate changes

Changes in V5
-renames ofono_call_compare_by_id to ofono_call_match_by_id
-updated new_entry pointer to fix use-after-free in
ofono_call_list_notify
-Renamed several constants
-in all_call_status_ind removed error label/gotos after restructuring
-When creating the call list, ensure an upper bound of 16
-Fix out-of-mound-read on remote number due to QMI string not being null
terminated
---
---
 drivers/qmimodem/voice.h     |  17 ++
 drivers/qmimodem/voicecall.c | 426 ++++++++++++++++++++++++++++++++++-
 2 files changed, 442 insertions(+), 1 deletion(-)

diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h
index 917e72f7..2344fd50 100644
--- a/drivers/qmimodem/voice.h
+++ b/drivers/qmimodem/voice.h
@@ -48,6 +48,9 @@ enum qmi_ussd_user_required {
 
 /* QMI service voice. Using an enum to prevent doublicated entries */
 enum voice_commands {
+	QMI_VOICE_DIAL_CALL =			0x20,
+	QMI_VOICE_ALL_CALL_STATUS_IND =		0x2e,
+	QMI_VOICE_GET_ALL_CALL_INFO =		0x2f,
 	QMI_VOICE_SUPS_NOTIFICATION_IND =	0x32,
 	QMI_VOICE_SET_SUPS_SERVICE =		0x33,
 	QMI_VOICE_GET_CALL_WAITING =		0x34,
@@ -66,6 +69,20 @@ enum voice_commands {
 	QMI_VOICE_GET_CNAP =			0x4d
 };
 
+enum qmi_voice_call_state {
+	QMI_VOICE_CALL_STATE_IDLE = 0x0,
+	QMI_VOICE_CALL_STATE_ORIG,
+	QMI_VOICE_CALL_STATE_INCOMING,
+	QMI_VOICE_CALL_STATE_CONV,
+	QMI_VOICE_CALL_STATE_CC_IN_PROG,
+	QMI_VOICE_CALL_STATE_ALERTING,
+	QMI_VOICE_CALL_STATE_HOLD,
+	QMI_VOICE_CALL_STATE_WAITING,
+	QMI_VOICE_CALL_STATE_DISCONNECTING,
+	QMI_VOICE_CALL_STATE_END,
+	QMI_VOICE_CALL_STATE_SETUP
+};
+
 struct qmi_ussd_data {
 	uint8_t dcs;
 	uint8_t length;
diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c
index aa34fc25..7b4813c9 100644
--- a/drivers/qmimodem/voicecall.c
+++ b/drivers/qmimodem/voicecall.c
@@ -3,6 +3,7 @@
  *  oFono - Open Source Telephony
  *
  *  Copyright (C) 2011-2012  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2024 Adam Pigg <adam@piggz.co.uk>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License version 2 as
@@ -26,6 +27,10 @@
 #include <ofono/log.h>
 #include <ofono/modem.h>
 #include <ofono/voicecall.h>
+#include <src/common.h>
+#include <ell/ell.h>
+
+#include "voice.h"
 
 #include "qmi.h"
 
@@ -35,8 +40,422 @@ struct voicecall_data {
 	struct qmi_service *voice;
 	uint16_t major;
 	uint16_t minor;
+	struct l_queue *call_list;
+	struct ofono_phone_number dialed;
 };
 
+struct qmi_voice_call_information_instance {
+	uint8_t id;
+	uint8_t state;
+	uint8_t type;
+	uint8_t direction;
+	uint8_t mode;
+	uint8_t multipart_indicator;
+	uint8_t als;
+} __attribute__((__packed__));
+
+struct qmi_voice_call_information {
+	uint8_t size;
+	struct qmi_voice_call_information_instance instance[0];
+} __attribute__((__packed__));
+
+struct qmi_voice_remote_party_number_instance {
+	uint8_t call_id;
+	uint8_t presentation_indicator;
+	uint8_t number_size;
+	char number[0];
+} __attribute__((__packed__));
+
+struct qmi_voice_remote_party_number {
+	uint8_t size;
+	struct qmi_voice_remote_party_number_instance instance[0];
+} __attribute__((__packed__));
+
+static int ofono_call_compare(const void *a, const void *b, void *data)
+{
+	const struct ofono_call *ca = a;
+	const struct ofono_call *cb = b;
+
+	if (ca->id < cb->id)
+		return -1;
+
+	if (ca->id > cb->id)
+		return 1;
+
+	return 0;
+}
+
+static bool ofono_call_match_by_id(const void *a, const void *b)
+{
+	const struct ofono_call *call = a;
+	unsigned int id = L_PTR_TO_UINT(b);
+
+	return (call->id == id);
+}
+
+static void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
+						int call_id)
+{
+	struct ofono_call *call;
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct l_queue *call_list = vd->call_list;
+	const struct ofono_phone_number *ph = &vd->dialed;
+
+	/* check if call_id already present */
+	call = l_queue_find(call_list, ofono_call_match_by_id,
+				L_UINT_TO_PTR(call_id));
+
+	if (call)
+		return;
+
+	call = l_new(struct ofono_call, 1);
+	call->id = call_id;
+
+	memcpy(&call->called_number, ph, sizeof(*ph));
+	call->direction = CALL_DIRECTION_MOBILE_ORIGINATED;
+	call->status = CALL_STATUS_DIALING;
+	call->type = 0; /* voice */
+
+	l_queue_insert(call_list, call, ofono_call_compare, NULL);
+
+	ofono_voicecall_notify(vc, call);
+}
+
+static void ofono_call_list_notify(struct ofono_voicecall *vc,
+					struct l_queue *calls)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct l_queue *old_calls = vd->call_list;
+	struct l_queue *new_calls = calls;
+	struct ofono_call *new_call, *old_call;
+	const struct l_queue_entry *old_entry, *new_entry;
+	uint i;
+
+	uint loop_length =
+		MAX(l_queue_length(old_calls), l_queue_length(new_calls));
+
+	old_entry = l_queue_get_entries(old_calls);
+	new_entry = l_queue_get_entries(new_calls);
+
+	for (i = 0; i < loop_length; ++i) {
+		old_call = old_entry ? old_entry->data : NULL;
+		new_call = new_entry ? new_entry->data : NULL;
+
+		if (new_call && new_call->status == CALL_STATUS_DISCONNECTED) {
+			ofono_voicecall_disconnected(
+				vc, new_call->id,
+				OFONO_DISCONNECT_REASON_REMOTE_HANGUP, NULL);
+			new_entry = new_entry->next;
+			l_queue_remove(calls, new_call);
+			l_free(new_call);
+			continue;
+		}
+
+		if (old_call &&
+				(!new_call || (new_call->id > old_call->id)))
+			ofono_voicecall_disconnected(
+				vc, old_call->id,
+				OFONO_DISCONNECT_REASON_REMOTE_HANGUP, NULL);
+		else if (new_call &&
+				(!old_call || (new_call->id < old_call->id))) {
+			DBG("Notify new call %d", new_call->id);
+			/* new call, signal it */
+			if (new_call->type == 0)
+				ofono_voicecall_notify(vc, new_call);
+		} else if (memcmp(new_call, old_call, sizeof(*new_call)) &&
+				new_call->type == 0)
+			ofono_voicecall_notify(vc, new_call);
+
+		if (old_entry)
+			old_entry = old_entry->next;
+		if (new_entry)
+			new_entry = new_entry->next;
+	}
+
+	l_queue_destroy(old_calls, l_free);
+	vd->call_list = calls;
+}
+
+static const char *qmi_voice_call_state_name(enum qmi_voice_call_state value)
+{
+	switch (value) {
+	case QMI_VOICE_CALL_STATE_IDLE:
+		return "QMI_VOICE_CALL_STATE_IDLE";
+	case QMI_VOICE_CALL_STATE_ORIG:
+		return "QMI_VOICE_CALL_STATE_ORIG";
+	case QMI_VOICE_CALL_STATE_INCOMING:
+		return "QMI_VOICE_CALL_STATE_INCOMING";
+	case QMI_VOICE_CALL_STATE_CONV:
+		return "QMI_VOICE_CALL_STATE_CONV";
+	case QMI_VOICE_CALL_STATE_CC_IN_PROG:
+		return "QMI_VOICE_CALL_STATE_CC_IN_PROG";
+	case QMI_VOICE_CALL_STATE_ALERTING:
+		return "QMI_VOICE_CALL_STATE_ALERTING";
+	case QMI_VOICE_CALL_STATE_HOLD:
+		return "QMI_VOICE_CALL_STATE_HOLD";
+	case QMI_VOICE_CALL_STATE_WAITING:
+		return "QMI_VOICE_CALL_STATE_WAITING";
+	case QMI_VOICE_CALL_STATE_DISCONNECTING:
+		return "QMI_VOICE_CALL_STATE_DISCONNECTING";
+	case QMI_VOICE_CALL_STATE_END:
+		return "QMI_VOICE_CALL_STATE_END";
+	case QMI_VOICE_CALL_STATE_SETUP:
+		return "QMI_VOICE_CALL_STATE_SETUP";
+	}
+	return "QMI_CALL_STATE_<UNKNOWN>";
+}
+
+static bool qmi_to_ofono_status(uint8_t status, int *ret)
+{
+	int err = false;
+
+	switch (status) {
+	case QMI_VOICE_CALL_STATE_IDLE:
+	case QMI_VOICE_CALL_STATE_END:
+	case QMI_VOICE_CALL_STATE_DISCONNECTING:
+		*ret = CALL_STATUS_DISCONNECTED;
+		break;
+	case QMI_VOICE_CALL_STATE_HOLD:
+		*ret = CALL_STATUS_HELD;
+		break;
+	case QMI_VOICE_CALL_STATE_WAITING:
+		*ret = CALL_STATUS_WAITING;
+		break;
+	case QMI_VOICE_CALL_STATE_ORIG:
+		*ret = CALL_STATUS_DIALING;
+		break;
+	case QMI_VOICE_CALL_STATE_SETUP:
+	case QMI_VOICE_CALL_STATE_INCOMING:
+		*ret = CALL_STATUS_INCOMING;
+		break;
+	case QMI_VOICE_CALL_STATE_CONV:
+		*ret = CALL_STATUS_ACTIVE;
+		break;
+	case QMI_VOICE_CALL_STATE_CC_IN_PROG:
+		*ret = CALL_STATUS_DIALING;
+		break;
+	case QMI_VOICE_CALL_STATE_ALERTING:
+		*ret = CALL_STATUS_ALERTING;
+		break;
+	default:
+		err = true;
+	}
+	return err;
+}
+
+static enum call_direction qmi_to_ofono_direction(uint8_t qmi_direction)
+{
+	return qmi_direction - 1;
+}
+
+static void all_call_status_ind(struct qmi_result *result, void *user_data)
+{
+	struct ofono_voicecall *vc = user_data;
+
+	int i;
+	int offset;
+	uint16_t len;
+	bool status = true;
+	int instance_size;
+	const struct qmi_voice_call_information *call_information;
+	const struct qmi_voice_remote_party_number *remote_party_number;
+	const struct qmi_voice_remote_party_number_instance *remote_party_number_inst[16];
+
+	static const uint8_t RESULT_CALL_STATUS_CALL_INFORMATION = 0x01;
+	static const uint8_t RESULT_CALL_STATUS_REMOTE_NUMBER = 0x10;
+	static const uint8_t RESULT_CALL_INFO_CALL_INFORMATION = 0x10;
+	static const uint8_t RESULT_CALL_INFO_REMOTE_NUMBER = 0x11;
+
+	DBG("");
+
+	/* mandatory */
+	call_information = qmi_result_get(
+		result, RESULT_CALL_STATUS_CALL_INFORMATION, &len);
+
+	if (!call_information) {
+		call_information = qmi_result_get(
+			result, RESULT_CALL_INFO_CALL_INFORMATION,
+			&len);
+		status = false;
+	}
+
+	if (!call_information || len < sizeof(call_information->size)) {
+		DBG("Parsing of all call status indication failed");
+		return;
+	}
+
+	if (!call_information->size) {
+		DBG("No call informations received!");
+		return;
+	}
+
+	if (len != call_information->size *
+			sizeof(struct qmi_voice_call_information_instance) +
+			sizeof(call_information->size)) {
+		DBG("Call information size incorrect");
+		return;
+	}
+
+	/* mandatory */
+	remote_party_number = qmi_result_get(
+		result,
+		status ? RESULT_CALL_STATUS_REMOTE_NUMBER :
+			 RESULT_CALL_INFO_REMOTE_NUMBER,
+		&len);
+
+	if (!remote_party_number) {
+		DBG("Unable to retrieve remote numbers");
+		return;
+	}
+
+	/* verify the length */
+	if (len < sizeof(remote_party_number->size)) {
+		DBG("Parsing of remote numbers failed");
+		return;
+	}
+
+	/* expect we have valid fields for every call */
+	if (call_information->size != remote_party_number->size) {
+		DBG("Not all fields have the same size");
+		return;
+	}
+
+	/* pull the remote call info into a local array */
+	instance_size = sizeof(struct qmi_voice_remote_party_number_instance);
+
+	for (i = 0, offset = sizeof(remote_party_number->size);
+			offset < len && i < 16 && i < remote_party_number->size;
+			i++) {
+		const struct qmi_voice_remote_party_number_instance *instance;
+
+		if (offset + instance_size > len) {
+			DBG("Error parsing remote numbers");
+			return;
+		}
+
+		instance = (void *)remote_party_number + offset;
+		if (offset + instance_size + instance->number_size > len) {
+			DBG("Error parsing remote numbers");
+			return;
+		}
+
+		remote_party_number_inst[i] = instance;
+		offset +=
+			sizeof(struct qmi_voice_remote_party_number_instance) +
+			instance->number_size;
+	}
+
+	struct l_queue *calls = l_queue_new();
+
+	for (i = 0; i < call_information->size && i < 16; i++) {
+		struct ofono_call *call = l_new(struct ofono_call, 1);
+		struct qmi_voice_call_information_instance call_info;
+		const struct qmi_voice_remote_party_number_instance
+			*remote_party = remote_party_number_inst[i];
+		int number_size;
+
+		call_info = call_information->instance[i];
+
+		call->id = call_info.id;
+		call->direction = qmi_to_ofono_direction(call_info.direction);
+		call->type = 0; /* always voice */
+
+		number_size = MIN(remote_party->number_size, OFONO_MAX_PHONE_NUMBER_LENGTH);
+		char *tmp = l_strndup(remote_party->number, number_size);
+		l_strlcpy(call->phone_number.number, tmp, sizeof(call->phone_number.number));
+		l_free(tmp);
+
+		if (strlen(call->phone_number.number) > 0)
+			call->clip_validity = 0;
+		else
+			call->clip_validity = 2;
+
+		if (qmi_to_ofono_status(call_info.state, &call->status)) {
+			DBG("Ignore call id %d, because can not convert QMI state 0x%x to ofono.",
+				call_info.id, call_info.state);
+			l_free(call);
+			continue;
+		}
+
+		DBG("Call %d in state %s(%d)", call_info.id,
+			qmi_voice_call_state_name(call_info.state),
+			call_info.state);
+
+		l_queue_push_tail(calls, call);
+	}
+
+	ofono_call_list_notify(vc, calls);
+}
+
+static void dial_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	struct ofono_voicecall *vc = cbd->user;
+	ofono_voicecall_cb_t cb = cbd->cb;
+	uint16_t error;
+	uint8_t call_id;
+
+	static const uint8_t RESULT_CALL_ID = 0x10;
+
+	DBG("");
+
+	if (qmi_result_set_error(result, &error)) {
+		DBG("QMI Error %d", error);
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	if (!qmi_result_get_uint8(result, RESULT_CALL_ID,
+			&call_id)) {
+		ofono_error("No call id in dial result");
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	DBG("New call QMI id %d", call_id);
+	ofono_call_list_dial_callback(vc, call_id);
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+}
+
+static void dial(struct ofono_voicecall *vc,
+			const struct ofono_phone_number *ph,
+			enum ofono_clir_option clir, ofono_voicecall_cb_t cb,
+			void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	struct qmi_param *param;
+	const char *calling_number = phone_number_to_string(ph);
+
+	static const uint8_t PARAM_CALL_NUMBER = 0x01;
+	static const uint8_t PARAM_CALL_TYPE = 0x10;
+	static const uint8_t QMI_VOICE_CALL_TYPE_VOICE = 0x00;
+
+	DBG("");
+
+	cbd->user = vc;
+	memcpy(&vd->dialed, ph, sizeof(*ph));
+
+	param = qmi_param_new();
+
+	if (!qmi_param_append(param, PARAM_CALL_NUMBER,
+			strlen(calling_number), calling_number))
+		goto error;
+
+	qmi_param_append_uint8(param, PARAM_CALL_TYPE,
+				QMI_VOICE_CALL_TYPE_VOICE);
+
+	if (qmi_service_send(vd->voice, QMI_VOICE_DIAL_CALL, param, dial_cb,
+				cbd, l_free) > 0)
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
+	l_free(cbd);
+	l_free(param);
+}
+
 static void create_voice_cb(struct qmi_service *service, void *user_data)
 {
 	struct ofono_voicecall *vc = user_data;
@@ -58,6 +477,9 @@ static void create_voice_cb(struct qmi_service *service, void *user_data)
 
 	data->voice = qmi_service_ref(service);
 
+	qmi_service_register(data->voice, QMI_VOICE_ALL_CALL_STATUS_IND,
+				all_call_status_ind, vc, NULL);
+
 	ofono_voicecall_register(vc);
 }
 
@@ -70,6 +492,7 @@ static int qmi_voicecall_probe(struct ofono_voicecall *vc,
 	DBG("");
 
 	data = l_new(struct voicecall_data, 1);
+	data->call_list = l_queue_new();
 
 	ofono_voicecall_set_data(vc, data);
 
@@ -77,7 +500,6 @@ static int qmi_voicecall_probe(struct ofono_voicecall *vc,
 					create_voice_cb, vc, NULL);
 
 	return 0;
-
 }
 
 static void qmi_voicecall_remove(struct ofono_voicecall *vc)
@@ -92,12 +514,14 @@ static void qmi_voicecall_remove(struct ofono_voicecall *vc)
 
 	qmi_service_unref(data->voice);
 
+	l_queue_destroy(data->call_list, l_free);
 	l_free(data);
 }
 
 static const struct ofono_voicecall_driver driver = {
 	.probe		= qmi_voicecall_probe,
 	.remove		= qmi_voicecall_remove,
+	.dial		= dial,
 };
 
 OFONO_ATOM_DRIVER_BUILTIN(voicecall, qmimodem, &driver)
-- 
2.44.0

[PATCH 2/4] qmimodem: voicecall: Implement call answer

[Adam Pigg]

The answer function setup the parameters for a call to the service
function QMI_VOICE_ANSWER_CALL.  The only parameter is the call-id.
answer_cb will then be called which retrieves the call-id and checks
the status of the result.

---
Changes in V4
-Minor rework to the answer function

Changes in V5
-Renamed ofono_call_compare_by_status ofono_call_match_by_status
-Renamed parameters
-Made answer() ofono_error if call is null
---
---
 drivers/qmimodem/voice.h     |  1 +
 drivers/qmimodem/voicecall.c | 70 ++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+)

diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h
index 2344fd50..a524cf98 100644
--- a/drivers/qmimodem/voice.h
+++ b/drivers/qmimodem/voice.h
@@ -51,6 +51,7 @@ enum voice_commands {
 	QMI_VOICE_DIAL_CALL =			0x20,
 	QMI_VOICE_ALL_CALL_STATUS_IND =		0x2e,
 	QMI_VOICE_GET_ALL_CALL_INFO =		0x2f,
+	QMI_VOICE_ANSWER_CALL =			0x22,
 	QMI_VOICE_SUPS_NOTIFICATION_IND =	0x32,
 	QMI_VOICE_SET_SUPS_SERVICE =		0x33,
 	QMI_VOICE_GET_CALL_WAITING =		0x34,
diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c
index 7b4813c9..99ec97f9 100644
--- a/drivers/qmimodem/voicecall.c
+++ b/drivers/qmimodem/voicecall.c
@@ -93,6 +93,14 @@ static bool ofono_call_match_by_id(const void *a, const void *b)
 	return (call->id == id);
 }
 
+static bool ofono_call_match_by_status(const void *a, const void *b)
+{
+	const struct ofono_call *call = a;
+	int status = L_PTR_TO_INT(b);
+
+	return status == call->status;
+}
+
 static void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
 						int call_id)
 {
@@ -456,6 +464,67 @@ error:
 	l_free(param);
 }
 
+static void answer_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_voicecall_cb_t cb = cbd->cb;
+	uint16_t error;
+	uint8_t call_id;
+
+	static const uint8_t RESULT_CALL_ID = 0x10;
+
+	DBG("");
+
+	if (qmi_result_set_error(result, &error)) {
+		DBG("QMI Error %d", error);
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	if (qmi_result_get_uint8(result, RESULT_CALL_ID, &call_id))
+		DBG("Received answer result with call id %d", call_id);
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+}
+
+static void answer(struct ofono_voicecall *vc, ofono_voicecall_cb_t cb, void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct cb_data *cbd;
+	struct ofono_call *call;
+	struct qmi_param *param = NULL;
+
+	static const uint8_t PARAM_CALL_ID = 0x01;
+
+	DBG("");
+
+	call = l_queue_find(vd->call_list,
+					ofono_call_match_by_status,
+					L_UINT_TO_PTR(CALL_STATUS_INCOMING));
+
+	param = qmi_param_new();
+	cbd = cb_data_new(cb, data);
+	cbd->user = vc;
+
+	if (call == NULL) {
+		ofono_error("Can not find a call to pick up");
+		goto error;
+	}
+
+	if (!qmi_param_append_uint8(param, PARAM_CALL_ID,
+			call->id))
+		goto error;
+
+	if (qmi_service_send(vd->voice, QMI_VOICE_ANSWER_CALL, param,
+			answer_cb, cbd, l_free) > 0)
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
+	l_free(cbd);
+	l_free(param);
+}
+
 static void create_voice_cb(struct qmi_service *service, void *user_data)
 {
 	struct ofono_voicecall *vc = user_data;
@@ -522,6 +591,7 @@ static const struct ofono_voicecall_driver driver = {
 	.probe		= qmi_voicecall_probe,
 	.remove		= qmi_voicecall_remove,
 	.dial		= dial,
+	.answer		= answer,
 };
 
 OFONO_ATOM_DRIVER_BUILTIN(voicecall, qmimodem, &driver)
-- 
2.44.0

[PATCH 3/4] qmimodem: voicecall: Implement active call hangup

[Adam Pigg]

hangup_active iterates the current list of calls, looking for the first
active call and then calls release_specific.  This then sets up the
parameters for a call to QMI_VOICE_END_CALL, with the only parameters
being the call-id.

end_call_cb will then be called and will parse out the call-id and check
for success.

---
Changes in V4
-Minor rework to hangup_active
-Removed unnescessary changes

Changes in V5
-Renamed parameters
---
---
 drivers/qmimodem/voice.h     |  1 +
 drivers/qmimodem/voicecall.c | 82 ++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+)

diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h
index a524cf98..caedb079 100644
--- a/drivers/qmimodem/voice.h
+++ b/drivers/qmimodem/voice.h
@@ -51,6 +51,7 @@ enum voice_commands {
 	QMI_VOICE_DIAL_CALL =			0x20,
 	QMI_VOICE_ALL_CALL_STATUS_IND =		0x2e,
 	QMI_VOICE_GET_ALL_CALL_INFO =		0x2f,
+	QMI_VOICE_END_CALL =			0x21,
 	QMI_VOICE_ANSWER_CALL =			0x22,
 	QMI_VOICE_SUPS_NOTIFICATION_IND =	0x32,
 	QMI_VOICE_SET_SUPS_SERVICE =		0x33,
diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c
index 99ec97f9..6b80f576 100644
--- a/drivers/qmimodem/voicecall.c
+++ b/drivers/qmimodem/voicecall.c
@@ -525,6 +525,86 @@ error:
 	l_free(param);
 }
 
+static void end_call_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_voicecall_cb_t cb = cbd->cb;
+	uint16_t error;
+	uint8_t call_id;
+
+	static const uint8_t RESULT_CALL_ID = 0x10;
+
+	if (qmi_result_set_error(result, &error)) {
+		DBG("QMI Error %d", error);
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	if (qmi_result_get_uint8(result, RESULT_CALL_ID, &call_id))
+		DBG("Received end call result with call id %d", call_id);
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+}
+
+static void release_specific(struct ofono_voicecall *vc, int id,
+			ofono_voicecall_cb_t cb, void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct cb_data *cbd;
+	struct qmi_param *param = NULL;
+
+	static const uint8_t PARAM_CALL_ID = 0x01;
+
+	DBG("");
+
+	param = qmi_param_new();
+	cbd = cb_data_new(cb, data);
+	cbd->user = vc;
+
+	if (!qmi_param_append_uint8(param, PARAM_CALL_ID, id))
+		goto error;
+
+	if (qmi_service_send(vd->voice, QMI_VOICE_END_CALL, param, end_call_cb,
+			cbd, l_free) > 0)
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
+	l_free(cbd);
+	l_free(param);
+}
+
+static void hangup_active(struct ofono_voicecall *vc, ofono_voicecall_cb_t cb,
+				void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct ofono_call *call;
+	enum call_status active[] = {
+		CALL_STATUS_ACTIVE,
+		CALL_STATUS_DIALING,
+		CALL_STATUS_ALERTING,
+		CALL_STATUS_INCOMING,
+	};
+
+	DBG("");
+
+	for (uint32_t i = 0; i < L_ARRAY_SIZE(active); i++) {
+		call = l_queue_find(vd->call_list, ofono_call_match_by_status,
+			L_INT_TO_PTR(active[i]));
+
+		if (call)
+			break;
+	}
+
+	if (call == NULL) {
+		DBG("Can not find a call to hang up");
+		CALLBACK_WITH_FAILURE(cb, data);
+		return;
+	}
+
+	release_specific(vc, call->id, cb, data);
+}
+
 static void create_voice_cb(struct qmi_service *service, void *user_data)
 {
 	struct ofono_voicecall *vc = user_data;
@@ -592,6 +672,8 @@ static const struct ofono_voicecall_driver driver = {
 	.remove		= qmi_voicecall_remove,
 	.dial		= dial,
 	.answer		= answer,
+	.hangup_active  = hangup_active,
+	.release_specific  = release_specific,
 };
 
 OFONO_ATOM_DRIVER_BUILTIN(voicecall, qmimodem, &driver)
-- 
2.44.0

[PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[Adam Pigg]

The send_dtmf function sets up a call to send_one_dtmf, which will call
the QMI_VOICE_START_CONTINUOUS_DTMF service function.  The parameters to
this call are a hard coded call-id and the DTMF character to send.
start_cont_dtmf_cb will then be called which will set up a call to
QMI_VOICE_STOP_CONTINUOUS_DTMF to stop the tone.  Finally,
stop_cont_dtmf_cb will check the final status.

---
Changes in V4
-Removed unused enum
-Minor formatting fixes
-Ensure data->full_dtmf is free'd
-Use cb_data_ref/unref between chains of dtmf calls

Changes in V5
-Store the cb and cbd obejects in the voicall_data  struct
---
---
 drivers/qmimodem/voice.h     |   6 ++
 drivers/qmimodem/voicecall.c | 114 +++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+)

diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h
index caedb079..961fe19f 100644
--- a/drivers/qmimodem/voice.h
+++ b/drivers/qmimodem/voice.h
@@ -53,6 +53,8 @@ enum voice_commands {
 	QMI_VOICE_GET_ALL_CALL_INFO =		0x2f,
 	QMI_VOICE_END_CALL =			0x21,
 	QMI_VOICE_ANSWER_CALL =			0x22,
+	QMI_VOICE_START_CONTINUOUS_DTMF =	0x29,
+	QMI_VOICE_STOP_CONTINUOUS_DTMF =	0x2A,
 	QMI_VOICE_SUPS_NOTIFICATION_IND =	0x32,
 	QMI_VOICE_SET_SUPS_SERVICE =		0x33,
 	QMI_VOICE_GET_CALL_WAITING =		0x34,
@@ -85,6 +87,10 @@ enum qmi_voice_call_state {
 	QMI_VOICE_CALL_STATE_SETUP
 };
 
+enum qmi_voice_call_dtmf_param {
+	QMI_VOICE_DTMF_DATA = 0x01,
+};
+
 struct qmi_ussd_data {
 	uint8_t dcs;
 	uint8_t length;
diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c
index 6b80f576..f51452e5 100644
--- a/drivers/qmimodem/voicecall.c
+++ b/drivers/qmimodem/voicecall.c
@@ -42,6 +42,10 @@ struct voicecall_data {
 	uint16_t minor;
 	struct l_queue *call_list;
 	struct ofono_phone_number dialed;
+	char *full_dtmf;
+	const char *next_dtmf;
+	ofono_voicecall_cb_t send_dtmf_cb;
+	struct cb_data *send_dtmf_data;
 };
 
 struct qmi_voice_call_information_instance {
@@ -605,6 +609,114 @@ static void hangup_active(struct ofono_voicecall *vc, ofono_voicecall_cb_t cb,
 	release_specific(vc, call->id, cb, data);
 }
 
+static void stop_cont_dtmf_cb(struct qmi_result *result, void *user_data)
+{
+	struct voicecall_data *vd = user_data;
+	uint16_t error;
+
+	DBG("");
+
+	if (qmi_result_set_error(result, &error)) {
+		DBG("QMI Error %d", error);
+		CALLBACK_WITH_FAILURE(vd->send_dtmf_cb, vd->send_dtmf_data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(vd->send_dtmf_cb, vd->send_dtmf_data);
+}
+
+static void start_cont_dtmf_cb(struct qmi_result *result, void *user_data)
+{
+	struct voicecall_data *vd = user_data;
+	uint16_t error;
+	struct qmi_param *param;
+
+	DBG("");
+
+	param = qmi_param_new();
+
+	if (qmi_result_set_error(result, &error))
+		goto error;
+
+	if (!qmi_param_append_uint8(param, QMI_VOICE_DTMF_DATA, 0xff))
+		goto error;
+
+	if (qmi_service_send(vd->voice, QMI_VOICE_STOP_CONTINUOUS_DTMF, param,
+			stop_cont_dtmf_cb, vd, NULL) > 0)
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(vd->send_dtmf_cb, vd->send_dtmf_data);
+	l_free(param);
+}
+
+static void send_one_dtmf(struct ofono_voicecall *vc, const char dtmf,
+				ofono_voicecall_cb_t cb, void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct qmi_param *param = NULL;
+	uint8_t param_body[2];
+
+	DBG("");
+
+	param = qmi_param_new();
+
+	param_body[0] = 0xff;
+	param_body[1] = (uint8_t)dtmf;
+
+	if (!qmi_param_append(param, QMI_VOICE_DTMF_DATA, sizeof(param_body),
+			param_body))
+		goto error;
+
+	if (qmi_service_send(vd->voice, QMI_VOICE_START_CONTINUOUS_DTMF, param,
+			start_cont_dtmf_cb, vd, NULL) > 0)
+		return;
+
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
+	l_free(param);
+}
+
+static void send_one_dtmf_cb(const struct ofono_error *error, void *data)
+{
+	struct cb_data *cbd = data;
+	struct voicecall_data *vd = ofono_voicecall_get_data(cbd->user);
+	ofono_voicecall_cb_t cb = vd->send_dtmf_cb;
+
+	DBG("");
+
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR ||
+			*vd->next_dtmf == 0) {
+		if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
+			CALLBACK_WITH_SUCCESS(cb, cbd->data);
+		else
+			CALLBACK_WITH_FAILURE(cb, cbd->data);
+
+		l_free(vd->full_dtmf);
+		vd->full_dtmf = NULL;
+	} else
+		send_one_dtmf(cbd->user,
+				*(vd->next_dtmf++),
+				send_one_dtmf_cb, vd->send_dtmf_data);
+}
+
+static void send_dtmf(struct ofono_voicecall *vc, const char *dtmf,
+			ofono_voicecall_cb_t cb, void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+
+	DBG("");
+
+	l_free(vd->full_dtmf);
+	vd->full_dtmf = l_strdup(dtmf);
+	vd->next_dtmf = &vd->full_dtmf[1];
+
+	vd->send_dtmf_data = data;
+	vd->send_dtmf_cb = cb;
+
+	send_one_dtmf(vc, *dtmf, send_one_dtmf_cb, NULL);
+}
+
 static void create_voice_cb(struct qmi_service *service, void *user_data)
 {
 	struct ofono_voicecall *vc = user_data;
@@ -664,6 +776,7 @@ static void qmi_voicecall_remove(struct ofono_voicecall *vc)
 	qmi_service_unref(data->voice);
 
 	l_queue_destroy(data->call_list, l_free);
+	l_free(data->full_dtmf);
 	l_free(data);
 }
 
@@ -674,6 +787,7 @@ static const struct ofono_voicecall_driver driver = {
 	.answer		= answer,
 	.hangup_active  = hangup_active,
 	.release_specific  = release_specific,
+	.send_tones	= send_dtmf,
 };
 
 OFONO_ATOM_DRIVER_BUILTIN(voicecall, qmimodem, &driver)
-- 
2.44.0

Re: [PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[Adam Pigg]

This one took the longest, but I think I implemented it correctly as you 
suggested.  I have ran this through valgrind and didnt catch any leaks.

On Sunday 21 April 2024 20:49:06 BST you wrote:
> The send_dtmf function sets up a call to send_one_dtmf, which will call
> the QMI_VOICE_START_CONTINUOUS_DTMF service function.  The parameters to
> this call are a hard coded call-id and the DTMF character to send.
> start_cont_dtmf_cb will then be called which will set up a call to
> QMI_VOICE_STOP_CONTINUOUS_DTMF to stop the tone.  Finally,
> stop_cont_dtmf_cb will check the final status.
> 
> ---
> Changes in V4
> -Removed unused enum
> -Minor formatting fixes
> -Ensure data->full_dtmf is free'd
> -Use cb_data_ref/unref between chains of dtmf calls
> 
> Changes in V5
> -Store the cb and cbd obejects in the voicall_data  struct
> ---
> ---
>  drivers/qmimodem/voice.h     |   6 ++
>  drivers/qmimodem/voicecall.c | 114 +++++++++++++++++++++++++++++++++++
>  2 files changed, 120 insertions(+)
> 
> diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h
> index caedb079..961fe19f 100644
> --- a/drivers/qmimodem/voice.h
> +++ b/drivers/qmimodem/voice.h
> @@ -53,6 +53,8 @@ enum voice_commands {
>  	QMI_VOICE_GET_ALL_CALL_INFO =		0x2f,
>  	QMI_VOICE_END_CALL =			0x21,
>  	QMI_VOICE_ANSWER_CALL =			0x22,
> +	QMI_VOICE_START_CONTINUOUS_DTMF =	0x29,
> +	QMI_VOICE_STOP_CONTINUOUS_DTMF =	0x2A,
>  	QMI_VOICE_SUPS_NOTIFICATION_IND =	0x32,
>  	QMI_VOICE_SET_SUPS_SERVICE =		0x33,
>  	QMI_VOICE_GET_CALL_WAITING =		0x34,
> @@ -85,6 +87,10 @@ enum qmi_voice_call_state {
>  	QMI_VOICE_CALL_STATE_SETUP
>  };
> 
> +enum qmi_voice_call_dtmf_param {
> +	QMI_VOICE_DTMF_DATA = 0x01,
> +};
> +
>  struct qmi_ussd_data {
>  	uint8_t dcs;
>  	uint8_t length;
> diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c
> index 6b80f576..f51452e5 100644
> --- a/drivers/qmimodem/voicecall.c
> +++ b/drivers/qmimodem/voicecall.c
> @@ -42,6 +42,10 @@ struct voicecall_data {
>  	uint16_t minor;
>  	struct l_queue *call_list;
>  	struct ofono_phone_number dialed;
> +	char *full_dtmf;
> +	const char *next_dtmf;
> +	ofono_voicecall_cb_t send_dtmf_cb;
> +	struct cb_data *send_dtmf_data;
>  };
> 
>  struct qmi_voice_call_information_instance {
> @@ -605,6 +609,114 @@ static void hangup_active(struct ofono_voicecall *vc,
> ofono_voicecall_cb_t cb, release_specific(vc, call->id, cb, data);
>  }
> 
> +static void stop_cont_dtmf_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct voicecall_data *vd = user_data;
> +	uint16_t error;
> +
> +	DBG("");
> +
> +	if (qmi_result_set_error(result, &error)) {
> +		DBG("QMI Error %d", error);
> +		CALLBACK_WITH_FAILURE(vd->send_dtmf_cb, vd-
>send_dtmf_data);
> +		return;
> +	}
> +
> +	CALLBACK_WITH_SUCCESS(vd->send_dtmf_cb, vd->send_dtmf_data);
> +}
> +
> +static void start_cont_dtmf_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct voicecall_data *vd = user_data;
> +	uint16_t error;
> +	struct qmi_param *param;
> +
> +	DBG("");
> +
> +	param = qmi_param_new();
> +
> +	if (qmi_result_set_error(result, &error))
> +		goto error;
> +
> +	if (!qmi_param_append_uint8(param, QMI_VOICE_DTMF_DATA, 0xff))
> +		goto error;
> +
> +	if (qmi_service_send(vd->voice, QMI_VOICE_STOP_CONTINUOUS_DTMF, 
param,
> +			stop_cont_dtmf_cb, vd, NULL) > 0)
> +		return;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(vd->send_dtmf_cb, vd->send_dtmf_data);
> +	l_free(param);
> +}
> +
> +static void send_one_dtmf(struct ofono_voicecall *vc, const char dtmf,
> +				ofono_voicecall_cb_t cb, void 
*data)
> +{
> +	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
> +	struct qmi_param *param = NULL;
> +	uint8_t param_body[2];
> +
> +	DBG("");
> +
> +	param = qmi_param_new();
> +
> +	param_body[0] = 0xff;
> +	param_body[1] = (uint8_t)dtmf;
> +
> +	if (!qmi_param_append(param, QMI_VOICE_DTMF_DATA, 
sizeof(param_body),
> +			param_body))
> +		goto error;
> +
> +	if (qmi_service_send(vd->voice, QMI_VOICE_START_CONTINUOUS_DTMF, 
param,
> +			start_cont_dtmf_cb, vd, NULL) > 0)
> +		return;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cb, data);
> +	l_free(param);
> +}
> +
> +static void send_one_dtmf_cb(const struct ofono_error *error, void *data)
> +{
> +	struct cb_data *cbd = data;
> +	struct voicecall_data *vd = ofono_voicecall_get_data(cbd->user);
> +	ofono_voicecall_cb_t cb = vd->send_dtmf_cb;
> +
> +	DBG("");
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR ||
> +			*vd->next_dtmf == 0) {
> +		if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
> +			CALLBACK_WITH_SUCCESS(cb, cbd->data);
> +		else
> +			CALLBACK_WITH_FAILURE(cb, cbd->data);
> +
> +		l_free(vd->full_dtmf);
> +		vd->full_dtmf = NULL;
> +	} else
> +		send_one_dtmf(cbd->user,
> +				*(vd->next_dtmf++),
> +				send_one_dtmf_cb, vd-
>send_dtmf_data);
> +}
> +
> +static void send_dtmf(struct ofono_voicecall *vc, const char *dtmf,
> +			ofono_voicecall_cb_t cb, void *data)
> +{
> +	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
> +
> +	DBG("");
> +
> +	l_free(vd->full_dtmf);
> +	vd->full_dtmf = l_strdup(dtmf);
> +	vd->next_dtmf = &vd->full_dtmf[1];
> +
> +	vd->send_dtmf_data = data;
> +	vd->send_dtmf_cb = cb;
> +
> +	send_one_dtmf(vc, *dtmf, send_one_dtmf_cb, NULL);
> +}
> +
>  static void create_voice_cb(struct qmi_service *service, void *user_data)
>  {
>  	struct ofono_voicecall *vc = user_data;
> @@ -664,6 +776,7 @@ static void qmi_voicecall_remove(struct ofono_voicecall
> *vc) qmi_service_unref(data->voice);
> 
>  	l_queue_destroy(data->call_list, l_free);
> +	l_free(data->full_dtmf);
>  	l_free(data);
>  }
> 
> @@ -674,6 +787,7 @@ static const struct ofono_voicecall_driver driver = {
>  	.answer		= answer,
>  	.hangup_active  = hangup_active,
>  	.release_specific  = release_specific,
> +	.send_tones	= send_dtmf,
>  };
> 
>  OFONO_ATOM_DRIVER_BUILTIN(voicecall, qmimodem, &driver)

Re: [PATCH 1/4] qmimodem: voicecall: Implement call dialing

[Adam Pigg]

Apologies, i missed the -v5 flag

On Sunday 21 April 2024 20:49:03 BST you wrote:
> Add voicecall dialling to the qmimodem driver
> Includes required infrastructure and setup of the QMI services
> 
> Call State Handling
> ===================
> On initialisation, register the all_call_status_ind callback to be
> called for QMI_VOICE_IND_ALL_STATUS.  This will handle notification of
> call status to the rest of the system
> 
> Dial Handling
> =============
> The dial function sets up the parameters for the QMI_VOICE_DIAL_CALL
> service.  The parameters are the number to be called and the call type,
> which is currently hard coded to be QMI_VOICE_CALL_TYPE_VOICE.  The
> dial_cb callback will then be called and will receive the call-id.
> 
> ---
> Changes in V4
> -merged qmi_voice_call_status and all_call_status_ind
> -several minor structure/formate changes
> 
> Changes in V5
> -renames ofono_call_compare_by_id to ofono_call_match_by_id
> -updated new_entry pointer to fix use-after-free in
> ofono_call_list_notify
> -Renamed several constants
> -in all_call_status_ind removed error label/gotos after restructuring
> -When creating the call list, ensure an upper bound of 16
> -Fix out-of-mound-read on remote number due to QMI string not being null
> terminated
> ---
> ---
>  drivers/qmimodem/voice.h     |  17 ++
>  drivers/qmimodem/voicecall.c | 426 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 442 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h
> index 917e72f7..2344fd50 100644
> --- a/drivers/qmimodem/voice.h
> +++ b/drivers/qmimodem/voice.h
> @@ -48,6 +48,9 @@ enum qmi_ussd_user_required {
> 
>  /* QMI service voice. Using an enum to prevent doublicated entries */
>  enum voice_commands {
> +	QMI_VOICE_DIAL_CALL =			0x20,
> +	QMI_VOICE_ALL_CALL_STATUS_IND =		0x2e,
> +	QMI_VOICE_GET_ALL_CALL_INFO =		0x2f,
>  	QMI_VOICE_SUPS_NOTIFICATION_IND =	0x32,
>  	QMI_VOICE_SET_SUPS_SERVICE =		0x33,
>  	QMI_VOICE_GET_CALL_WAITING =		0x34,
> @@ -66,6 +69,20 @@ enum voice_commands {
>  	QMI_VOICE_GET_CNAP =			0x4d
>  };
> 
> +enum qmi_voice_call_state {
> +	QMI_VOICE_CALL_STATE_IDLE = 0x0,
> +	QMI_VOICE_CALL_STATE_ORIG,
> +	QMI_VOICE_CALL_STATE_INCOMING,
> +	QMI_VOICE_CALL_STATE_CONV,
> +	QMI_VOICE_CALL_STATE_CC_IN_PROG,
> +	QMI_VOICE_CALL_STATE_ALERTING,
> +	QMI_VOICE_CALL_STATE_HOLD,
> +	QMI_VOICE_CALL_STATE_WAITING,
> +	QMI_VOICE_CALL_STATE_DISCONNECTING,
> +	QMI_VOICE_CALL_STATE_END,
> +	QMI_VOICE_CALL_STATE_SETUP
> +};
> +
>  struct qmi_ussd_data {
>  	uint8_t dcs;
>  	uint8_t length;
> diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c
> index aa34fc25..7b4813c9 100644
> --- a/drivers/qmimodem/voicecall.c
> +++ b/drivers/qmimodem/voicecall.c
> @@ -3,6 +3,7 @@
>   *  oFono - Open Source Telephony
>   *
>   *  Copyright (C) 2011-2012  Intel Corporation. All rights reserved.
> + *  Copyright (C) 2024 Adam Pigg <adam@piggz.co.uk>
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License version 2 as
> @@ -26,6 +27,10 @@
>  #include <ofono/log.h>
>  #include <ofono/modem.h>
>  #include <ofono/voicecall.h>
> +#include <src/common.h>
> +#include <ell/ell.h>
> +
> +#include "voice.h"
> 
>  #include "qmi.h"
> 
> @@ -35,8 +40,422 @@ struct voicecall_data {
>  	struct qmi_service *voice;
>  	uint16_t major;
>  	uint16_t minor;
> +	struct l_queue *call_list;
> +	struct ofono_phone_number dialed;
>  };
> 
> +struct qmi_voice_call_information_instance {
> +	uint8_t id;
> +	uint8_t state;
> +	uint8_t type;
> +	uint8_t direction;
> +	uint8_t mode;
> +	uint8_t multipart_indicator;
> +	uint8_t als;
> +} __attribute__((__packed__));
> +
> +struct qmi_voice_call_information {
> +	uint8_t size;
> +	struct qmi_voice_call_information_instance instance[0];
> +} __attribute__((__packed__));
> +
> +struct qmi_voice_remote_party_number_instance {
> +	uint8_t call_id;
> +	uint8_t presentation_indicator;
> +	uint8_t number_size;
> +	char number[0];
> +} __attribute__((__packed__));
> +
> +struct qmi_voice_remote_party_number {
> +	uint8_t size;
> +	struct qmi_voice_remote_party_number_instance instance[0];
> +} __attribute__((__packed__));
> +
> +static int ofono_call_compare(const void *a, const void *b, void *data)
> +{
> +	const struct ofono_call *ca = a;
> +	const struct ofono_call *cb = b;
> +
> +	if (ca->id < cb->id)
> +		return -1;
> +
> +	if (ca->id > cb->id)
> +		return 1;
> +
> +	return 0;
> +}
> +
> +static bool ofono_call_match_by_id(const void *a, const void *b)
> +{
> +	const struct ofono_call *call = a;
> +	unsigned int id = L_PTR_TO_UINT(b);
> +
> +	return (call->id == id);
> +}
> +
> +static void ofono_call_list_dial_callback(struct ofono_voicecall *vc,
> +						int call_id)
> +{
> +	struct ofono_call *call;
> +	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
> +	struct l_queue *call_list = vd->call_list;
> +	const struct ofono_phone_number *ph = &vd->dialed;
> +
> +	/* check if call_id already present */
> +	call = l_queue_find(call_list, ofono_call_match_by_id,
> +				L_UINT_TO_PTR(call_id));
> +
> +	if (call)
> +		return;
> +
> +	call = l_new(struct ofono_call, 1);
> +	call->id = call_id;
> +
> +	memcpy(&call->called_number, ph, sizeof(*ph));
> +	call->direction = CALL_DIRECTION_MOBILE_ORIGINATED;
> +	call->status = CALL_STATUS_DIALING;
> +	call->type = 0; /* voice */
> +
> +	l_queue_insert(call_list, call, ofono_call_compare, NULL);
> +
> +	ofono_voicecall_notify(vc, call);
> +}
> +
> +static void ofono_call_list_notify(struct ofono_voicecall *vc,
> +					struct l_queue *calls)
> +{
> +	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
> +	struct l_queue *old_calls = vd->call_list;
> +	struct l_queue *new_calls = calls;
> +	struct ofono_call *new_call, *old_call;
> +	const struct l_queue_entry *old_entry, *new_entry;
> +	uint i;
> +
> +	uint loop_length =
> +		MAX(l_queue_length(old_calls), 
l_queue_length(new_calls));
> +
> +	old_entry = l_queue_get_entries(old_calls);
> +	new_entry = l_queue_get_entries(new_calls);
> +
> +	for (i = 0; i < loop_length; ++i) {
> +		old_call = old_entry ? old_entry->data : NULL;
> +		new_call = new_entry ? new_entry->data : NULL;
> +
> +		if (new_call && new_call->status == 
CALL_STATUS_DISCONNECTED) {
> +			ofono_voicecall_disconnected(
> +				vc, new_call->id,
> +				
OFONO_DISCONNECT_REASON_REMOTE_HANGUP, NULL);
> +			new_entry = new_entry->next;
> +			l_queue_remove(calls, new_call);
> +			l_free(new_call);
> +			continue;
> +		}
> +
> +		if (old_call &&
> +				(!new_call || (new_call->id > 
old_call->id)))
> +			ofono_voicecall_disconnected(
> +				vc, old_call->id,
> +				
OFONO_DISCONNECT_REASON_REMOTE_HANGUP, NULL);
> +		else if (new_call &&
> +				(!old_call || (new_call->id < 
old_call->id))) {
> +			DBG("Notify new call %d", new_call->id);
> +			/* new call, signal it */
> +			if (new_call->type == 0)
> +				ofono_voicecall_notify(vc, 
new_call);
> +		} else if (memcmp(new_call, old_call, sizeof(*new_call)) 
&&
> +				new_call->type == 0)
> +			ofono_voicecall_notify(vc, new_call);
> +
> +		if (old_entry)
> +			old_entry = old_entry->next;
> +		if (new_entry)
> +			new_entry = new_entry->next;
> +	}
> +
> +	l_queue_destroy(old_calls, l_free);
> +	vd->call_list = calls;
> +}
> +
> +static const char *qmi_voice_call_state_name(enum qmi_voice_call_state
> value) +{
> +	switch (value) {
> +	case QMI_VOICE_CALL_STATE_IDLE:
> +		return "QMI_VOICE_CALL_STATE_IDLE";
> +	case QMI_VOICE_CALL_STATE_ORIG:
> +		return "QMI_VOICE_CALL_STATE_ORIG";
> +	case QMI_VOICE_CALL_STATE_INCOMING:
> +		return "QMI_VOICE_CALL_STATE_INCOMING";
> +	case QMI_VOICE_CALL_STATE_CONV:
> +		return "QMI_VOICE_CALL_STATE_CONV";
> +	case QMI_VOICE_CALL_STATE_CC_IN_PROG:
> +		return "QMI_VOICE_CALL_STATE_CC_IN_PROG";
> +	case QMI_VOICE_CALL_STATE_ALERTING:
> +		return "QMI_VOICE_CALL_STATE_ALERTING";
> +	case QMI_VOICE_CALL_STATE_HOLD:
> +		return "QMI_VOICE_CALL_STATE_HOLD";
> +	case QMI_VOICE_CALL_STATE_WAITING:
> +		return "QMI_VOICE_CALL_STATE_WAITING";
> +	case QMI_VOICE_CALL_STATE_DISCONNECTING:
> +		return "QMI_VOICE_CALL_STATE_DISCONNECTING";
> +	case QMI_VOICE_CALL_STATE_END:
> +		return "QMI_VOICE_CALL_STATE_END";
> +	case QMI_VOICE_CALL_STATE_SETUP:
> +		return "QMI_VOICE_CALL_STATE_SETUP";
> +	}
> +	return "QMI_CALL_STATE_<UNKNOWN>";
> +}
> +
> +static bool qmi_to_ofono_status(uint8_t status, int *ret)
> +{
> +	int err = false;
> +
> +	switch (status) {
> +	case QMI_VOICE_CALL_STATE_IDLE:
> +	case QMI_VOICE_CALL_STATE_END:
> +	case QMI_VOICE_CALL_STATE_DISCONNECTING:
> +		*ret = CALL_STATUS_DISCONNECTED;
> +		break;
> +	case QMI_VOICE_CALL_STATE_HOLD:
> +		*ret = CALL_STATUS_HELD;
> +		break;
> +	case QMI_VOICE_CALL_STATE_WAITING:
> +		*ret = CALL_STATUS_WAITING;
> +		break;
> +	case QMI_VOICE_CALL_STATE_ORIG:
> +		*ret = CALL_STATUS_DIALING;
> +		break;
> +	case QMI_VOICE_CALL_STATE_SETUP:
> +	case QMI_VOICE_CALL_STATE_INCOMING:
> +		*ret = CALL_STATUS_INCOMING;
> +		break;
> +	case QMI_VOICE_CALL_STATE_CONV:
> +		*ret = CALL_STATUS_ACTIVE;
> +		break;
> +	case QMI_VOICE_CALL_STATE_CC_IN_PROG:
> +		*ret = CALL_STATUS_DIALING;
> +		break;
> +	case QMI_VOICE_CALL_STATE_ALERTING:
> +		*ret = CALL_STATUS_ALERTING;
> +		break;
> +	default:
> +		err = true;
> +	}
> +	return err;
> +}
> +
> +static enum call_direction qmi_to_ofono_direction(uint8_t qmi_direction)
> +{
> +	return qmi_direction - 1;
> +}
> +
> +static void all_call_status_ind(struct qmi_result *result, void *user_data)
> +{
> +	struct ofono_voicecall *vc = user_data;
> +
> +	int i;
> +	int offset;
> +	uint16_t len;
> +	bool status = true;
> +	int instance_size;
> +	const struct qmi_voice_call_information *call_information;
> +	const struct qmi_voice_remote_party_number *remote_party_number;
> +	const struct qmi_voice_remote_party_number_instance
> *remote_party_number_inst[16]; +
> +	static const uint8_t RESULT_CALL_STATUS_CALL_INFORMATION = 0x01;
> +	static const uint8_t RESULT_CALL_STATUS_REMOTE_NUMBER = 0x10;
> +	static const uint8_t RESULT_CALL_INFO_CALL_INFORMATION = 0x10;
> +	static const uint8_t RESULT_CALL_INFO_REMOTE_NUMBER = 0x11;
> +
> +	DBG("");
> +
> +	/* mandatory */
> +	call_information = qmi_result_get(
> +		result, RESULT_CALL_STATUS_CALL_INFORMATION, &len);
> +
> +	if (!call_information) {
> +		call_information = qmi_result_get(
> +			result, RESULT_CALL_INFO_CALL_INFORMATION,
> +			&len);
> +		status = false;
> +	}
> +
> +	if (!call_information || len < sizeof(call_information->size)) {
> +		DBG("Parsing of all call status indication failed");
> +		return;
> +	}
> +
> +	if (!call_information->size) {
> +		DBG("No call informations received!");
> +		return;
> +	}
> +
> +	if (len != call_information->size *
> +			sizeof(struct 
qmi_voice_call_information_instance) +
> +			sizeof(call_information->size)) {
> +		DBG("Call information size incorrect");
> +		return;
> +	}
> +
> +	/* mandatory */
> +	remote_party_number = qmi_result_get(
> +		result,
> +		status ? RESULT_CALL_STATUS_REMOTE_NUMBER :
> +			 RESULT_CALL_INFO_REMOTE_NUMBER,
> +		&len);
> +
> +	if (!remote_party_number) {
> +		DBG("Unable to retrieve remote numbers");
> +		return;
> +	}
> +
> +	/* verify the length */
> +	if (len < sizeof(remote_party_number->size)) {
> +		DBG("Parsing of remote numbers failed");
> +		return;
> +	}
> +
> +	/* expect we have valid fields for every call */
> +	if (call_information->size != remote_party_number->size) {
> +		DBG("Not all fields have the same size");
> +		return;
> +	}
> +
> +	/* pull the remote call info into a local array */
> +	instance_size = sizeof(struct 
qmi_voice_remote_party_number_instance);
> +
> +	for (i = 0, offset = sizeof(remote_party_number->size);
> +			offset < len && i < 16 && i < 
remote_party_number->size;
> +			i++) {
> +		const struct qmi_voice_remote_party_number_instance 
*instance;
> +
> +		if (offset + instance_size > len) {
> +			DBG("Error parsing remote numbers");
> +			return;
> +		}
> +
> +		instance = (void *)remote_party_number + offset;
> +		if (offset + instance_size + instance->number_size > len) 
{
> +			DBG("Error parsing remote numbers");
> +			return;
> +		}
> +
> +		remote_party_number_inst[i] = instance;
> +		offset +=
> +			sizeof(struct 
qmi_voice_remote_party_number_instance) +
> +			instance->number_size;
> +	}
> +
> +	struct l_queue *calls = l_queue_new();
> +
> +	for (i = 0; i < call_information->size && i < 16; i++) {
> +		struct ofono_call *call = l_new(struct ofono_call, 1);
> +		struct qmi_voice_call_information_instance call_info;
> +		const struct qmi_voice_remote_party_number_instance
> +			*remote_party = remote_party_number_inst[i];
> +		int number_size;
> +
> +		call_info = call_information->instance[i];
> +
> +		call->id = call_info.id;
> +		call->direction = 
qmi_to_ofono_direction(call_info.direction);
> +		call->type = 0; /* always voice */
> +
> +		number_size = MIN(remote_party->number_size,
> OFONO_MAX_PHONE_NUMBER_LENGTH); +		char *tmp =
> l_strndup(remote_party->number, number_size);
> +		l_strlcpy(call->phone_number.number, tmp,
> sizeof(call->phone_number.number)); +		l_free(tmp);
> +
> +		if (strlen(call->phone_number.number) > 0)
> +			call->clip_validity = 0;
> +		else
> +			call->clip_validity = 2;
> +
> +		if (qmi_to_ofono_status(call_info.state, &call->status)) 
{
> +			DBG("Ignore call id %d, because can not 
convert QMI state 0x%x to
> ofono.", +				call_info.id, call_info.state);
> +			l_free(call);
> +			continue;
> +		}
> +
> +		DBG("Call %d in state %s(%d)", call_info.id,
> +			qmi_voice_call_state_name(call_info.state),
> +			call_info.state);
> +
> +		l_queue_push_tail(calls, call);
> +	}
> +
> +	ofono_call_list_notify(vc, calls);
> +}
> +
> +static void dial_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	struct ofono_voicecall *vc = cbd->user;
> +	ofono_voicecall_cb_t cb = cbd->cb;
> +	uint16_t error;
> +	uint8_t call_id;
> +
> +	static const uint8_t RESULT_CALL_ID = 0x10;
> +
> +	DBG("");
> +
> +	if (qmi_result_set_error(result, &error)) {
> +		DBG("QMI Error %d", error);
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);
> +		return;
> +	}
> +
> +	if (!qmi_result_get_uint8(result, RESULT_CALL_ID,
> +			&call_id)) {
> +		ofono_error("No call id in dial result");
> +		CALLBACK_WITH_FAILURE(cb, cbd->data);
> +		return;
> +	}
> +
> +	DBG("New call QMI id %d", call_id);
> +	ofono_call_list_dial_callback(vc, call_id);
> +
> +	CALLBACK_WITH_SUCCESS(cb, cbd->data);
> +}
> +
> +static void dial(struct ofono_voicecall *vc,
> +			const struct ofono_phone_number *ph,
> +			enum ofono_clir_option clir, 
ofono_voicecall_cb_t cb,
> +			void *data)
> +{
> +	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
> +	struct cb_data *cbd = cb_data_new(cb, data);
> +	struct qmi_param *param;
> +	const char *calling_number = phone_number_to_string(ph);
> +
> +	static const uint8_t PARAM_CALL_NUMBER = 0x01;
> +	static const uint8_t PARAM_CALL_TYPE = 0x10;
> +	static const uint8_t QMI_VOICE_CALL_TYPE_VOICE = 0x00;
> +
> +	DBG("");
> +
> +	cbd->user = vc;
> +	memcpy(&vd->dialed, ph, sizeof(*ph));
> +
> +	param = qmi_param_new();
> +
> +	if (!qmi_param_append(param, PARAM_CALL_NUMBER,
> +			strlen(calling_number), calling_number))
> +		goto error;
> +
> +	qmi_param_append_uint8(param, PARAM_CALL_TYPE,
> +				QMI_VOICE_CALL_TYPE_VOICE);
> +
> +	if (qmi_service_send(vd->voice, QMI_VOICE_DIAL_CALL, param, 
dial_cb,
> +				cbd, l_free) > 0)
> +		return;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cb, data);
> +	l_free(cbd);
> +	l_free(param);
> +}
> +
>  static void create_voice_cb(struct qmi_service *service, void *user_data)
>  {
>  	struct ofono_voicecall *vc = user_data;
> @@ -58,6 +477,9 @@ static void create_voice_cb(struct qmi_service *service,
> void *user_data)
> 
>  	data->voice = qmi_service_ref(service);
> 
> +	qmi_service_register(data->voice, QMI_VOICE_ALL_CALL_STATUS_IND,
> +				all_call_status_ind, vc, NULL);
> +
>  	ofono_voicecall_register(vc);
>  }
> 
> @@ -70,6 +492,7 @@ static int qmi_voicecall_probe(struct ofono_voicecall
> *vc, DBG("");
> 
>  	data = l_new(struct voicecall_data, 1);
> +	data->call_list = l_queue_new();
> 
>  	ofono_voicecall_set_data(vc, data);
> 
> @@ -77,7 +500,6 @@ static int qmi_voicecall_probe(struct ofono_voicecall
> *vc, create_voice_cb, vc, NULL);
> 
>  	return 0;
> -
>  }
> 
>  static void qmi_voicecall_remove(struct ofono_voicecall *vc)
> @@ -92,12 +514,14 @@ static void qmi_voicecall_remove(struct ofono_voicecall
> *vc)
> 
>  	qmi_service_unref(data->voice);
> 
> +	l_queue_destroy(data->call_list, l_free);
>  	l_free(data);
>  }
> 
>  static const struct ofono_voicecall_driver driver = {
>  	.probe		= qmi_voicecall_probe,
>  	.remove		= qmi_voicecall_remove,
> +	.dial		= dial,
>  };
> 
>  OFONO_ATOM_DRIVER_BUILTIN(voicecall, qmimodem, &driver)

Re: [PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[Denis Kenzior]

Hi Adam,

On 4/21/24 14:49, Adam Pigg wrote:
> The send_dtmf function sets up a call to send_one_dtmf, which will call
> the QMI_VOICE_START_CONTINUOUS_DTMF service function.  The parameters to
> this call are a hard coded call-id and the DTMF character to send.
> start_cont_dtmf_cb will then be called which will set up a call to
> QMI_VOICE_STOP_CONTINUOUS_DTMF to stop the tone.  Finally,
> stop_cont_dtmf_cb will check the final status.
> 
> ---
> Changes in V4
> -Removed unused enum
> -Minor formatting fixes
> -Ensure data->full_dtmf is free'd
> -Use cb_data_ref/unref between chains of dtmf calls
> 
> Changes in V5
> -Store the cb and cbd obejects in the voicall_data  struct
> ---
> ---
>   drivers/qmimodem/voice.h     |   6 ++
>   drivers/qmimodem/voicecall.c | 114 +++++++++++++++++++++++++++++++++++
>   2 files changed, 120 insertions(+)
> 

<snip>

> @@ -605,6 +609,114 @@ static void hangup_active(struct ofono_voicecall *vc, ofono_voicecall_cb_t cb,
>   	release_specific(vc, call->id, cb, data);
>   }
>   
> +static void stop_cont_dtmf_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct voicecall_data *vd = user_data;
> +	uint16_t error;
> +
> +	DBG("");
> +
> +	if (qmi_result_set_error(result, &error)) {
> +		DBG("QMI Error %d", error);
> +		CALLBACK_WITH_FAILURE(vd->send_dtmf_cb, vd->send_dtmf_data);
> +		return;
> +	}
> +
> +	CALLBACK_WITH_SUCCESS(vd->send_dtmf_cb, vd->send_dtmf_data);

This calls back into oFono core unconditionally.  Doesn't this imply that you're 
handling only a single DTMF character?

> +}
> +

<snip>

> +static void send_one_dtmf(struct ofono_voicecall *vc, const char dtmf,
> +				ofono_voicecall_cb_t cb, void *data)

Both cb and data are not saved anywhere and are only used on the error path. 
The initial invocation in send_dtmf() also passes in NULL for data...

So again, I think this implies only a request which uses a single DTMF tone 
character would work?

> +{
> +	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
> +	struct qmi_param *param = NULL;
> +	uint8_t param_body[2];
> +
> +	DBG("");
> +
> +	param = qmi_param_new();
> +
> +	param_body[0] = 0xff;
> +	param_body[1] = (uint8_t)dtmf;
> +
> +	if (!qmi_param_append(param, QMI_VOICE_DTMF_DATA, sizeof(param_body),
> +			param_body))
> +		goto error;
> +
> +	if (qmi_service_send(vd->voice, QMI_VOICE_START_CONTINUOUS_DTMF, param,
> +			start_cont_dtmf_cb, vd, NULL) > 0)
> +		return;
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cb, data);
> +	l_free(param);
> +}
> +
> +static void send_one_dtmf_cb(const struct ofono_error *error, void *data)
> +{
> +	struct cb_data *cbd = data;
> +	struct voicecall_data *vd = ofono_voicecall_get_data(cbd->user);
> +	ofono_voicecall_cb_t cb = vd->send_dtmf_cb;
> +
> +	DBG("");
> +
> +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR ||
> +			*vd->next_dtmf == 0) {
> +		if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
> +			CALLBACK_WITH_SUCCESS(cb, cbd->data);
> +		else
> +			CALLBACK_WITH_FAILURE(cb, cbd->data);
> +
> +		l_free(vd->full_dtmf);
> +		vd->full_dtmf = NULL;
> +	} else
> +		send_one_dtmf(cbd->user,
> +				*(vd->next_dtmf++),
> +				send_one_dtmf_cb, vd->send_dtmf_data);

This function doesn't seem to be invoked?

> +}
> +

<snip>

Regards,
-Denis

Re: [PATCH 1/4] qmimodem: voicecall: Implement call dialing

[Denis Kenzior]

On 4/21/24 14:49, Adam Pigg wrote:
> Add voicecall dialling to the qmimodem driver
> Includes required infrastructure and setup of the QMI services
> 
> Call State Handling
> ===================
> On initialisation, register the all_call_status_ind callback to be
> called for QMI_VOICE_IND_ALL_STATUS.  This will handle notification of
> call status to the rest of the system
> 
> Dial Handling
> =============
> The dial function sets up the parameters for the QMI_VOICE_DIAL_CALL
> service.  The parameters are the number to be called and the call type,
> which is currently hard coded to be QMI_VOICE_CALL_TYPE_VOICE.  The
> dial_cb callback will then be called and will receive the call-id.
> 
> ---
> Changes in V4
> -merged qmi_voice_call_status and all_call_status_ind
> -several minor structure/formate changes
> 
> Changes in V5
> -renames ofono_call_compare_by_id to ofono_call_match_by_id
> -updated new_entry pointer to fix use-after-free in
> ofono_call_list_notify
> -Renamed several constants
> -in all_call_status_ind removed error label/gotos after restructuring
> -When creating the call list, ensure an upper bound of 16
> -Fix out-of-mound-read on remote number due to QMI string not being null
> terminated
> ---
> ---
>   drivers/qmimodem/voice.h     |  17 ++
>   drivers/qmimodem/voicecall.c | 426 ++++++++++++++++++++++++++++++++++-
>   2 files changed, 442 insertions(+), 1 deletion(-)
> 

I had the following when this patch was applied:

[denkenz@archdev ofono]$ make
make --no-print-directory all-am
   CC       drivers/qmimodem/voicecall.o
vdrivers/qmimodem/voicecall.c: In function ‘all_call_status_ind’:
drivers/qmimodem/voicecall.c:348:9: error: ISO C90 forbids mixed declarations 
and code [-Werror=declaration-after-statement]
   348 |         struct l_queue *calls = l_queue_new();
       |         ^~~~~~
drivers/qmimodem/voicecall.c:364:17: error: ISO C90 forbids mixed declarations 
and code [-Werror=declaration-after-statement]
   364 |                 char *tmp = l_strndup(remote_party->number, number_size);
       |                 ^~~~
icc1: all warnings being treated as errors

I fixed this up manually to declare calls & tmp in the variable block to avoid 
ISO C90 warning.

Patches 1-3 applied, thanks.

Regards,
-Denis

Re: [PATCH 1/4] qmimodem: voicecall: Implement call dialing

[patchwork-bot+ofono]

Hello:

This series was applied to ofono.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Sun, 21 Apr 2024 20:49:03 +0100 you wrote:
> Add voicecall dialling to the qmimodem driver
> Includes required infrastructure and setup of the QMI services
> 
> Call State Handling
> ===================
> On initialisation, register the all_call_status_ind callback to be
> called for QMI_VOICE_IND_ALL_STATUS.  This will handle notification of
> call status to the rest of the system
> 
> [...]

Here is the summary with links:
  - [1/4] qmimodem: voicecall: Implement call dialing
    (no matching commit)
  - [2/4] qmimodem: voicecall: Implement call answer
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=3680df684593
  - [3/4] qmimodem: voicecall: Implement active call hangup
    https://git.kernel.org/pub/scm/network/ofono/ofono.git/?id=752d286a5ce3
  - [4/4] qmimodem: voicecall: Implement DTMF tones
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[Adam Pigg]

Hi Dennis

Thanks for the thorough review and merging the others....

On Monday 22 April 2024 22:00:41 BST Denis Kenzior wrote:
> Hi Adam,
> 
> On 4/21/24 14:49, Adam Pigg wrote:
> > The send_dtmf function sets up a call to send_one_dtmf, which will call
> > the QMI_VOICE_START_CONTINUOUS_DTMF service function.  The parameters to
> > this call are a hard coded call-id and the DTMF character to send.
> > start_cont_dtmf_cb will then be called which will set up a call to
> > QMI_VOICE_STOP_CONTINUOUS_DTMF to stop the tone.  Finally,
> > stop_cont_dtmf_cb will check the final status.
> > 
> > ---
> > Changes in V4
> > -Removed unused enum
> > -Minor formatting fixes
> > -Ensure data->full_dtmf is free'd
> > -Use cb_data_ref/unref between chains of dtmf calls
> > 
> > Changes in V5
> > -Store the cb and cbd obejects in the voicall_data  struct
> > ---
> > ---
> > 
> >   drivers/qmimodem/voice.h     |   6 ++
> >   drivers/qmimodem/voicecall.c | 114 +++++++++++++++++++++++++++++++++++
> >   2 files changed, 120 insertions(+)
> 
> <snip>
> 
> > @@ -605,6 +609,114 @@ static void hangup_active(struct ofono_voicecall
> > *vc, ofono_voicecall_cb_t cb,> 
> >   	release_specific(vc, call->id, cb, data);
> >   
> >   }
> > 
> > +static void stop_cont_dtmf_cb(struct qmi_result *result, void *user_data)
> > +{
> > +	struct voicecall_data *vd = user_data;
> > +	uint16_t error;
> > +
> > +	DBG("");
> > +
> > +	if (qmi_result_set_error(result, &error)) {
> > +		DBG("QMI Error %d", error);
> > +		CALLBACK_WITH_FAILURE(vd->send_dtmf_cb, vd-
>send_dtmf_data);
> > +		return;
> > +	}
> > +
> > +	CALLBACK_WITH_SUCCESS(vd->send_dtmf_cb, vd->send_dtmf_data);
> 
> This calls back into oFono core unconditionally.  Doesn't this imply that
> you're handling only a single DTMF character?
> 
Currently, ive only been able to test using single character invocations, 
using the num-pad on the phone app.  Do you have any suggestions on how to 
trigger multiple characters?


> > +}
> > +
> 
> <snip>
> 
> > +static void send_one_dtmf(struct ofono_voicecall *vc, const char dtmf,
> > +				ofono_voicecall_cb_t cb, void 
*data)
> 
> Both cb and data are not saved anywhere and are only used on the error path.
> The initial invocation in send_dtmf() also passes in NULL for data...
> 
> So again, I think this implies only a request which uses a single DTMF tone
> character would work?

Im saving the cb and data params in the initial call, and used them as vd-
>,,,, where needed (i think!)
All the user_data params are being set to the vd object so I can get a handle 
on the params in there.
> 
> > +{
> > +	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
> > +	struct qmi_param *param = NULL;
> > +	uint8_t param_body[2];
> > +
> > +	DBG("");
> > +
> > +	param = qmi_param_new();
> > +
> > +	param_body[0] = 0xff;
> > +	param_body[1] = (uint8_t)dtmf;
> > +
> > +	if (!qmi_param_append(param, QMI_VOICE_DTMF_DATA, 
sizeof(param_body),
> > +			param_body))
> > +		goto error;
> > +
> > +	if (qmi_service_send(vd->voice, QMI_VOICE_START_CONTINUOUS_DTMF, 
param,
> > +			start_cont_dtmf_cb, vd, NULL) > 0)
> > +		return;
> > +
> > +error:
> > +	CALLBACK_WITH_FAILURE(cb, data);
> > +	l_free(param);
> > +}
> > +
> > +static void send_one_dtmf_cb(const struct ofono_error *error, void *data)
> > +{
> > +	struct cb_data *cbd = data;
> > +	struct voicecall_data *vd = ofono_voicecall_get_data(cbd->user);
> > +	ofono_voicecall_cb_t cb = vd->send_dtmf_cb;
> > +
> > +	DBG("");
> > +
> > +	if (error->type != OFONO_ERROR_TYPE_NO_ERROR ||
> > +			*vd->next_dtmf == 0) {
> > +		if (error->type == OFONO_ERROR_TYPE_NO_ERROR)
> > +			CALLBACK_WITH_SUCCESS(cb, cbd->data);
> > +		else
> > +			CALLBACK_WITH_FAILURE(cb, cbd->data);
> > +
> > +		l_free(vd->full_dtmf);
> > +		vd->full_dtmf = NULL;
> > +	} else
> > +		send_one_dtmf(cbd->user,
> > +				*(vd->next_dtmf++),
> > +				send_one_dtmf_cb, vd-
>send_dtmf_data);
> 
> This function doesn't seem to be invoked?
This function is passed into send_one_dtmf but yes, potentially goes unused, 
unless it gets invoked in the macro call?  Ill double-check the original 
implementation to see if i messed anything up, but open to suggestions :)
> 
> > +}
> > +
> 
> <snip>
> 
> Regards,
> -Denis

Re: [PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[Denis Kenzior]

Hi Adam,

On 4/23/24 05:10, Adam Pigg wrote:
> Hi Dennis
> 
> Thanks for the thorough review and merging the others....
> 

And thanks for doing all the hard work :)

<snip>

>>> +
>>> +	CALLBACK_WITH_SUCCESS(vd->send_dtmf_cb, vd->send_dtmf_data);
>>
>> This calls back into oFono core unconditionally.  Doesn't this imply that
>> you're handling only a single DTMF character?
>>
> Currently, ive only been able to test using single character invocations,
> using the num-pad on the phone app.  Do you have any suggestions on how to
> trigger multiple characters?

Take a look at doc/voicecallmanager-api.txt.  The SendTones() method takes a 
string of tones, so there can be multiple tones per request.  If your phone 
dialer UI does not support sending multiple tones, then the easiest way is to 
use a command line utility like 'dbus-send' (from dbus-1) or 'busctl' (from 
systemd), or a graphical one like d-feet or qdbus.

Something like:

dbus-send --print-reply --system --dest=org.ofono /phonesim 
org.ofono.VoicecallManager.SendTones string:"1234"

Replace /phonesim with the path of your modem device.

<snip>

>>
>> This function doesn't seem to be invoked?
> This function is passed into send_one_dtmf but yes, potentially goes unused,
> unless it gets invoked in the macro call?  Ill double-check the original
> implementation to see if i messed anything up, but open to suggestions :)

  I think your v4 implementation handled multiple characters correctly, just 
FYI.  Just the memory free paths were not completely correct.

Regards,
-Denis

Re: [PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[Adam Pigg]

So, I decided to have  a re-think and try the burst dtmf method, which accepts 
a string of chars, and should be simpler....

I created a packet struct for the parameter data:
struct qmi_dtmf_burst_info {
	uint8_t call_id;
	uint8_t length;
	char digits[0];
} __attribute__((__packed__));

and set it up as follows
	struct qmi_dtmf_burst_info *di;
	di = alloca(2 + strlen(dtmf));
	di->call_id = 0xff;
	di->length = strlen(dtmf);
	l_strlcpy(di->digits, dtmf, strlen(dtmf));
	param = qmi_param_new();
	if (!qmi_param_append(param, QMI_VOICE_DTMF_DATA, di->length + 2,
			di))
		goto error;
	if (qmi_service_send(vd->voice, QMI_VOICE_BURST_DTMF, param,
			send_dtmf_cb, cbd, l_free) > 0)
		return;
But,  in the callback, in a call to qmi_result_set_error, i get back error 24, 
which is QMI_PROTOCOL_ERROR_NETWORK_UNSUPPORTED right?

Not sure where ivce gone wrong, the struct seems to match the format I found 
online for this function call, but is the likely culprit, or its not possible 
to do a burst dtmf, and its better to go back to the original style of 
individual characters?

Thanks


On Tuesday 23 April 2024 16:01:10 BST Denis Kenzior wrote:
> Hi Adam,
> 
> On 4/23/24 05:10, Adam Pigg wrote:
> > Hi Dennis
> > 
> > Thanks for the thorough review and merging the others....
> 
> And thanks for doing all the hard work :)
> 
> <snip>
> 
> >>> +
> >>> +	CALLBACK_WITH_SUCCESS(vd->send_dtmf_cb, vd->send_dtmf_data);
> >> 
> >> This calls back into oFono core unconditionally.  Doesn't this imply that
> >> you're handling only a single DTMF character?
> > 
> > Currently, ive only been able to test using single character invocations,
> > using the num-pad on the phone app.  Do you have any suggestions on how to
> > trigger multiple characters?
> 
> Take a look at doc/voicecallmanager-api.txt.  The SendTones() method takes a
> string of tones, so there can be multiple tones per request.  If your phone
> dialer UI does not support sending multiple tones, then the easiest way is
> to use a command line utility like 'dbus-send' (from dbus-1) or 'busctl'
> (from systemd), or a graphical one like d-feet or qdbus.
> 
> Something like:
> 
> dbus-send --print-reply --system --dest=org.ofono /phonesim
> org.ofono.VoicecallManager.SendTones string:"1234"
> 
> Replace /phonesim with the path of your modem device.
> 
> <snip>
> 
> >> This function doesn't seem to be invoked?
> > 
> > This function is passed into send_one_dtmf but yes, potentially goes
> > unused, unless it gets invoked in the macro call?  Ill double-check the
> > original implementation to see if i messed anything up, but open to
> > suggestions :)
>   I think your v4 implementation handled multiple characters correctly, just
> FYI.  Just the memory free paths were not completely correct.
> 
> Regards,
> -Denis

Re: [PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[adam]

On Tuesday 23 April 2024 22:54:57 BST Adam Pigg wrote:
> So, I decided to have  a re-think and try the burst dtmf method, which
> accepts a string of chars, and should be simpler....
> 
> I created a packet struct for the parameter data:
> struct qmi_dtmf_burst_info {
> 	uint8_t call_id;
> 	uint8_t length;
> 	char digits[0];
> } __attribute__((__packed__));
> 
> and set it up as follows
> 	struct qmi_dtmf_burst_info *di;
> 	di = alloca(2 + strlen(dtmf));
> 	di->call_id = 0xff;
> 	di->length = strlen(dtmf);
> 	l_strlcpy(di->digits, dtmf, strlen(dtmf));
> 	param = qmi_param_new();
> 	if (!qmi_param_append(param, QMI_VOICE_DTMF_DATA, di->length + 2,
> 			di))
> 		goto error;
> 	if (qmi_service_send(vd->voice, QMI_VOICE_BURST_DTMF, param,
> 			send_dtmf_cb, cbd, l_free) > 0)
> 		return;
> But,  in the callback, in a call to qmi_result_set_error, i get back error
> 24, which is QMI_PROTOCOL_ERROR_NETWORK_UNSUPPORTED right?
> 
Some further research suggested burst dtmf is only supported on 3gpp2 
(cdma2000?) .. so maybe the original implementation is more suitable.


> Not sure where ivce gone wrong, the struct seems to match the format I found
> online for this function call, but is the likely culprit, or its not
> possible to do a burst dtmf, and its better to go back to the original
> style of individual characters?
> 
> Thanks
> 
> On Tuesday 23 April 2024 16:01:10 BST Denis Kenzior wrote:
> > Hi Adam,
> > 
> > On 4/23/24 05:10, Adam Pigg wrote:
> > > Hi Dennis
> > > 
> > > Thanks for the thorough review and merging the others....
> > 
> > And thanks for doing all the hard work :)
> > 
> > <snip>
> > 
> > >>> +
> > >>> +	CALLBACK_WITH_SUCCESS(vd->send_dtmf_cb, vd->send_dtmf_data);
> > >> 
> > >> This calls back into oFono core unconditionally.  Doesn't this imply
> > >> that
> > >> you're handling only a single DTMF character?
> > > 
> > > Currently, ive only been able to test using single character
> > > invocations,
> > > using the num-pad on the phone app.  Do you have any suggestions on how
> > > to
> > > trigger multiple characters?
> > 
> > Take a look at doc/voicecallmanager-api.txt.  The SendTones() method takes
> > a string of tones, so there can be multiple tones per request.  If your
> > phone dialer UI does not support sending multiple tones, then the easiest
> > way is to use a command line utility like 'dbus-send' (from dbus-1) or
> > 'busctl' (from systemd), or a graphical one like d-feet or qdbus.
> > 
> > Something like:
> > 
> > dbus-send --print-reply --system --dest=org.ofono /phonesim
> > org.ofono.VoicecallManager.SendTones string:"1234"
> > 
> > Replace /phonesim with the path of your modem device.
> > 
> > <snip>
> > 
> > >> This function doesn't seem to be invoked?
> > > 
> > > This function is passed into send_one_dtmf but yes, potentially goes
> > > unused, unless it gets invoked in the macro call?  Ill double-check the
> > > original implementation to see if i messed anything up, but open to
> > > suggestions :)
> > > 
> >   I think your v4 implementation handled multiple characters correctly,
> >   just
> > 
> > FYI.  Just the memory free paths were not completely correct.
> > 
> > Regards,
> > -Denis

Re: [PATCH 4/4] qmimodem: voicecall: Implement DTMF tones

[Denis Kenzior]

Hi Adam,

On 4/25/24 2:07 PM, adam@piggz.co.uk wrote:
> On Tuesday 23 April 2024 22:54:57 BST Adam Pigg wrote:
>> So, I decided to have  a re-think and try the burst dtmf method, which
>> accepts a string of chars, and should be simpler....
>>

Yeah, that's what I was wondering as well, whether this Burst DTMF command was 
an easier approach.

>> I created a packet struct for the parameter data:
>> struct qmi_dtmf_burst_info {
>> 	uint8_t call_id;
>> 	uint8_t length;
>> 	char digits[0];
>> } __attribute__((__packed__));
>>
>> and set it up as follows
>> 	struct qmi_dtmf_burst_info *di;
>> 	di = alloca(2 + strlen(dtmf));
>> 	di->call_id = 0xff;
>> 	di->length = strlen(dtmf);
>> 	l_strlcpy(di->digits, dtmf, strlen(dtmf));
>> 	param = qmi_param_new();
>> 	if (!qmi_param_append(param, QMI_VOICE_DTMF_DATA, di->length + 2,
>> 			di))
>> 		goto error;
>> 	if (qmi_service_send(vd->voice, QMI_VOICE_BURST_DTMF, param,
>> 			send_dtmf_cb, cbd, l_free) > 0)
>> 		return;

Looks reasonable.

>> But,  in the callback, in a call to qmi_result_set_error, i get back error
>> 24, which is QMI_PROTOCOL_ERROR_NETWORK_UNSUPPORTED right?
>> > Some further research suggested burst dtmf is only supported on 3gpp2
> (cdma2000?) .. so maybe the original implementation is more suitable.
> 

Yeah, ERROR_NETWORK_UNSUPPORTED would seem to support this conclusion.  I think 
your v4 version was pretty close to being ready to be upstreamed.

Regards,
-Denis

[PATCH 3/4] [qmimodem][voicecall] Implement active call hangup

[Adam Pigg]

---
 drivers/qmimodem/voice.h     |   9 +++
 drivers/qmimodem/voicecall.c | 113 +++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+)

diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h
index 9c31297b..244a6f85 100644
--- a/drivers/qmimodem/voice.h
+++ b/drivers/qmimodem/voice.h
@@ -54,6 +54,7 @@ enum qmi_ussd_user_required {
 /* QMI service voice. Using an enum to prevent doublicated entries */
 enum voice_commands {
 	QMI_VOICE_DIAL_CALL =			0x20,
+	QMI_VOICE_END_CALL =			0x21,
 	QMI_VOICE_ANSWER_CALL =			0x22,
 	QMI_VOICE_SUPS_NOTIFICATION_IND =	0x32,
 	QMI_VOICE_SET_SUPS_SERVICE =		0x33,
@@ -119,6 +120,14 @@ enum qmi_voice_call_answer_return {
 	QMI_VOICE_ANSWER_RETURN_CALL_ID = 0x10,
 };
 
+enum qmi_voice_call_end_param {
+	QMI_VOICE_END_CALL_ID = 0x01,
+};
+
+enum qmi_voice_call_end_return {
+	QMI_VOICE_END_RETURN_CALL_ID = 0x10,
+};
+
 enum parse_error {
 	NONE = 0,
 	MISSING_MANDATORY = 1,
diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c
index 7c6bc113..4ef8adc1 100644
--- a/drivers/qmimodem/voicecall.c
+++ b/drivers/qmimodem/voicecall.c
@@ -107,6 +107,16 @@ struct qmi_voice_answer_call_result {
 	uint8_t call_id;
 };
 
+struct qmi_voice_end_call_arg {
+	bool call_id_set;
+	uint8_t call_id;
+};
+
+struct qmi_voice_end_call_result {
+	bool call_id_set;
+	uint8_t call_id;
+};
+
 int ofono_call_compare(const void *a, const void *b, void *data)
 {
 	const struct ofono_call *ca = a;
@@ -631,6 +641,107 @@ error:
 	l_free(param);
 }
 
+enum parse_error
+qmi_voice_end_call_parse(struct qmi_result *qmi_result,
+			 struct qmi_voice_end_call_result *result)
+{
+	int err = NONE;
+
+	/* optional */
+	if (qmi_result_get_uint8(qmi_result, QMI_VOICE_END_RETURN_CALL_ID, &result->call_id))
+		result->call_id_set = 1;
+
+	return err;
+}
+
+static void end_call_cb(struct qmi_result *result, void *user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_voicecall_cb_t cb = cbd->cb;
+	uint16_t error;
+	struct qmi_voice_end_call_result end_result;
+
+	if (qmi_result_set_error(result, &error)) {
+		DBG("QMI Error %d", error);
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	if (qmi_voice_end_call_parse(result, &end_result) != NONE) {
+		DBG("Received invalid Result");
+		CALLBACK_WITH_FAILURE(cb, cbd->data);
+		return;
+	}
+
+	CALLBACK_WITH_SUCCESS(cb, cbd->data);
+}
+
+static void release_specific(struct ofono_voicecall *vc, int id,
+		     ofono_voicecall_cb_t cb, void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct cb_data *cbd = cb_data_new(cb, data);
+	struct qmi_voice_end_call_arg arg;
+	struct qmi_param *param = NULL;
+
+	DBG("");
+	cbd->user = vc;
+
+	arg.call_id_set = true;
+	arg.call_id = id;
+
+	param = qmi_param_new();
+	if (!param)
+		goto error;
+
+	if (arg.call_id_set) {
+		if (!qmi_param_append_uint8(param, QMI_VOICE_END_CALL_ID, arg.call_id)) {
+			goto error;
+		}
+	}
+
+	if (qmi_service_send(vd->voice, QMI_VOICE_END_CALL, param, end_call_cb, cbd, l_free) >
+	    0) {
+		return;
+	}
+
+error:
+	CALLBACK_WITH_FAILURE(cb, data);
+	l_free(cbd);
+	l_free(param);
+}
+
+static void hangup_active(struct ofono_voicecall *vc, ofono_voicecall_cb_t cb,
+			  void *data)
+{
+	struct voicecall_data *vd = ofono_voicecall_get_data(vc);
+	struct ofono_call *call;
+	enum call_status active[] = {
+		CALL_STATUS_ACTIVE,
+		CALL_STATUS_DIALING,
+		CALL_STATUS_ALERTING,
+		CALL_STATUS_INCOMING,
+	};
+	int i;
+
+	DBG("");
+	for (i = 0; i < L_ARRAY_SIZE(active); i++) {
+		call = l_queue_find(vd->call_list, ofono_call_compare_by_status,
+				    L_INT_TO_PTR(active[i]));
+
+		if (call)
+			break;
+	}
+
+	if (call == NULL) {
+		DBG("Can not find a call to hang up");
+		CALLBACK_WITH_FAILURE(cb, data);
+		return;
+	}
+
+	release_specific(vc, call->id, cb, data);
+}
+
 static void create_voice_cb(struct qmi_service *service, void *user_data)
 {
 	struct ofono_voicecall *vc = user_data;
@@ -697,6 +808,8 @@ static const struct ofono_voicecall_driver driver = {
 	.remove		= qmi_voicecall_remove,
 	.dial		= dial,
 	.answer		= answer,
+	.hangup_active  = hangup_active,
+	.release_specific  = release_specific,
 };
 
 OFONO_ATOM_DRIVER_BUILTIN(voicecall, qmimodem, &driver)
-- 
2.44.0