[PATCH 1/8][RFC] Add functions to notify core of SIM insertion/removal or request polling.

[PATCH 1/8][RFC] Add functions to notify core of SIM insertion/removal or request polling.

[Andrzej Zaborowski]

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

Implement optional polling.  Polling is mandatory according to the
specification but on most hardware it will me impractical because
there will be vendor specific unsolicited notifications that can be
used instead.  Modem plugins need to handle them.
---
 include/sim.h |   21 ++++++-
 src/sim.c     |  197 ++++++++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 195 insertions(+), 23 deletions(-)

diff --git a/include/sim.h b/include/sim.h
index 6ff29f7..fdfe2e8 100644
--- a/include/sim.h
+++ b/include/sim.h
@@ -106,6 +106,8 @@ typedef void (*ofono_sim_lock_unlock_cb_t)(const struct ofono_error *error,
 typedef void (*ofono_sim_locked_cb_t)(const struct ofono_error *error,
 					int locked, void *data);
 
+typedef void (*ofono_sim_cb_t)(const struct ofono_error *error, void *data);
+
 struct ofono_sim_driver {
 	const char *name;
 	int (*probe)(struct ofono_sim *sim, unsigned int vendor, void *data);
@@ -152,6 +154,12 @@ struct ofono_sim_driver {
 	void (*envelope)(struct ofono_sim *sim, int length,
 				const guint8 *command,
 				ofono_sim_read_cb_t cb, void *data);
+	void (*status)(struct ofono_sim *sim, ofono_sim_cb_t cb, void *data);
+	void (*fetch)(struct ofono_sim *sim, int length,
+			ofono_sim_read_cb_t cb, void *data);
+	void (*terminal_response)(struct ofono_sim *sim, int length,
+			const unsigned char *value, ofono_sim_write_cb_t cb,
+			void *data);
 };
 
 int ofono_sim_driver_register(const struct ofono_sim_driver *d);
@@ -179,8 +187,19 @@ unsigned int ofono_sim_add_ready_watch(struct ofono_sim *sim,
 
 void ofono_sim_remove_ready_watch(struct ofono_sim *sim, unsigned int id);
 
+unsigned int ofono_sim_add_removed_watch(struct ofono_sim *sim,
+				ofono_sim_ready_notify_cb_t cb,
+				void *data, ofono_destroy_func destroy);
+
+void ofono_sim_remove_removed_watch(struct ofono_sim *sim, unsigned int id);
+
 int ofono_sim_get_ready(struct ofono_sim *sim);
-void ofono_sim_set_ready(struct ofono_sim *sim);
+
+void ofono_sim_inserted(struct ofono_sim *sim);
+void ofono_sim_removed(struct ofono_sim *sim);
+void ofono_sim_proactive_command_notify(struct ofono_sim *sim, int length);
+
+void ofono_sim_poll_enable(struct ofono_sim *sim);
 
 /* This will queue an operation to read all available records with id from the
  * SIM.  Callback cb will be called every time a record has been read, or once
diff --git a/src/sim.c b/src/sim.c
index 1ab8a8b..733e876 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -57,6 +57,8 @@ static gboolean sim_op_next(gpointer user_data);
 static gboolean sim_op_retrieve_next(gpointer user);
 static void sim_own_numbers_update(struct ofono_sim *sim);
 static void sim_pin_check(struct ofono_sim *sim);
+static void sim_set_ready(struct ofono_sim *sim);
+static gboolean sim_status_poll(gpointer user_data);
 
 struct sim_file_op {
 	int id;
@@ -92,6 +94,13 @@ struct ofono_sim {
 	enum ofono_sim_cphs_phase cphs_phase;
 	unsigned char cphs_service_table[2];
 	struct ofono_watchlist *ready_watches;
+	struct ofono_watchlist *removed_watches;
+	int idle_poll_interval;
+	int incall_poll_interval;
+	gint status_timeout;
+	gint poll_timeout;
+	gboolean poll_enabled;
+	gboolean inserted;
 	const struct ofono_sim_driver *driver;
 	void *driver_data;
 	struct ofono_atom *atom;
@@ -973,7 +982,7 @@ static void sim_cphs_information_read_cb(int ok, int length, int record,
 	memcpy(sim->cphs_service_table, data + 1, 2);
 
 ready:
-	ofono_sim_set_ready(sim);
+	sim_set_ready(sim);
 }
 
 static void sim_imsi_cb(const struct ofono_error *error, const char *imsi,
@@ -1743,6 +1752,117 @@ const unsigned char *ofono_sim_get_cphs_service_table(struct ofono_sim *sim)
 	return sim->cphs_service_table;
 }
 
+void ofono_sim_inserted(struct ofono_sim *sim)
+{
+	sim->inserted = TRUE;
+
+	/* Perform SIM initialization according to 3GPP 31.102 Section 5.1.1.2
+	 * The assumption here is that if sim manager is being initialized,
+	 * then sim commands are implemented, and the sim manager is then
+	 * responsible for checking the PIN, reading the IMSI and signaling
+	 * SIM ready condition.
+	 *
+	 * The procedure according to 31.102 is roughly:
+	 * Read EFecc
+	 * Read EFli and EFpl
+	 * SIM Pin check
+	 * Request SIM phase (only in 51.011)
+	 * Read EFust
+	 * Read EFest
+	 * Read IMSI
+	 *
+	 * At this point we signal the SIM ready condition and allow
+	 * arbitrary files to be written or read, assuming their presence
+	 * in the EFust
+	 */
+	sim_determine_phase(sim);
+}
+
+void ofono_sim_removed(struct ofono_sim *sim)
+{
+	GSList *l;
+	ofono_sim_ready_notify_cb_t notify;
+
+	if (sim == NULL)
+		return;
+
+	sim->inserted = FALSE;
+
+	if (sim->ready == FALSE)
+		return;
+
+	sim->ready = FALSE;
+
+	for (l = sim->removed_watches->items; l; l = l->next) {
+		struct ofono_watchlist_item *item = l->data;
+		notify = item->notify;
+
+		notify(item->notify_data);
+	}
+}
+
+void ofono_sim_proactive_command_notify(struct ofono_sim *sim, int length)
+{
+}
+
+static void sim_status_poll_schedule(struct ofono_sim *sim)
+{
+	/* TODO: Decide on the interval based on whether any call is active */
+	/* TODO: On idle, possibly only schedule if proactive commands enabled
+	 * as indicated by EFphase + EFsst (51.011: 11.6.1) */
+	int interval = sim->idle_poll_interval;
+
+	sim->poll_timeout = g_timeout_add_seconds(interval,
+			sim_status_poll, sim);
+}
+
+static void sim_status_cb(const struct ofono_error *error, void *data)
+{
+	struct ofono_sim *sim = data;
+
+	if (!sim->status_timeout)
+		/* The STATUS already timed out */
+		goto done;
+
+	g_source_remove(sim->status_timeout);
+	sim->status_timeout = 0;
+
+	if (sim->inserted != TRUE)
+		ofono_sim_inserted(sim);
+
+done:
+	sim_status_poll_schedule(sim);
+}
+
+static gboolean sim_status_timeout(gpointer user_data)
+{
+	struct ofono_sim *sim = user_data;
+
+	sim->status_timeout = 0;
+	if (sim->inserted == TRUE)
+		ofono_sim_removed(sim);
+
+	sim_status_poll_schedule(sim);
+
+	return FALSE;
+}
+
+static gboolean sim_status_poll(gpointer user_data)
+{
+	struct ofono_sim *sim = user_data;
+
+	sim->poll_timeout = 0;
+
+	/* The SIM must respond in a given time frame which is of at
+	 * least 5 seconds in TS 11.11.  */
+	sim->status_timeout = g_timeout_add_seconds(5,
+			sim_status_timeout, sim);
+
+	sim->driver->status(sim, sim_status_cb, sim);
+
+	return FALSE;
+}
+
 unsigned int ofono_sim_add_ready_watch(struct ofono_sim *sim,
 				ofono_sim_ready_notify_cb_t notify,
 				void *data, ofono_destroy_func destroy)
@@ -1771,6 +1891,34 @@ void ofono_sim_remove_ready_watch(struct ofono_sim *sim, unsigned int id)
 	__ofono_watchlist_remove_item(sim->ready_watches, id);
 }
 
+unsigned int ofono_sim_add_removed_watch(struct ofono_sim *sim,
+				ofono_sim_ready_notify_cb_t notify,
+				void *data, ofono_destroy_func destroy)
+{
+	struct ofono_watchlist_item *item;
+
+	DBG("%p", sim);
+
+	if (sim == NULL)
+		return 0;
+
+	if (notify == NULL)
+		return 0;
+
+	item = g_new0(struct ofono_watchlist_item, 1);
+
+	item->notify = notify;
+	item->destroy = destroy;
+	item->notify_data = data;
+
+	return __ofono_watchlist_add_item(sim->removed_watches, item);
+}
+
+void ofono_sim_remove_removed_watch(struct ofono_sim *sim, unsigned int id)
+{
+	__ofono_watchlist_remove_item(sim->removed_watches, id);
+}
+
 int ofono_sim_get_ready(struct ofono_sim *sim)
 {
 	if (sim == NULL)
@@ -1782,7 +1930,7 @@ int ofono_sim_get_ready(struct ofono_sim *sim)
 	return 0;
 }
 
-void ofono_sim_set_ready(struct ofono_sim *sim)
+static void sim_set_ready(struct ofono_sim *sim)
 {
 	GSList *l;
 	ofono_sim_ready_notify_cb_t notify;
@@ -1867,6 +2015,8 @@ static void sim_unregister(struct ofono_atom *atom)
 
 	__ofono_watchlist_free(sim->ready_watches);
 	sim->ready_watches = NULL;
+	__ofono_watchlist_free(sim->removed_watches);
+	sim->removed_watches = NULL;
 
 	g_dbus_unregister_interface(conn, path,
 					SIM_MANAGER_INTERFACE);
@@ -1925,6 +2075,16 @@ static void sim_remove(struct ofono_atom *atom)
 		sim->simop_q = NULL;
 	}
 
+	if (sim->status_timeout) {
+		g_source_remove(sim->status_timeout);
+		sim->status_timeout = 0;
+	}
+
+	if (sim->poll_timeout) {
+		g_source_remove(sim->poll_timeout);
+		sim->poll_timeout = 0;
+	}
+
 	g_free(sim);
 }
 
@@ -1982,31 +2142,19 @@ void ofono_sim_register(struct ofono_sim *sim)
 
 	ofono_modem_add_interface(modem, SIM_MANAGER_INTERFACE);
 	sim->ready_watches = __ofono_watchlist_new(g_free);
+	sim->removed_watches = __ofono_watchlist_new(g_free);
 
 	__ofono_atom_register(sim->atom, sim_unregister);
 
 	ofono_sim_add_ready_watch(sim, sim_ready, sim, NULL);
 
-	/* Perform SIM initialization according to 3GPP 31.102 Section 5.1.1.2
-	 * The assumption here is that if sim manager is being initialized,
-	 * then sim commands are implemented, and the sim manager is then
-	 * responsible for checking the PIN, reading the IMSI and signaling
-	 * SIM ready condition.
-	 *
-	 * The procedure according to 31.102 is roughly:
-	 * Read EFecc
-	 * Read EFli and EFpl
-	 * SIM Pin check
-	 * Request SIM phase (only in 51.011)
-	 * Read EFust
-	 * Read EFest
-	 * Read IMSI
-	 *
-	 * At this point we signal the SIM ready condition and allow
-	 * arbitrary files to be written or read, assuming their presence
-	 * in the EFust
-	 */
-	sim_determine_phase(sim);
+	if (sim->poll_enabled && sim->driver->status) {
+		sim->idle_poll_interval = 30;
+		sim->incall_poll_interval = 30;
+
+		sim_status_poll(sim);
+	} else
+		ofono_sim_inserted(sim);
 }
 
 void ofono_sim_remove(struct ofono_sim *sim)
@@ -2023,3 +2171,8 @@ void *ofono_sim_get_data(struct ofono_sim *sim)
 {
 	return sim->driver_data;
 }
+
+void ofono_sim_poll_enable(struct ofono_sim *sim)
+{
+	sim->poll_enabled = TRUE;
+}
-- 
1.6.1

Re: [PATCH 1/8][RFC] Add functions to notify core of SIM insertion/removal or request polling.

[Denis Kenzior]

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

Hi Andrew,

> Implement optional polling.  Polling is mandatory according to the
> specification but on most hardware it will me impractical because
> there will be vendor specific unsolicited notifications that can be
> used instead.  Modem plugins need to handle them.
> ---

So I've been going back and forth on this one and my opinion is we should not 
do the poll in the core.  The modem has to do it anyway, so let the modem do 
it and tell us when it happens.  Most vendors provide unsolicited notifications 
for SIM Toolkit events anyway.  Let us treat it the same way as voicecalls.  
The standard does not define any unsolicited responses for call state 
notifications, yet in practice all modem vendors provide this.

Keep this code around and let us re-examine this decision if we ever need to 
implement POLLING ON / POLLING OFF / POLLING INTERVAL commands.  Based on all 
the vendor specific docs I have these are intercepted by the modem and we never 
see them anyway.

>  include/sim.h |   21 ++++++-
>  src/sim.c     |  197
>  ++++++++++++++++++++++++++++++++++++++++++++++++++------- 2 files 
changed,
>  195 insertions(+), 23 deletions(-)
> 
> diff --git a/include/sim.h b/include/sim.h
> index 6ff29f7..fdfe2e8 100644
> --- a/include/sim.h
> +++ b/include/sim.h
> @@ -106,6 +106,8 @@ typedef void (*ofono_sim_lock_unlock_cb_t)(const struct
>  ofono_error *error, typedef void (*ofono_sim_locked_cb_t)(const struct
>  ofono_error *error, int locked, void *data);
> 
> +typedef void (*ofono_sim_cb_t)(const struct ofono_error *error, void
>  *data); +
>  struct ofono_sim_driver {
>  	const char *name;
>  	int (*probe)(struct ofono_sim *sim, unsigned int vendor, void *data);
> @@ -152,6 +154,12 @@ struct ofono_sim_driver {
>  	void (*envelope)(struct ofono_sim *sim, int length,
>  				const guint8 *command,
>  				ofono_sim_read_cb_t cb, void *data);
> +	void (*status)(struct ofono_sim *sim, ofono_sim_cb_t cb, void *data);
> +	void (*fetch)(struct ofono_sim *sim, int length,
> +			ofono_sim_read_cb_t cb, void *data);

So three things here: First the STK details should be moved to a separate atom 
since they are highly vendor specific.

Second, why are we bothering with fetch, can't we simply use 
ofono_sim_proactive_command_notify to send us the apdu directly?

And we should get rid of status if we're not doing polling.

> +	void (*terminal_response)(struct ofono_sim *sim, int length,
> +			const unsigned char *value, ofono_sim_write_cb_t cb,

to be more consistent, value first, then length.

> +			void *data);
>  };

Regards,
-Denis

Re: [PATCH 1/8][RFC] Add functions to notify core of SIM insertion/removal or request polling.

[andrzej zaborowski]

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

Hi Denis,

On 18 March 2010 00:01, Denis Kenzior <denkenz@gmail.com> wrote:
>> Implement optional polling.  Polling is mandatory according to the
>> specification but on most hardware it will me impractical because
>> there will be vendor specific unsolicited notifications that can be
>> used instead.  Modem plugins need to handle them.
>> ---
>
> So I've been going back and forth on this one and my opinion is we should not
> do the poll in the core.  The modem has to do it anyway, so let the modem do
> it and tell us when it happens.  Most vendors provide unsolicited notifications
> for SIM Toolkit events anyway.  Let us treat it the same way as voicecalls.
> The standard does not define any unsolicited responses for call state
> notifications, yet in practice all modem vendors provide this.

I'm fine with that, but note that we will lose the ability to have STK
working out of the box on a AT compliant modem or emulator, this would
be useful even just during the early development.  The spec
specifically says the ME that supports STK needs to poll and with what
interval.

>
> Keep this code around and let us re-examine this decision if we ever need to
> implement POLLING ON / POLLING OFF / POLLING INTERVAL commands.  Based on all
> the vendor specific docs I have these are intercepted by the modem and we never
> see them anyway.
>
>>  include/sim.h |   21 ++++++-
>>  src/sim.c     |  197
>>  ++++++++++++++++++++++++++++++++++++++++++++++++++------- 2 files
> changed,
>>  195 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/sim.h b/include/sim.h
>> index 6ff29f7..fdfe2e8 100644
>> --- a/include/sim.h
>> +++ b/include/sim.h
>> @@ -106,6 +106,8 @@ typedef void (*ofono_sim_lock_unlock_cb_t)(const struct
>>  ofono_error *error, typedef void (*ofono_sim_locked_cb_t)(const struct
>>  ofono_error *error, int locked, void *data);
>>
>> +typedef void (*ofono_sim_cb_t)(const struct ofono_error *error, void
>>  *data); +
>>  struct ofono_sim_driver {
>>       const char *name;
>>       int (*probe)(struct ofono_sim *sim, unsigned int vendor, void *data);
>> @@ -152,6 +154,12 @@ struct ofono_sim_driver {
>>       void (*envelope)(struct ofono_sim *sim, int length,
>>                               const guint8 *command,
>>                               ofono_sim_read_cb_t cb, void *data);
>> +     void (*status)(struct ofono_sim *sim, ofono_sim_cb_t cb, void *data);
>> +     void (*fetch)(struct ofono_sim *sim, int length,
>> +                     ofono_sim_read_cb_t cb, void *data);
>
> So three things here: First the STK details should be moved to a separate atom
> since they are highly vendor specific.
>
> Second, why are we bothering with fetch, can't we simply use
> ofono_sim_proactive_command_notify to send us the apdu directly?

Yes, good idea.  Do we want to still call
ofono_sim_proactive_command_notify from atmodem/sim.c or will we just
count on modem driver to do that?

I noticed that we also need to cancel all commands already sent to
GAtChat when we conclude that STATUS timed out and am not sure where
to do this cancellation.

Regards