Hi,
On 1 July 2010 21:57, Denis Kenzior <[email protected]> wrote:
> Hi Andrew,
>
>> +
>> + gboolean envelope_q_busy;
>
> In my opinion we can get rid of this variable. The SMS tx_queue does
> almost the same thing without requiring such a variable.
True, I didn't think of it.
>
>> + GQueue *envelope_q;
>> +};
>> +
>> +struct envelope_op {
>> + struct stk_envelope e;
>> + int retries;
>> + void (*cb)(struct ofono_stk *stk, gboolean ok,
>> + const unsigned char *data, int length);
>
> Is the callback really needed? What can we intelligently do besides
> printing an error to the log?
In the generic case we should inform whoever asked us to submit the
information to the UICC. We don't currently have any such case (in
case of Cell Broadcast there's no one to inform. In case of the
SimAppAgent our d-bus api doesn't let us do it. In case of SMS-PP
download it's a technical difficulty). But, for example the Timer
Expiration event is more complicated because it needs to be retried
until it succeeds, and every time we retry sending the envelope it
will be different because it contains current time. So the Timer
Expiration retrying has to be imlemented separately.
>
>> };
>>
>> +#define ENVELOPE_RETRIES_DEFAULT 5
>> +
>> +static void envelope_queue_run(struct ofono_stk *stk);
>> +
>> static int stk_respond(struct ofono_stk *stk, struct stk_response *rsp,
>> void (*cb)(const struct ofono_error *error,
>> struct ofono_stk *stk))
>> @@ -72,37 +86,87 @@ static int stk_respond(struct ofono_stk *stk, struct
>> stk_response *rsp,
>> return 0;
>> }
>>
>> -static int stk_send_envelope(struct ofono_stk *stk, struct stk_envelope *e,
>> - void (*cb)(const struct ofono_error *error,
>> - const unsigned char *data,
>> - int length,
>> - struct ofono_stk *stk))
>> +static void envelope_cb(const struct ofono_error *error, const uint8_t
>> *data,
>> + int length, void *user_data)
>> +{
>> + struct ofono_stk *stk = user_data;
>> + struct envelope_op *op = g_queue_peek_head(stk->envelope_q);
>> + gboolean result = TRUE;
>> +
>> + stk->envelope_q_busy = FALSE;
>> +
>> + if (op->retries > 0 && error->type == OFONO_ERROR_TYPE_SIM &&
>> + error->error == 0x9300) {
>> + op->retries--;
>> + goto out;
>
> You might really want to use an increasing retry timeout here.
For now I'm hoping that this retrying is purely theoretical, and it
never happens in practice. The problem with increasing timeouts is
that there's a period where we're not doing anything. And if we have
an envelope like Menu Selection or Timer Expiration later in the
queue, we don't want to delay it.
>
>> + }
>> +
>> + if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
>> + result = FALSE;
>> +
>> + g_queue_pop_head(stk->envelope_q);
>> +
>> + if (op->cb)
>> + op->cb(stk, result, data, length);
>> + g_free(op);
>> +
>> +out:
>> + envelope_queue_run(stk);
>> +}
>> +
>> +static void envelope_queue_run(struct ofono_stk *stk)
>> {
>> const guint8 *tlv;
>> unsigned int tlv_len;
>>
>> - e->dst = STK_DEVICE_IDENTITY_TYPE_UICC;
>> + while (stk->envelope_q_busy == FALSE &&
>> + g_queue_get_length(stk->envelope_q) > 0) {
>> + struct envelope_op *op = g_queue_peek_head(stk->envelope_q);
>> +
>> + tlv = stk_pdu_from_envelope(&op->e, &tlv_len);
>
> Do you think it is efficient for us to re-encode the envelope every time
> the queue is retried? What about encoding once during send (and
> erroring out right away if that fails) and then simply storing the PDU?
> Might make the code a bit simpler too.
Yes, good idea.
Regards
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
