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

Proposed by Ricardo Salveti on 2013-06-25
Status: Merged
Approved by: Tony Espy on 2013-06-25
Approved revision: 46
Merged at revision: 42
Proposed branch: lp:~rsalveti/phablet-extras/ofono-imei-revision
Merge into: lp:phablet-extras/ofono
Diff against target: 146 lines (+84/-17)
2 files modified
debian/changelog (+6/-0)
drivers/rilmodem/devinfo.c (+78/-17)
To merge this branch: bzr merge lp:~rsalveti/phablet-extras/ofono-imei-revision
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2013-06-25
Tony Espy 2013-06-25 Approve on 2013-06-25
Simon Fels Approve on 2013-06-25
Ubuntu Phablet Team 2013-06-25 Pending
Review via email: mp+171221@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
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

Ricardo Salveti (rsalveti) wrote :

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

Simon Fels (morphis) wrote :

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) wrote :

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
46. By Ricardo Salveti on 2013-06-25

rilmodem/devinfo.c: fixes from code review

Tony Espy (awe) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2013-06-17 17:40:00 +0000
3+++ debian/changelog 2013-06-25 18:32:25 +0000
4@@ -1,3 +1,9 @@
5+ofono (1.12phablet6) saucy; urgency=low
6+
7+ * [ril/rilmodem] Add real calls for revision and IMEI probe
8+
9+ -- Ricardo Salveti de Araujo <ricardo.salveti@canonical.com> Tue, 25 Jun 2013 02:46:40 -0300
10+
11 ofono (1.12phablet5) saucy; urgency=low
12
13 * plugins/ril.c: Delay modem initialization until RIL_CONNECTED
14
15=== modified file 'drivers/rilmodem/devinfo.c'
16--- drivers/rilmodem/devinfo.c 2013-06-10 16:43:50 +0000
17+++ drivers/rilmodem/devinfo.c 2013-06-25 18:32:25 +0000
18@@ -76,34 +76,95 @@
19 g_free(cbd);
20 }
21
22+static void query_revision_cb(struct ril_msg *message, gpointer user_data)
23+{
24+ struct cb_data *cbd = user_data;
25+ ofono_devinfo_query_cb_t cb = cbd->cb;
26+ struct ofono_error error;
27+ struct parcel rilp;
28+ gchar *revision;
29+
30+ if (message->error == RIL_E_SUCCESS) {
31+ decode_ril_error(&error, "OK");
32+ } else {
33+ decode_ril_error(&error, "FAIL");
34+ cb(&error, NULL, cbd->data);
35+ return;
36+ }
37+
38+ ril_util_init_parcel(message, &rilp);
39+ revision = parcel_r_string(&rilp);
40+
41+ cb(&error, revision, cbd->data);
42+
43+ g_free(revision);
44+}
45+
46 static void ril_query_revision(struct ofono_devinfo *info,
47 ofono_devinfo_query_cb_t cb,
48 void *data)
49 {
50- const char *attr = "Fake Revision";
51 struct cb_data *cbd = cb_data_new(cb, data);
52+ GRil *ril = ofono_devinfo_get_data(info);
53+ int request = RIL_REQUEST_BASEBAND_VERSION;
54+ int ret;
55+
56+ ret = g_ril_send(ril, request, NULL, 0,
57+ query_revision_cb, cbd, g_free);
58+
59+ g_ril_print_request_no_args(ril, ret, request);
60+
61+ if (ret <= 0) {
62+ g_free(cbd);
63+ CALLBACK_WITH_FAILURE(cb, NULL, data);
64+ }
65+}
66+
67+static void query_serial_cb(struct ril_msg *message, gpointer user_data)
68+{
69+ struct cb_data *cbd = user_data;
70+ ofono_devinfo_query_cb_t cb = cbd->cb;
71 struct ofono_error error;
72- decode_ril_error(&error, "OK");
73-
74- cb(&error, attr, cbd->data);
75-
76- /* Note: this will need to change if cbd passed to gril layer */
77- g_free(cbd);
78+ struct parcel rilp;
79+ gchar *imei;
80+
81+ if (message->error == RIL_E_SUCCESS) {
82+ decode_ril_error(&error, "OK");
83+ } else {
84+ decode_ril_error(&error, "FAIL");
85+ cb(&error, NULL, cbd->data);
86+ return;
87+ }
88+
89+ ril_util_init_parcel(message, &rilp);
90+
91+ imei = parcel_r_string(&rilp);
92+
93+ cb(&error, imei, cbd->data);
94+
95+ g_free(imei);
96 }
97
98 static void ril_query_serial(struct ofono_devinfo *info,
99 ofono_devinfo_query_cb_t cb,
100 void *data)
101 {
102- const char *attr = "THIS-IS-A-FAKE-SERIAL-NO";
103 struct cb_data *cbd = cb_data_new(cb, data);
104- struct ofono_error error;
105- decode_ril_error(&error, "OK");
106-
107- cb(&error, attr, cbd->data);
108-
109- /* Note: this will need to change if cbd passed to gril layer */
110- g_free(cbd);
111+ GRil *ril = ofono_devinfo_get_data(info);
112+ /* TODO: make it support both RIL_REQUEST_GET_IMEI (deprecated) and
113+ * RIL_REQUEST_DEVICE_IDENTITY depending on the rild version used */
114+ int request = RIL_REQUEST_GET_IMEI;
115+ int ret;
116+
117+ ret = g_ril_send(ril, request, NULL, 0,
118+ query_serial_cb, cbd, g_free);
119+
120+ g_ril_print_request_no_args(ril, ret, request);
121+
122+ if (ret <= 0) {
123+ g_free(cbd);
124+ CALLBACK_WITH_FAILURE(cb, NULL, data);
125+ }
126 }
127
128 static gboolean ril_delayed_register(gpointer user_data)
129@@ -126,7 +187,7 @@
130
131 ofono_devinfo_set_data(info, ril);
132
133- /*
134+ /*
135 * TODO: analyze if capability check is needed
136 * and/or timer should be adjusted.
137 *
138@@ -136,7 +197,7 @@
139 * some kind of capabilities query to the modem, and then
140 * call register in the callback; we use a timer instead.
141 */
142- g_timeout_add_seconds(1, ril_delayed_register, info);
143+ g_timeout_add_seconds(1, ril_delayed_register, info);
144
145 return 0;
146 }

Subscribers

People subscribed via source and target branches