Hi Andrew,
> +void __ofono_sim_refresh(struct ofono_sim *sim, GSList *file_list,
> + ofono_bool_t full_file_change, ofono_bool_t naa_init)
> +{
> + GSList *l, *fl;
> + enum sim_reset {
> + RESET_REMOVE_ATOMS = OFONO_SIM_STATE_INSERTED,
> + RESET_SIGNAL_READY = OFONO_SIM_STATE_READY,
> + RESET_NONE,
> + } reset_state = RESET_NONE;
> +
> + /* Flush cached content for affected files */
> + if (full_file_change)
> + sim_fs_cache_flush(sim->simfs);
> + else {
> + for (fl = file_list; fl; fl = fl->next) {
> + struct stk_file *file = fl->data;
> + int id = (file->file[file->len - 2] << 8) |
> + (file->file[file->len - 1] << 0);
> +
> + sim_file_changed_flush(sim, id);
> + }
> + }
> +
> + if (naa_init)
> + reset_state = RESET_REMOVE_ATOMS;
> +
> + /*
> + * Check if we have file change handlers for all of the affected
> + * files. If not, we will fall back to re-initialising the
> + * application which ensures that all files are re-read.
> + */
> + for (l = sim->fs_watches; l; l = l->next) {
> + struct fs_watch *w = l->data;
> +
> + if (full_file_change) {
> + if (w->notify)
> + continue;
> +
> + if (w->reset_state < reset_state)
> + reset_state = reset_state;
> +
> + continue;
> + }
What exactly is this if statement accomplishing? If we have a
full_file_change notification we have to always re-init the SIM.
> +
> + for (fl = file_list; fl; fl = fl->next) {
> + struct stk_file *file = fl->data;
> + int id = (file->file[file->len - 2] << 8) |
> + (file->file[file->len - 1] << 0);
> +
> + if (id != w->id)
> + continue;
> +
> + if (w->notify)
> + break;
> +
> + if (w->reset_state < reset_state)
> + reset_state = reset_state;
> +
> + break;
> + }
> + }
> +
And I'm pretty sure we can just skip this one as well, a NULL callback
is just that.
> + /*
> + * Notify the subscribers of files that have changed unless we
> + * have determined that a re-initialisation is necessary and
> + * will trigger re-reading of those files anyway.
> + */
> + for (l = sim->fs_watches; l; l = l->next) {
> + struct fs_watch *w = l->data;
> +
> + if (full_file_change) {
> + if (w->reset_state < reset_state)
> + w->notify(w->id, w->notify_data);
> +
> + continue;
> + }
> +
> + for (fl = file_list; fl; fl = fl->next) {
> + struct stk_file *file = fl->data;
> + int id = (file->file[file->len - 2] << 8) |
> + (file->file[file->len - 1] << 0);
> +
> + if (id != w->id)
> + continue;
> +
> + if (w->reset_state < reset_state)
> + w->notify(w->id, w->notify_data);
> +
> + break;
> + }
> + }
> +
So again, this seems entirely way too complicated. I'd suggest
something like:
if full_file_change or changed ef in (EFbdn, EFfdn, EFest, EFust, EFsst,
EFphase, EFad, EFcphs_info):
Remove all atoms in post_sim and post_online
Restart SIM initialization procedure
if full_file_change:
notify all remaining watches
else
for each file changed:
notify matching remaining watches
Incidentally the above list of files to re-read is exactly the same
files we need to re-initialize when we lock ourselves out when trying to
lock/unlock/change the PIN.
> + switch (reset_state) {
> + case RESET_REMOVE_ATOMS:
> + /*
> + * 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.
> + */
> + ofono_sim_inserted_notify(sim, FALSE);
> + ofono_sim_inserted_notify(sim, TRUE);
> + break;
This is not really a good idea semantics wise. The SIM hasn't really
been removed and a UICC reset / NAA application reset / NAA session
reset is not being performed either.
> + case RESET_SIGNAL_READY:
> + sim->state = OFONO_SIM_STATE_INSERTED;
> + sim_set_ready(sim);
> + break;
I still don't see a reason for this one, isn't the file_changed watch
enough?
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
