RE: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi, To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now? AFAIK, this is still a product requirement for us. I can recheck, but I just asked last week and the answer was a very definite Yes, this is still required. MikaL ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Mika, To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now? AFAIK, this is still a product requirement for us. I can recheck, but I just asked last week and the answer was a very definite Yes, this is still required. as Denis and I mentioned, we have been told that this is not needed anymore. So where is this still needed? And again, this is something that the modem firmware should be doing anyway (if it is needed). The modem firmware knows best. And we do have the GPRS suspend support already to signal this back to the user. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Marcel, Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now? AFAIK, this is still a product requirement for us. I can recheck, but I just asked last week and the answer was a very definite Yes, this is still required. as Denis and I mentioned, we have been told that this is not needed anymore. So where is this still needed? Ok, I double checked the product requirements and it turns out that you're right. The requirement is no longer there for oFono based stuff. Seems we've had some wires crossed at our end, so the situation was not entirely clear. Sorry for the confusion, MikaL ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Denis, On 10/29/2010 08:32 PM, ext Denis Kenzior wrote: Hi Andras, Just some general comments, but this patch seems to be backwards from the earlier proposal. Namely EmergencyMode is a property on the Modem interface, not on the VoiceCallManager. See doc/modem-api.txt, Emergency property. I thought it was more logical to have the EmergencyMode property linked to voicecall since it is about a special call case, but I am fine with moving that property up to modem level. Oh I can see that as well, but I think earlier we agreed that it should be on the Modem interface. This has several advantages: offline / online toggling has to happen in the modem and it is easier for the power management framework to monitor it there. So unless you feel really strongly against that I suggest we stick with the earlier proposal. I see the advantage of using the Modem interface and I am fine with basing the implementation on it. In general I think that the emergency_watch is unnecessary. Having a reference counted emergency tracking inside the modem object and a modem online state watch should be sufficient. The idea with the emergency watch is that any subsystem can get the notification when the emergency mode is entered and react on it. Don't get me wrong, it might be useful in the future. But for the context of supporting emergency calls in the voicecall driver the emergency watch is not really needed. In general I prefer to review code which has an immediate or foreseeable need. In this case if we detect an emergency call dial and we're offline, we: - Save the pending call - establish an online watch - ofono_modem_inc_emergency_mode() Once we are online: - Dial the call Once the call ends - ofono_modem_dec_emergency_mode() Nowhere do we actually need to use an emergency watch itself. To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now? If this is still required, I suggest you group the emergency watch functions with patches implementing the above functionality. available by gprs and to deal with the gprs connection within voicecall (or somewhere else). The online/offline mode change handling in fact is bringing up the some issue, how the mode change handling should be implemented when making the emergency call. My idea was let every subsystem deal with the specifics of its own subsystem. Let the modem figure out the specifics. Basically as long as the count for emergency is greater than 1, Offline mode should not be entered. Once it reaches 0, the online mode should go back to the previous value. Based on the comments and after some clarifications made in our team, I consider the emergency watch unnecessary. We can forget about the very corner case and go with the simpler approach. OK, I am going to come up with a new set of patches. Regards, -Denis Regards, Andras ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Denis,
Thank you for your comments, here is my response.
On 10/28/2010 06:48 AM, ext Denis Kenzior wrote:
Hi Andras,
On 10/25/2010 03:03 AM, Andras Domokos wrote:
From: Andras Domokosandras.domo...@nokia.com
Signed-off-by: Andras Domokosandras.domo...@nokia.com
---
include/voicecall.h | 12 +++
src/voicecall.c | 221 ++
2 files changed, 215 insertions(+), 18 deletions(-)
diff --git a/include/voicecall.h b/include/voicecall.h
index 2356fcf..d530148 100644
--- a/include/voicecall.h
+++ b/include/voicecall.h
@@ -38,6 +38,9 @@ typedef void (*ofono_call_list_cb_t)(const struct ofono_error
*error,
const struct ofono_call *call_list,
void *data);
+typedef void (*ofono_voicecall_emergency_notify_cb_t)(ofono_bool_t state,
+ void *data);
+
/* Voice call related functionality, including ATD, ATA, +CHLD, CTFR, CLCC
* and VTS.
*
@@ -116,6 +119,15 @@ void ofono_voicecall_set_data(struct ofono_voicecall *vc,
void *data);
void *ofono_voicecall_get_data(struct ofono_voicecall *vc);
int ofono_voicecall_get_next_callid(struct ofono_voicecall *vc);
+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall *vc,
+ ofono_voicecall_emergency_notify_cb_t notify,
+ void *data, ofono_destroy_func destroy);
+void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
+ unsigned int id);
+ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc);
+void __ofono_voicecall_set_emergency_state(struct ofono_voicecall *vc,
+ int state);
+
Just some general comments, but this patch seems to be backwards from
the earlier proposal. Namely EmergencyMode is a property on the Modem
interface, not on the VoiceCallManager. See doc/modem-api.txt,
Emergency property.
I thought it was more logical to have the EmergencyMode property
linked to voicecall since it is about a special call case, but I am fine
with moving that property up to modem level.
In general I think that the emergency_watch is unnecessary. Having a
reference counted emergency tracking inside the modem object and a modem
online state watch should be sufficient.
The idea with the emergency watch is that any subsystem can get the
notification when the emergency mode is entered and react on it.
To give you a more complex example, it might well be that the gprs
connection needs to be torn down when making an emergency call in
2G mode, there are such networks out there that prevents you from
making an emergency call if your device is attached to a PDP context.
In this given situation it comes to the question how to bring down the
gprs connection. It can be done such that the gprs atom will tear down
the connection after receiving the EmergencyMode notification, or
another option is to have gprs connection handling functions made
available by gprs and to deal with the gprs connection within voicecall
(or somewhere else). The online/offline mode change handling in fact is
bringing up the some issue, how the mode change handling should be
implemented when making the emergency call. My idea was let every
subsystem deal with the specifics of its own subsystem.
#ifdef __cplusplus
}
#endif
diff --git a/src/voicecall.c b/src/voicecall.c
index 7b5fe3b..a619b30 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -25,6 +25,7 @@
#includestring.h
#includestdio.h
+#includestdlib.h
#includetime.h
#includeerrno.h
#includestdint.h
@@ -56,6 +57,9 @@ struct ofono_voicecall {
void *driver_data;
struct ofono_atom *atom;
struct dial_request *dial_req;
+ struct ofono_watchlist *emergency_watches;
+ unsigned int emergency_state;
+ unsigned int modem_state_watch;
};
struct voicecall {
@@ -85,6 +89,8 @@ static const char *default_en_list_no_sim[] = { 119, 118, 999,
110,
static void generic_callback(const struct ofono_error *error, void *data);
static void multirelease_callback(const struct ofono_error *err, void *data);
+static const char *voicecall_build_path(struct ofono_voicecall *vc,
+ const struct ofono_call *call);
It is generally against the coding style to forward-declare static
functions. If this function is needed, please simply move it higher.
OK, I'll conform to the agreed coding policy.
static gint call_compare_by_id(gconstpointer a, gconstpointer b)
{
@@ -121,6 +127,145 @@ static void add_to_en_list(GSList **l, const char **list)
*l = g_slist_prepend(*l, g_strdup(list[i++]));
}
+static gint number_compare(gconstpointer a, gconstpointer b)
+{
+ const char *s1 = a, *s2 = b;
+ return strcmp(s1, s2);
+}
+
+static ofono_bool_t
Re: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Andras, In general I think that the emergency_watch is unnecessary. Having a reference counted emergency tracking inside the modem object and a modem online state watch should be sufficient. The idea with the emergency watch is that any subsystem can get the notification when the emergency mode is entered and react on it. To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made available by gprs and to deal with the gprs connection within voicecall (or somewhere else). The online/offline mode change handling in fact is bringing up the some issue, how the mode change handling should be implemented when making the emergency call. My idea was let every subsystem deal with the specifics of its own subsystem. we had this specific discussion before and my understanding is that all these networks have been updated by now anyway. So this is not an issue anymore. However in the end this is not something oFono should be doing in the first place. The modem should do this. oFono does not know if something is or becomes an emergency call. Only the modem itself knows if it starts the procedure to make an emergency call. Regards Marcel ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Andras, Just some general comments, but this patch seems to be backwards from the earlier proposal. Namely EmergencyMode is a property on the Modem interface, not on the VoiceCallManager. See doc/modem-api.txt, Emergency property. I thought it was more logical to have the EmergencyMode property linked to voicecall since it is about a special call case, but I am fine with moving that property up to modem level. Oh I can see that as well, but I think earlier we agreed that it should be on the Modem interface. This has several advantages: offline / online toggling has to happen in the modem and it is easier for the power management framework to monitor it there. So unless you feel really strongly against that I suggest we stick with the earlier proposal. In general I think that the emergency_watch is unnecessary. Having a reference counted emergency tracking inside the modem object and a modem online state watch should be sufficient. The idea with the emergency watch is that any subsystem can get the notification when the emergency mode is entered and react on it. Don't get me wrong, it might be useful in the future. But for the context of supporting emergency calls in the voicecall driver the emergency watch is not really needed. In general I prefer to review code which has an immediate or foreseeable need. In this case if we detect an emergency call dial and we're offline, we: - Save the pending call - establish an online watch - ofono_modem_inc_emergency_mode() Once we are online: - Dial the call Once the call ends - ofono_modem_dec_emergency_mode() Nowhere do we actually need to use an emergency watch itself. To give you a more complex example, it might well be that the gprs connection needs to be torn down when making an emergency call in 2G mode, there are such networks out there that prevents you from making an emergency call if your device is attached to a PDP context. In this given situation it comes to the question how to bring down the gprs connection. It can be done such that the gprs atom will tear down the connection after receiving the EmergencyMode notification, or another option is to have gprs connection handling functions made Have we established that this is actually still needed? I thought you guys said all the networks that have this problem have been fixed by now? If this is still required, I suggest you group the emergency watch functions with patches implementing the above functionality. available by gprs and to deal with the gprs connection within voicecall (or somewhere else). The online/offline mode change handling in fact is bringing up the some issue, how the mode change handling should be implemented when making the emergency call. My idea was let every subsystem deal with the specifics of its own subsystem. Let the modem figure out the specifics. Basically as long as the count for emergency is greater than 1, Offline mode should not be entered. Once it reaches 0, the online mode should go back to the previous value. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [RFC PATCH 2/3] voicecall: emergency call handling added
Hi Andras,
On 10/25/2010 03:03 AM, Andras Domokos wrote:
From: Andras Domokos andras.domo...@nokia.com
Signed-off-by: Andras Domokos andras.domo...@nokia.com
---
include/voicecall.h | 12 +++
src/voicecall.c | 221 ++
2 files changed, 215 insertions(+), 18 deletions(-)
diff --git a/include/voicecall.h b/include/voicecall.h
index 2356fcf..d530148 100644
--- a/include/voicecall.h
+++ b/include/voicecall.h
@@ -38,6 +38,9 @@ typedef void (*ofono_call_list_cb_t)(const struct
ofono_error *error,
const struct ofono_call *call_list,
void *data);
+typedef void (*ofono_voicecall_emergency_notify_cb_t)(ofono_bool_t state,
+ void *data);
+
/* Voice call related functionality, including ATD, ATA, +CHLD, CTFR, CLCC
* and VTS.
*
@@ -116,6 +119,15 @@ void ofono_voicecall_set_data(struct ofono_voicecall
*vc, void *data);
void *ofono_voicecall_get_data(struct ofono_voicecall *vc);
int ofono_voicecall_get_next_callid(struct ofono_voicecall *vc);
+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall
*vc,
+ ofono_voicecall_emergency_notify_cb_t notify,
+ void *data, ofono_destroy_func destroy);
+void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
+ unsigned int id);
+ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc);
+void __ofono_voicecall_set_emergency_state(struct ofono_voicecall *vc,
+ int state);
+
Just some general comments, but this patch seems to be backwards from
the earlier proposal. Namely EmergencyMode is a property on the Modem
interface, not on the VoiceCallManager. See doc/modem-api.txt,
Emergency property.
In general I think that the emergency_watch is unnecessary. Having a
reference counted emergency tracking inside the modem object and a modem
online state watch should be sufficient.
#ifdef __cplusplus
}
#endif
diff --git a/src/voicecall.c b/src/voicecall.c
index 7b5fe3b..a619b30 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -25,6 +25,7 @@
#include string.h
#include stdio.h
+#include stdlib.h
#include time.h
#include errno.h
#include stdint.h
@@ -56,6 +57,9 @@ struct ofono_voicecall {
void *driver_data;
struct ofono_atom *atom;
struct dial_request *dial_req;
+ struct ofono_watchlist *emergency_watches;
+ unsigned int emergency_state;
+ unsigned int modem_state_watch;
};
struct voicecall {
@@ -85,6 +89,8 @@ static const char *default_en_list_no_sim[] = { 119,
118, 999, 110,
static void generic_callback(const struct ofono_error *error, void *data);
static void multirelease_callback(const struct ofono_error *err, void *data);
+static const char *voicecall_build_path(struct ofono_voicecall *vc,
+ const struct ofono_call *call);
It is generally against the coding style to forward-declare static
functions. If this function is needed, please simply move it higher.
static gint call_compare_by_id(gconstpointer a, gconstpointer b)
{
@@ -121,6 +127,145 @@ static void add_to_en_list(GSList **l, const char
**list)
*l = g_slist_prepend(*l, g_strdup(list[i++]));
}
+static gint number_compare(gconstpointer a, gconstpointer b)
+{
+ const char *s1 = a, *s2 = b;
+ return strcmp(s1, s2);
+}
+
+static ofono_bool_t emergency_number(struct ofono_voicecall *vc,
+ const char *number)
+{
+ if (!number)
+ return FALSE;
+
+ return g_slist_find_custom(vc-en_list,
+ number, number_compare) ? TRUE : FALSE;
+}
+
Please add this as a separate patch handling the 'Extend the voicecall
interface with a property indicating whether this call is an emergency
call' task.
+static void notify_emergency_watches(struct ofono_voicecall *vc,
+ ofono_bool_t state)
+{
+ struct ofono_watchlist_item *item;
+ GSList *l;
+ ofono_voicecall_emergency_notify_cb_t notify;
+
+ if (vc-emergency_watches == NULL)
+ return;
+
+ for (l = vc-emergency_watches-items; l; l = l-next) {
+ item = l-data;
+ notify = item-notify;
+ notify(state, item-notify_data);
+ }
+}
+
+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall
*vc,
+ ofono_voicecall_emergency_notify_cb_t notify,
+ void *data, ofono_destroy_func destroy)
+{
+ struct ofono_watchlist_item *item;
+
+ if (vc == NULL || notify == NULL)
+ return 0;
+
+ item =
[RFC PATCH 2/3] voicecall: emergency call handling added
Signed-off-by: Andras Domokos andras.domo...@nokia.com
---
include/voicecall.h | 12 +++
src/voicecall.c | 221 ++
2 files changed, 215 insertions(+), 18 deletions(-)
diff --git a/include/voicecall.h b/include/voicecall.h
index 2356fcf..d530148 100644
--- a/include/voicecall.h
+++ b/include/voicecall.h
@@ -38,6 +38,9 @@ typedef void (*ofono_call_list_cb_t)(const struct ofono_error
*error,
const struct ofono_call *call_list,
void *data);
+typedef void (*ofono_voicecall_emergency_notify_cb_t)(ofono_bool_t state,
+ void *data);
+
/* Voice call related functionality, including ATD, ATA, +CHLD, CTFR, CLCC
* and VTS.
*
@@ -116,6 +119,15 @@ void ofono_voicecall_set_data(struct ofono_voicecall *vc,
void *data);
void *ofono_voicecall_get_data(struct ofono_voicecall *vc);
int ofono_voicecall_get_next_callid(struct ofono_voicecall *vc);
+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall *vc,
+ ofono_voicecall_emergency_notify_cb_t notify,
+ void *data, ofono_destroy_func destroy);
+void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
+ unsigned int id);
+ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc);
+void __ofono_voicecall_set_emergency_state(struct ofono_voicecall *vc,
+ int state);
+
#ifdef __cplusplus
}
#endif
diff --git a/src/voicecall.c b/src/voicecall.c
index 7b5fe3b..a619b30 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -25,6 +25,7 @@
#include string.h
#include stdio.h
+#include stdlib.h
#include time.h
#include errno.h
#include stdint.h
@@ -56,6 +57,9 @@ struct ofono_voicecall {
void *driver_data;
struct ofono_atom *atom;
struct dial_request *dial_req;
+ struct ofono_watchlist *emergency_watches;
+ unsigned int emergency_state;
+ unsigned int modem_state_watch;
};
struct voicecall {
@@ -85,6 +89,8 @@ static const char *default_en_list_no_sim[] = { 119, 118,
999, 110,
static void generic_callback(const struct ofono_error *error, void *data);
static void multirelease_callback(const struct ofono_error *err, void *data);
+static const char *voicecall_build_path(struct ofono_voicecall *vc,
+ const struct ofono_call *call);
static gint call_compare_by_id(gconstpointer a, gconstpointer b)
{
@@ -121,6 +127,145 @@ static void add_to_en_list(GSList **l, const char **list)
*l = g_slist_prepend(*l, g_strdup(list[i++]));
}
+static gint number_compare(gconstpointer a, gconstpointer b)
+{
+ const char *s1 = a, *s2 = b;
+ return strcmp(s1, s2);
+}
+
+static ofono_bool_t emergency_number(struct ofono_voicecall *vc,
+ const char *number)
+{
+ if (!number)
+ return FALSE;
+
+ return g_slist_find_custom(vc-en_list,
+ number, number_compare) ? TRUE : FALSE;
+}
+
+static void notify_emergency_watches(struct ofono_voicecall *vc,
+ ofono_bool_t state)
+{
+ struct ofono_watchlist_item *item;
+ GSList *l;
+ ofono_voicecall_emergency_notify_cb_t notify;
+
+ if (vc-emergency_watches == NULL)
+ return;
+
+ for (l = vc-emergency_watches-items; l; l = l-next) {
+ item = l-data;
+ notify = item-notify;
+ notify(state, item-notify_data);
+ }
+}
+
+unsigned int __ofono_voicecall_add_emergency_watch(struct ofono_voicecall *vc,
+ ofono_voicecall_emergency_notify_cb_t notify,
+ void *data, ofono_destroy_func destroy)
+{
+ struct ofono_watchlist_item *item;
+
+ if (vc == NULL || notify == NULL)
+ return 0;
+
+ item = g_new0(struct ofono_watchlist_item, 1);
+
+ item-notify = notify;
+ item-destroy = destroy;
+ item-notify_data = data;
+
+ return __ofono_watchlist_add_item(vc-emergency_watches, item);
+}
+
+void __ofono_voicecall_remove_emergency_watch(struct ofono_voicecall *vc,
+ unsigned int id)
+{
+ __ofono_watchlist_remove_item(vc-emergency_watches, id);
+}
+
+ofono_bool_t ofono_voicecall_get_emergency_state(struct ofono_voicecall *vc)
+{
+ return vc-emergency_state ? 1 : 0;
+}
+
+void __ofono_voicecall_inc_emergency_state(struct ofono_voicecall *vc)
+{
+ DBusConnection *conn = ofono_dbus_get_connection();
+ const char *path = __ofono_atom_get_path(vc-atom);
+ gboolean state = TRUE;
+
+ if (!vc-emergency_state++) {
+
