FW: [RFC 3/3] STE-plugin: Adding STE plugin
Hi Denis. Denis Kenzior denk...@gmail.com wrote: +static void ste_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 release_id_req *req = g_try_new0(struct release_id_req, 1); +char buf[32]; + struct ofono_call *call; +GSList *l; +int ac = 0; + +if (!req) +goto error; + +req-vc = vc; +req-cb = cb; +req-data = data; +req-id = id; + +/* Count active calls. If there is more than one active call + * we cannot use ATH, as it will terminate all calls. + * The reason for using ATH and not CHLD is that + * emergency calls can not be terminated with AT+CHLD. + */ +for (l = vd-calls; l; l = l-next) { +call = l-data; + +if (call-status == CALL_STATUS_ACTIVE) +ac++; +} + +l = g_slist_find_custom(vd-calls, GUINT_TO_POINTER(id), +call_compare_by_id); + if (l == NULL) { +ofono_error(Hangup request for unknow call); +goto error; +} +call = l-data; +/* Check the state of the call, as AT+CHLD does not terminate + * calls in state Dialing, Alerting and Incoming */ +if (call-status == CALL_STATUS_DIALING || +call-status == CALL_STATUS_ALERTING || +call-status == CALL_STATUS_INCOMING) +sprintf(buf, ATH); + +/* Waiting call can not be terminated by at+chld=1x, + * have to use at+chld = 0, but that will also terminate + * other held calls. Bug in STE's AT module. + */ +else if (call-status == CALL_STATUS_WAITING) +sprintf(buf, AT+CHLD=0); + +else { +/* A held call can not be released by ATH, need to use CHLD */ +if (ac 1 || call-status == CALL_STATUS_HELD) +sprintf(buf, AT+CHLD=1%d, id); +else /* This is the last call, ok to use ATH. */ +sprintf(buf, ATH); +} + +if (g_at_chat_send(vd-chat, buf, none_prefix, +release_id_cb, req, g_free) 0) +return; + +error: +if (req) +g_free(req); + +CALLBACK_WITH_FAILURE(cb, data); +} All of this logic needs to go away. The core already handles all of this, including selection of ATH/CHLD, waiting/held. Please review src/voicecall.c. If the core logic is not sufficient or does not properly handle a certain case, then lets see if we can fix the core first. Drivers should not concern themselves with this stuff. OK, we can remove the state logic, but STE modem cannot terminate calls in state DIALING and ALERTING with CHLD=1X. We need to use ATH (or AT+CHUP) instead. For now I think we will remove the state logic from ste_release_specific as you suggest. Terminating calls in state DIALING and ALERTING must then be handled by the Core part. The simplest would probably be to use hangup in this case, but I suspect hangup work differently for different modems. So if you cannot use hangup as the general approach, maybe it could be implemented by adding callback functions release_dialing and release_alerting in struct ofono_voicecall_driver. The STE driver could send ATH from release_dialing and release_alerting, other drivers could leave them empty and this could default to use release_specific instead? With this in mind, you might not need to track any state in this driver at all. See drivers/calypsomodem/voicecall.c for details. TI's CPI notifications are almost exactly the same as the STE ECAV. The STE ECAV update is giving delta updates on the state information, right now I don't see how we can get the ofono_voicall_notify right without keeping some state information in ecav_notify. BR/Sjur ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH 1/2] Remove unneeded code to disable the modem on hfp
ofono_modem_remove() already disables the modem. --- plugins/hfp.c |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/plugins/hfp.c b/plugins/hfp.c index 867b194..2d0faa8 100644 --- a/plugins/hfp.c +++ b/plugins/hfp.c @@ -397,9 +397,6 @@ static DBusMessage *hfp_agent_release(DBusConnection *conn, DBusMessage *msg, vo { struct ofono_modem *modem = data; - if (ofono_modem_get_powered(modem)) - hfp_disable(modem); - ofono_modem_remove(modem); return dbus_message_new_method_return(msg); -- 1.6.4.4 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH 2/2] Handle the error path from service_level_connection
--- plugins/hfp.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/hfp.c b/plugins/hfp.c index 2d0faa8..d9d3bda 100644 --- a/plugins/hfp.c +++ b/plugins/hfp.c @@ -377,7 +377,7 @@ static int service_level_connection(struct ofono_modem *modem, int fd) static DBusMessage *hfp_agent_new_connection(DBusConnection *conn, DBusMessage *msg, void *data) { - int fd; + int fd, err; struct ofono_modem *modem = data; struct hfp_data *hfp_data = ofono_modem_get_data(modem); @@ -385,7 +385,9 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn, DBusMessage * DBUS_TYPE_INVALID)) return __ofono_error_invalid_args(msg); - service_level_connection(modem, fd); + err = service_level_connection(modem, fd); + if (err 0 err != -EINPROGRESS) + return __ofono_error_failed(msg); hfp_data-slc_msg = msg; dbus_message_ref(msg); -- 1.6.4.4 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Add STE voice call support.
From: Sjur Brændeland sjur.brandel...@stericsson.com --- Makefile.am |1 + drivers/stemodem/stemodem.c |2 + drivers/stemodem/stemodem.h |3 + drivers/stemodem/voicecall.c | 596 ++ plugins/ste.c|2 +- 5 files changed, 603 insertions(+), 1 deletions(-) create mode 100644 drivers/stemodem/voicecall.c diff --git a/Makefile.am b/Makefile.am index ac13d73..ecd2660 100644 --- a/Makefile.am +++ b/Makefile.am @@ -159,6 +159,7 @@ builtin_sources += drivers/atmodem/atutil.h \ drivers/stemodem/stemodem.h \ drivers/stemodem/stemodem.c \ drivers/stemodem/gprs-context.c \ + drivers/stemodem/voicecall.c \ drivers/stemodem/caif_socket.h \ drivers/stemodem/if_caif.h diff --git a/drivers/stemodem/stemodem.c b/drivers/stemodem/stemodem.c index 53207db..9184a42 100644 --- a/drivers/stemodem/stemodem.c +++ b/drivers/stemodem/stemodem.c @@ -37,6 +37,7 @@ static int stemodem_init(void) { ste_gprs_context_init(); + ste_voicecall_init(); return 0; } @@ -44,6 +45,7 @@ static int stemodem_init(void) static void stemodem_exit(void) { ste_gprs_context_exit(); + ste_voicecall_exit(); } OFONO_PLUGIN_DEFINE(stemodem, STE modem driver, VERSION, diff --git a/drivers/stemodem/stemodem.h b/drivers/stemodem/stemodem.h index e55a2c3..e7c6934 100644 --- a/drivers/stemodem/stemodem.h +++ b/drivers/stemodem/stemodem.h @@ -25,3 +25,6 @@ extern void ste_gprs_context_init(); extern void ste_gprs_context_exit(); +extern void ste_voicecall_init(); +extern void ste_voicecall_exit(); + diff --git a/drivers/stemodem/voicecall.c b/drivers/stemodem/voicecall.c new file mode 100644 index 000..e74aa3d --- /dev/null +++ b/drivers/stemodem/voicecall.c @@ -0,0 +1,596 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. + * Copyright (C) 2010 ST-Ericsson AB. + * + * 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 + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA + * + */ + +#ifdef HAVE_CONFIG_H +#include config.h +#endif + +#define _GNU_SOURCE +#include string.h +#include stdlib.h +#include stdio.h + +#include glib.h + +#include ofono/log.h +#include ofono/modem.h +#include ofono/voicecall.h + +#include gatchat.h +#include gatresult.h +#include common.h + +#include stemodem.h + +enum call_status_ste { + STE_CALL_STATUS_IDLE = 0, + STE_CALL_STATUS_CALLING = 1, + STE_CALL_STATUS_CONNECTING = 2, + STE_CALL_STATUS_ACTIVE = 3, + STE_CALL_STATUS_HOLD = 4, + STE_CALL_STATUS_WAITING = 5, + STE_CALL_STATUS_ALERTING = 6, + STE_CALL_STATUS_BUSY = 7 +}; + +static const char *none_prefix[] = { NULL }; + +struct voicecall_data { + GSList *calls; + unsigned int local_release; + GAtChat *chat; +}; + +struct release_id_req { + struct ofono_voicecall *vc; + ofono_voicecall_cb_t cb; + void *data; + int id; +}; + +struct change_state_req { + struct ofono_voicecall *vc; + ofono_voicecall_cb_t cb; + void *data; + int affected_types; +}; + +/* Translate from the ECAV-based STE-status to CLCC based status */ +static int call_status_ste_to_ofono(int status) +{ + switch (status) { + case STE_CALL_STATUS_IDLE: + return CALL_STATUS_DISCONNECTED; + case STE_CALL_STATUS_CALLING: + return CALL_STATUS_DIALING; + case STE_CALL_STATUS_CONNECTING: + return CALL_STATUS_ALERTING; + case STE_CALL_STATUS_ACTIVE: + return CALL_STATUS_ACTIVE; + case STE_CALL_STATUS_HOLD: + return CALL_STATUS_HELD; + case STE_CALL_STATUS_WAITING: + return CALL_STATUS_WAITING; + case STE_CALL_STATUS_ALERTING: + return CALL_STATUS_INCOMING; + case STE_CALL_STATUS_BUSY: + return CALL_STATUS_DISCONNECTED; + default: + return CALL_STATUS_DISCONNECTED; + } +} + +static struct ofono_call *create_call(struct ofono_voicecall *vc, int type, + int direction, int status, + const char *num, int num_type, int clip) +{ + struct
Re: [PATCH] Add STE voice call support.
Hi Sjur, --- Makefile.am |1 + drivers/stemodem/stemodem.c |2 + drivers/stemodem/stemodem.h |3 + drivers/stemodem/voicecall.c | 596 ++ plugins/ste.c|2 +- 5 files changed, 603 insertions(+), 1 deletions(-) create mode 100644 drivers/stemodem/voicecall.c diff --git a/Makefile.am b/Makefile.am index ac13d73..ecd2660 100644 --- a/Makefile.am +++ b/Makefile.am @@ -159,6 +159,7 @@ builtin_sources += drivers/atmodem/atutil.h \ drivers/stemodem/stemodem.h \ drivers/stemodem/stemodem.c \ drivers/stemodem/gprs-context.c \ + drivers/stemodem/voicecall.c \ drivers/stemodem/caif_socket.h \ drivers/stemodem/if_caif.h diff --git a/drivers/stemodem/stemodem.c b/drivers/stemodem/stemodem.c index 53207db..9184a42 100644 --- a/drivers/stemodem/stemodem.c +++ b/drivers/stemodem/stemodem.c @@ -37,6 +37,7 @@ static int stemodem_init(void) { ste_gprs_context_init(); + ste_voicecall_init(); return 0; } @@ -44,6 +45,7 @@ static int stemodem_init(void) static void stemodem_exit(void) { ste_gprs_context_exit(); + ste_voicecall_exit(); } OFONO_PLUGIN_DEFINE(stemodem, STE modem driver, VERSION, diff --git a/drivers/stemodem/stemodem.h b/drivers/stemodem/stemodem.h index e55a2c3..e7c6934 100644 --- a/drivers/stemodem/stemodem.h +++ b/drivers/stemodem/stemodem.h @@ -25,3 +25,6 @@ extern void ste_gprs_context_init(); extern void ste_gprs_context_exit(); +extern void ste_voicecall_init(); +extern void ste_voicecall_exit(); + just a small style issue. Can you put voicecall before GPRS in the init routines and the Makefile. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: FW: [RFC 3/3] STE-plugin: Adding STE plugin
Hi Sjur, All of this logic needs to go away. The core already handles all of this, including selection of ATH/CHLD, waiting/held. Please review src/voicecall.c. If the core logic is not sufficient or does not properly handle a certain case, then lets see if we can fix the core first. Drivers should not concern themselves with this stuff. OK, we can remove the state logic, but STE modem cannot terminate calls in state DIALING and ALERTING with CHLD=1X. We need to use ATH (or AT+CHUP) instead. oFono already takes care of this for single calls (see src/voicecall.c voicecall_hangup.) So this is only an issue in the case of three way calls, is this what you're referring to here? For now I think we will remove the state logic from ste_release_specific as you suggest. Terminating calls in state DIALING and ALERTING must then be handled by the Core part. The simplest would probably be to use hangup in this case, but I suspect hangup work differently for different modems. So if you cannot use hangup as the general approach, maybe it could be implemented by adding callback functions release_dialing and release_alerting in struct ofono_voicecall_driver. The STE driver could send ATH from release_dialing and release_alerting, other drivers could leave them empty and this could default to use release_specific instead? What I have been considering to take care of this case is to add end_all and end_all_active callbacks. According to 27.007/22.030 ATH should end all calls (active + held) except waiting calls, while +CHUP should only end the currently active call. At least on one TI modem I tried this works as expected. Do your modems implement the same behavior? If so then we can always use end_all_active for dialing/incoming/alerting cases. With this in mind, you might not need to track any state in this driver at all. See drivers/calypsomodem/voicecall.c for details. TI's CPI notifications are almost exactly the same as the STE ECAV. The STE ECAV update is giving delta updates on the state information, right now I don't see how we can get the ofono_voicall_notify right without keeping some state information in ecav_notify. Yes you're right, I was assuming ECAV gives you more information on whether the call was released locally or remotely. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 2/2] Handle the error path from service_level_connection
Hi Gustavo, --- plugins/hfp.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) Patch looks good, but doesn't apply: Applying: Handle the error path from service_level_connection error: patch failed: plugins/hfp.c:377 error: plugins/hfp.c: patch does not apply Patch failed at 0001 Handle the error path from service_level_connection When you have resolved this problem run git am --resolved. If you would prefer to skip this patch, instead run git am --skip. To restore the original branch and stop patching run git am --abort. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Handle the error path from service_level_connection
--- plugins/hfp.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/hfp.c b/plugins/hfp.c index 5b2cdae..0e2e359 100644 --- a/plugins/hfp.c +++ b/plugins/hfp.c @@ -367,7 +367,7 @@ static int service_level_connection(struct ofono_modem *modem, int fd) static DBusMessage *hfp_agent_new_connection(DBusConnection *conn, DBusMessage *msg, void *data) { - int fd; + int fd, err; struct ofono_modem *modem = data; struct hfp_data *hfp_data = ofono_modem_get_data(modem); @@ -375,7 +375,9 @@ static DBusMessage *hfp_agent_new_connection(DBusConnection *conn, DBUS_TYPE_INVALID)) return __ofono_error_invalid_args(msg); - service_level_connection(modem, fd); + err = service_level_connection(modem, fd); + if (err 0 err != -EINPROGRESS) + return __ofono_error_failed(msg); hfp_data-slc_msg = msg; dbus_message_ref(msg); -- 1.6.4.4 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH] Handle the error path from service_level_connection
Hi Gustavo, --- plugins/hfp.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) Patch has been applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: FW: [RFC 3/3] STE-plugin: Adding STE plugin
Hi Sjur, oFono already takes care of this for single calls (see src/voicecall.c voicecall_hangup.) So this is only an issue in the case of three way calls, is this what you're referring to here? Kind of. This is very good, it takes care of the situation with emergency call which cannot be terminated with CHLD commands. But I think there are more issues. If I am not mistaken STE-modems have the following behavior: CHLD=1X can only terminate call in state ACTIVE or HELD. (I think this is as STE interprets the standards). The standards specify that CHLD=1X can only terminate an ACTIVE call. Most modems implement it this way. There are vendor extensions that provide this functionality (e.g. CHLD=7X on TI.) By default oFono assumes that release_specific will simply fail if a user attempts to use it on an e.g. HELD call with no modem support. a) If you are in a active call and receives a new incoming call (ALERTING) and want to reject the new ALERTING call, then STE modem cannot terminate this call with CHLD=1X. It has to be terminated with CHLD=0 (cause=BUSY) or ATH (possible CHUP). Ok, lets get the terminology clear here. In this case the incoming call is not ALERTING, it is WAITING. WAITING calls are always rejected by using CHLD=0. ALERTING calls are always outgoing calls that transitioned from DIALING to alerting the user. b) Or you may have the following situation. One call on HOLD, another ACTIVE call, and then you receive a new incoming call ALERTING. If you try to terminate the new incoming (ALERTING) call with CHLD=0, I think you as a side effect will terminate the call on hold as well. If I am not mistaken ATH (possible CHUP) would be the correct in this situation for STE modems The standards are quite clear here, the WAITING call always takes precedence and thus only the WAITING call is affected. Can you check that STE modems do indeed get this wrong? If the modem is standards compliant, oFono does the right thing here. c) If you have an call on hold and initiate a new call, but want to terminate the newly initiated call (DIALING), then this call cannot be terminated with CHLD=1X, but you would have to use ATH (or possible CHUP). Yes, so this is the case that we do need to take care of in the core. Most modems let us get away with sending release_specific up to this point. What I have been considering to take care of this case is to add end_all and end_all_active callbacks. According to 27.007/22.030 ATH should end all calls (active + held) except waiting calls, while +CHUP should only end the currently active call. At least on one TI modem I tried this works as expected. Do your modems implement the same behavior? No, I don't think so. I think ATH will only terminate one call. In order to terminate all calls you would probably need to do something like: AT+CHLD=0;H But I'm not sure this works in all possible scenarios either... Can you check the behavior of ATH vs CHUP on STE modems? We need to send the right one here or both HELD and ACTIVE/DIALING/ALERTING will be terminated. If using CHUP and ATH doesn't work out we'll have to come up with another solution. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
[PATCH] Add ability to select modem on test-voicecall
--- test/test-voicecall | 14 ++ 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/test/test-voicecall b/test/test-voicecall index 13f371a..2da7703 100755 --- a/test/test-voicecall +++ b/test/test-voicecall @@ -39,11 +39,9 @@ if __name__ == __main__: global vcmanager if (len(sys.argv) 2): - print Useage: %s number % (sys.argv[0]) + print Useage: %s [modem] number % (sys.argv[0]) sys.exit(1) - number = sys.argv[1] - dbus.mainloop.glib.DBusGMainLoop(set_as_default=True) bus = dbus.SystemBus() @@ -52,9 +50,17 @@ if __name__ == __main__: 'org.ofono.Manager') modems = manager.GetProperties()['Modems'] + modem = modems[0] print modems - vcmanager = dbus.Interface(bus.get_object('org.ofono', modems[0]), + if (len(sys.argv) == 3): + modem = sys.argv[1] + number = sys.argv[2] + else: + number = sys.argv[1] + print Using modem %s % modem + + vcmanager = dbus.Interface(bus.get_object('org.ofono', modem), 'org.ofono.VoiceCallManager') vcmanager.connect_to_signal(PropertyChanged, -- 1.6.3.3 ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono