Use enums for MobileNetworkType in nsIMobileConnectionProvider.idl

RESOLVED FIXED

Status

Firefox OS
RIL
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Anshul, Assigned: jessica)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
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.

Updated

4 years ago
Blocks: 1047203
We will need to care "MobilePreferredNetworkType."
(Assignee)

Updated

4 years ago
Assignee: nobody → jjong
(Assignee)

Comment 2

4 years ago
Created attachment 8522137 [details] [diff] [review]
Part 1: idl changes, v1.

Hsinyi, may I have your review on the interface? Thanks.
Attachment #8522137 - Flags: review?(htsai)
(Assignee)

Comment 3

4 years ago
Created attachment 8522647 [details] [diff] [review]
[final] Part 2: dom/ipc changes. r=echen

Edgar, may I have your review? Thanks.
Attachment #8522647 - Flags: review?(echen)
(Assignee)

Comment 4

4 years ago
Created attachment 8522648 [details] [diff] [review]
Part 3: gonk changes, v1.

Edgar, may I have your review? Thanks.
Attachment #8522648 - Flags: review?(echen)

Updated

4 years ago
Attachment #8522137 - Flags: review?(htsai) → review+

Comment 5

4 years ago
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 6

4 years ago
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+
(Assignee)

Comment 7

4 years ago
(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!
(Assignee)

Comment 8

4 years ago
Created attachment 8523765 [details] [diff] [review]
[final] Part 1: idl changes. r=hsinyi

aligned consts.
Attachment #8522137 - Attachment is obsolete: true
Attachment #8523765 - Flags: review+
(Assignee)

Updated

4 years ago
Attachment #8522647 - Attachment description: Part 2: dom/ipc changes, v1. → [final] Part 2: dom/ipc changes. r=echen
(Assignee)

Comment 9

4 years ago
Created attachment 8523768 [details] [diff] [review]
[final] Part 3: gonk 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.