phablet-extras

Merge lp://qastaging/~phablet-team/phablet-extras/ofono-sim-pin-support into lp://qastaging/phablet-extras/ofono

ofono-sim-pin-support
Merge into ofono

Proposed by Tony Espy on 2013-06-29

Status:	Merged
Approved by:	Ricardo Salveti on 2013-07-10
Approved revision:	53
Merged at revision:	46
Proposed branch:	lp://qastaging/~phablet-team/phablet-extras/ofono-sim-pin-support
Merge into:	lp://qastaging/phablet-extras/ofono
Diff against target:	860 lines (+511/-107) 5 files modified debian/changelog (+8/-0) drivers/rilmodem/rilutil.c (+81/-61) drivers/rilmodem/rilutil.h (+42/-11) drivers/rilmodem/sim.c (+368/-30) plugins/ril.c (+12/-5)
To merge this branch:	bzr merge lp://qastaging/~phablet-team/phablet-extras/ofono-sim-pin-support
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Ricardo Salveti (community)		2013-06-29	Approve on 2013-07-10
PS Jenkins bot	continuous-integration		Approve on 2013-07-09
Review via email: mp+172204@code.qastaging.launchpad.net

Commit message

[rilmodem] Add SIM PIN/PUK support.

Description of the change

This merge adds SIM PIN/PUK support. It's based on code from Jolla that's been re-factored due to the fact that they're working on a older version of our code, and to remove some of the changes they made which were questionable.

https://github.com/nemomobile-packages/ofono/tree/master/ofono

Testing was performed on an un-flipped maguro, using the 20130629 build.

I tested an unlocked SIM, and only verified that I was able to make voice calls.

I tested a locked SIM. I manually started ofono. Before unlocked, the ofono DBus interfaces are restricted to VoiceCallManager and SimManager. I manually entered a pin via dbus-send:

I resorted to dbus-send, as I couldn't get the python enter-pin test script to run, but this was because it took me awhile to figure out that the 'pin_type' argument was the same as reported by the property ( eg. "pin" ). Here's the command I used:

# dbus-send --print-reply --system --dest=org.ofono /ril_0 org.ofono.SimManager.EnterPin string:"pin" string:"2112"

After entering the PIN, I verified that the rest of the modem initialization occurs, and I was able to make a phone call.

NOTE -- I did some minor cleanup of the branch before submitting, so this code definitely needs to be fully re-tested on maguro. It also needs to be tested on mako.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-06-29:

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

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

review: Approve (continuous-integration)

Revision history for this message

Ricardo Salveti (rsalveti) wrote on 2013-07-08:

435 + if (ril_util_parse_sim_status(sd->ril, message, &status, apps)) {
436 +
437 + if (status.num_apps) {

As you only cares about status.num_apps, you could check both at the same line (with &&), and that will save you one indentation level.

review: Needs Fixing

Revision history for this message

Ricardo Salveti (rsalveti) wrote on 2013-07-08:

521 +static void ril_query_passwd_state(struct ofono_sim *sim,
522 + ofono_sim_passwd_cb_t cb, void *data)
523 +{
524 + struct sim_data *sd = ofono_sim_get_data(sim);
525 +
526 + DBG("passwd_state %u", sd->passwd_state);
527 + CALLBACK_WITH_SUCCESS(cb, sd->passwd_state, data);
528 +}

You also might want to handle the case which the passwd_state is invalid, check how this is done with isi and qmimodem.

review: Needs Fixing

lp://qastaging/~phablet-team/phablet-extras/ofono-sim-pin-support updated on 2013-07-08

48. By Tony Espy on 2013-07-08: [rilmodem] Minor re-factoring of PIN code.
49. By Tony Espy on 2013-07-08: Releasing 1.12phablet8

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-07-08:

FAILED: Continuous integration, rev:49
http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-ci/50/
Executed test runs:
FAILURE: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/25/console

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~phablet-team/phablet-extras/ofono-sim-pin-support updated on 2013-07-08

50. By Tony Espy on 2013-07-08: [rilmodem] Fix compile error.

Revision history for this message

Tony Espy (awe) wrote on 2013-07-08:

I re-factored sim_status_cb() and fixed the line length problem which still existed even after combining the "if" statements per the initial review comment.

I also fixed ril_query_passwd_state() to use CALLBACK_WITH_FAILURE if the passwd_state is invalid.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-07-08:

FAILED: Continuous integration, rev:50
http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-ci/52/
Executed test runs:
FAILURE: http://jenkins.qa.ubuntu.com/job/phablet-extras-ofono-saucy-armhf-ci/27/console

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

review: Needs Fixing (continuous-integration)

lp://qastaging/~phablet-team/phablet-extras/ofono-sim-pin-support updated on 2013-07-08

51. By Tony Espy on 2013-07-08: [ril] Fix compile error.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-07-08:

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

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

review: Approve (continuous-integration)

Revision history for this message

Ricardo Salveti (rsalveti) wrote on 2013-07-09:

750 + g_ril_append_print_buf(sd->ril, "(puk=%s,pin=%s,aid=%s)",
751 + old, new,
752 + sd->aid_str);

The debug is wrong here, it should be old=%s,new=%d instead.

Also, there's a memory corruption when copying the aid_str string to sd, as you basically just assign the pointer (line 435), instead of copying the value (the corruption happens once ril_util_free_sim_apps is called). Same happens with app_str.

With such fix in place I'm able to lock/unlock my sim card.

review: Needs Fixing

lp://qastaging/~phablet-team/phablet-extras/ofono-sim-pin-support updated on 2013-07-09