Merge lp:~rsalveti/phablet-extras/ofono-imei-revision into lp:phablet-extras/ofono

[Ricardo Salveti]

[ril/rilmodem] Add real calls for revision and IMEI probe

[ril/rilmodem] Add real calls for revision and IMEI probe

Also making a new release so it can be used by the system-settings application.

[lp:~rsalveti/phablet-extras/ofono-imei-revision]

44. By Ricardo Salveti on 2013-06-25

rilmodem/devinfo.c: using RIL_REQUEST_GET_IMEI for imei (works for both maguro and mako)

45. By Ricardo Salveti on 2013-06-25

rilmodem/devinfo.c: only free revision/imei if it's valid

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:43
http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-ci/38/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/13
        deb: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/13/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/phablet-extras-ofono-ci/38/rebuild

review: Approve (continuous-integration)

[Ricardo Salveti (rsalveti)]

How to test:

phablet@ubuntu-phablet:/usr/share/ofono/scripts$ ./list-modems
[ /ril_0 ]
    Features = sms net gprs sim
    Emergency = 0
    Powered = 1
    Lockdown = 0
    Interfaces = org.ofono.CallVolume org.ofono.SmartMessaging org.ofono.PushNotification org.ofono.MessageManager org.ofono.NetworkRegistration org.ofono.VoiceCallManager org.ofono.ConnectionManager org.ofono.SimManager
    Online = 1
    Model = Fake Modem Model
    Revision = M9615A-CEFWMAZM-2.XXXXX
    Type = hardware
    Serial = 3539XXXXXXXXXX

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:45
http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-ci/39/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/14
        deb: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/14/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/phablet-extras-ofono-ci/39/rebuild

review: Approve (continuous-integration)

[Simon Fels (morphis)]

Works fine for me here. Testing gave me the output I get within android too.

[ /ril_0 ]
    Features = sms net gprs sim
    Emergency = 0
    Powered = 1
    Lockdown = 0
    Interfaces = org.ofono.CallVolume org.ofono.SmartMessaging org.ofono.PushNotification org.ofono.MessageManager org.ofono.VoiceCallManager org.ofono.NetworkRegistration org.ofono.ConnectionManager org.ofono.SimManager
    Online = 1
    Model = Fake Modem Model
    Revision = I9250XXLJ1
    Type = hardware
    Serial = 35XXXXXXXXXXXXX
    Manufacturer = Fake Manufacturer

review: Approve

[Tony Espy (awe)]

A couple comments...

1. Since it's safe to call g_free() with a NULL pointer, the general approach I've seen in the ofono code is to just call it and not check for pointers. This feels wrong to me, but that seems to be the general convention for glib based code ( including the rest of ofono ).

So, could you remove line 43 & 96?

2. The second comment is due to the overly complicated tracing functions. On lines 60 & 122, you should call g_print_request_no_args(), otherwise you could end up with incorrect output from a previous request/response.

As part of our major re-factor, I have a plan to fix this by moving the g_ril_print_request() calls into g_ril_send *and* I will also change the macros to clear print_buf after print_request/print_response or print_unsol() calls.

review: Needs Fixing

[lp:~rsalveti/phablet-extras/ofono-imei-revision]

46. By Ricardo Salveti on 2013-06-25

rilmodem/devinfo.c: fixes from code review

[Tony Espy (awe)]

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:46
http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-ci/43/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/18
        deb: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/18/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/phablet-extras-ofono-ci/43/rebuild

review: Approve (continuous-integration)

=== modified file 'debian/changelog'
--- debian/changelog	2013-06-17 17:40:00 +0000
+++ debian/changelog	2013-06-25 18:32:25 +0000
@@ -1,3 +1,9 @@
+ofono (1.12phablet6) saucy; urgency=low
+
+  * [ril/rilmodem] Add real calls for revision and IMEI probe
+
+ -- Ricardo Salveti de Araujo <[e-mail]>  Tue, 25 Jun 2013 02:46:40 -0300
+
 ofono (1.12phablet5) saucy; urgency=low
 
   * plugins/ril.c: Delay modem initialization until RIL_CONNECTED

=== modified file 'drivers/rilmodem/devinfo.c'
--- drivers/rilmodem/devinfo.c	2013-06-10 16:43:50 +0000
+++ drivers/rilmodem/devinfo.c	2013-06-25 18:32:25 +0000
@@ -76,34 +76,95 @@
 	g_free(cbd);
 }
 
+static void query_revision_cb(struct ril_msg *message, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_devinfo_query_cb_t cb = cbd->cb;
+	struct ofono_error error;
+	struct parcel rilp;
+	gchar *revision;
+
+	if (message->error == RIL_E_SUCCESS) {
+		decode_ril_error(&error, "OK");
+	} else {
+		decode_ril_error(&error, "FAIL");
+		cb(&error, NULL, cbd->data);
+		return;
+	}
+
+	ril_util_init_parcel(message, &rilp);
+	revision = parcel_r_string(&rilp);
+
+	cb(&error, revision, cbd->data);
+
+	g_free(revision);
+}
+
 static void ril_query_revision(struct ofono_devinfo *info,
 				ofono_devinfo_query_cb_t cb,
 				void *data)
 {
-	const char *attr = "Fake Revision";
 	struct cb_data *cbd = cb_data_new(cb, data);
+	GRil *ril = ofono_devinfo_get_data(info);
+	int request = RIL_REQUEST_BASEBAND_VERSION;
+	int ret;
+
+	ret = g_ril_send(ril, request, NULL, 0,
+					query_revision_cb, cbd, g_free);
+
+	g_ril_print_request_no_args(ril, ret, request);
+
+	if (ret <= 0) {
+		g_free(cbd);
+		CALLBACK_WITH_FAILURE(cb, NULL, data);
+	}
+}
+
+static void query_serial_cb(struct ril_msg *message, gpointer user_data)
+{
+	struct cb_data *cbd = user_data;
+	ofono_devinfo_query_cb_t cb = cbd->cb;
 	struct ofono_error error;
-	decode_ril_error(&error, "OK");
-
-	cb(&error, attr, cbd->data);
-
-	/* Note: this will need to change if cbd passed to gril layer */
-	g_free(cbd);
+	struct parcel rilp;
+	gchar *imei;
+
+	if (message->error == RIL_E_SUCCESS) {
+		decode_ril_error(&error, "OK");
+	} else {
+		decode_ril_error(&error, "FAIL");
+		cb(&error, NULL, cbd->data);
+		return;
+	}
+
+	ril_util_init_parcel(message, &rilp);
+
+	imei = parcel_r_string(&rilp);
+
+	cb(&error, imei, cbd->data);
+
+	g_free(imei);
 }
 
 static void ril_query_serial(struct ofono_devinfo *info,
 				ofono_devinfo_query_cb_t cb,
 				void *data)
 {
-	const char *attr = "THIS-IS-A-FAKE-SERIAL-NO";
 	struct cb_data *cbd = cb_data_new(cb, data);
-	struct ofono_error error;
-	decode_ril_error(&error, "OK");
-
-	cb(&error, attr, cbd->data);
-
-	/* Note: this will need to change if cbd passed to gril layer */
-	g_free(cbd);
+	GRil *ril = ofono_devinfo_get_data(info);
+	/* TODO: make it support both RIL_REQUEST_GET_IMEI (deprecated) and
+	 * RIL_REQUEST_DEVICE_IDENTITY depending on the rild version used */
+	int request = RIL_REQUEST_GET_IMEI;
+	int ret;
+
+	ret = g_ril_send(ril, request, NULL, 0,
+					query_serial_cb, cbd, g_free);
+
+	g_ril_print_request_no_args(ril, ret, request);
+
+	if (ret <= 0) {
+		g_free(cbd);
+		CALLBACK_WITH_FAILURE(cb, NULL, data);
+	}
 }
 
 static gboolean ril_delayed_register(gpointer user_data)
@@ -126,7 +187,7 @@
 
 	ofono_devinfo_set_data(info, ril);
 
-        /*
+	/*
 	 * TODO: analyze if capability check is needed
 	 * and/or timer should be adjusted.
 	 *
@@ -136,7 +197,7 @@
 	 * some kind of capabilities query to the modem, and then
 	 * call register in the callback; we use a timer instead.
 	 */
-        g_timeout_add_seconds(1, ril_delayed_register, info);
+	g_timeout_add_seconds(1, ril_delayed_register, info);
 
 	return 0;
 }