3:24 p.m.
On 17 February 2011 22:08, Denis Kenzior <denkenz(a)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.