RE: [PATCH] sim: check if lock is locked after code changing attempt

2011-01-13 Thread Jussi.Kangas
Hi Denis, 

On Wed, 2011-01-12 at 07:07 +0200, Denis Kenzior wrote:
 Hi Jussi,
 
 On 01/11/2011 06:17 AM, jussi.kan...@tieto.com wrote:
  Hi,
  
  This is fix to Marit Henriksen's TODO item Check SIM pin status if 
  sim_change_pin fails. I've discussed with Marit and it's ok for her if I 
  fix the issue. Problem here is that issue could perhaps also be fixed with 
  retry counter solution introduced by Lucas De Marchi couple of tasks ago. 
  That would seem however require some extra implementation in ste modem ( at 
  least I was not able get the ofono show correct values without extra 
  modifications ) and I think that isimodems don't have that sort of retry 
  counter at all. Because of that and since I had this solution already 
  implemented I propose it to be added as well. 
  
  Br,
  -Jussi
  
  ---
   src/sim.c |   54 ++
   1 files changed, 54 insertions(+), 0 deletions(-)
  
 
 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. 

 
  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. 

 
   struct ofono_sim *sim = data;
  +DBusConnection *conn = ofono_dbus_get_connection();
  +const char *path = __ofono_atom_get_path(sim-atom);
  +struct ofono_modem *modem = __ofono_atom_get_modem(sim-atom);
  +const char *pin_name;
   
   if (error-type != OFONO_ERROR_TYPE_NO_ERROR) {
  +if (error-error == 12) {
  +sim-locked_pins[sim-pin_type] = TRUE;
  +switch (sim-pin_type) {
  +case OFONO_SIM_PASSWORD_SIM_PIN:
  +sim-pin_type = OFONO_SIM_PASSWORD_SIM_PUK;
  +pin_name = sim_passwd_name(
  +OFONO_SIM_PASSWORD_SIM_PUK);
  +break;
  +case OFONO_SIM_PASSWORD_PHFSIM_PIN:
  +sim-pin_type = OFONO_SIM_PASSWORD_PHFSIM_PUK;
  +pin_name = sim_passwd_name(
  +OFONO_SIM_PASSWORD_PHFSIM_PUK);
  +break;
  +case OFONO_SIM_PASSWORD_PHCORP_PIN:
  +sim-pin_type = OFONO_SIM_PASSWORD_PHCORP_PUK;
  +pin_name = sim_passwd_name(
  +OFONO_SIM_PASSWORD_PHCORP_PUK);
  +break;
  +case OFONO_SIM_PASSWORD_PHNET_PIN:
  +sim-pin_type = OFONO_SIM_PASSWORD_PHNET_PUK;
  +pin_name = sim_passwd_name(
  +OFONO_SIM_PASSWORD_PHNET_PUK);
  +case OFONO_SIM_PASSWORD_PHNETSUB_PIN:
  +sim-pin_type = OFONO_SIM_PASSWORD_PHNETSUB_PUK;
  +pin_name = sim_passwd_name(
  +OFONO_SIM_PASSWORD_PHNETSUB_PUK);
  +break;
  +case OFONO_SIM_PASSWORD_PHSP_PIN:
  +sim-pin_type = OFONO_SIM_PASSWORD_PHSP_PUK;
  +pin_name = sim_passwd_name(
  +OFONO_SIM_PASSWORD_PHSP_PUK);
  +break;
  +case OFONO_SIM_PASSWORD_SIM_PIN2:
  +sim-pin_type = OFONO_SIM_PASSWORD_SIM_PUK2;
  +pin_name = sim_passwd_name(
  +OFONO_SIM_PASSWORD_SIM_PUK2);
  +break;
  +default:
  +break;
  +}
  +ofono_dbus_signal_property_changed(conn, path,
  +OFONO_SIM_MANAGER_INTERFACE,
  +PinRequired, DBUS_TYPE_STRING,
  +pin_name);
 
 Have you considered querying the PIN state on 

Re: [PATCH] sim: check if lock is locked after code changing attempt

2011-01-13 Thread Lucas De Marchi
Ho Jussi

On Thu, Jan 13, 2011 at 6:34 AM,  jussi.kan...@tieto.com wrote:
 On Wed, 2011-01-12 at 07:07 +0200, Denis Kenzior wrote:
 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

This is `git am'

 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.

You can use whatever client you want to write email. But for sending
patches, git send-email is the best one since you won't need to copy
and paste, nor attach anything. Give it a try and you'll regret not
using it first.



Lucas De Marchi
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] sim: check if lock is locked after code changing attempt

2011-01-12 Thread Lucas De Marchi
Hi Jussi,

On Wed, Jan 12, 2011 at 5:39 AM,  jussi.kan...@tieto.com wrote:
 Hi Lucas,

 On Tue, 2011-01-11 at 16:08 +0200, Lucas De Marchi wrote:
 Hi Jussi

 On Tue, Jan 11, 2011 at 10:17 AM,  jussi.kan...@tieto.com wrote:
  Hi,
 
  This is fix to Marit Henriksen's TODO item Check SIM pin status if 
  sim_change_pin fails. I've discussed with Marit and it's ok for her if I 
  fix the issue. Problem here is that issue could perhaps also be fixed with 
  retry counter solution introduced by Lucas De Marchi couple of tasks ago. 
  That would seem however require some extra implementation in ste modem ( 
  at least I was not able get the ofono show correct values without extra 
  modifications ) and I think that isimodems don't have that sort of retry 
  counter at all. Because of that and since I had this solution already 
  implemented I propose it to be added as well.
 

 I failed to understand why you are hard-coding this and why you'd like
 to use retry counters. Can't you just check the pin state by calling
 sim_pin_check()?

 Moreover, this is not really implementing what the TODO item says.


 regards,
 Lucas De Marchi

 Marit already proposed sim_pin_check approach 2010/11/2. Denis Kenzior
 thought that initializing sim interface would not be a good idea. See
 mail Re: [Patch] sim: Check SIM pin status after changing pin from
 Friday 5th in November 2010.

 True, fix proposal does not do exactly what TODO item says. It trusts to
 returned error value instead of going to check the sim state once more.
 In my testings this value has been trustable so far and there has not
 been reason to go check the state. Of course I've tested only with one
 modem so this might not be a universal truth.

 I don't particularly wish to use the retry counters. I was just thinking that 
 same
 information ( PUK required ) could be deduced by counting the retries.

IMO you should try querying the pin state on failure as Denis and I
suggested rather than using retry counters and/or hard-coding this
here. You might do this without re-initializing the sim interface.
Then Denis' worries in
http://www.mail-archive.com/ofono@ofono.org/msg05294.html are gone.



Lucas De Marchi
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH] sim: check if lock is locked after code changing attempt

2011-01-12 Thread Denis Kenzior
Hi Jussi,

 I don't particularly wish to use the retry counters. I was just thinking that 
 same
 information ( PUK required ) could be deduced by counting the retries. 

To my knowledge you cannot.  Correct me if I'm wrong, but the retry
counters are stored on the SIM and the SIM can be removed, and later
re-inserted.  You would not know how the counters have been affected.

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


Re: [PATCH] sim: check if lock is locked after code changing attempt

2011-01-11 Thread Lucas De Marchi
Hi Jussi

On Tue, Jan 11, 2011 at 10:17 AM,  jussi.kan...@tieto.com wrote:
 Hi,

 This is fix to Marit Henriksen's TODO item Check SIM pin status if 
 sim_change_pin fails. I've discussed with Marit and it's ok for her if I fix 
 the issue. Problem here is that issue could perhaps also be fixed with retry 
 counter solution introduced by Lucas De Marchi couple of tasks ago. That 
 would seem however require some extra implementation in ste modem ( at least 
 I was not able get the ofono show correct values without extra modifications 
 ) and I think that isimodems don't have that sort of retry counter at all. 
 Because of that and since I had this solution already implemented I propose 
 it to be added as well.


I failed to understand why you are hard-coding this and why you'd like
to use retry counters. Can't you just check the pin state by calling
sim_pin_check()?

Moreover, this is not really implementing what the TODO item says.


regards,
Lucas De Marchi
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


RE: [PATCH] sim: check if lock is locked after code changing attempt

2011-01-11 Thread Jussi.Kangas
Hi Lucas,

On Tue, 2011-01-11 at 16:08 +0200, Lucas De Marchi wrote:
 Hi Jussi
 
 On Tue, Jan 11, 2011 at 10:17 AM,  jussi.kan...@tieto.com wrote:
  Hi,
 
  This is fix to Marit Henriksen's TODO item Check SIM pin status if 
  sim_change_pin fails. I've discussed with Marit and it's ok for her if I 
  fix the issue. Problem here is that issue could perhaps also be fixed with 
  retry counter solution introduced by Lucas De Marchi couple of tasks ago. 
  That would seem however require some extra implementation in ste modem ( at 
  least I was not able get the ofono show correct values without extra 
  modifications ) and I think that isimodems don't have that sort of retry 
  counter at all. Because of that and since I had this solution already 
  implemented I propose it to be added as well.
 
 
 I failed to understand why you are hard-coding this and why you'd like
 to use retry counters. Can't you just check the pin state by calling
 sim_pin_check()?
 
 Moreover, this is not really implementing what the TODO item says.
 
 
 regards,
 Lucas De Marchi

Marit already proposed sim_pin_check approach 2010/11/2. Denis Kenzior
thought that initializing sim interface would not be a good idea. See
mail Re: [Patch] sim: Check SIM pin status after changing pin from
Friday 5th in November 2010. 

True, fix proposal does not do exactly what TODO item says. It trusts to
returned error value instead of going to check the sim state once more.
In my testings this value has been trustable so far and there has not
been reason to go check the state. Of course I've tested only with one
modem so this might not be a universal truth.

I don't particularly wish to use the retry counters. I was just thinking that 
same
information ( PUK required ) could be deduced by counting the retries. 

Br,
-Jussi



___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono