diff mbox series

[v2,3/4,qmimodem,voicecall] Implement active call hangup

Message ID 20240326102054.30946-3-adam@piggz.co.uk (mailing list archive)
State Superseded
Headers show
Series [v2,1/4,qmimodem,voicecall] Implement call dialing | expand

Commit Message

Adam Pigg March 26, 2024, 10:19 a.m. UTC 
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 paramters
being the call-id.

end_call_cb will then be called and will parse out the call-id and check
for success.
---
 drivers/qmimodem/voice.h     |   9 +++
 drivers/qmimodem/voicecall.c | 113 +++++++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+)

Comments

Denis Kenzior March 27, 2024, 6:49 p.m. UTC | #1 
Hi Adam,

On 3/26/24 05:19, Adam Pigg wrote:
> 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 paramters
> being the call-id.
> 
> end_call_cb will then be called and will parse out the call-id and check
> for success.
> ---
>   drivers/qmimodem/voice.h     |   9 +++
>   drivers/qmimodem/voicecall.c | 113 +++++++++++++++++++++++++++++++++++
>   2 files changed, 122 insertions(+)
> 

Reported by CI:
0003-Implement-active-call-hangup.patch:8: WARNING: 'paramters' may be 
misspelled - perhaps 'parameters'?
0003-Implement-active-call-hangup.patch:124: WARNING: braces {} are not 
necessary for single statement blocks

<snip>

> 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;
> +};

Again, totally unneeded structures.

> +
>   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;
> +

inline this in the end_call_cb

> +	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) {

This function doesn't actually fail, so again, is this block even needed?

> +		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;

Fill out qmi_param directly, no need for this intermediate arg structure.

> +
> +	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;
> +	}

Might be easier to do something like:

const struct l_queue_entry *entry;

for (entry = l_queue_get_entries(vd->call_list); entry; entry = entry->next) {
	struct ofono_call *call = entry->data;

	if (L_IN_SET(call->status, CALL_STATUS_ACTIVE,
			CALL_STATUS_DIALING,
			CALL_STATUS_ALERTING,
			CALL_STATUS_INCOMING)) {
		release_specific(vc, call->id, cb, data);
		return;
	}
}

DBG("Can not find...");
CALLBACK_WITH_FAILURE(...);

> +
> +	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)

Regards,
-Denis
diff mbox series

Patch

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)