| Message ID | 20240326102054.30946-4-adam@piggz.co.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/4,qmimodem,voicecall] Implement call dialing | expand |
Hi Adam, On 3/26/24 05:19, 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 paramters 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. > --- > drivers/qmimodem/voice.h | 6 ++ > drivers/qmimodem/voicecall.c | 154 +++++++++++++++++++++++++++++++++++ > 2 files changed, 160 insertions(+) > CI says: 0004-Implement-DTMF-tones.patch:7: WARNING: 'paramters' may be misspelled - perhaps 'parameters'? 0004-Implement-DTMF-tones.patch:110: WARNING: braces {} are not necessary for single statement blocks 0004-Implement-DTMF-tones.patch:114: WARNING: braces {} are not necessary for single statement blocks 0004-Implement-DTMF-tones.patch:177: WARNING: braces {} are not necessary for any arm of this statement <snip> > diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c > index 4ef8adc1..ab4bd655 100644 > --- a/drivers/qmimodem/voicecall.c > +++ b/drivers/qmimodem/voicecall.c > @@ -117,6 +117,21 @@ struct qmi_voice_end_call_result { > uint8_t call_id; > }; > > +struct send_one_dtmf_cb_data { > + const char *full_dtmf; > + const char *next_dtmf; > + struct ofono_voicecall *vc; > +}; > + > +struct qmi_voice_start_cont_dtmf_arg { > + uint8_t call_id; > + uint8_t dtmf_char; > +}; > + > +struct qmi_voice_stop_cont_dtmf_arg { > + uint8_t call_id; > +}; > + The two arg structures can be removed, they don't really serve any purpose. > int ofono_call_compare(const void *a, const void *b, void *data) > { > const struct ofono_call *ca = a; > @@ -742,6 +757,144 @@ 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 cb_data *cbd = user_data; > + ofono_voicecall_cb_t cb = cbd->cb; > + > + uint16_t error; > + > + DBG(""); > + > + if (qmi_result_set_error(result, &error)) { > + DBG("QMI Error %d", error); > + CALLBACK_WITH_FAILURE(cb, cbd->data); > + return; > + } > + > + CALLBACK_WITH_SUCCESS(cb, cbd->data); > +} > + > +static void start_cont_dtmf_cb(struct qmi_result *result, void *user_data) > +{ > + struct cb_data *cbd = user_data; > + ofono_voicecall_cb_t cb = cbd->cb; > + struct ofono_voicecall *vc = cbd->user; > + struct voicecall_data *vd = ofono_voicecall_get_data(vc); > + struct qmi_voice_stop_cont_dtmf_arg arg; > + uint16_t error; > + struct qmi_param *param = NULL; > + > + DBG(""); > + > + if (qmi_result_set_error(result, &error)) { > + DBG("QMI Error %d", error); > + CALLBACK_WITH_FAILURE(cb, cbd->data); > + return; > + } > + > + arg.call_id = 0xff; > + > + param = qmi_param_new(); > + if (!param) { > + goto error; > + } > + > + if (!qmi_param_append_uint8(param, QMI_VOICE_DTMF_DATA, arg.call_id)) { > + goto error; > + } > + > + if (qmi_service_send(vd->voice, QMI_VOICE_STOP_CONTINUOUS_DTMF, param, > + stop_cont_dtmf_cb, cbd, NULL) > 0) { > + return; > + } > + > +error: > + CALLBACK_WITH_FAILURE(cb, cbd->data); > + l_free(param); > +} > + > +static void send_one_dtmf(struct ofono_voicecall *vc, const char dtmf, > + ofono_voicecall_cb_t cb, void *data) > +{ > + struct qmi_voice_start_cont_dtmf_arg arg; > + struct voicecall_data *vd = ofono_voicecall_get_data(vc); > + struct qmi_param *param = NULL; > + uint8_t param_body[2]; > + struct cb_data *cbd = cb_data_new(cb, data); > + > + DBG(""); > + > + arg.call_id = 0xff; > + arg.dtmf_char = (uint8_t)dtmf; > + > + cbd->user = vc; > + > + param = qmi_param_new(); > + if (!param) > + goto error; > + > + param_body[0] = arg.call_id; > + param_body[1] = arg.dtmf_char; > + > + 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, cbd, NULL) > 0) { > + return; > + } > + > +error: > + CALLBACK_WITH_FAILURE(cb, data); > + l_free(cbd); > + l_free(param); > +} > + > +static void send_one_dtmf_cb(const struct ofono_error *error, void *data) > +{ > + struct cb_data *cbd = data; > + ofono_voicecall_cb_t cb = cbd->cb; > + struct send_one_dtmf_cb_data *send_one_dtmf_cb_data = cbd->user; > + > + DBG(""); > + > + if (error->type != OFONO_ERROR_TYPE_NO_ERROR || > + *send_one_dtmf_cb_data->next_dtmf == 0) { > + if (error->type == OFONO_ERROR_TYPE_NO_ERROR) { > + CALLBACK_WITH_SUCCESS(cb, cbd->data); > + } else { > + CALLBACK_WITH_FAILURE(cb, cbd->data); > + } If any DTMF fails, you should return an error right away, cleaning up regardless. Then, if it is the last entry, callback with success and cleanup. Otherwise, send the next DTMF. > + l_free((void *)send_one_dtmf_cb_data->full_dtmf); > + l_free(send_one_dtmf_cb_data); > + l_free(cbd); > + } else { > + send_one_dtmf(send_one_dtmf_cb_data->vc, > + *(send_one_dtmf_cb_data->next_dtmf++), > + send_one_dtmf_cb, data); > + } > +} > + > +static void send_dtmf(struct ofono_voicecall *vc, const char *dtmf, > + ofono_voicecall_cb_t cb, void *data) > +{ > + struct cb_data *cbd = cb_data_new(cb, data); > + struct send_one_dtmf_cb_data *send_one_dtmf_cb_data = > + l_new(struct send_one_dtmf_cb_data, 1); > + > + DBG(""); > + > + send_one_dtmf_cb_data->full_dtmf = l_strdup(dtmf); full_dtmf is declared const. How are you freeing the string? > + send_one_dtmf_cb_data->next_dtmf = &send_one_dtmf_cb_data->full_dtmf[1]; > + send_one_dtmf_cb_data->vc = vc; > + cbd->user = send_one_dtmf_cb_data; cb_data is not setup to destroy cbd->user on unref. So it is likely this structure is leaked on some of the code paths. This is something we can add, for example by introducing a destroy member to struct cb_data. However, it may be far easier to just put dtmf information into voicecall_data. Have you looked into using 'Burst DTMF' instead of Start/Stop Continuous DTMF? Looks like that one takes a string. Might be an easier implementation. > + > + send_one_dtmf(vc, *dtmf, send_one_dtmf_cb, cbd); > +} > + > static void create_voice_cb(struct qmi_service *service, void *user_data) > { > struct ofono_voicecall *vc = user_data; > @@ -810,6 +963,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) Regards, -Denis
diff --git a/drivers/qmimodem/voice.h b/drivers/qmimodem/voice.h index 244a6f85..42b6b9b6 100644 --- a/drivers/qmimodem/voice.h +++ b/drivers/qmimodem/voice.h @@ -56,6 +56,8 @@ enum voice_commands { QMI_VOICE_DIAL_CALL = 0x20, 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, @@ -128,6 +130,10 @@ enum qmi_voice_call_end_return { QMI_VOICE_END_RETURN_CALL_ID = 0x10, }; +enum qmi_voice_call_dtmf_param { + QMI_VOICE_DTMF_DATA = 0x01, +}; + enum parse_error { NONE = 0, MISSING_MANDATORY = 1, diff --git a/drivers/qmimodem/voicecall.c b/drivers/qmimodem/voicecall.c index 4ef8adc1..ab4bd655 100644 --- a/drivers/qmimodem/voicecall.c +++ b/drivers/qmimodem/voicecall.c @@ -117,6 +117,21 @@ struct qmi_voice_end_call_result { uint8_t call_id; }; +struct send_one_dtmf_cb_data { + const char *full_dtmf; + const char *next_dtmf; + struct ofono_voicecall *vc; +}; + +struct qmi_voice_start_cont_dtmf_arg { + uint8_t call_id; + uint8_t dtmf_char; +}; + +struct qmi_voice_stop_cont_dtmf_arg { + uint8_t call_id; +}; + int ofono_call_compare(const void *a, const void *b, void *data) { const struct ofono_call *ca = a; @@ -742,6 +757,144 @@ 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 cb_data *cbd = user_data; + ofono_voicecall_cb_t cb = cbd->cb; + + uint16_t error; + + DBG(""); + + if (qmi_result_set_error(result, &error)) { + DBG("QMI Error %d", error); + CALLBACK_WITH_FAILURE(cb, cbd->data); + return; + } + + CALLBACK_WITH_SUCCESS(cb, cbd->data); +} + +static void start_cont_dtmf_cb(struct qmi_result *result, void *user_data) +{ + struct cb_data *cbd = user_data; + ofono_voicecall_cb_t cb = cbd->cb; + struct ofono_voicecall *vc = cbd->user; + struct voicecall_data *vd = ofono_voicecall_get_data(vc); + struct qmi_voice_stop_cont_dtmf_arg arg; + uint16_t error; + struct qmi_param *param = NULL; + + DBG(""); + + if (qmi_result_set_error(result, &error)) { + DBG("QMI Error %d", error); + CALLBACK_WITH_FAILURE(cb, cbd->data); + return; + } + + arg.call_id = 0xff; + + param = qmi_param_new(); + if (!param) { + goto error; + } + + if (!qmi_param_append_uint8(param, QMI_VOICE_DTMF_DATA, arg.call_id)) { + goto error; + } + + if (qmi_service_send(vd->voice, QMI_VOICE_STOP_CONTINUOUS_DTMF, param, + stop_cont_dtmf_cb, cbd, NULL) > 0) { + return; + } + +error: + CALLBACK_WITH_FAILURE(cb, cbd->data); + l_free(param); +} + +static void send_one_dtmf(struct ofono_voicecall *vc, const char dtmf, + ofono_voicecall_cb_t cb, void *data) +{ + struct qmi_voice_start_cont_dtmf_arg arg; + struct voicecall_data *vd = ofono_voicecall_get_data(vc); + struct qmi_param *param = NULL; + uint8_t param_body[2]; + struct cb_data *cbd = cb_data_new(cb, data); + + DBG(""); + + arg.call_id = 0xff; + arg.dtmf_char = (uint8_t)dtmf; + + cbd->user = vc; + + param = qmi_param_new(); + if (!param) + goto error; + + param_body[0] = arg.call_id; + param_body[1] = arg.dtmf_char; + + 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, cbd, NULL) > 0) { + return; + } + +error: + CALLBACK_WITH_FAILURE(cb, data); + l_free(cbd); + l_free(param); +} + +static void send_one_dtmf_cb(const struct ofono_error *error, void *data) +{ + struct cb_data *cbd = data; + ofono_voicecall_cb_t cb = cbd->cb; + struct send_one_dtmf_cb_data *send_one_dtmf_cb_data = cbd->user; + + DBG(""); + + if (error->type != OFONO_ERROR_TYPE_NO_ERROR || + *send_one_dtmf_cb_data->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((void *)send_one_dtmf_cb_data->full_dtmf); + l_free(send_one_dtmf_cb_data); + l_free(cbd); + } else { + send_one_dtmf(send_one_dtmf_cb_data->vc, + *(send_one_dtmf_cb_data->next_dtmf++), + send_one_dtmf_cb, data); + } +} + +static void send_dtmf(struct ofono_voicecall *vc, const char *dtmf, + ofono_voicecall_cb_t cb, void *data) +{ + struct cb_data *cbd = cb_data_new(cb, data); + struct send_one_dtmf_cb_data *send_one_dtmf_cb_data = + l_new(struct send_one_dtmf_cb_data, 1); + + DBG(""); + + send_one_dtmf_cb_data->full_dtmf = l_strdup(dtmf); + send_one_dtmf_cb_data->next_dtmf = &send_one_dtmf_cb_data->full_dtmf[1]; + send_one_dtmf_cb_data->vc = vc; + cbd->user = send_one_dtmf_cb_data; + + send_one_dtmf(vc, *dtmf, send_one_dtmf_cb, cbd); +} + static void create_voice_cb(struct qmi_service *service, void *user_data) { struct ofono_voicecall *vc = user_data; @@ -810,6 +963,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)