11:05 p.m.
---
Hi,
On Mon, 2011-02-21 at 17:21 +0200, Denis Kenzior wrote:
It seems to be showing the states correctly except in case when you
make
> PIN2 blocked using call meter API. If PIN2 goes blocked when using
> callmeter nothing is shown in API. (My patch did not do that either.
> Plan was to fix it in separate patch)
>
Please do. Most modems do not support PIN2 entry via CPIN, and
implement this using a vendor extension. PIN2 / PUK2 support seems to
be a rather recent addition to the specs.
Here it is. Also after this patch having PUK2 blocked does not prevent
going online anymore in start up and it does not hide the interfaces if
PIN2 goes blocked during usage.
There seems to be also some problem when API shows PUK2 required and
PIN
> and PIN2 locked. Reset-pin opens the PUK2 but API does not get the
> response it's looking for and oFono needs to be booted before states are
> showed correctly again. I suspect that's a separate issue however and
> not caused by this patch.
>
I'm having trouble visualizing this one. Can you provide an AT debug
log to illustrate?
I found the problem already. Looks like modem problem which has nothing to
do with having multiple locks on/blocked or with your change. Reseting
the PIN2 does not seem to trigger expected event in startup.
Br,
Jussi
include/sim.h | 2 ++
src/call-meter.c | 27 +++++++++++++++++++++++++++
src/sim.c | 8 ++++----
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/include/sim.h b/include/sim.h
index 412ae44..a56056e 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -227,6 +227,8 @@ unsigned int ofono_sim_add_file_watch(struct ofono_sim_context
*context,
void ofono_sim_remove_file_watch(struct ofono_sim_context *context,
unsigned int id);
+void sim_pin_check(struct ofono_sim *sim);
+
#ifdef __cplusplus
}
#endif
diff --git a/src/call-meter.c b/src/call-meter.c
index 0789935..61678d8 100644
--- a/src/call-meter.c
+++ b/src/call-meter.c
@@ -335,11 +335,20 @@ static void set_acm_max_query_callback(const struct ofono_error
*error,
static void set_acm_max_callback(const struct ofono_error *error, void *data)
{
struct ofono_call_meter *cm = data;
+ struct ofono_atom *sim_atom;
+ struct ofono_sim *sim = NULL;
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
DBG("Setting acm_max failed");
__ofono_dbus_pending_reply(&cm->pending,
__ofono_error_failed(cm->pending));
+ sim_atom = __ofono_modem_find_atom(
+ __ofono_atom_get_modem(cm->atom),
+ OFONO_ATOM_TYPE_SIM);
+ if (!sim_atom)
+ return;
+ sim = __ofono_atom_get_data(sim_atom);
+ sim_pin_check(sim);
return;
}
@@ -396,11 +405,20 @@ static void set_puct_query_callback(const struct ofono_error
*error,
static void set_puct_callback(const struct ofono_error *error, void *data)
{
struct ofono_call_meter *cm = data;
+ struct ofono_atom *sim_atom;
+ struct ofono_sim *sim = NULL;
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
DBG("setting puct failed");
__ofono_dbus_pending_reply(&cm->pending,
__ofono_error_failed(cm->pending));
+ sim_atom = __ofono_modem_find_atom(
+ __ofono_atom_get_modem(cm->atom),
+ OFONO_ATOM_TYPE_SIM);
+ if (!sim_atom)
+ return;
+ sim = __ofono_atom_get_data(sim_atom);
+ sim_pin_check(sim);
return;
}
@@ -593,11 +611,20 @@ static void reset_acm_query_callback(const struct ofono_error
*error, int value,
static void acm_reset_callback(const struct ofono_error *error, void *data)
{
struct ofono_call_meter *cm = data;
+ struct ofono_atom *sim_atom;
+ struct ofono_sim *sim = NULL;
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
DBG("reseting acm failed");
__ofono_dbus_pending_reply(&cm->pending,
__ofono_error_failed(cm->pending));
+ sim_atom = __ofono_modem_find_atom(
+ __ofono_atom_get_modem(cm->atom),
+ OFONO_ATOM_TYPE_SIM);
+ if (!sim_atom)
+ return;
+ sim = __ofono_atom_get_data(sim_atom);
+ sim_pin_check(sim);
return;
}
diff --git a/src/sim.c b/src/sim.c
index c39269d..08236f2 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -49,7 +49,6 @@
static GSList *g_drivers = NULL;
static void sim_own_numbers_update(struct ofono_sim *sim);
-static void sim_pin_check(struct ofono_sim *sim);
struct ofono_sim {
/* Contents of the SIM file system, in rough initialization order */
@@ -2231,8 +2230,9 @@ static void sim_pin_query_cb(const struct ofono_error *error,
&pin_name);
}
- if (pin_type != OFONO_SIM_PASSWORD_NONE &&
- sim->state == OFONO_SIM_STATE_READY) {
+ if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
+ sim->state == OFONO_SIM_STATE_READY) &&
+ (pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {
/* Force the sim state out of READY */
sim_free_main_state(sim);
@@ -2247,7 +2247,7 @@ checkdone:
sim_initialize_after_pin(sim);
}
-static void sim_pin_check(struct ofono_sim *sim)
+void sim_pin_check(struct ofono_sim *sim)
{
if (sim->driver->query_passwd_state == NULL) {
sim_initialize_after_pin(sim);
--
1.7.1
5:15 p.m.
Hi Jussi,
include/sim.h | 2 ++
src/call-meter.c | 27 +++++++++++++++++++++++++++
src/sim.c | 8 ++++----
3 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/include/sim.h b/include/sim.h
index 412ae44..a56056e 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -227,6 +227,8 @@ unsigned int ofono_sim_add_file_watch(struct ofono_sim_context
*context,
void ofono_sim_remove_file_watch(struct ofono_sim_context *context,
unsigned int id);
+void sim_pin_check(struct ofono_sim *sim);
+
Please name this __ofono_sim_recheck_pin(struct ofono_sim *sim);
#ifdef __cplusplus
}
#endif
diff --git a/src/call-meter.c b/src/call-meter.c
index 0789935..61678d8 100644
--- a/src/call-meter.c
+++ b/src/call-meter.c
@@ -335,11 +335,20 @@ static void set_acm_max_query_callback(const struct ofono_error
*error,
static void set_acm_max_callback(const struct ofono_error *error, void *data)
{
struct ofono_call_meter *cm = data;
+ struct ofono_atom *sim_atom;
+ struct ofono_sim *sim = NULL;
if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
DBG("Setting acm_max failed");
__ofono_dbus_pending_reply(&cm->pending,
__ofono_error_failed(cm->pending));
+ sim_atom = __ofono_modem_find_atom(
+ __ofono_atom_get_modem(cm->atom),
+ OFONO_ATOM_TYPE_SIM);
+ if (!sim_atom)
doc/coding-style.txt M1, and M13
+ return;
+ sim = __ofono_atom_get_data(sim_atom);
doc/coding-style.txt M1
+ sim_pin_check(sim);
return;
}
And please factor out this code into a separate function, you repeat it
three times.
> @@ -396,11 +405,20 @@ static void set_puct_query_callback(const struct ofono_error
*error,
> static void set_puct_callback(const struct ofono_error *error, void *data)
> {
> struct ofono_call_meter *cm = data;
> + struct ofono_atom *sim_atom;
> + struct ofono_sim *sim = NULL;
>
> if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> DBG("setting puct failed");
> __ofono_dbus_pending_reply(&cm->pending,
> __ofono_error_failed(cm->pending));
> + sim_atom = __ofono_modem_find_atom(
> + __ofono_atom_get_modem(cm->atom),
> + OFONO_ATOM_TYPE_SIM);
> + if (!sim_atom)
+ return;
+ sim = __ofono_atom_get_data(sim_atom);
> + sim_pin_check(sim);
> return;
> }
>
> @@ -593,11 +611,20 @@ static void reset_acm_query_callback(const struct ofono_error
*error, int value,
> static void acm_reset_callback(const struct ofono_error *error, void *data)
> {
> struct ofono_call_meter *cm = data;
> + struct ofono_atom *sim_atom;
> + struct ofono_sim *sim = NULL;
>
> if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> DBG("reseting acm failed");
> __ofono_dbus_pending_reply(&cm->pending,
> __ofono_error_failed(cm->pending));
> + sim_atom = __ofono_modem_find_atom(
> + __ofono_atom_get_modem(cm->atom),
> + OFONO_ATOM_TYPE_SIM);
> + if (!sim_atom)
+ return;
+ sim = __ofono_atom_get_data(sim_atom);
> + sim_pin_check(sim);
> return;
> }
>
> diff --git a/src/sim.c b/src/sim.c
> index c39269d..08236f2 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -49,7 +49,6 @@
> static GSList *g_drivers = NULL;
>
> static void sim_own_numbers_update(struct ofono_sim *sim);
> -static void sim_pin_check(struct ofono_sim *sim);
>
> struct ofono_sim {
> /* Contents of the SIM file system, in rough initialization order */
> @@ -2231,8 +2230,9 @@ static void sim_pin_query_cb(const struct ofono_error *error,
> &pin_name);
> }
>
> - if (pin_type != OFONO_SIM_PASSWORD_NONE &&
> - sim->state == OFONO_SIM_STATE_READY) {
> + if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
> + sim->state == OFONO_SIM_STATE_READY) &&
> + (pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {
I don't see how this can work. You need to check for pin_type != NONE,
PIN2 and PUK2 AND state == READY here. This also only covers the case
of the PIN2 or PUK2 being triggered when call-meter is active. You do
not take care of the case where PIN2 or PUK2 are already required during
sim initialization.
/* Force the sim state out of READY */
sim_free_main_state(sim);
@@ -2247,7 +2247,7 @@ checkdone:
sim_initialize_after_pin(sim);
}
-static void sim_pin_check(struct ofono_sim *sim)
+void sim_pin_check(struct ofono_sim *sim)
{
if (sim->driver->query_passwd_state == NULL) {
sim_initialize_after_pin(sim);
Regards,
-Denis
2:24 a.m.
Hi,
On Wed, 2011-02-23 at 08:15 +0200, Denis Kenzior wrote:
>
> - if (pin_type != OFONO_SIM_PASSWORD_NONE &&
> - sim->state == OFONO_SIM_STATE_READY) {
> + if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
> + sim->state == OFONO_SIM_STATE_READY) &&
> + (pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {
I don't see how this can work. You need to check for pin_type != NONE,
PIN2 and PUK2 AND state == READY here. This also only covers the case
of the PIN2 or PUK2 being triggered when call-meter is active. You do
not take care of the case where PIN2 or PUK2 are already required during
sim initialization.
Hmm. It seems to work here. If I try reset call meter with wrong
password I get pin_type OFONO_SIM_PASSWORD_SIM_PIN2 and
OFONO_SIM_STATE_READY. APIs stay up and SIM manager shows pin2 locked
and pin2 required. (Which is kinda wrong I admit, u cannot unlock the
pin2, pin2 should always be visible in LockedPins or not at all, I would
prefer the later. However it is showed as it is showed and it was
working that way already before this patch). If I run same case three
times I get same values but API shows that puk2 is required and all
API:s stay up. If I run the case with correct password it does not come
to this code at all all.
PIN2 cannot be required in startup. It is only required if u do
something with certain features like fdn or call meter. PUK2 can be
blocked in startup all right but it does not matter from startup point
of view. If I start the oFono with PIN2 blocked I get pin_type
OFONO_SIM_PASSWORD_SIM_PIN2 and OFONO_SIM_STATE_INSERTED states and API
shows pin2 locked and puk2 required as required. (does not seem work if
I boot up the modem and not just oFono though. Looks like modem bug
where it does not read voluntarily PIN2 state.)
Br,
-Jussi
7:17 a.m.
Hi Jussi,
On 02/23/2011 09:24 AM, Jussi Kangas wrote:
Hi,
On Wed, 2011-02-23 at 08:15 +0200, Denis Kenzior wrote:
>>
>> - if (pin_type != OFONO_SIM_PASSWORD_NONE &&
>> - sim->state == OFONO_SIM_STATE_READY) {
>> + if ((pin_type != OFONO_SIM_PASSWORD_NONE &&
>> + sim->state == OFONO_SIM_STATE_READY) &&
>> + (pin_type != OFONO_SIM_PASSWORD_SIM_PIN2)) {
>
> I don't see how this can work. You need to check for pin_type != NONE,
> PIN2 and PUK2 AND state == READY here. This also only covers the case
> of the PIN2 or PUK2 being triggered when call-meter is active. You do
> not take care of the case where PIN2 or PUK2 are already required during
> sim initialization.
Hmm. It seems to work here. If I try reset call meter with wrong
password I get pin_type OFONO_SIM_PASSWORD_SIM_PIN2 and
OFONO_SIM_STATE_READY. APIs stay up and SIM manager shows pin2 locked
and pin2 required. (Which is kinda wrong I admit, u cannot unlock the
pin2, pin2 should always be visible in LockedPins or not at all, I would
prefer the later. However it is showed as it is showed and it was
working that way already before this patch). If I run same case three
times I get same values but API shows that puk2 is required and all
API:s stay up. If I run the case with correct password it does not come
to this code at all all.
What I'm saying here is that it would be safer to do it like this:
switch (pin_type) {
case OFONO_SIM_PASSWORD_NONE:
case OFONO_SIM_PASSWORD_SIM_PIN2:
case OFONO_SIM_PASSWORD_SIM_PUK2:
break;
default:
if (sim->state == OFONO_SIM_STATE_READY)
...
}
We have to check for the PUK2 case above since we might be in PUK2
required state already. If we fail to unlock the PUK2, then
pin_type = puk2pin(pin_type);
will never be executed.
And I'm fine with your suggestion of blacklisting PIN2 from LockedPins,
but lets handle this separately.
PIN2 cannot be required in startup. It is only required if u do
something with certain features like fdn or call meter. PUK2 can be
blocked in startup all right but it does not matter from startup point
of view. If I start the oFono with PIN2 blocked I get pin_type
OFONO_SIM_PASSWORD_SIM_PIN2 and OFONO_SIM_STATE_INSERTED states and API
shows pin2 locked and puk2 required as required. (does not seem work if
I boot up the modem and not just oFono though. Looks like modem bug
where it does not read voluntarily PIN2 state.)
Again, my point here is that you have this code at the bottom of the
function:
if (pin_type == OFONO_SIM_PASSWORD_NONE)
sim_initialize_after_pin(sim);
If you want the SIM initialization to proceed then you need to add a
check for SIM PIN2/PUK2.
Regards,
-Denis
12:19 a.m.
Hi,
On Thu, 2011-02-24 at 22:17 +0200, Denis Kenzior wrote:
We have to check for the PUK2 case above since we might be in PUK2
required state already. If we fail to unlock the PUK2, then
pin_type = puk2pin(pin_type);
will never be executed.
And I'm fine with your suggestion of blacklisting PIN2 from
LockedPins,
but lets handle this separately.
Yes let's handle that as separate issue if at all. What I was trying to
say was that there was no situation in my testings where PUK2 would be
required in this part of code. If PUK2 is required pin_type is still
PIN2 here. Situation you describe has to happen with other modems.
Again, my point here is that you have this code at the bottom of the
function:
if (pin_type == OFONO_SIM_PASSWORD_NONE)
sim_initialize_after_pin(sim);
If you want the SIM initialization to proceed then you need to add a
check for SIM PIN2/PUK2.
You are writing about adding PIN2 and PUK2 to this check? I rechecked
my AT command API specification and PIN2 and PUK2 cannot be required in
startup. (Speculation about bug in my previous mail was wrong) And if
PIN2 or PUK2 is required in other time there is no need for
sim_initialization. You must be writing about situation where oFono is
run workstation environment and restarted without booting the modem? Ok,
I'll change that but I think I'll add a check to prevent sim
initialization in sim state READY situation.
What is your general opinion about making amends in oFono code to enable
nice working in workstation? My previous hotswap patch does not work if
you disable modem without restarting oFono. It lacks initialization of
have_sim parameter in init_sim_reporting method.
Br,
-Jussi
4:07 a.m.
Hi Jussi,
On 02/25/2011 07:19 AM, Jussi Kangas wrote:
Hi,
On Thu, 2011-02-24 at 22:17 +0200, Denis Kenzior wrote:
> We have to check for the PUK2 case above since we might be in PUK2
> required state already. If we fail to unlock the PUK2, then
> pin_type = puk2pin(pin_type);
> will never be executed.
>
> And I'm fine with your suggestion of blacklisting PIN2 from
LockedPins,
> but lets handle this separately.
Yes let's handle that as separate issue if at all. What I was trying to
say was that there was no situation in my testings where PUK2 would be
required in this part of code. If PUK2 is required pin_type is still
PIN2 here. Situation you describe has to happen with other modems.
Looking at the code there might be possibilities when pin_type is in
fact PUK2. The puk2pin code is executed only if pin_type does not match
sim->pin_type. So if you're already in PUK2 state, and fail to unlock
again, you need to check against PUK2, not PIN2.
> Again, my point here is that you have this code at the bottom of
the
> function:
>
> if (pin_type == OFONO_SIM_PASSWORD_NONE)
> sim_initialize_after_pin(sim);
>
> If you want the SIM initialization to proceed then you need to add a
> check for SIM PIN2/PUK2.
You are writing about adding PIN2 and PUK2 to this check? I rechecked
my AT command API specification and PIN2 and PUK2 cannot be required in
startup. (Speculation about bug in my previous mail was wrong) And if
PIN2 or PUK2 is required in other time there is no need for
sim_initialization. You must be writing about situation where oFono is
run workstation environment and restarted without booting the modem? Ok,
I'll change that but I think I'll add a check to prevent sim
initialization in sim state READY situation.
What is your general opinion about making amends in oFono code to enable
nice working in workstation? My previous hotswap patch does not work if
you disable modem without restarting oFono. It lacks initialization of
have_sim parameter in init_sim_reporting method.
We need to make every reasonable effort to make oFono work in these
cases. We can't assume that the driver can reset the modem properly
when powering up. And in fact most USB data sticks on the market do not
support modem resets short of pulling the hardware. If the modem
firmware is buggy, then there's obviously nothing we can do.
Regards,
-Denis
4223
days inactive
4226
days old
5 comments
2 participants
participants (2)
-
Denis Kenzior -
Jussi Kangas