B2G Emulator: add test cases for REQUEST_SCREEN_STATE

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vicamo, Assigned: tzimmermann)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Bug 899885 left a typo on sending "setScreenState" worker message and was fixed in bug 905090.  We currently have no marionette test cases for this request because neither hardware/ril nor external/qemu is capable of handling it.  A xpcshel test case here won't really help because we may still have similar mistakes in other places.

The purpose of REQUEST_SCREEN_STATE can be found in ril.h[1]:

  When the screen is off, the RIL should notify the baseband to suppress certain
  notifications (eg, signal strength and changes in LAC/CID or
  BID/SID/NID/latitude/longitude) in an effort to conserve power. These
  notifications should resume when the screen is on.

Related AT commands could be:

  1. 3GPP 27.007 subclause 7.2 "Network registration +CREG"
  2. 3GPP 27.007 subclause 8.10 "Mobile termination event reporting +CMER"
  3. 3GPP 27.007 subclause 8.41 "Time zone reporting +CTZR"
  4. 3GPP 27.007 subclause 8.50 "Mobile originated location request +CMOLR"
  5. 3GPP 27.007 subclause 8.56 "Positioning reporting +CPOSR"
  6. 3GPP 27.007 subclause 8.59 "Battery capacity +CBCAP"
  7. 3GPP 27.007 subclause 8.60 "Battery connection status +CBCON"
  8. 3GPP 27.007 subclause 8.61 "Battery charger status +CBCHG"
  ...

We're most likely to use "+CREG=1" for screen off and "+CREG=2" for screen on.

[1]: https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L2330
Created attachment 793506 [details]
Github pull request

I created a patch for the reference RIL to send AT+CREG when it receives REQUEST_SCREEN_STATE. I'm not sure how to test this, though. Is there a way for retrieving the network state?
Attachment #793506 - Flags: feedback?(vyang)
Assignee: nobody → tzimmermann
Status: NEW → ASSIGNED
(Reporter)

Comment 2

5 years ago
Comment on attachment 793506 [details]
Github pull request

Then we need a patch to qemu[1], revealing current |modem->voice_mode| to determine whether we're in A_REGISTRATION_UNSOL_ENABLED or A_REGISTRATION_UNSOL_ENABLED_FULL state.  I suggest a "gsm report [type1[,type2[,..]]] [on,off]" console command for this.

  gsm report          - print the statuses of all supported types.
  gsm report creg     - print creg verbose reporting status only
  gsm report creg on  - turn on creg verbose reporting
  gsm report creg off - turn off creg verbose reporting

Also need to ensure that ril_worker accepts CREG without cell info[2]. But since hardware/ril[3] returns (-1, -1) in this case, I think it will be fine.

[1]: https://github.com/mozilla-b2g/platform_external_qemu/blob/master/telephony/android_modem.c#L1640
[2]: http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#3730
[3]: https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/reference-ril/reference-ril.c#L1234
Attachment #793506 - Flags: feedback?(vyang) → feedback+
(Reporter)

Comment 3

5 years ago
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #2)
>   gsm report creg on  - turn on creg verbose reporting
>   gsm report creg off - turn off creg verbose reporting

The ability to enable/disable reporting is not necessary for this bug.
(Reporter)

Updated

5 years ago
Component: General → Emulator
Created attachment 814406 [details]
Github pull request for QEmu
Attachment #814406 - Flags: review?(vyang)
Created attachment 814407 [details]
Github pull request for RIL
Attachment #793506 - Attachment is obsolete: true
Attachment #814407 - Flags: review?(vyang)
Created attachment 814409 [details] [diff] [review]
[01] Bug 905107: Fix options in RIL.setScreenState

This fixes a bug in the RIL worker.
Attachment #814409 - Flags: review?(vyang)
Created attachment 814410 [details] [diff] [review]
[02] Bug 905107: Added test case for screen-state handling in RIL
Attachment #814410 - Flags: review?(vyang)
Sorry that it took me so long to update this bug. I was distracted by NFC.
(Reporter)

Comment 9

5 years ago
Comment on attachment 814409 [details] [diff] [review]
[01] Bug 905107: Fix options in RIL.setScreenState

Review of attachment 814409 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thank you :)
Attachment #814409 - Flags: review?(vyang) → review+
(Reporter)

Updated

5 years ago
Attachment #814410 - Flags: review?(vyang) → review+
(Reporter)

Comment 10

5 years ago
Comment on attachment 814406 [details]
Github pull request for QEmu

`modem->voice_mode` is a ARegistrationUnsolMode-typed variable, not ARegistrationState.  Please move the definition of ARegistrationUnsolMode from telephony/android_modem.c to telephony/android_modem.h and return correct type here.  Besides, function name `amodem_get_voice_unsol_mode` is more preferable for me because there is also a data member called `data_mode` in AModemRec structure.
(Reporter)

Comment 11

5 years ago
Comment on attachment 814407 [details]
Github pull request for RIL

`at_response_free(p_response);` missed.
Attachment #814406 - Flags: review?(vyang)
Comment on attachment 814406 [details]
Github pull request for QEmu

I updated the patch according to your review.
Attachment #814406 - Flags: review?(vyang)
Created attachment 814841 [details]
Github pull request for RIL

A new pull request for the RIL with the changes you requested.
Attachment #814407 - Attachment is obsolete: true
Attachment #814407 - Flags: review?(vyang)
Attachment #814841 - Flags: review?(vyang)
(Reporter)

Updated

5 years ago
Depends on: 879695
(Reporter)

Updated

5 years ago
Attachment #814406 - Flags: review?(vyang) → review+
(Reporter)

Updated

5 years ago
Attachment #814841 - Flags: review?(vyang) → review+
https://hg.mozilla.org/mozilla-central/rev/84fd14b8f86d
https://hg.mozilla.org/mozilla-central/rev/321b38da7bc4
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.