from:"Jussi.Kangas"

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

[PATCH] test: add change-pin test case

2011-01-11 Thread Jussi.Kangas

---
 Makefile.am |3 ++-
 test/change-pin |   29 +
 2 files changed, 31 insertions(+), 1 deletions(-)
 create mode 100755 test/change-pin

diff --git a/Makefile.am b/Makefile.am
index cc30624..4c9add4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -446,7 +446,8 @@ test_scripts = test/backtrace \
 test/set-tty \
 test/set-gsm-band \
 test/set-umts-band \
-test/lockdown-modem
+test/lockdown-modem \
+test/change-pin
 
 if TEST
 testdir = $(pkglibdir)/test
diff --git a/test/change-pin b/test/change-pin new file mode 100755 index 
000..a2444d6
--- /dev/null
+++ b/test/change-pin
@@ -0,0 +1,29 @@
+#!/usr/bin/python
+
+import dbus
+import sys
+
+bus = dbus.SystemBus()
+
+if len(sys.argv) == 5:
+path = sys.argv[1]
+pin_type = sys.argv[2]
+orig_pin = sys.argv[3]
+new_pin = sys.argv[4]
+elif len(sys.argv) == 4:
+manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+'org.ofono.Manager')
+modems = manager.GetModems()
+path = modems[0][0]
+pin_type = sys.argv[1]
+orig_pin = sys.argv[2]
+new_pin = sys.argv[3]
+else:
+print %s [PATH] pin_type pin % (sys.argv[0])
+sys.exit(0)
+
+print Change Pin for modem %s... % path simmanager = 
+dbus.Interface(bus.get_object('org.ofono', path),
+'org.ofono.SimManager')
+
+simmanager.ChangePin(pin_type, orig_pin, new_pin)
--
1.7.1
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

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

2011-01-11 Thread Jussi.Kangas

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(-)

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)
 {
 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);
+
+if (sim-pin_type != OFONO_SIM_PASSWORD_SIM_PUK2)
+ofono_modem_reset(modem);
+}
 __ofono_dbus_pending_reply(sim-pending,
 __ofono_error_failed(sim-pending));
 
@@ -722,6 +774,7 @@ static void sim_change_pin_cb(const struct ofono_error 
*error, void *data)
 return;
 }
 
+sim-pin_type = OFONO_SIM_PASSWORD_NONE;
 __ofono_dbus_pending_reply(sim-pending,
 dbus_message_new_method_return(sim-pending));
 
@@ -764,6 +817,7 @@ static DBusMessage *sim_change_pin(DBusConnection *conn, 
DBusMessage *msg,
 return dbus_message_new_method_return(msg);
 
 sim-pending = dbus_message_ref(msg);
+sim-pin_type = type;
 sim-driver-change_passwd(sim, type, old, new,
 sim_change_pin_cb, sim);
 
-- 
1.7.1

___
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

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

[PATCH] test: add change-pin test case

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

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

4 matches

Site Navigation

Mail list logo

Footer information