Closed Bug 1052846 Opened 5 years ago Closed 5 years ago

Use enums for MobileNetworkType in nsIMobileConnectionProvider.idl

Categories

(Firefox OS Graveyard :: RIL, defect)

x86_64
Windows 7
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: anshulj, Assigned: jessica)

References

Details

Attachments

(3 files, 2 obsolete files)

The bug 898445 introduced the  MobileNetworkType enum in the MozMobileConnection.webidl but no corresponding constants were added in nsIMobileConnectionProvider.idl. 

Please consider adding enums for network type for getSupportedNetworkTypes API in nsIMobileConnectionProvider.idl to strictly define the IPDL interface.
Blocks: 1047203
We will need to care "MobilePreferredNetworkType."
Assignee: nobody → jjong
Attached patch Part 1: idl changes, v1. (obsolete) — Splinter Review
Hsinyi, may I have your review on the interface? Thanks.
Attachment #8522137 - Flags: review?(htsai)
Edgar, may I have your review? Thanks.
Attachment #8522647 - Flags: review?(echen)
Attached patch Part 3: gonk changes, v1. (obsolete) — Splinter Review
Edgar, may I have your review? Thanks.
Attachment #8522648 - Flags: review?(echen)
Attachment #8522137 - Flags: review?(htsai) → review+
Comment on attachment 8522647 [details] [diff] [review]
[final] Part 2: dom/ipc changes. r=echen

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

Looks good to me, thank you.
Attachment #8522647 - Flags: review?(echen) → review+
Comment on attachment 8522648 [details] [diff] [review]
Part 3: gonk changes, v1.

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

Thank you.

::: dom/mobileconnection/gonk/MobileConnectionService.js
@@ +406,5 @@
>          if (DEBUG) {
>            this._debug("Unknown network type: " + type);
>          }
> +        RIL.GECKO_SUPPORTED_NETWORK_TYPES_DEFAULT.split(",").forEach(aType => {
> +          enumNetworkTypes.push(RIL.GECKO_SUPPORTED_NETWORK_TYPES.indexOf(aType));

Please help to add comments in ril_consts.js about the items in `GECKO_SUPPORTED_NETWORK_TYPES` should be in sync with nsIMobileConnection.MOBILE_NETWORK_TYPE_*. Thank you.
Attachment #8522648 - Flags: review?(echen) → review+
(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> Comment on attachment 8522648 [details] [diff] [review]
> Part 3: gonk changes, v1.
> 
> Review of attachment 8522648 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thank you.
> 
> ::: dom/mobileconnection/gonk/MobileConnectionService.js
> @@ +406,5 @@
> >          if (DEBUG) {
> >            this._debug("Unknown network type: " + type);
> >          }
> > +        RIL.GECKO_SUPPORTED_NETWORK_TYPES_DEFAULT.split(",").forEach(aType => {
> > +          enumNetworkTypes.push(RIL.GECKO_SUPPORTED_NETWORK_TYPES.indexOf(aType));
> 
> Please help to add comments in ril_consts.js about the items in
> `GECKO_SUPPORTED_NETWORK_TYPES` should be in sync with
> nsIMobileConnection.MOBILE_NETWORK_TYPE_*. Thank you.

Will do, thanks!
aligned consts.
Attachment #8522137 - Attachment is obsolete: true
Attachment #8523765 - Flags: review+
Attachment #8522647 - Attachment description: Part 2: dom/ipc changes, v1. → [final] Part 2: dom/ipc changes. r=echen
added comments for `GECKO_SUPPORTED_NETWORK_TYPES` in ril_consts.js, see review comment 6.
Attachment #8522648 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.