Re: [PATCH 1/2] [sim] Emit LockedPins after pin_type is queried

Re: [PATCH 1/2] [sim] Emit LockedPins after pin_type is queried

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2162 bytes --]

Hi Tommi,

On 06/18/2015 06:46 AM, Tommi Kenakkala wrote:
> Fixes property change not being emited when hot-swapping a
> PIN-enabled card.
> ---
>   src/sim.c |   20 +++++++++++++++++++-
>   1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/src/sim.c b/src/sim.c
> index b5badf1..0bcafe0 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -2737,6 +2737,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
>   	DBusConnection *conn = ofono_dbus_get_connection();
>   	const char *path = __ofono_atom_get_path(sim->atom);
>   	const char *pin_name;
> +	char **locked_pins;
> +	gboolean lock_changed;
> +
> +	DBG("sim->pin_type: %d, pin_type: %d", sim->pin_type, pin_type);
>
>   	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
>   		ofono_error("Querying PIN authentication state failed");
> @@ -2751,9 +2755,23 @@ static void sim_pin_query_cb(const struct ofono_error *error,
>   				password_is_pin(pin_type) == FALSE)
>   			pin_type = puk2pin(pin_type);
>
> -		if (pin_type != OFONO_SIM_PASSWORD_INVALID)
> +		if (pin_type != OFONO_SIM_PASSWORD_INVALID) {

you might want to add && pin_type != OFONO_SIM_PASSWORD_NONE here.  See 
below.

> +			lock_changed = !sim->locked_pins[pin_type];
> +

So when do you want to emit LockedPins here?  Only when the list is 
non-empty or always?

There is a subtlety here.  OFONO_SIM_PASSWORD_NONE is never considered 
when emitting LockedPins.  However, in this proposal you can trigger a 
LockedPins emission even if there's no PIN set.

To me it seems like emitting LockedPins with an empty value seems 
unnecessary.  Thoughts?

>   			sim->locked_pins[pin_type] = TRUE;
>
> +			if (lock_changed) {
> +				locked_pins = get_locked_pins(sim);
> +
> +				ofono_dbus_signal_array_property_changed(conn,
> +						path,
> +						OFONO_SIM_MANAGER_INTERFACE,
> +						"LockedPins", DBUS_TYPE_STRING,
> +						&locked_pins);
> +
> +				g_strfreev(locked_pins);
> +			}
> +		}
>   		ofono_dbus_signal_property_changed(conn, path,
>   						OFONO_SIM_MANAGER_INTERFACE,
>   						"PinRequired", DBUS_TYPE_STRING,
>

Regards,
-Denis

Re: [PATCH 2/2] [sim] Reset pin_type on card remove

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 259 bytes --]

Hi Tommi,

On 06/18/2015 06:46 AM, Tommi Kenakkala wrote:
> Fixes PinRequired not being emitted when a card is inserted
> ---
>   src/sim.c |    7 +++++--
>   1 file changed, 5 insertions(+), 2 deletions(-)
>

Applied, thanks.

Regards,
-Denis

[PATCH 0/2] Improve support for card hot-swapping

[Tommi Kenakkala]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

Events after inserting a card are as follows. Problem is
async steps 2 and 3+4 complete independently so client can't
rely on Present, CardIdentifier or other properties to know 
when PinRequired and LockedPins are updated.
1. ofono_sim_inserted_notify > emit "Present" ==> sim_initialize
2. sim_iccid_read_cb > emit "CardIdentifier"
3. sim_efpl_read_cb > __ofono_sim_recheck_pin
4. sim_pin_query_cb
    ==> "PinRequired" emitted at startup but not on pin-enabled hotswaps
    ==> "LockedPins" never emitted 

These patches improve the situation.

Tommi Kenakkala (2):
  [sim] Emit LockedPins after pin_type is queried
  [sim] Reset pin_type on card remove

 src/sim.c |   27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] [sim] Emit LockedPins after pin_type is queried

[Tommi Kenakkala]

[-- Attachment #1: Type: text/plain, Size: 1522 bytes --]

Fixes property change not being emited when hot-swapping a
PIN-enabled card.
---
 src/sim.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/sim.c b/src/sim.c
index b5badf1..0bcafe0 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2737,6 +2737,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path = __ofono_atom_get_path(sim->atom);
 	const char *pin_name;
+	char **locked_pins;
+	gboolean lock_changed;
+
+	DBG("sim->pin_type: %d, pin_type: %d", sim->pin_type, pin_type);
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		ofono_error("Querying PIN authentication state failed");
@@ -2751,9 +2755,23 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 				password_is_pin(pin_type) == FALSE)
 			pin_type = puk2pin(pin_type);
 
-		if (pin_type != OFONO_SIM_PASSWORD_INVALID)
+		if (pin_type != OFONO_SIM_PASSWORD_INVALID) {
+			lock_changed = !sim->locked_pins[pin_type];
+
 			sim->locked_pins[pin_type] = TRUE;
 
+			if (lock_changed) {
+				locked_pins = get_locked_pins(sim);
+
+				ofono_dbus_signal_array_property_changed(conn,
+						path,
+						OFONO_SIM_MANAGER_INTERFACE,
+						"LockedPins", DBUS_TYPE_STRING,
+						&locked_pins);
+
+				g_strfreev(locked_pins);
+			}
+		}
 		ofono_dbus_signal_property_changed(conn, path,
 						OFONO_SIM_MANAGER_INTERFACE,
 						"PinRequired", DBUS_TYPE_STRING,
-- 
1.7.9.5

[PATCH 2/2] [sim] Reset pin_type on card remove

[Tommi Kenakkala]

[-- Attachment #1: Type: text/plain, Size: 654 bytes --]

Fixes PinRequired not being emitted when a card is inserted
---
 src/sim.c |    7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index 0bcafe0..997ac27 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2485,10 +2485,13 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
 	sim_inserted_update(sim);
 	call_state_watches(sim);
 
-	if (inserted)
+	if (inserted) {
 		sim_initialize(sim);
-	else
+	} else {
+		sim->pin_type = OFONO_SIM_PASSWORD_NONE;
+
 		sim_free_state(sim);
+	}
 }
 
 unsigned int ofono_sim_add_state_watch(struct ofono_sim *sim,
-- 
1.7.9.5

Re: [PATCH 2/2] [sim] Emit LockedPins after pin_type is queried

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 294 bytes --]

Hi Tommi,

On 06/22/2015 08:37 AM, Tommi Kenakkala wrote:
> Fixes property change not being emited when hot-swapping a
> PIN-enabled card.
> ---
>   src/sim.c |   22 +++++++++++++++++++++-
>   1 file changed, 21 insertions(+), 1 deletion(-)
>

Applied, thanks.

Regards,
-Denis

[PATCH 2/2] [sim] Emit LockedPins after pin_type is queried

[Tommi Kenakkala]

[-- Attachment #1: Type: text/plain, Size: 1572 bytes --]

Fixes property change not being emited when hot-swapping a
PIN-enabled card.
---
 src/sim.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/sim.c b/src/sim.c
index 715f3c0..8341af0 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2740,6 +2740,10 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 	DBusConnection *conn = ofono_dbus_get_connection();
 	const char *path = __ofono_atom_get_path(sim->atom);
 	const char *pin_name;
+	char **locked_pins;
+	gboolean lock_changed;
+
+	DBG("sim->pin_type: %d, pin_type: %d", sim->pin_type, pin_type);
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
 		ofono_error("Querying PIN authentication state failed");
@@ -2754,9 +2758,25 @@ static void sim_pin_query_cb(const struct ofono_error *error,
 				password_is_pin(pin_type) == FALSE)
 			pin_type = puk2pin(pin_type);
 
-		if (pin_type != OFONO_SIM_PASSWORD_INVALID)
+
+		if (pin_type != OFONO_SIM_PASSWORD_INVALID
+				&& pin_type != OFONO_SIM_PASSWORD_NONE) {
+			lock_changed = !sim->locked_pins[pin_type];
+
 			sim->locked_pins[pin_type] = TRUE;
 
+			if (lock_changed) {
+				locked_pins = get_locked_pins(sim);
+
+				ofono_dbus_signal_array_property_changed(conn,
+						path,
+						OFONO_SIM_MANAGER_INTERFACE,
+						"LockedPins", DBUS_TYPE_STRING,
+						&locked_pins);
+
+				g_strfreev(locked_pins);
+			}
+		}
 		ofono_dbus_signal_property_changed(conn, path,
 						OFONO_SIM_MANAGER_INTERFACE,
 						"PinRequired", DBUS_TYPE_STRING,
-- 
1.7.9.5