[PATCH v2 1/2] hfp_ag: start server on sim 'ready' state

[PATCH v2 1/2] hfp_ag: start server on sim 'ready' state

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

[-- Attachment #1: Type: text/plain, Size: 4390 bytes --]

update HFP AG server to start only when a modem has its SIM atom
in 'ready' state and has voice call capability
---
 plugins/hfp_ag.c |   85 ++++++++++++++++++++++++++++++++++++++++++------------
 1 files changed, 66 insertions(+), 19 deletions(-)

diff --git a/plugins/hfp_ag.c b/plugins/hfp_ag.c
index 191708b..9fea282 100644
--- a/plugins/hfp_ag.c
+++ b/plugins/hfp_ag.c
@@ -40,6 +40,7 @@
 static struct server *server;
 static guint modemwatch_id;
 static GList *modems;
+static GHashTable *sim_hash = NULL;
 
 static const gchar *hfp_ag_record =
 "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
@@ -85,6 +86,11 @@ static const gchar *hfp_ag_record =
 "  </attribute>\n"
 "</record>\n";
 
+struct sim_entry {
+	int watch;
+	struct ofono_modem *modem;
+};
+
 static void hfp_ag_connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 {
 	struct ofono_modem *modem;
@@ -115,27 +121,68 @@ static void hfp_ag_connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 	ofono_emulator_register(em, fd);
 }
 
-static void voicecall_watch(struct ofono_atom *atom,
-				enum ofono_atom_watch_condition cond,
-				void *data)
+static void sim_state_watch(enum ofono_sim_state new_state, void *data)
 {
 	struct ofono_modem *modem = data;
 
-	if (cond == OFONO_ATOM_WATCH_CONDITION_REGISTERED) {
-		modems = g_list_append(modems, modem);
-
-		if (modems->next == NULL)
-			server = bluetooth_register_server(HFP_AG_CHANNEL,
-							hfp_ag_record,
-							hfp_ag_connect_cb,
-							NULL);
-	} else {
+	if (new_state != OFONO_SIM_STATE_READY) {
 		modems = g_list_remove(modems, modem);
 		if (modems == NULL && server != NULL) {
 			bluetooth_unregister_server(server);
 			server = NULL;
 		}
+		return;
 	}
+
+	if (__ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_VOICECALL)
+			== NULL)
+		return;
+
+	modems = g_list_append(modems, modem);
+
+	if (modems->next == NULL)
+		server = bluetooth_register_server(HFP_AG_CHANNEL,
+							hfp_ag_record,
+							hfp_ag_connect_cb,
+							NULL);
+}
+
+static gboolean sim_watch_remove(gpointer key, gpointer value,
+				gpointer user_data)
+{
+	struct ofono_sim *sim = key;
+	struct sim_entry *s = value;
+
+	ofono_sim_remove_state_watch(sim, s->watch);
+	sim_state_watch(OFONO_SIM_STATE_NOT_PRESENT, s->modem);
+
+	return TRUE;
+}
+
+static void sim_watch(struct ofono_atom *atom,
+				enum ofono_atom_watch_condition cond,
+				void *data)
+{
+	struct ofono_sim *sim = __ofono_atom_get_data(atom);
+	struct ofono_modem *modem = data;
+	struct sim_entry *s;
+
+	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
+		sim_watch_remove(sim, g_hash_table_lookup(sim_hash, sim), NULL);
+		g_hash_table_remove(sim_hash, sim);
+		return;
+	}
+
+	s = g_try_new0(struct sim_entry, 1);
+	if (s == NULL) {
+		ofono_error("Unable to allocate sim entry structure");
+		return;
+	}
+
+	s->modem = modem;
+	s->watch = ofono_sim_add_state_watch(sim, sim_state_watch, modem, NULL);
+	g_hash_table_insert(sim_hash, sim, s);
+	sim_state_watch(ofono_sim_get_state(sim), modem);
 }
 
 static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
@@ -145,8 +192,8 @@ static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
 	if (added == FALSE)
 		return;
 
-	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_VOICECALL,
-					voicecall_watch, modem, NULL);
+	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SIM,
+					sim_watch, modem, NULL);
 }
 
 static void call_modemwatch(struct ofono_modem *modem, void *user)
@@ -156,6 +203,9 @@ static void call_modemwatch(struct ofono_modem *modem, void *user)
 
 static int hfp_ag_init()
 {
+	sim_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
+								g_free);
+
 	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
 	__ofono_modem_foreach(call_modemwatch, NULL);
 
@@ -165,12 +215,9 @@ static int hfp_ag_init()
 static void hfp_ag_exit()
 {
 	__ofono_modemwatch_remove(modemwatch_id);
+	g_hash_table_foreach_remove(sim_hash, sim_watch_remove, NULL);
+	g_hash_table_destroy(sim_hash);
 	g_list_free(modems);
-
-	if (server) {
-		bluetooth_unregister_server(server);
-		server = NULL;
-	}
 }
 
 OFONO_PLUGIN_DEFINE(hfp_ag, "Hands-Free Audio Gateway Profile Plugins", VERSION,
-- 
1.7.1

[PATCH v2 2/2] voicecall: remove usage of em_atd_number

[=?unknown-8bit?q?Fr=C3=A9d=C3=A9ric?= Danis]

[-- Attachment #1: Type: text/plain, Size: 2508 bytes --]

as emulator atom can only run with a 'ready' SIM,
use saved number instead of em_atd_number
---
 src/voicecall.c |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 9160ce8..f4c2358 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -74,7 +74,6 @@ struct ofono_voicecall {
 	ofono_voicecall_cb_t release_queue_done_cb;
 	struct ofono_emulator *pending_em;
 	unsigned int pending_id;
-	char *em_atd_number;
 };
 
 struct voicecall {
@@ -1506,14 +1505,14 @@ static int voicecall_dial(struct ofono_voicecall *vc, const char *number,
 
 	string_to_phone_number(number, &ph);
 
-	vc->driver->dial(vc, &ph, clir, cb, vc);
-
 	if (vc->settings) {
 		g_key_file_set_string(vc->settings, SETTINGS_GROUP,
 					"Number", number);
 		storage_sync(vc->imsi, SETTINGS_STORE, vc->settings);
 	}
 
+	vc->driver->dial(vc, &ph, clir, cb, vc);
+
 	return 0;
 }
 
@@ -3154,13 +3153,18 @@ static void emulator_dial_callback(const struct ofono_error *error, void *data)
 	struct ofono_voicecall *vc = data;
 	gboolean need_to_emit;
 	struct voicecall *v;
+	const char *number;
+	GError *err = NULL;
+
+	number = g_key_file_get_string(vc->settings, SETTINGS_GROUP,
+					"Number", &err);
 
-	v = dial_handle_result(vc, error, vc->em_atd_number, &need_to_emit);
+	v = dial_handle_result(vc, error, number, &need_to_emit);
 
 	if (v == NULL) {
 		struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom);
 
-		if (is_emergency_number(vc, vc->em_atd_number) == TRUE)
+		if (is_emergency_number(vc, number) == TRUE)
 			__ofono_modem_dec_emergency_mode(modem);
 	}
 
@@ -3168,8 +3172,6 @@ static void emulator_dial_callback(const struct ofono_error *error, void *data)
 		ofono_emulator_send_final(vc->pending_em, error);
 
 	vc->pending_em = NULL;
-	g_free(vc->em_atd_number);
-	vc->em_atd_number = NULL;
 
 	notify_emulator_call_status(vc);
 
@@ -3191,7 +3193,6 @@ static void emulator_dial(struct ofono_emulator *em, struct ofono_voicecall *vc,
 	}
 
 	vc->pending_em = em;
-	vc->em_atd_number = g_strdup(number);
 
 	err = voicecall_dial(vc, number, OFONO_CLIR_OPTION_DEFAULT,
 					emulator_dial_callback, vc);
@@ -3200,8 +3201,6 @@ static void emulator_dial(struct ofono_emulator *em, struct ofono_voicecall *vc,
 		return;
 
 	vc->pending_em = NULL;
-	g_free(vc->em_atd_number);
-	vc->em_atd_number = NULL;
 
 	switch (err) {
 	case -ENETDOWN:
-- 
1.7.1

Re: [PATCH v2 1/2] hfp_ag: start server on sim 'ready' state

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 5148 bytes --]

Hi Frédéric,

On 07/20/2011 09:34 AM, Frédéric Danis wrote:
> update HFP AG server to start only when a modem has its SIM atom
> in 'ready' state and has voice call capability
> ---
>  plugins/hfp_ag.c |   85 ++++++++++++++++++++++++++++++++++++++++++------------
>  1 files changed, 66 insertions(+), 19 deletions(-)
> 
> diff --git a/plugins/hfp_ag.c b/plugins/hfp_ag.c
> index 191708b..9fea282 100644
> --- a/plugins/hfp_ag.c
> +++ b/plugins/hfp_ag.c
> @@ -40,6 +40,7 @@
>  static struct server *server;
>  static guint modemwatch_id;
>  static GList *modems;
> +static GHashTable *sim_hash = NULL;
>  
>  static const gchar *hfp_ag_record =
>  "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
> @@ -85,6 +86,11 @@ static const gchar *hfp_ag_record =
>  "  </attribute>\n"
>  "</record>\n";
>  
> +struct sim_entry {
> +	int watch;
> +	struct ofono_modem *modem;
> +};
> +

Are you sure you need this structure?  I'd rather not become this
complicated.  Your original patch was fine, it just wasn't unregistering
the sim watches in hfp_ag_exit.

>  static void hfp_ag_connect_cb(GIOChannel *io, GError *err, gpointer user_data)
>  {
>  	struct ofono_modem *modem;
> @@ -115,27 +121,68 @@ static void hfp_ag_connect_cb(GIOChannel *io, GError *err, gpointer user_data)
>  	ofono_emulator_register(em, fd);
>  }
>  
> -static void voicecall_watch(struct ofono_atom *atom,
> -				enum ofono_atom_watch_condition cond,
> -				void *data)
> +static void sim_state_watch(enum ofono_sim_state new_state, void *data)
>  {
>  	struct ofono_modem *modem = data;
>  
> -	if (cond == OFONO_ATOM_WATCH_CONDITION_REGISTERED) {
> -		modems = g_list_append(modems, modem);
> -
> -		if (modems->next == NULL)
> -			server = bluetooth_register_server(HFP_AG_CHANNEL,
> -							hfp_ag_record,
> -							hfp_ag_connect_cb,
> -							NULL);
> -	} else {
> +	if (new_state != OFONO_SIM_STATE_READY) {
>  		modems = g_list_remove(modems, modem);
>  		if (modems == NULL && server != NULL) {
>  			bluetooth_unregister_server(server);
>  			server = NULL;
>  		}

A newline here please

> +		return;
>  	}
> +
> +	if (__ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_VOICECALL)
> +			== NULL)
> +		return;
> +
> +	modems = g_list_append(modems, modem);
> +
> +	if (modems->next == NULL)
> +		server = bluetooth_register_server(HFP_AG_CHANNEL,
> +							hfp_ag_record,
> +							hfp_ag_connect_cb,
> +							NULL);
> +}
> +
> +static gboolean sim_watch_remove(gpointer key, gpointer value,
> +				gpointer user_data)
> +{
> +	struct ofono_sim *sim = key;
> +	struct sim_entry *s = value;
> +
> +	ofono_sim_remove_state_watch(sim, s->watch);
> +	sim_state_watch(OFONO_SIM_STATE_NOT_PRESENT, s->modem);
> +
> +	return TRUE;
> +}
> +
> +static void sim_watch(struct ofono_atom *atom,
> +				enum ofono_atom_watch_condition cond,
> +				void *data)
> +{
> +	struct ofono_sim *sim = __ofono_atom_get_data(atom);
> +	struct ofono_modem *modem = data;
> +	struct sim_entry *s;
> +
> +	if (cond == OFONO_ATOM_WATCH_CONDITION_UNREGISTERED) {
> +		sim_watch_remove(sim, g_hash_table_lookup(sim_hash, sim), NULL);

You already have the sim and modem here, so this lookup is useless.

> +		g_hash_table_remove(sim_hash, sim);
> +		return;
> +	}
> +
> +	s = g_try_new0(struct sim_entry, 1);
> +	if (s == NULL) {
> +		ofono_error("Unable to allocate sim entry structure");
> +		return;
> +	}
> +
> +	s->modem = modem;
> +	s->watch = ofono_sim_add_state_watch(sim, sim_state_watch, modem, NULL);
> +	g_hash_table_insert(sim_hash, sim, s);
> +	sim_state_watch(ofono_sim_get_state(sim), modem);
>  }
>  
>  static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
> @@ -145,8 +192,8 @@ static void modem_watch(struct ofono_modem *modem, gboolean added, void *user)
>  	if (added == FALSE)
>  		return;
>  
> -	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_VOICECALL,
> -					voicecall_watch, modem, NULL);
> +	__ofono_modem_add_atom_watch(modem, OFONO_ATOM_TYPE_SIM,
> +					sim_watch, modem, NULL);
>  }
>  
>  static void call_modemwatch(struct ofono_modem *modem, void *user)
> @@ -156,6 +203,9 @@ static void call_modemwatch(struct ofono_modem *modem, void *user)
>  
>  static int hfp_ag_init()
>  {
> +	sim_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
> +								g_free);
> +
>  	modemwatch_id = __ofono_modemwatch_add(modem_watch, NULL, NULL);
>  	__ofono_modem_foreach(call_modemwatch, NULL);
>  
> @@ -165,12 +215,9 @@ static int hfp_ag_init()
>  static void hfp_ag_exit()
>  {
>  	__ofono_modemwatch_remove(modemwatch_id);
> +	g_hash_table_foreach_remove(sim_hash, sim_watch_remove, NULL);
> +	g_hash_table_destroy(sim_hash);

Here you're clearing everything, so clearing the hash and later calling
unregister_server seems just fine.

>  	g_list_free(modems);
> -
> -	if (server) {
> -		bluetooth_unregister_server(server);
> -		server = NULL;
> -	}
>  }
>  
>  OFONO_PLUGIN_DEFINE(hfp_ag, "Hands-Free Audio Gateway Profile Plugins", VERSION,

Regards,
-Denis