[PATCH 1/9] modem.c: Go to PRE_SIM modem state on OFONO_SIM_STATE_INSERTED.

[PATCH 1/9] modem.c: Go to PRE_SIM modem state on OFONO_SIM_STATE_INSERTED.

[Andrzej Zaborowski]

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

The assumption so far was that SIM state would only go from
NOT_PRESENT -> INSERTED -> READY -> NOT_PRESENT because there's
no method to unauthenticate from SIM without removing.  We need to
be able to switch to INSERTED when SIM is being reset,
reinitialised, etc. because going to NOT_PRESENT and then
immediately INSERTED would be logically wrong.
---
 src/modem.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/src/modem.c b/src/modem.c
index 3c1bb52..d1c7905 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -517,7 +517,7 @@ static void common_online_cb(const struct ofono_error *error, void *data)
 	 *
 	 * Additionally, this process can be interrupted by the following
 	 * events:
-	 *	- Sim being removed
+	 *	- Sim being removed or reset
 	 *	- SetProperty(Powered, False) being called
 	 *	- SetProperty(Lockdown, True) being called
 	 *
@@ -531,7 +531,7 @@ static void common_online_cb(const struct ofono_error *error, void *data)
 		/* The powered operation is pending */
 		break;
 	case MODEM_STATE_PRE_SIM:
-		/* Go back offline if the sim was removed */
+		/* Go back offline if the sim was removed or reset */
 		modem->driver->set_online(modem, 0, NULL, NULL);
 		break;
 	case MODEM_STATE_ONLINE:
@@ -583,9 +583,13 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *user)
 
 	switch (new_state) {
 	case OFONO_SIM_STATE_NOT_PRESENT:
-		modem_change_state(modem, MODEM_STATE_PRE_SIM);
-		break;
 	case OFONO_SIM_STATE_INSERTED:
+		if (modem->modem_state != MODEM_STATE_PRE_SIM) {
+			if (modem->modem_state == MODEM_STATE_ONLINE)
+				modem->get_online = TRUE;
+
+			modem_change_state(modem, MODEM_STATE_PRE_SIM);
+		}
 		break;
 	case OFONO_SIM_STATE_READY:
 		modem_change_state(modem, MODEM_STATE_OFFLINE);
-- 
1.7.1.86.g0e460.dirty

[PATCH 2/9] sim: Implement basic Refresh.

[Andrzej Zaborowski]

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

---
 src/ofono.h |    4 ++
 src/sim.c   |  163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 155 insertions(+), 12 deletions(-)

diff --git a/src/ofono.h b/src/ofono.h
index dbe1862..8f3b684 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -317,6 +317,10 @@ ofono_bool_t __ofono_is_valid_sim_pin(const char *pin,
 
 ofono_bool_t __ofono_is_valid_net_pin(const char *pin);
 
+void __ofono_sim_refresh(struct ofono_sim *sim, GSList *file_list,
+				ofono_bool_t full_file_change,
+				ofono_bool_t naa_init);
+
 #include <ofono/stk.h>
 
 typedef void (*__ofono_sms_sim_download_cb_t)(ofono_bool_t ok,
diff --git a/src/sim.c b/src/sim.c
index c2e8631..7c2e151 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -26,6 +26,7 @@
 #define _GNU_SOURCE
 #include <string.h>
 #include <stdio.h>
+#include <stdint.h>
 
 #include <glib.h>
 #include <gdbus.h>
@@ -2078,13 +2079,27 @@ static void sim_inserted_update(struct ofono_sim *sim)
 						DBUS_TYPE_BOOLEAN, &present);
 }
 
-static void sim_free_state(struct ofono_sim *sim)
+static void sim_free_early_state(struct ofono_sim *sim)
 {
 	if (sim->iccid) {
 		g_free(sim->iccid);
 		sim->iccid = NULL;
 	}
 
+	if (sim->efli) {
+		g_free(sim->efli);
+		sim->efli = NULL;
+		sim->efli_length = 0;
+	}
+
+	if (sim->language_prefs) {
+		g_strfreev(sim->language_prefs);
+		sim->language_prefs = NULL;
+	}
+}
+
+static void sim_free_main_state(struct ofono_sim *sim)
+{
 	if (sim->imsi) {
 		g_free(sim->imsi);
 		sim->imsi = NULL;
@@ -2106,17 +2121,6 @@ static void sim_free_state(struct ofono_sim *sim)
 		sim->service_numbers = NULL;
 	}
 
-	if (sim->efli) {
-		g_free(sim->efli);
-		sim->efli = NULL;
-		sim->efli_length = 0;
-	}
-
-	if (sim->language_prefs) {
-		g_strfreev(sim->language_prefs);
-		sim->language_prefs = NULL;
-	}
-
 	if (sim->efust) {
 		g_free(sim->efust);
 		sim->efust = NULL;
@@ -2150,6 +2154,12 @@ static void sim_free_state(struct ofono_sim *sim)
 	sim->barred_dialing = FALSE;
 }
 
+static void sim_free_state(struct ofono_sim *sim)
+{
+	sim_free_early_state(sim);
+	sim_free_main_state(sim);
+}
+
 void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
 {
 	ofono_sim_state_event_cb_t notify;
@@ -2442,3 +2452,132 @@ ofono_bool_t __ofono_is_valid_net_pin(const char *pin)
 {
 	return is_valid_pin(pin, 4, 4);
 }
+
+static void sim_file_changed_flush(struct ofono_sim *sim, int id)
+{
+	int i, imgid;
+
+	if (id == SIM_EFIMG_FILEID)
+		/* All cached images become invalid */
+		sim_fs_image_cache_flush(sim->simfs);
+	else if (sim->efimg) {
+		/*
+		 * Data and CLUT for image instances stored in the changed
+		 * file need to be re-read.
+		 */
+		for (i = sim->efimg_length / 9 - 1; i >= 0; i--) {
+			imgid = (sim->efimg[i * 9 + 3] << 8) |
+				sim->efimg[i * 9 + 4];
+
+			if (imgid == id)
+				sim_fs_image_cache_flush_file(sim->simfs, i);
+		}
+	}
+
+	sim_fs_cache_flush_file(sim->simfs, id);
+}
+
+void __ofono_sim_refresh(struct ofono_sim *sim, GSList *file_list,
+			ofono_bool_t full_file_change, ofono_bool_t naa_init)
+{
+	GSList *l;
+	ofono_sim_state_event_cb_t notify;
+	gboolean reinit_naa = naa_init || full_file_change;
+
+	/*
+	 * Check if any files used in SIM initialisation procedure
+	 * are affected, except EFiccid, EFpl, EFli.
+	 */
+	for (l = file_list; l; l = l->next) {
+		struct stk_file *file = l->data;
+		uint32_t mf, df, ef;
+
+		if (file->len != 6)
+			continue;
+
+		mf = (file->file[0] << 8) | (file->file[1] << 0);
+		df = (file->file[2] << 8) | (file->file[3] << 0);
+		ef = (file->file[4] << 8) | (file->file[5] << 0);
+
+		if (mf != 0x3f00)
+			continue;
+
+		/*
+		 * 8.18: "the path '3F007FFF' indicates the relevant
+		 * NAA Application dedicated file;".
+		 */
+		if (df == 0x7fff)
+			df = 0x7f20;
+
+#define DFGSM (0x7f20 << 16)
+#define DFTEL (0x7f10 << 16)
+
+		switch ((df << 16) | ef) {
+		case DFGSM | SIM_EFEST_FILEID:
+		case DFGSM | SIM_EFUST_FILEID: /* aka. EFSST */
+		case DFGSM | SIM_EFPHASE_FILEID:
+		case DFGSM | SIM_EFAD_FILEID:
+		case DFTEL | SIM_EFBDN_FILEID:
+		case DFTEL | SIM_EFADN_FILEID:
+		case DFGSM | SIM_EF_CPHS_INFORMATION_FILEID:
+			reinit_naa = TRUE;
+			break;
+		}
+	}
+
+	/* Flush cached content for affected files */
+	if (full_file_change)
+		sim_fs_cache_flush(sim->simfs);
+	else {
+		for (l = file_list; l; l = l->next) {
+			struct stk_file *file = l->data;
+			int id = (file->file[file->len - 2] << 8) |
+				(file->file[file->len - 1] << 0);
+
+			sim_file_changed_flush(sim, id);
+		}
+	}
+
+	if (reinit_naa) {
+		/* Force the sim state out of READY */
+
+		sim_free_main_state(sim);
+
+		sim->state = OFONO_SIM_STATE_INSERTED;
+		for (l = sim->state_watches->items; l; l = l->next) {
+			struct ofono_watchlist_item *item = l->data;
+			notify = item->notify;
+
+			notify(sim->state, item->notify_data);
+		}
+	}
+
+	/*
+	 * Notify the subscribers of files that have changed and who
+	 * haven't unsubsribed during the SIM state change.
+	 */
+	if (full_file_change)
+		sim_fs_notify_file_watches(sim->simfs, -1);
+	else {
+		for (l = file_list; l; l = l->next) {
+			struct stk_file *file = l->data;
+			int id = (file->file[file->len - 2] << 8) |
+				(file->file[file->len - 1] << 0);
+
+			sim_fs_notify_file_watches(sim->simfs, id);
+		}
+	}
+
+	if (reinit_naa) {
+		/*
+		 * REVISIT: There's some concern that on re-insertion the
+		 * atoms will start to talk to the SIM before it becomes
+		 * ready, on certain SIMs.
+		 */
+		/*
+		 * Start initialization procedure from after EFiccid,
+		 * EFli and EFpl are retrieved.
+		 */
+		sim_pin_check(sim);
+	}
+}
-- 
1.7.1.86.g0e460.dirty

[PATCH 3/9] sim: Watch for changes to relevant SIM files.

[Andrzej Zaborowski]

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

The watch callbacks are notified when a watched file is changed
during a Refresh.  Currently in most of the callbacks we free all
the information read from the file, and schedule a re-read.  I
wonder if we need some sort of check if a re-read is already in
progress.  We may also need to send PropertyChanged indicating
that the value is invalid until the file read finishes at which
point we send another PropertyChanged.  Otherwise the value of
the property is for a short while inconsistent with what
GetProperties would return (but I note that D-bus is already racy
so maybe it doesn't matter).
---
 src/sim.c |  153 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 143 insertions(+), 10 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index 7c2e151..63c1ce9 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -59,6 +59,7 @@ struct ofono_sim {
 	char **language_prefs;
 	unsigned char *efli;
 	unsigned char efli_length;
+	gboolean language_prefs_update;
 
 	enum ofono_sim_password_type pin_type;
 	gboolean locked_pins[OFONO_SIM_PASSWORD_SIM_PUK]; /* Number of PINs */
@@ -929,6 +930,11 @@ static void sim_iidf_read_cb(int ok, int length, int record,
 					sim_iidf_read_clut_cb, sim);
 }
 
+static void sim_image_data_changed(int id, void *userdata)
+{
+	/* TODO: notify D-bus clients */
+}
+
 static void sim_get_image(struct ofono_sim *sim, unsigned char id,
 				gpointer user_data)
 {
@@ -959,6 +965,8 @@ static void sim_get_image(struct ofono_sim *sim, unsigned char id,
 	/* read the image data */
 	ofono_sim_read_bytes(sim->context, iidf_id, iidf_offset, iidf_len,
 				sim_iidf_read_cb, sim);
+	ofono_sim_add_file_watch(sim->context, iidf_id,
+					sim_image_data_changed, sim, NULL);
 }
 
 static DBusMessage *sim_get_icon(DBusConnection *conn,
@@ -1198,10 +1206,12 @@ out:
 check:
 	/* All records retrieved */
 	if (sim->service_numbers) {
-		char **service_numbers;
-
 		sim->service_numbers = g_slist_reverse(sim->service_numbers);
 		sim->sdn_ready = TRUE;
+	}
+
+	if (sim->sdn_ready) {
+		char **service_numbers;
 
 		service_numbers = get_service_numbers(sim->service_numbers);
 
@@ -1214,6 +1224,21 @@ check:
 	}
 }
 
+static void sim_service_numbers_changed(int id, void *userdata)
+{
+	struct ofono_sim *sim = userdata;
+
+	if (sim->service_numbers) {
+		g_slist_foreach(sim->service_numbers,
+				(GFunc)service_number_free, NULL);
+		g_slist_free(sim->service_numbers);
+		sim->service_numbers = NULL;
+	}
+
+	ofono_sim_read(sim->context, SIM_EFSDN_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_sdn_read_cb, sim);
+}
+
 static void sim_own_numbers_update(struct ofono_sim *sim)
 {
 	ofono_sim_read(sim->context, SIM_EFMSISDN_FILEID,
@@ -1221,6 +1246,13 @@ static void sim_own_numbers_update(struct ofono_sim *sim)
 			sim);
 }
 
+static void sim_own_numbers_changed(int id, void *userdata)
+{
+	struct ofono_sim *sim = userdata;
+
+	sim_own_numbers_update(sim);
+}
+
 static void sim_efimg_read_cb(int ok, int length, int record,
 				const unsigned char *data,
 				int record_length, void *userdata)
@@ -1262,19 +1294,69 @@ static void sim_efimg_read_cb(int ok, int length, int record,
 	memcpy(efimg, &data[1], 9);
 }
 
+static void sim_efimg_changed(int id, void *userdata)
+{
+	struct ofono_sim *sim = userdata;
+	int i, iidf_id;
+
+	if (sim->efimg != NULL) {
+		for (i = sim->efimg_length / 9 - 1; i >= 0; i--) {
+			iidf_id = (sim->efimg[i * 9 + 3] << 8) |
+				sim->efimg[i * 9 + 4];
+
+			ofono_sim_remove_file_watch(sim->context, iidf_id);
+		}
+
+		g_free(sim->efimg);
+		sim->efimg = NULL;
+		sim->efimg_length = 0;
+	}
+
+	ofono_sim_read(sim->context, SIM_EFIMG_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_efimg_read_cb, sim);
+
+	/* TODO: notify D-bus clients */
+}
+
 static void sim_ready(enum ofono_sim_state new_state, void *user)
 {
 	struct ofono_sim *sim = user;
+	int i, iidf_id;
+
+	if (new_state != OFONO_SIM_STATE_READY) {
+		if (sim->context == NULL)
+			return;
+
+		ofono_sim_remove_file_watch(sim->context, SIM_EFMSISDN_FILEID);
+		ofono_sim_remove_file_watch(sim->context, SIM_EFSDN_FILEID);
+		ofono_sim_remove_file_watch(sim->context, SIM_EFIMG_FILEID);
+
+		if (sim->efimg == NULL)
+			return;
+
+		for (i = sim->efimg_length / 9 - 1; i >= 0; i--) {
+			iidf_id = (sim->efimg[i * 9 + 3] << 8) |
+				sim->efimg[i * 9 + 4];
+
+			ofono_sim_remove_file_watch(sim->context, iidf_id);
+		}
 
-	if (new_state != OFONO_SIM_STATE_READY)
 		return;
+	}
 
 	sim_own_numbers_update(sim);
+	ofono_sim_add_file_watch(sim->context, SIM_EFMSISDN_FILEID,
+					sim_own_numbers_changed, sim, NULL);
 
 	ofono_sim_read(sim->context, SIM_EFSDN_FILEID,
 			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_sdn_read_cb, sim);
+	ofono_sim_add_file_watch(sim->context, SIM_EFSDN_FILEID,
+					sim_service_numbers_changed, sim, NULL);
+
 	ofono_sim_read(sim->context, SIM_EFIMG_FILEID,
 			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_efimg_read_cb, sim);
+	ofono_sim_add_file_watch(sim->context, SIM_EFIMG_FILEID,
+					sim_efimg_changed, sim, NULL);
 }
 
 static void sim_imsi_cb(const struct ofono_error *error, const char *imsi,
@@ -1873,7 +1955,11 @@ skip_efpl:
 						DBUS_TYPE_STRING,
 						&sim->language_prefs);
 
-	sim_pin_check(sim);
+	/* Proceed with sim initialization if we're not merely updating */
+	if (!sim->language_prefs_update)
+		sim_pin_check(sim);
+
+	sim->language_prefs_update = FALSE;
 }
 
 static void sim_iccid_read_cb(int ok, int length, int record,
@@ -1899,6 +1985,43 @@ static void sim_iccid_read_cb(int ok, int length, int record,
 						&sim->iccid);
 }
 
+static void sim_iccid_changed(int id, void *userdata)
+{
+	struct ofono_sim *sim = userdata;
+
+	if (sim->iccid) {
+		g_free(sim->iccid);
+		sim->iccid = NULL;
+	}
+
+	ofono_sim_read(sim->context, SIM_EF_ICCID_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			sim_iccid_read_cb, sim);
+}
+
+static void sim_efli_efpl_changed(int id, void *userdata)
+{
+	struct ofono_sim *sim = userdata;
+
+	if (sim->efli != NULL) /* This shouldn't happen */
+		return;
+
+	if (sim->language_prefs) {
+		g_strfreev(sim->language_prefs);
+		sim->language_prefs = NULL;
+	}
+
+	sim->language_prefs_update = TRUE;
+
+	ofono_sim_read(sim->context, SIM_EFLI_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			sim_efli_read_cb, sim);
+
+	ofono_sim_read(sim->context, SIM_EFPL_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			sim_efpl_read_cb, sim);
+}
+
 static void sim_initialize(struct ofono_sim *sim)
 {
 	/*
@@ -1927,10 +2050,15 @@ static void sim_initialize(struct ofono_sim *sim)
 	 * in the EFust
 	 */
 
+	if (sim->context == NULL)
+		sim->context = ofono_sim_context_create(sim);
+
 	/* Grab the EFiccid which is always available */
 	ofono_sim_read(sim->context, SIM_EF_ICCID_FILEID,
 			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 			sim_iccid_read_cb, sim);
+	ofono_sim_add_file_watch(sim->context, SIM_EF_ICCID_FILEID,
+					sim_iccid_changed, sim, NULL);
 
 	/* EFecc is read by the voicecall atom */
 
@@ -1945,9 +2073,14 @@ static void sim_initialize(struct ofono_sim *sim)
 	ofono_sim_read(sim->context, SIM_EFLI_FILEID,
 			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 			sim_efli_read_cb, sim);
+	ofono_sim_add_file_watch(sim->context, SIM_EFLI_FILEID,
+					sim_efli_efpl_changed, sim, NULL);
+
 	ofono_sim_read(sim->context, SIM_EFPL_FILEID,
 			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 			sim_efpl_read_cb, sim);
+	ofono_sim_add_file_watch(sim->context, SIM_EFPL_FILEID,
+					sim_efli_efpl_changed, sim, NULL);
 }
 
 struct ofono_sim_context *ofono_sim_context_create(struct ofono_sim *sim)
@@ -2096,6 +2229,11 @@ static void sim_free_early_state(struct ofono_sim *sim)
 		g_strfreev(sim->language_prefs);
 		sim->language_prefs = NULL;
 	}
+
+	if (sim->context) {
+		ofono_sim_context_free(sim->context);
+		sim->context = NULL;
+	}
 }
 
 static void sim_free_main_state(struct ofono_sim *sim)
@@ -2119,6 +2257,7 @@ static void sim_free_main_state(struct ofono_sim *sim)
 				(GFunc)service_number_free, NULL);
 		g_slist_free(sim->service_numbers);
 		sim->service_numbers = NULL;
+		sim->sdn_ready = FALSE;
 	}
 
 	if (sim->efust) {
@@ -2296,11 +2435,6 @@ static void sim_remove(struct ofono_atom *atom)
 
 	sim_free_state(sim);
 
-	if (sim->context) {
-		ofono_sim_context_free(sim->context);
-		sim->context = NULL;
-	}
-
 	sim_fs_free(sim->simfs);
 	sim->simfs = NULL;
 
@@ -2366,7 +2500,6 @@ void ofono_sim_register(struct ofono_sim *sim)
 	ofono_modem_add_interface(modem, OFONO_SIM_MANAGER_INTERFACE);
 	sim->state_watches = __ofono_watchlist_new(g_free);
 	sim->simfs = sim_fs_new(sim, sim->driver);
-	sim->context = ofono_sim_context_create(sim);
 
 	__ofono_atom_register(sim->atom, sim_unregister);
 
-- 
1.7.1.86.g0e460.dirty

[PATCH 4/9] voicecall: Watch for changes to relevant SIM files.

[Andrzej Zaborowski]

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

---
 src/voicecall.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/src/voicecall.c b/src/voicecall.c
index 4606668..d6e8539 100644
--- a/src/voicecall.c
+++ b/src/voicecall.c
@@ -2279,6 +2279,19 @@ struct ofono_voicecall *ofono_voicecall_create(struct ofono_modem *modem,
 	return vc;
 }
 
+static void read_ecc_numbers(int id, void *userdata)
+{
+	struct ofono_voicecall *vc = userdata;
+
+	/* Try both formats, only one or none will work */
+	ofono_sim_read(vc->sim_context, SIM_EFECC_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			ecc_g2_read_cb, vc);
+	ofono_sim_read(vc->sim_context, SIM_EFECC_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_FIXED,
+			ecc_g3_read_cb, vc);
+}
+
 static void sim_state_watch(enum ofono_sim_state new_state, void *user)
 {
 	struct ofono_voicecall *vc = user;
@@ -2288,13 +2301,10 @@ static void sim_state_watch(enum ofono_sim_state new_state, void *user)
 		if (vc->sim_context == NULL)
 			vc->sim_context = ofono_sim_context_create(vc->sim);
 
-		/* Try both formats, only one or none will work */
-		ofono_sim_read(vc->sim_context, SIM_EFECC_FILEID,
-				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
-				ecc_g2_read_cb, vc);
-		ofono_sim_read(vc->sim_context, SIM_EFECC_FILEID,
-				OFONO_SIM_FILE_STRUCTURE_FIXED,
-				ecc_g3_read_cb, vc);
+		read_ecc_numbers(SIM_EFECC_FILEID, vc);
+
+		ofono_sim_add_file_watch(vc->sim_context, SIM_EFECC_FILEID,
+						read_ecc_numbers, vc, NULL);
 		break;
 	case OFONO_SIM_STATE_NOT_PRESENT:
 		/* TODO: Must release all non-emergency calls */
-- 
1.7.1.86.g0e460.dirty

[PATCH 5/9] network: Watch for changes to relevant SIM files.

[Andrzej Zaborowski]

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

---
 src/network.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 66 insertions(+), 0 deletions(-)

diff --git a/src/network.c b/src/network.c
index 9d1d05f..c059906 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1784,6 +1784,59 @@ static void netreg_load_settings(struct ofono_netreg *netreg)
 				"Mode", netreg->mode);
 }
 
+static void sim_pnn_opl_changed(int id, void *userdata)
+{
+	struct ofono_netreg *netreg = userdata;
+	GSList *l;
+
+	/*
+	 * Free references to structures on the netreg->eons list and
+	 * update the operator info on D-bus.  If EFpnn/EFopl read succeeds,
+	 * operator info will be updated again, otherwise it won't be
+	 * updated again.
+	 */
+	for (l = netreg->operator_list; l; l = l->next)
+		set_network_operator_eons_info(l->data, NULL);
+
+	sim_eons_free(netreg->eons);
+	netreg->eons = NULL;
+
+	ofono_sim_read(netreg->sim_context, SIM_EFPNN_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_FIXED,
+			sim_pnn_read_cb, netreg);
+}
+
+static void sim_spn_spdi_changed(int id, void *userdata)
+{
+	struct ofono_netreg *netreg = userdata;
+
+	netreg->flags &= ~(NETWORK_REGISTRATION_FLAG_HOME_SHOW_PLMN |
+			NETWORK_REGISTRATION_FLAG_ROAMING_SHOW_SPN);
+
+	g_free(netreg->spname);
+	netreg->spname = NULL;
+
+	sim_spdi_free(netreg->spdi);
+	netreg->spdi = NULL;
+
+	if (netreg->current_operator) {
+		DBusConnection *conn = ofono_dbus_get_connection();
+		const char *path = __ofono_atom_get_path(netreg->atom);
+		const char *operator;
+
+		operator = get_operator_display_name(netreg);
+
+		ofono_dbus_signal_property_changed(conn, path,
+					OFONO_NETWORK_REGISTRATION_INTERFACE,
+					"Name", DBUS_TYPE_STRING,
+					&operator);
+	}
+
+	ofono_sim_read(netreg->sim_context, SIM_EFSPN_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			sim_spn_read_cb, netreg);
+}
+
 void ofono_netreg_register(struct ofono_netreg *netreg)
 {
 	DBusConnection *conn = ofono_dbus_get_connection();
@@ -1822,9 +1875,22 @@ void ofono_netreg_register(struct ofono_netreg *netreg)
 		ofono_sim_read(netreg->sim_context, SIM_EFPNN_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_FIXED,
 				sim_pnn_read_cb, netreg);
+		ofono_sim_add_file_watch(netreg->sim_context, SIM_EFPNN_FILEID,
+						sim_pnn_opl_changed, netreg,
+						NULL);
+		ofono_sim_add_file_watch(netreg->sim_context, SIM_EFOPL_FILEID,
+						sim_pnn_opl_changed, netreg,
+						NULL);
+
 		ofono_sim_read(netreg->sim_context, SIM_EFSPN_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 				sim_spn_read_cb, netreg);
+		ofono_sim_add_file_watch(netreg->sim_context, SIM_EFSPN_FILEID,
+						sim_spn_spdi_changed, netreg,
+						NULL);
+		ofono_sim_add_file_watch(netreg->sim_context, SIM_EFSPDI_FILEID,
+						sim_spn_spdi_changed, netreg,
+						NULL);
 	}
 
 	__ofono_atom_register(netreg->atom, netreg_unregister);
-- 
1.7.1.86.g0e460.dirty

[PATCH 6/9] cbs: Watch for changes to relevant SIM files.

[Andrzej Zaborowski]

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

---
 src/cbs.c |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/src/cbs.c b/src/cbs.c
index cf8fc10..c58ff89 100644
--- a/src/cbs.c
+++ b/src/cbs.c
@@ -68,6 +68,7 @@ struct ofono_cbs {
 	GSList *efcbmir_contents;
 	unsigned short efcbmid_length;
 	GSList *efcbmid_contents;
+	gboolean efcbmid_update;
 	guint reset_source;
 	int lac;
 	int ci;
@@ -852,6 +853,7 @@ static void sim_cbmid_read_cb(int ok, int length, int record,
 	int i;
 	char *str;
 	GSList *contents = NULL;
+	char *topic_str;
 
 	if (!ok)
 		goto done;
@@ -886,7 +888,33 @@ static void sim_cbmid_read_cb(int ok, int length, int record,
 	g_free(str);
 
 done:
-	cbs_got_file_contents(cbs);
+	if (cbs->efcbmid_update) {
+		topic_str = cbs_topics_to_str(cbs, cbs->topics);
+		cbs->driver->set_topics(cbs, topic_str,
+					cbs_set_powered_cb, cbs);
+		g_free(topic_str);
+
+		cbs->efcbmid_update = FALSE;
+	} else
+		cbs_got_file_contents(cbs);
+}
+
+static void cbs_efcbmid_changed(int id, void *userdata)
+{
+	struct ofono_cbs *cbs = userdata;
+
+	if (cbs->efcbmid_length) {
+		cbs->efcbmid_length = 0;
+		g_slist_foreach(cbs->efcbmid_contents, (GFunc) g_free, NULL);
+		g_slist_free(cbs->efcbmid_contents);
+		cbs->efcbmid_contents = NULL;
+	}
+
+	cbs->efcbmid_update = TRUE;
+
+	ofono_sim_read(cbs->sim_context, SIM_EFCBMID_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			sim_cbmid_read_cb, cbs);
 }
 
 static void cbs_got_imsi(struct ofono_cbs *cbs)
@@ -929,6 +957,8 @@ static void cbs_got_imsi(struct ofono_cbs *cbs)
 	ofono_sim_read(cbs->sim_context, SIM_EFCBMID_FILEID,
 			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 			sim_cbmid_read_cb, cbs);
+	ofono_sim_add_file_watch(cbs->sim_context, SIM_EFCBMID_FILEID,
+					cbs_efcbmid_changed, cbs, NULL);
 }
 
 static gboolean reset_base_station_name(gpointer user)
-- 
1.7.1.86.g0e460.dirty

[PATCH 7/9] message-waiting: Watch for changes to relevant SIM files.

[Andrzej Zaborowski]

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

---
 src/message-waiting.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 87 insertions(+), 1 deletions(-)

diff --git a/src/message-waiting.c b/src/message-waiting.c
index 72cf582..f5406ca 100644
--- a/src/message-waiting.c
+++ b/src/message-waiting.c
@@ -576,6 +576,33 @@ static void mw_mbdn_read_cb(int ok, int total_length, int record,
 	mw->efmbdn_length = record_length;
 }
 
+static void mw_mbdn_changed(int id, void *userdata)
+{
+	struct ofono_message_waiting *mw = userdata;
+	int err;
+
+	mw->efmbdn_length = 0;
+	mw->mbdn_not_provided = FALSE;
+
+	err = ofono_sim_read(mw->sim_context, SIM_EFMBDN_FILEID,
+				OFONO_SIM_FILE_STRUCTURE_FIXED,
+				mw_mbdn_read_cb, mw);
+	if (err != 0)
+		ofono_error("Unable to read EF-MBDN from SIM");
+}
+
+static void mw_cphs_mbdn_changed(int id, void *userdata)
+{
+	struct ofono_message_waiting *mw = userdata;
+
+	mw->ef_cphs_mbdn_length = 0;
+	mw->cphs_mbdn_not_provided = FALSE;
+
+	ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MBDN_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_FIXED,
+			mw_cphs_mbdn_read_cb, mw);
+}
+
 static void mw_mbi_read_cb(int ok, int total_length, int record,
 				const unsigned char *data,
 				int record_length, void *userdata)
@@ -604,6 +631,8 @@ static void mw_mbi_read_cb(int ok, int total_length, int record,
 	err = ofono_sim_read(mw->sim_context, SIM_EFMBDN_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_FIXED,
 				mw_mbdn_read_cb, mw);
+	ofono_sim_add_file_watch(mw->sim_context, SIM_EFMBDN_FILEID,
+					mw_mbdn_changed, mw, NULL);
 
 	if (err != 0)
 		ofono_error("Unable to read EF-MBDN from SIM");
@@ -615,10 +644,14 @@ out:
 	 */
 	st = ofono_sim_get_cphs_service_table(mw->sim);
 
-	if (st && bit_field(st[0], 4, 2) == 3)
+	if (st && bit_field(st[0], 4, 2) == 3) {
 		ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MBDN_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_FIXED,
 				mw_cphs_mbdn_read_cb, mw);
+		ofono_sim_add_file_watch(mw->sim_context,
+						SIM_EF_CPHS_MBDN_FILEID,
+						mw_cphs_mbdn_changed, mw, NULL);
+	}
 }
 
 static void mw_mwis_write_cb(int ok, void *userdata)
@@ -935,6 +968,46 @@ static void message_waiting_unregister(struct ofono_atom *atom)
 	ofono_modem_remove_interface(modem, OFONO_MESSAGE_WAITING_INTERFACE);
 }
 
+static void mw_mwis_changed(int id, void *userdata)
+{
+	struct ofono_message_waiting *mw = userdata;
+
+	mw->efmwis_length = 0;
+
+	ofono_sim_read(mw->sim_context, SIM_EFMWIS_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_FIXED,
+			mw_mwis_read_cb, mw);
+}
+
+static void mw_cphs_mwis_changed(int id, void *userdata)
+{
+	struct ofono_message_waiting *mw = userdata;
+
+	mw->ef_cphs_mwis_length = 0;
+
+	ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MWIS_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+			mw_cphs_mwis_read_cb, mw);
+}
+
+static void mw_mbi_changed(int id, void *userdata)
+{
+	struct ofono_message_waiting *mw = userdata;
+
+	mw->efmbdn_length = 0;
+	mw->mbdn_not_provided = FALSE;
+
+	mw->ef_cphs_mbdn_length = 0;
+	mw->cphs_mbdn_not_provided = FALSE;
+
+	ofono_sim_remove_file_watch(mw->sim_context, SIM_EFMBDN_FILEID);
+	ofono_sim_remove_file_watch(mw->sim_context, SIM_EF_CPHS_MBDN_FILEID);
+
+	ofono_sim_read(mw->sim_context, SIM_EFMBI_FILEID,
+			OFONO_SIM_FILE_STRUCTURE_FIXED,
+			mw_mbi_read_cb, mw);
+}
+
 void ofono_message_waiting_register(struct ofono_message_waiting *mw)
 {
 	DBusConnection *conn;
@@ -980,6 +1053,19 @@ void ofono_message_waiting_register(struct ofono_message_waiting *mw)
 		ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MWIS_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 				mw_cphs_mwis_read_cb, mw);
+
+		/*
+		 * The operator could send us SMS mwi updates, but let's be
+		 * extra careful and track the file contents too.
+		 */
+		ofono_sim_add_file_watch(mw->sim_context, SIM_EFMWIS_FILEID,
+						mw_mwis_changed, mw, NULL);
+		ofono_sim_add_file_watch(mw->sim_context,
+						SIM_EF_CPHS_MWIS_FILEID,
+						mw_cphs_mwis_changed, mw, NULL);
+
+		ofono_sim_add_file_watch(mw->sim_context, SIM_EFMBI_FILEID,
+						mw_mbi_changed, mw, NULL);
 	}
 
 	__ofono_atom_register(mw->atom, message_waiting_unregister);
-- 
1.7.1.86.g0e460.dirty

[PATCH 8/9] call-forwarding: Watch for changes to relevant SIM files.

[Andrzej Zaborowski]

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

---
 src/call-forwarding.c |   39 +++++++++++++++++++++++++++++++++++++--
 1 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index d13f990..d610521 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -74,6 +74,7 @@ static void get_query_next_cf_cond(struct ofono_call_forwarding *cf);
 static void set_query_next_cf_cond(struct ofono_call_forwarding *cf);
 static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf);
 static void cf_unregister_ss_controls(struct ofono_call_forwarding *cf);
+static void sim_read_cf_indicator(struct ofono_call_forwarding *cf);
 
 struct cf_ss_request {
 	int ss_type;
@@ -1358,18 +1359,52 @@ static void sim_cphs_cff_read_cb(int ok, int total_length, int record,
 					DBUS_TYPE_BOOLEAN, &cfu_voice);
 }
 
+static void sim_cfis_changed(int id, void *userdata)
+{
+	struct ofono_call_forwarding *cf = userdata;
+
+	if (((cf->flags & CALL_FORWARDING_FLAG_CPHS_CFF) ||
+			cf->cfis_record_id > 0) && is_cfu_enabled(cf, NULL)) {
+		DBusConnection *conn = ofono_dbus_get_connection();
+		const char *path = __ofono_atom_get_path(cf->atom);
+		ofono_bool_t status = FALSE;
+
+		ofono_dbus_signal_property_changed(conn, path,
+					OFONO_CALL_FORWARDING_INTERFACE,
+					"ForwardingFlagOnSim",
+					DBUS_TYPE_BOOLEAN, &status);
+	}
+
+	cf->cfis_record_id = 0;
+	cf->flags &= ~CALL_FORWARDING_FLAG_CPHS_CFF;
+
+	/*
+	 * TODO: remove forwarding rules in
+	 * cf->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL] that
+	 * originate from EFcfis before adding the new rules?
+	 */
+
+	sim_read_cf_indicator(cf);
+}
+
 static void sim_read_cf_indicator(struct ofono_call_forwarding *cf)
 {
 	if (__ofono_sim_service_available(cf->sim,
 			SIM_UST_SERVICE_CFIS,
-			SIM_SST_SERVICE_CFIS) == TRUE)
+			SIM_SST_SERVICE_CFIS) == TRUE) {
 		ofono_sim_read(cf->sim_context, SIM_EFCFIS_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_FIXED,
 				sim_cfis_read_cb, cf);
-	else
+		ofono_sim_add_file_watch(cf->sim_context, SIM_EFCFIS_FILEID,
+						sim_cfis_changed, cf, NULL);
+	} else {
 		ofono_sim_read(cf->sim_context, SIM_EF_CPHS_CFF_FILEID,
 				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
 				sim_cphs_cff_read_cb, cf);
+		ofono_sim_add_file_watch(cf->sim_context,
+						SIM_EF_CPHS_CFF_FILEID,
+						sim_cfis_changed, cf, NULL);
+	}
 }
 
 int ofono_call_forwarding_driver_register(const struct ofono_call_forwarding_driver *d)
-- 
1.7.1.86.g0e460.dirty

[PATCH 9/9] stk: Partially handle Refresh command.

[Andrzej Zaborowski]

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

Only the four "NAA initialisation" modes are handled at the moment.
---
 src/stk.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/src/stk.c b/src/stk.c
index 199e02b..bc46b2f 100644
--- a/src/stk.c
+++ b/src/stk.c
@@ -1973,10 +1973,22 @@ static gboolean handle_command_send_ussd(const struct stk_command *cmd,
 	return FALSE;
 }
 
+static void free_idle_mode_text(struct ofono_stk *stk)
+{
+	g_free(stk->idle_mode_text);
+	stk->idle_mode_text = NULL;
+
+	memset(&stk->idle_mode_icon, 0, sizeof(stk->idle_mode_icon));
+}
+
 static gboolean handle_command_refresh(const struct stk_command *cmd,
 					struct stk_response *rsp,
 					struct ofono_stk *stk)
 {
+	struct ofono_error failure = { .type = OFONO_ERROR_TYPE_FAILURE };
+	struct ofono_sim *sim = NULL;
+	struct ofono_atom *sim_atom;
+	int err;
 	GSList *l;
 
 	DBG("");
@@ -2030,6 +2042,59 @@ static gboolean handle_command_refresh(const struct stk_command *cmd,
 					cmd->refresh.icon_id.qualifier);
 	DBG("Alpha ID: %s", cmd->refresh.alpha_id);
 
+	sim_atom = __ofono_modem_find_atom(__ofono_atom_get_modem(stk->atom),
+						OFONO_ATOM_TYPE_SIM);
+	if (sim_atom)
+		sim = __ofono_atom_get_data(sim_atom);
+
+	if (sim == NULL) {
+		rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
+		return TRUE;
+	}
+
+	if (cmd->qualifier < 4) {
+		int qualifier = stk->pending_cmd->qualifier;
+		GSList *file_list = stk->pending_cmd->refresh.file_list;
+
+		/* Don't free the list yet */
+		stk->pending_cmd->refresh.file_list = NULL;
+
+		/*
+		 * Queue the TERMINAL RESPONSE before triggering potential
+		 * file accesses.
+		 */
+		err = stk_respond(stk, rsp, stk_command_cb);
+		if (err)
+			stk_command_cb(&failure, stk);
+
+		/* TODO: use the alphaId / icon */
+		/* TODO: if AID is supplied, check its value */
+		/* TODO: possibly check if a D-bus call is pending or
+		 * an STK session ongoing. */
+
+		/* TODO: free some elements of the atom state */
+
+		switch (qualifier) {
+		case 0:
+			free_idle_mode_text(stk);
+			__ofono_sim_refresh(sim, file_list, TRUE, TRUE);
+			break;
+		case 1:
+			__ofono_sim_refresh(sim, file_list, FALSE, FALSE);
+			break;
+		case 2:
+		case 3:
+			free_idle_mode_text(stk);
+			__ofono_sim_refresh(sim, file_list, FALSE, TRUE);
+			break;
+		}
+
+		g_slist_foreach(file_list, (GFunc) g_free, NULL);
+		g_slist_free(file_list);
+
+		return FALSE;
+	}
+
 	rsp->result.type = STK_RESULT_TYPE_NOT_CAPABLE;
 	return TRUE;
 }
-- 
1.7.1.86.g0e460.dirty

Re: [PATCH 7/9] message-waiting: Watch for changes to relevant SIM files.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> ---
>  src/message-waiting.c |   88 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 87 insertions(+), 1 deletions(-)
> 
> diff --git a/src/message-waiting.c b/src/message-waiting.c
> index 72cf582..f5406ca 100644
> --- a/src/message-waiting.c
> +++ b/src/message-waiting.c
> @@ -576,6 +576,33 @@ static void mw_mbdn_read_cb(int ok, int total_length, int record,
>  	mw->efmbdn_length = record_length;
>  }
>  
> +static void mw_mbdn_changed(int id, void *userdata)
> +{
> +	struct ofono_message_waiting *mw = userdata;
> +	int err;
> +
> +	mw->efmbdn_length = 0;
> +	mw->mbdn_not_provided = FALSE;
> +
> +	err = ofono_sim_read(mw->sim_context, SIM_EFMBDN_FILEID,
> +				OFONO_SIM_FILE_STRUCTURE_FIXED,
> +				mw_mbdn_read_cb, mw);
> +	if (err != 0)
> +		ofono_error("Unable to read EF-MBDN from SIM");
> +}
> +
> +static void mw_cphs_mbdn_changed(int id, void *userdata)
> +{
> +	struct ofono_message_waiting *mw = userdata;
> +
> +	mw->ef_cphs_mbdn_length = 0;
> +	mw->cphs_mbdn_not_provided = FALSE;
> +
> +	ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MBDN_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_FIXED,
> +			mw_cphs_mbdn_read_cb, mw);
> +}
> +
>  static void mw_mbi_read_cb(int ok, int total_length, int record,
>  				const unsigned char *data,
>  				int record_length, void *userdata)
> @@ -604,6 +631,8 @@ static void mw_mbi_read_cb(int ok, int total_length, int record,
>  	err = ofono_sim_read(mw->sim_context, SIM_EFMBDN_FILEID,
>  				OFONO_SIM_FILE_STRUCTURE_FIXED,
>  				mw_mbdn_read_cb, mw);
> +	ofono_sim_add_file_watch(mw->sim_context, SIM_EFMBDN_FILEID,
> +					mw_mbdn_changed, mw, NULL);
>  
>  	if (err != 0)
>  		ofono_error("Unable to read EF-MBDN from SIM");
> @@ -615,10 +644,14 @@ out:
>  	 */
>  	st = ofono_sim_get_cphs_service_table(mw->sim);
>  
> -	if (st && bit_field(st[0], 4, 2) == 3)
> +	if (st && bit_field(st[0], 4, 2) == 3) {
>  		ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MBDN_FILEID,
>  				OFONO_SIM_FILE_STRUCTURE_FIXED,
>  				mw_cphs_mbdn_read_cb, mw);
> +		ofono_sim_add_file_watch(mw->sim_context,
> +						SIM_EF_CPHS_MBDN_FILEID,
> +						mw_cphs_mbdn_changed, mw, NULL);
> +	}
>  }
>  
>  static void mw_mwis_write_cb(int ok, void *userdata)
> @@ -935,6 +968,46 @@ static void message_waiting_unregister(struct ofono_atom *atom)
>  	ofono_modem_remove_interface(modem, OFONO_MESSAGE_WAITING_INTERFACE);
>  }
>  
> +static void mw_mwis_changed(int id, void *userdata)
> +{
> +	struct ofono_message_waiting *mw = userdata;
> +
> +	mw->efmwis_length = 0;
> +
> +	ofono_sim_read(mw->sim_context, SIM_EFMWIS_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_FIXED,
> +			mw_mwis_read_cb, mw);
> +}
> +
> +static void mw_cphs_mwis_changed(int id, void *userdata)
> +{
> +	struct ofono_message_waiting *mw = userdata;
> +
> +	mw->ef_cphs_mwis_length = 0;
> +
> +	ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MWIS_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +			mw_cphs_mwis_read_cb, mw);
> +}
> +
> +static void mw_mbi_changed(int id, void *userdata)
> +{
> +	struct ofono_message_waiting *mw = userdata;
> +
> +	mw->efmbdn_length = 0;
> +	mw->mbdn_not_provided = FALSE;
> +
> +	mw->ef_cphs_mbdn_length = 0;
> +	mw->cphs_mbdn_not_provided = FALSE;
> +
> +	ofono_sim_remove_file_watch(mw->sim_context, SIM_EFMBDN_FILEID);
> +	ofono_sim_remove_file_watch(mw->sim_context, SIM_EF_CPHS_MBDN_FILEID);

This won't actually work, the id returned by add_file_watch and taken by
remove_file_watch is just an unsigned int.  Not the id of the watched
file.  In theory we can try to make it work this way, but it doesn't
right now.

> +
> +	ofono_sim_read(mw->sim_context, SIM_EFMBI_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_FIXED,
> +			mw_mbi_read_cb, mw);
> +}
> +
>  void ofono_message_waiting_register(struct ofono_message_waiting *mw)
>  {
>  	DBusConnection *conn;
> @@ -980,6 +1053,19 @@ void ofono_message_waiting_register(struct ofono_message_waiting *mw)
>  		ofono_sim_read(mw->sim_context, SIM_EF_CPHS_MWIS_FILEID,
>  				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>  				mw_cphs_mwis_read_cb, mw);
> +
> +		/*
> +		 * The operator could send us SMS mwi updates, but let's be
> +		 * extra careful and track the file contents too.
> +		 */
> +		ofono_sim_add_file_watch(mw->sim_context, SIM_EFMWIS_FILEID,
> +						mw_mwis_changed, mw, NULL);
> +		ofono_sim_add_file_watch(mw->sim_context,
> +						SIM_EF_CPHS_MWIS_FILEID,
> +						mw_cphs_mwis_changed, mw, NULL);
> +
> +		ofono_sim_add_file_watch(mw->sim_context, SIM_EFMBI_FILEID,
> +						mw_mbi_changed, mw, NULL);
>  	}
>  
>  	__ofono_atom_register(mw->atom, message_waiting_unregister);

Regards,
-Denis

Re: [PATCH 8/9] call-forwarding: Watch for changes to relevant SIM files.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> ---
>  src/call-forwarding.c |   39 +++++++++++++++++++++++++++++++++++++--
>  1 files changed, 37 insertions(+), 2 deletions(-)
> 
> diff --git a/src/call-forwarding.c b/src/call-forwarding.c
> index d13f990..d610521 100644
> --- a/src/call-forwarding.c
> +++ b/src/call-forwarding.c
> @@ -74,6 +74,7 @@ static void get_query_next_cf_cond(struct ofono_call_forwarding *cf);
>  static void set_query_next_cf_cond(struct ofono_call_forwarding *cf);
>  static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf);
>  static void cf_unregister_ss_controls(struct ofono_call_forwarding *cf);
> +static void sim_read_cf_indicator(struct ofono_call_forwarding *cf);
>  
>  struct cf_ss_request {
>  	int ss_type;
> @@ -1358,18 +1359,52 @@ static void sim_cphs_cff_read_cb(int ok, int total_length, int record,
>  					DBUS_TYPE_BOOLEAN, &cfu_voice);
>  }
>  
> +static void sim_cfis_changed(int id, void *userdata)
> +{
> +	struct ofono_call_forwarding *cf = userdata;
> +
> +	if (((cf->flags & CALL_FORWARDING_FLAG_CPHS_CFF) ||
> +			cf->cfis_record_id > 0) && is_cfu_enabled(cf, NULL)) {
> +		DBusConnection *conn = ofono_dbus_get_connection();
> +		const char *path = __ofono_atom_get_path(cf->atom);
> +		ofono_bool_t status = FALSE;
> +
> +		ofono_dbus_signal_property_changed(conn, path,
> +					OFONO_CALL_FORWARDING_INTERFACE,
> +					"ForwardingFlagOnSim",
> +					DBUS_TYPE_BOOLEAN, &status);
> +	}
> +
> +	cf->cfis_record_id = 0;
> +	cf->flags &= ~CALL_FORWARDING_FLAG_CPHS_CFF;
> +
> +	/*
> +	 * TODO: remove forwarding rules in
> +	 * cf->cf_conditions[CALL_FORWARDING_TYPE_UNCONDITIONAL] that
> +	 * originate from EFcfis before adding the new rules?
> +	 */
> +
> +	sim_read_cf_indicator(cf);

Wouldn't this result in us adding a 2nd, or 3rd, or 4th, etc file watch
here?

> +}
> +
>  static void sim_read_cf_indicator(struct ofono_call_forwarding *cf)
>  {
>  	if (__ofono_sim_service_available(cf->sim,
>  			SIM_UST_SERVICE_CFIS,
> -			SIM_SST_SERVICE_CFIS) == TRUE)
> +			SIM_SST_SERVICE_CFIS) == TRUE) {
>  		ofono_sim_read(cf->sim_context, SIM_EFCFIS_FILEID,
>  				OFONO_SIM_FILE_STRUCTURE_FIXED,
>  				sim_cfis_read_cb, cf);
> -	else
> +		ofono_sim_add_file_watch(cf->sim_context, SIM_EFCFIS_FILEID,
> +						sim_cfis_changed, cf, NULL);
> +	} else {
>  		ofono_sim_read(cf->sim_context, SIM_EF_CPHS_CFF_FILEID,
>  				OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>  				sim_cphs_cff_read_cb, cf);
> +		ofono_sim_add_file_watch(cf->sim_context,
> +						SIM_EF_CPHS_CFF_FILEID,
> +						sim_cfis_changed, cf, NULL);
> +	}
>  }
>  
>  int ofono_call_forwarding_driver_register(const struct ofono_call_forwarding_driver *d)

Regards,
-Denis

Re: [PATCH 1/9] modem.c: Go to PRE_SIM modem state on OFONO_SIM_STATE_INSERTED.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> The assumption so far was that SIM state would only go from
> NOT_PRESENT -> INSERTED -> READY -> NOT_PRESENT because there's
> no method to unauthenticate from SIM without removing.  We need to
> be able to switch to INSERTED when SIM is being reset,
> reinitialised, etc. because going to NOT_PRESENT and then
> immediately INSERTED would be logically wrong.
> ---
>  src/modem.c |   12 ++++++++----
>  1 files changed, 8 insertions(+), 4 deletions(-)

I applied this patch, but partly refactored it in a later commit.  Can
you check that I didn't screw something up?

Regards,
-Denis

Re: [PATCH 2/9] sim: Implement basic Refresh.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> ---
>  src/ofono.h |    4 ++
>  src/sim.c   |  163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 155 insertions(+), 12 deletions(-)
> 

Patch has been applied, but some comments below:

> +	if (reinit_naa) {
> +		/* Force the sim state out of READY */
> +
> +		sim_free_main_state(sim);
> +
> +		sim->state = OFONO_SIM_STATE_INSERTED;
> +		for (l = sim->state_watches->items; l; l = l->next) {
> +			struct ofono_watchlist_item *item = l->data;
> +			notify = item->notify;
> +
> +			notify(sim->state, item->notify_data);
> +		}

I changed this to directly call the modem object and force it into
pre-sim state.  I think this is a bit more clear than your proposed
approach.

Another question here is whether we want to maintain two separate simfs
contexts.  One for the main state and one for the early state.  This
might make things a bit easier to implement for patch 3 in this series.

Regards,
-Denis

Re: [PATCH 3/9] sim: Watch for changes to relevant SIM files.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> The watch callbacks are notified when a watched file is changed
> during a Refresh.  Currently in most of the callbacks we free all
> the information read from the file, and schedule a re-read.  I
> wonder if we need some sort of check if a re-read is already in
> progress.  We may also need to send PropertyChanged indicating
> that the value is invalid until the file read finishes at which
> point we send another PropertyChanged.  Otherwise the value of
> the property is for a short while inconsistent with what
> GetProperties would return (but I note that D-bus is already racy
> so maybe it doesn't matter).
> ---
>  src/sim.c |  153 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 143 insertions(+), 10 deletions(-)
> 
> diff --git a/src/sim.c b/src/sim.c
> index 7c2e151..63c1ce9 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -59,6 +59,7 @@ struct ofono_sim {
>  	char **language_prefs;
>  	unsigned char *efli;
>  	unsigned char efli_length;
> +	gboolean language_prefs_update;
>  
>  	enum ofono_sim_password_type pin_type;
>  	gboolean locked_pins[OFONO_SIM_PASSWORD_SIM_PUK]; /* Number of PINs */
> @@ -929,6 +930,11 @@ static void sim_iidf_read_cb(int ok, int length, int record,
>  					sim_iidf_read_clut_cb, sim);
>  }
>  
> +static void sim_image_data_changed(int id, void *userdata)
> +{
> +	/* TODO: notify D-bus clients */
> +}
> +
>  static void sim_get_image(struct ofono_sim *sim, unsigned char id,
>  				gpointer user_data)
>  {
> @@ -959,6 +965,8 @@ static void sim_get_image(struct ofono_sim *sim, unsigned char id,
>  	/* read the image data */
>  	ofono_sim_read_bytes(sim->context, iidf_id, iidf_offset, iidf_len,
>  				sim_iidf_read_cb, sim);
> +	ofono_sim_add_file_watch(sim->context, iidf_id,
> +					sim_image_data_changed, sim, NULL);
>  }
>  
>  static DBusMessage *sim_get_icon(DBusConnection *conn,
> @@ -1198,10 +1206,12 @@ out:
>  check:
>  	/* All records retrieved */
>  	if (sim->service_numbers) {
> -		char **service_numbers;
> -
>  		sim->service_numbers = g_slist_reverse(sim->service_numbers);
>  		sim->sdn_ready = TRUE;
> +	}
> +
> +	if (sim->sdn_ready) {
> +		char **service_numbers;
>  
>  		service_numbers = get_service_numbers(sim->service_numbers);
>  
> @@ -1214,6 +1224,21 @@ check:
>  	}
>  }
>  
> +static void sim_service_numbers_changed(int id, void *userdata)
> +{
> +	struct ofono_sim *sim = userdata;
> +
> +	if (sim->service_numbers) {
> +		g_slist_foreach(sim->service_numbers,
> +				(GFunc)service_number_free, NULL);
> +		g_slist_free(sim->service_numbers);
> +		sim->service_numbers = NULL;
> +	}
> +
> +	ofono_sim_read(sim->context, SIM_EFSDN_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_sdn_read_cb, sim);
> +}
> +
>  static void sim_own_numbers_update(struct ofono_sim *sim)
>  {
>  	ofono_sim_read(sim->context, SIM_EFMSISDN_FILEID,
> @@ -1221,6 +1246,13 @@ static void sim_own_numbers_update(struct ofono_sim *sim)
>  			sim);
>  }
>  
> +static void sim_own_numbers_changed(int id, void *userdata)
> +{
> +	struct ofono_sim *sim = userdata;
> +
> +	sim_own_numbers_update(sim);
> +}
> +
>  static void sim_efimg_read_cb(int ok, int length, int record,
>  				const unsigned char *data,
>  				int record_length, void *userdata)
> @@ -1262,19 +1294,69 @@ static void sim_efimg_read_cb(int ok, int length, int record,
>  	memcpy(efimg, &data[1], 9);
>  }
>  
> +static void sim_efimg_changed(int id, void *userdata)
> +{
> +	struct ofono_sim *sim = userdata;
> +	int i, iidf_id;
> +
> +	if (sim->efimg != NULL) {
> +		for (i = sim->efimg_length / 9 - 1; i >= 0; i--) {
> +			iidf_id = (sim->efimg[i * 9 + 3] << 8) |
> +				sim->efimg[i * 9 + 4];
> +
> +			ofono_sim_remove_file_watch(sim->context, iidf_id);
> +		}
> +
> +		g_free(sim->efimg);
> +		sim->efimg = NULL;
> +		sim->efimg_length = 0;
> +	}
> +
> +	ofono_sim_read(sim->context, SIM_EFIMG_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_efimg_read_cb, sim);
> +
> +	/* TODO: notify D-bus clients */
> +}
> +
>  static void sim_ready(enum ofono_sim_state new_state, void *user)
>  {
>  	struct ofono_sim *sim = user;
> +	int i, iidf_id;
> +
> +	if (new_state != OFONO_SIM_STATE_READY) {
> +		if (sim->context == NULL)
> +			return;
> +
> +		ofono_sim_remove_file_watch(sim->context, SIM_EFMSISDN_FILEID);
> +		ofono_sim_remove_file_watch(sim->context, SIM_EFSDN_FILEID);
> +		ofono_sim_remove_file_watch(sim->context, SIM_EFIMG_FILEID);
> +

So two problems here.  The file_watch id is not related to the file EF.
 So this won't really work anyway.  It also sounds like we should be
canceling the entire context (and thus the related file watches) instead
of trying to cancel them one by one.

What do you think of using two separate sim contexts here?  Also, this
might really belong in sim_free_main_state instead of this function.

> +		if (sim->efimg == NULL)
> +			return;
> +
> +		for (i = sim->efimg_length / 9 - 1; i >= 0; i--) {
> +			iidf_id = (sim->efimg[i * 9 + 3] << 8) |
> +				sim->efimg[i * 9 + 4];
> +
> +			ofono_sim_remove_file_watch(sim->context, iidf_id);
> +		}
>  
> -	if (new_state != OFONO_SIM_STATE_READY)
>  		return;
> +	}
>  
>  	sim_own_numbers_update(sim);
> +	ofono_sim_add_file_watch(sim->context, SIM_EFMSISDN_FILEID,
> +					sim_own_numbers_changed, sim, NULL);
>  
>  	ofono_sim_read(sim->context, SIM_EFSDN_FILEID,
>  			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_sdn_read_cb, sim);
> +	ofono_sim_add_file_watch(sim->context, SIM_EFSDN_FILEID,
> +					sim_service_numbers_changed, sim, NULL);
> +
>  	ofono_sim_read(sim->context, SIM_EFIMG_FILEID,
>  			OFONO_SIM_FILE_STRUCTURE_FIXED, sim_efimg_read_cb, sim);
> +	ofono_sim_add_file_watch(sim->context, SIM_EFIMG_FILEID,
> +					sim_efimg_changed, sim, NULL);
>  }
>  
>  static void sim_imsi_cb(const struct ofono_error *error, const char *imsi,
> @@ -1873,7 +1955,11 @@ skip_efpl:
>  						DBUS_TYPE_STRING,
>  						&sim->language_prefs);
>  
> -	sim_pin_check(sim);
> +	/* Proceed with sim initialization if we're not merely updating */
> +	if (!sim->language_prefs_update)
> +		sim_pin_check(sim);
> +
> +	sim->language_prefs_update = FALSE;
>  }
>  
>  static void sim_iccid_read_cb(int ok, int length, int record,
> @@ -1899,6 +1985,43 @@ static void sim_iccid_read_cb(int ok, int length, int record,
>  						&sim->iccid);
>  }
>  
> +static void sim_iccid_changed(int id, void *userdata)
> +{
> +	struct ofono_sim *sim = userdata;
> +
> +	if (sim->iccid) {
> +		g_free(sim->iccid);
> +		sim->iccid = NULL;
> +	}
> +
> +	ofono_sim_read(sim->context, SIM_EF_ICCID_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +			sim_iccid_read_cb, sim);
> +}
> +
> +static void sim_efli_efpl_changed(int id, void *userdata)
> +{
> +	struct ofono_sim *sim = userdata;
> +
> +	if (sim->efli != NULL) /* This shouldn't happen */
> +		return;
> +
> +	if (sim->language_prefs) {
> +		g_strfreev(sim->language_prefs);
> +		sim->language_prefs = NULL;
> +	}
> +
> +	sim->language_prefs_update = TRUE;
> +
> +	ofono_sim_read(sim->context, SIM_EFLI_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +			sim_efli_read_cb, sim);
> +
> +	ofono_sim_read(sim->context, SIM_EFPL_FILEID,
> +			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +			sim_efpl_read_cb, sim);
> +}
> +
>  static void sim_initialize(struct ofono_sim *sim)
>  {
>  	/*
> @@ -1927,10 +2050,15 @@ static void sim_initialize(struct ofono_sim *sim)
>  	 * in the EFust
>  	 */
>  
> +	if (sim->context == NULL)
> +		sim->context = ofono_sim_context_create(sim);
> +
>  	/* Grab the EFiccid which is always available */
>  	ofono_sim_read(sim->context, SIM_EF_ICCID_FILEID,
>  			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>  			sim_iccid_read_cb, sim);
> +	ofono_sim_add_file_watch(sim->context, SIM_EF_ICCID_FILEID,
> +					sim_iccid_changed, sim, NULL);
>  
>  	/* EFecc is read by the voicecall atom */
>  
> @@ -1945,9 +2073,14 @@ static void sim_initialize(struct ofono_sim *sim)
>  	ofono_sim_read(sim->context, SIM_EFLI_FILEID,
>  			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>  			sim_efli_read_cb, sim);
> +	ofono_sim_add_file_watch(sim->context, SIM_EFLI_FILEID,
> +					sim_efli_efpl_changed, sim, NULL);
> +
>  	ofono_sim_read(sim->context, SIM_EFPL_FILEID,
>  			OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>  			sim_efpl_read_cb, sim);
> +	ofono_sim_add_file_watch(sim->context, SIM_EFPL_FILEID,
> +					sim_efli_efpl_changed, sim, NULL);
>  }
>  
>  struct ofono_sim_context *ofono_sim_context_create(struct ofono_sim *sim)
> @@ -2096,6 +2229,11 @@ static void sim_free_early_state(struct ofono_sim *sim)
>  		g_strfreev(sim->language_prefs);
>  		sim->language_prefs = NULL;
>  	}
> +
> +	if (sim->context) {
> +		ofono_sim_context_free(sim->context);
> +		sim->context = NULL;
> +	}
>  }
>  
>  static void sim_free_main_state(struct ofono_sim *sim)
> @@ -2119,6 +2257,7 @@ static void sim_free_main_state(struct ofono_sim *sim)
>  				(GFunc)service_number_free, NULL);
>  		g_slist_free(sim->service_numbers);
>  		sim->service_numbers = NULL;
> +		sim->sdn_ready = FALSE;
>  	}
>  
>  	if (sim->efust) {
> @@ -2296,11 +2435,6 @@ static void sim_remove(struct ofono_atom *atom)
>  
>  	sim_free_state(sim);
>  
> -	if (sim->context) {
> -		ofono_sim_context_free(sim->context);
> -		sim->context = NULL;
> -	}
> -

Out of curiosity, why are you moving this one?  It seems to have no
effect.  Either way, this belongs in a separate patch ;)

>  	sim_fs_free(sim->simfs);
>  	sim->simfs = NULL;
>  
> @@ -2366,7 +2500,6 @@ void ofono_sim_register(struct ofono_sim *sim)
>  	ofono_modem_add_interface(modem, OFONO_SIM_MANAGER_INTERFACE);
>  	sim->state_watches = __ofono_watchlist_new(g_free);
>  	sim->simfs = sim_fs_new(sim, sim->driver);
> -	sim->context = ofono_sim_context_create(sim);

Same comment as above.

>  
>  	__ofono_atom_register(sim->atom, sim_unregister);
>  

Regards,
-Denis

Re: [PATCH 4/9] voicecall: Watch for changes to relevant SIM files.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> ---
>  src/voicecall.c |   24 +++++++++++++++++-------
>  1 files changed, 17 insertions(+), 7 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 5/9] network: Watch for changes to relevant SIM files.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> ---
>  src/network.c |   66 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 66 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 6/9] cbs: Watch for changes to relevant SIM files.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> ---
>  src/cbs.c |   32 +++++++++++++++++++++++++++++++-
>  1 files changed, 31 insertions(+), 1 deletions(-)
> 

Patch has been applied.  One more comment below:

> @@ -886,7 +888,33 @@ static void sim_cbmid_read_cb(int ok, int length, int record,
>  	g_free(str);
>  
>  done:
> -	cbs_got_file_contents(cbs);
> +	if (cbs->efcbmid_update) {
> +		topic_str = cbs_topics_to_str(cbs, cbs->topics);
> +		cbs->driver->set_topics(cbs, topic_str,
> +					cbs_set_powered_cb, cbs);
> +		g_free(topic_str);

I think you meant to do this only if CBS atom was powered.  I fixed this
under that assumption.  Can you double check my logic?

> +
> +		cbs->efcbmid_update = FALSE;
> +	} else
> +		cbs_got_file_contents(cbs);
> +}
> +

Regards,
-Denis

Re: [PATCH 9/9] stk: Partially handle Refresh command.

[Denis Kenzior]

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

Hi Andrew,

On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
> Only the four "NAA initialisation" modes are handled at the moment.
> ---
>  src/stk.c |   65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 65 insertions(+), 0 deletions(-)
> 

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 2/9] sim: Implement basic Refresh.

[Andrzej Zaborowski]

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

Hi Denis,

On 17 February 2011 22:03, Denis Kenzior <denkenz@gmail.com> wrote:
> On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
>> ---
>>  src/ofono.h |    4 ++
>>  src/sim.c   |  163 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>>  2 files changed, 155 insertions(+), 12 deletions(-)
>>
>
> Patch has been applied, but some comments below:
>
>> +     if (reinit_naa) {
>> +             /* Force the sim state out of READY */
>> +
>> +             sim_free_main_state(sim);
>> +
>> +             sim->state = OFONO_SIM_STATE_INSERTED;
>> +             for (l = sim->state_watches->items; l; l = l->next) {
>> +                     struct ofono_watchlist_item *item = l->data;
>> +                     notify = item->notify;
>> +
>> +                     notify(sim->state, item->notify_data);
>> +             }
>
> I changed this to directly call the modem object and force it into
> pre-sim state.  I think this is a bit more clear than your proposed
> approach.

Looks good to me, maybe it would be even better if this was just a
different type of sim state change, like OFONO_SIM_STATE_NOT_READY so
it is handled in a similar way as other events.

>
> Another question here is whether we want to maintain two separate simfs
> contexts.  One for the main state and one for the early state.  This
> might make things a bit easier to implement for patch 3 in this series.

I was thinking about that too, I'll send a patch to implement this.

Best regards

Re: [PATCH 3/9] sim: Watch for changes to relevant SIM files.

[Andrzej Zaborowski]

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

On 17 February 2011 22:08, Denis Kenzior <denkenz@gmail.com> wrote:
> On 02/15/2011 01:58 AM, Andrzej Zaborowski wrote:
>> +             ofono_sim_remove_file_watch(sim->context, SIM_EFMSISDN_FILEID);
>> +             ofono_sim_remove_file_watch(sim->context, SIM_EFSDN_FILEID);
>> +             ofono_sim_remove_file_watch(sim->context, SIM_EFIMG_FILEID);
>> +
>
> So two problems here.  The file_watch id is not related to the file EF.
>  So this won't really work anyway.  It also sounds like we should be
> canceling the entire context (and thus the related file watches) instead
> of trying to cancel them one by one.

Oh right.  I read the commit where ofono_sim_add_file_watch and
ofono_sim_remove_file_watch were added but I didn't notice this change
from the earlier proposed api, where this would have worked.  I'll fix
that by using a second context.

>
> What do you think of using two separate sim contexts here?  Also, this
> might really belong in sim_free_main_state instead of this function.
>
>> +             if (sim->efimg == NULL)
>> +                     return;
>> +
>> +             for (i = sim->efimg_length / 9 - 1; i >= 0; i--) {
>> +                     iidf_id = (sim->efimg[i * 9 + 3] << 8) |
>> +                             sim->efimg[i * 9 + 4];
>> +
>> +                     ofono_sim_remove_file_watch(sim->context, iidf_id);
>> +             }
>>
>> -     if (new_state != OFONO_SIM_STATE_READY)
>>               return;
>> +     }
>>
>>       sim_own_numbers_update(sim);
>> +     ofono_sim_add_file_watch(sim->context, SIM_EFMSISDN_FILEID,
>> +                                     sim_own_numbers_changed, sim, NULL);
>>
>>       ofono_sim_read(sim->context, SIM_EFSDN_FILEID,
>>                       OFONO_SIM_FILE_STRUCTURE_FIXED, sim_sdn_read_cb, sim);
>> +     ofono_sim_add_file_watch(sim->context, SIM_EFSDN_FILEID,
>> +                                     sim_service_numbers_changed, sim, NULL);
>> +
>>       ofono_sim_read(sim->context, SIM_EFIMG_FILEID,
>>                       OFONO_SIM_FILE_STRUCTURE_FIXED, sim_efimg_read_cb, sim);
>> +     ofono_sim_add_file_watch(sim->context, SIM_EFIMG_FILEID,
>> +                                     sim_efimg_changed, sim, NULL);
>>  }
>>
>>  static void sim_imsi_cb(const struct ofono_error *error, const char *imsi,
>> @@ -1873,7 +1955,11 @@ skip_efpl:
>>                                               DBUS_TYPE_STRING,
>>                                               &sim->language_prefs);
>>
>> -     sim_pin_check(sim);
>> +     /* Proceed with sim initialization if we're not merely updating */
>> +     if (!sim->language_prefs_update)
>> +             sim_pin_check(sim);
>> +
>> +     sim->language_prefs_update = FALSE;
>>  }
>>
>>  static void sim_iccid_read_cb(int ok, int length, int record,
>> @@ -1899,6 +1985,43 @@ static void sim_iccid_read_cb(int ok, int length, int record,
>>                                               &sim->iccid);
>>  }
>>
>> +static void sim_iccid_changed(int id, void *userdata)
>> +{
>> +     struct ofono_sim *sim = userdata;
>> +
>> +     if (sim->iccid) {
>> +             g_free(sim->iccid);
>> +             sim->iccid = NULL;
>> +     }
>> +
>> +     ofono_sim_read(sim->context, SIM_EF_ICCID_FILEID,
>> +                     OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>> +                     sim_iccid_read_cb, sim);
>> +}
>> +
>> +static void sim_efli_efpl_changed(int id, void *userdata)
>> +{
>> +     struct ofono_sim *sim = userdata;
>> +
>> +     if (sim->efli != NULL) /* This shouldn't happen */
>> +             return;
>> +
>> +     if (sim->language_prefs) {
>> +             g_strfreev(sim->language_prefs);
>> +             sim->language_prefs = NULL;
>> +     }
>> +
>> +     sim->language_prefs_update = TRUE;
>> +
>> +     ofono_sim_read(sim->context, SIM_EFLI_FILEID,
>> +                     OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>> +                     sim_efli_read_cb, sim);
>> +
>> +     ofono_sim_read(sim->context, SIM_EFPL_FILEID,
>> +                     OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>> +                     sim_efpl_read_cb, sim);
>> +}
>> +
>>  static void sim_initialize(struct ofono_sim *sim)
>>  {
>>       /*
>> @@ -1927,10 +2050,15 @@ static void sim_initialize(struct ofono_sim *sim)
>>        * in the EFust
>>        */
>>
>> +     if (sim->context == NULL)
>> +             sim->context = ofono_sim_context_create(sim);
>> +
>>       /* Grab the EFiccid which is always available */
>>       ofono_sim_read(sim->context, SIM_EF_ICCID_FILEID,
>>                       OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>>                       sim_iccid_read_cb, sim);
>> +     ofono_sim_add_file_watch(sim->context, SIM_EF_ICCID_FILEID,
>> +                                     sim_iccid_changed, sim, NULL);
>>
>>       /* EFecc is read by the voicecall atom */
>>
>> @@ -1945,9 +2073,14 @@ static void sim_initialize(struct ofono_sim *sim)
>>       ofono_sim_read(sim->context, SIM_EFLI_FILEID,
>>                       OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>>                       sim_efli_read_cb, sim);
>> +     ofono_sim_add_file_watch(sim->context, SIM_EFLI_FILEID,
>> +                                     sim_efli_efpl_changed, sim, NULL);
>> +
>>       ofono_sim_read(sim->context, SIM_EFPL_FILEID,
>>                       OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
>>                       sim_efpl_read_cb, sim);
>> +     ofono_sim_add_file_watch(sim->context, SIM_EFPL_FILEID,
>> +                                     sim_efli_efpl_changed, sim, NULL);
>>  }
>>
>>  struct ofono_sim_context *ofono_sim_context_create(struct ofono_sim *sim)
>> @@ -2096,6 +2229,11 @@ static void sim_free_early_state(struct ofono_sim *sim)
>>               g_strfreev(sim->language_prefs);
>>               sim->language_prefs = NULL;
>>       }
>> +
>> +     if (sim->context) {
>> +             ofono_sim_context_free(sim->context);
>> +             sim->context = NULL;
>> +     }
>>  }
>>
>>  static void sim_free_main_state(struct ofono_sim *sim)
>> @@ -2119,6 +2257,7 @@ static void sim_free_main_state(struct ofono_sim *sim)
>>                               (GFunc)service_number_free, NULL);
>>               g_slist_free(sim->service_numbers);
>>               sim->service_numbers = NULL;
>> +             sim->sdn_ready = FALSE;
>>       }
>>
>>       if (sim->efust) {
>> @@ -2296,11 +2435,6 @@ static void sim_remove(struct ofono_atom *atom)
>>
>>       sim_free_state(sim);
>>
>> -     if (sim->context) {
>> -             ofono_sim_context_free(sim->context);
>> -             sim->context = NULL;
>> -     }
>> -
>
> Out of curiosity, why are you moving this one?  It seems to have no
> effect.  Either way, this belongs in a separate patch ;)

This is because the context was logically invalid once sim is removed
even though the atom wasn't unregistered, so the context needs to be
created on sim insertion and removed on sim removal (this also
releases the watches)

Best regards.