FW: [RFC 3/3] STE-plugin: Adding STE plugin

2010-01-28 Thread Sjur Brændeland
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

2010-01-28 Thread Gustavo F. Padovan
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

2010-01-28 Thread Gustavo F. Padovan
---
 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.

2010-01-28 Thread sjur . brandeland
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.

2010-01-28 Thread Marcel Holtmann
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

2010-01-28 Thread Denis Kenzior
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

2010-01-28 Thread Denis Kenzior
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

2010-01-28 Thread Gustavo F. Padovan
---
 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

2010-01-28 Thread Denis Kenzior
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

2010-01-28 Thread Denis Kenzior
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

2010-01-28 Thread João Paulo Rechi Vita
---
 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