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

[Denis Kenzior <denkenz@gmail.com>]

Hi Adam,

On 4/2/24 16:25, 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

typo: '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     |  1 +
>   drivers/qmimodem/voicecall.c | 84 +++++++++++++++++++++++++++++++++++-
>   2 files changed, 84 insertions(+), 1 deletion(-)
> 

<snip>

> +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_param *param = NULL;
> +
> +	static const uint8_t QMI_VOICE_END_CALL_ID = 0x01;
> +
> +	DBG("");
> +	cbd->user = vc;
> +
> +	param = qmi_param_new();

As mentioned before, qmi_param_new() can't fail, so feel free to drop !param check

> +	if (!param)
> +		goto error;
> +
> +	if (!qmi_param_append_uint8(param, QMI_VOICE_END_CALL_ID, id))
> +		goto error;
> +
> +	if (qmi_service_send(vd->voice, QMI_VOICE_END_CALL, param, end_call_cb,
> +		cbd, l_free) > 0)

item M4

> +		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("");

item M1

> +	for (uint32_t 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;
> @@ -594,7 +674,7 @@ 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);
> +				all_call_status_ind, vc, NULL);

This belongs in patch 1.

>   
>   	ofono_voicecall_register(vc);
>   }

Regards,
-Denis