Read EFad and expose MNC length in IMSI as a SimManager property.

[Gu, Yang]

Several minor comments on this patch:
1. How about unify all the mnc_length to be unsigned char?
2. For this piece of code:
>+		value = new_mnc_length;
>+		ofono_dbus_signal_property_changed(conn, modem->path,
>+						SIM_MANAGER_INTERFACE,
>+						"MNCLength", DBUS_TYPE_BYTE,
>+						&value);
Is it necessary to use temporary variable value? Why not use &sim->mnc_length instead of &value?

3. In function sim_ad_read_cb(), variable "ph" is defined by not used. Current build system is not smart enough to check if all the variables are used or not. I sugguest to add some option, such as -Wunused, in our build system to check this kind of situation.


Regards,
-Yang


>-----Original Message-----
>From: ofono-bounces at ofono.org [mailto:ofono-bounces at ofono.org] On Behalf
>Of Andrzej Zaborowski
>Sent: Thursday, August 20, 2009 12:00 AM
>To: ofono at ofono.org
>Subject: Read EFad and expose MNC length in IMSI as a SimManager property.
>
>Alternatively we could expose the MNC as a separate property.
>---
> src/sim.c     |   50
>++++++++++++++++++++++++++++++++++++++++++++++++++
> src/simutil.h |    1 +
> 2 files changed, 51 insertions(+), 0 deletions(-)
>
>diff --git a/src/sim.c b/src/sim.c
>index 39976b3..0af02cd 100644
>--- a/src/sim.c
>+++ b/src/sim.c
>@@ -77,6 +77,7 @@ struct sim_file_op {
> struct sim_manager_data {
> 	struct ofono_sim_ops *ops;
> 	char *imsi;
>+	int mnc_length;
> 	GSList *own_numbers;
> 	GSList *new_numbers;
> 	GSList *ready_notify;
>@@ -164,6 +165,7 @@ static DBusMessage
>*sim_get_properties(DBusConnection *conn,
> 	DBusMessageIter iter;
> 	DBusMessageIter dict;
> 	char **own_numbers;
>+	unsigned char mnc_len;
>
> 	reply = dbus_message_new_method_return(msg);
> 	if (!reply)
>@@ -179,6 +181,13 @@ static DBusMessage
>*sim_get_properties(DBusConnection *conn,
> 		ofono_dbus_dict_append(&dict, "SubscriberIdentity",
> 					DBUS_TYPE_STRING, &sim->imsi);
>
>+	if (sim->mnc_length) {
>+		mnc_len = sim->mnc_length;
>+
>+		ofono_dbus_dict_append(&dict, "MNCLength",
>+					DBUS_TYPE_BYTE, &mnc_len);
>+	}
>+
> 	own_numbers = get_own_numbers(sim->own_numbers);
>
> 	ofono_dbus_dict_append_array(&dict, "SubscriberNumbers",
>@@ -432,15 +441,56 @@ check:
> 	sim->new_numbers = NULL;
> }
>
>+static void sim_ad_read_cb(struct ofono_modem *modem, int ok,
>+				enum ofono_sim_file_structure structure,
>+				int length, int record,
>+				const unsigned char *data,
>+				int record_length, void *userdata)
>+{
>+	struct sim_manager_data *sim = userdata;
>+	DBusConnection *conn = ofono_dbus_get_connection();
>+	int new_mnc_length;
>+	unsigned char value;
>+	struct ofono_phone_number ph;
>+
>+	if (!ok)
>+		return;
>+
>+	if (structure != OFONO_SIM_FILE_STRUCTURE_TRANSPARENT)
>+		return;
>+
>+	if (length < 4)
>+		return;
>+
>+	new_mnc_length = data[3] & 0xf;
>+
>+	if (sim->mnc_length != new_mnc_length) {
>+		sim->mnc_length = new_mnc_length;
>+
>+		value = new_mnc_length;
>+		ofono_dbus_signal_property_changed(conn, modem->path,
>+						SIM_MANAGER_INTERFACE,
>+						"MNCLength", DBUS_TYPE_BYTE,
>+						&value);
>+	}
>+}
>+
> static void sim_own_numbers_update(struct ofono_modem *modem)
> {
> 	ofono_sim_read(modem, SIM_EFMSISDN_FILEID,
> 			sim_msisdn_read_cb, modem->sim_manager);
> }
>
>+static void sim_mnc_length_update(struct ofono_modem *modem)
>+{
>+	ofono_sim_read(modem, SIM_EFAD_FILEID,
>+			sim_ad_read_cb, modem->sim_manager);
>+}
>+
> static void sim_ready(struct ofono_modem *modem)
> {
> 	sim_own_numbers_update(modem);
>+	sim_mnc_length_update(modem);
> }
>
> static void sim_imsi_cb(const struct ofono_error *error, const char *imsi,
>diff --git a/src/simutil.h b/src/simutil.h
>index c0d3d52..9bb5323 100644
>--- a/src/simutil.h
>+++ b/src/simutil.h
>@@ -22,6 +22,7 @@
> enum sim_fileid {
> 	SIM_EFMSISDN_FILEID = 0x6f40,
> 	SIM_EFSPN_FILEID = 0x6f46,
>+	SIM_EFAD_FILEID = 0x6fad,
> 	SIM_EFPNN_FILEID = 0x6fc5,
> 	SIM_EFOPL_FILEID = 0x6fc6,
> 	SIM_EFMBDN_FILEID = 0x6fc7,
>--
>1.6.1
>
>_______________________________________________
>ofono mailing list
>ofono at ofono.org
>http://lists.ofono.org/listinfo/ofono