Hi Jussi,

>>
>> First, please fix your authorship information.  See the AUTHORS file 
>> to see what we expect.  Also, please make sure that you're using tabs 
>> for indentation and following the relevant coding style.  See the 
>> Submitting Patches section in the HACKING document in oFono git as 
>> well as doc/coding-style.txt.
> 
> I think whole this chapter should be translated as "Do not use Outlook".
> I asked around little bit and best guess seems to be that you are using 
> something that takes authorship information from mail address. Also since the 
> patch passes the checkpatch here with no problems whatsoever I guess Outlook 
> ruined the patch. Fine, I'll try Evolution next. If possible, I would like to 
> avoid using the git send-email. I prefer tools with GUI. 

git-send-email is the only sane approach.  Save yourself some time and
just use it.  Or find a GUI tool to do it for you.

> 
>>
>>> diff --git a/src/sim.c b/src/sim.c
>>> index d627647..789ddde 100644
>>> --- a/src/sim.c
>>> +++ b/src/sim.c
>>> @@ -712,8 +712,60 @@ static DBusMessage 
>>> *sim_unlock_pin(DBusConnection *conn, DBusMessage *msg,  static void 
>>> sim_change_pin_cb(const struct ofono_error *error, void *data)  {
>>
>> So my first concern is that whatever you do here also has to work for 
>> LockPin and UnlockPin.
> 
> I'm not after total solution here. I would like to implement this so that 
> first the ChangePin starts working, i.e fullfill the existing TODO task 
> first. Reasoning here is that I thought it would be easier to get the fixes 
> in if I keep them small. If u mean that stuff inside (error->error = 12) 
> condition should be as separate function to be usable for LockPin and 
> UnlockPin, I agree. I was just thinking that separation could be done when 
> fix is extended to LockPin and UnlockPin. 
> 

Whatever you do here applies to LockPin and UnlockPin, so you might as
well handle them too.

>> Have you considered querying the PIN state on a failure instead?  If 
>> the lock/unlock/change pin operation fails, then re-query the current PIN.
>> If the CPIN is no longer returning READY and we're in a state 
>> OFONO_SIM_STATE_READY, then tear us back down to an earlier state.
> 
> Yes, I have. As I answered to Lucas already at least with the modem I'm using 
> the return value I get seems to be trustable and there is no point to go 
> querying the information I already know. Also there could be other reasons 
> why sim state is not ready ( small chance of course ) so to satisfy my pedant 
> nature I would have to either check the return value anyways or use some 
> other means to get the reason why the sim is not ready in order to be 100 % 
> sure that PUK is required. At least Lucas seemed to favor query approach. I 
> guess that has to mean that return value is not trustable with all modems. I 
> can change this if required. 
> 
>>

In general you cannot trust the return value.  Most modems simply get
this wrong.  On a well setup modem driver you do not need to worry about
sim readiness conditions.  The driver should take care of that.  If the
modem firmware is buggy, well there's no strategy that will save you anyway.

So I'd prefer if we try out the query approach first.

Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Reply via email to