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

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Andrew,

> @@ -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);

So my question here is should we delay calling refresh until after our
terminal response has been accepted?

> +
> +		/* 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;
>  }

Should we be handling the case where refresh is notified via
ofono_stk_proactive_command_handled_notify as well?

Regards,
-Denis