RE: [PATCH] sim: check if lock is locked after code changing attempt
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
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
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
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
[PATCH] sim: check if lock is locked after code changing attempt
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
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
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