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

[Denis Kenzior <denkenz@gmail.com>]

[-- 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