Marcel Holtmann <[email protected]> writes:
> Hi Kalle,
>
>> On my Huawei E1552 when I plug in the modem (ie. cold start) with PIN locked
>> SIM, the sim state is 255 (HUAWEI_SIM_STATE_NOT_EXISTENT). As the modem
>> doesn't send ^SIMST notifications, poll the sim state until it's ready.
>>
>> In theory it might be possible to do this better, for example follow
>> ^BOOT notifications or something, but it's unknown what parameter we
>> should check for.
>
[...]
>> + if (sim_state == HUAWEI_SIM_STATE_NOT_EXISTENT) {
>> ofono_sim_inserted_notify(data->sim, FALSE);
>> +
>> + /* SIM is not ready, try again a bit later */
>> + return TRUE;
>> + }
>> else
>> ofono_sim_inserted_notify(data->sim, TRUE);
>
> Coding style. } else on the same line. You should know this by now ;)
Oops :)
> Also in this case since you do return TRUE anyway from the if block, why
> bother with else. Just don't.
Good point. I changed that.
>> + if (rerun && data->sim_poll_count < MAX_SIM_POLL_COUNT) {
>> + data->sim_poll_count++;
>> + data->query_sim_state = g_timeout_add_seconds(2,
>> + query_sim_state,
>> + modem);
>
> Now I don't like the query_sim_state name. It should sim_poll_timeout or
> something similar. At least make it clear that it is timeout.
I didn't know if you talked only about about the name of structure field
or also about the function name? I only changed the structure name.
>> +
>> + /* query_sim_state(modem); */
>> }
>
> What is this ???
A leftover from my tests. Removed
>> static GAtChat *create_port(const char *device)
>> @@ -546,6 +586,9 @@ static int huawei_disable(struct ofono_modem *modem)
>>
>> DBG("%p", modem);
>>
>> + if (data->query_sim_state)
>> + g_source_remove(data->query_sim_state);
>> +
>
> You need to set the timeout variable also to 0. Otherwise you have some
> funny side effect then next time enabling the modem.
>
> Also I prefer check for the timeout variables with > 0.
Fixed.
I'll send v2 shortly.
--
Kalle Valo
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono
