1058308 - Use enums for data settings types types in webidl

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: RIL, defect)

Description

[pgravel]

Just like it's done in bug 937485 I would like to request to use enums for data call settings coming from "ril.data.apnSettings"

These include:
- APN type
- Authentication type (None, Chap, PAP, PapOrChap)
- Protocol/Roaming Protocol (IPv4, IPv6, IPv4v6)

Comment 1

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to pgravel from comment #0)
> Just like it's done in bug 937485 I would like to request to use enums for
> data call settings coming from "ril.data.apnSettings"
> 
> These include:
> - APN type
> - Authentication type (None, Chap, PAP, PapOrChap)
> - Protocol/Roaming Protocol (IPv4, IPv6, IPv4v6)

Given the fact that we count on settings API for data connection, I don't see how we could define the settings "value" in an interface. :( I mean, we can of course write the possible values down in a certain API but since we don't really use that API for data connection, it can't really help and doesn't make much sense IMO. 

Any other idea?

Comment 2

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> (In reply to pgravel from comment #0)
> > Just like it's done in bug 937485 I would like to request to use enums for
> > data call settings coming from "ril.data.apnSettings"
> > 
> > These include:
> > - APN type
> > - Authentication type (None, Chap, PAP, PapOrChap)
> > - Protocol/Roaming Protocol (IPv4, IPv6, IPv4v6)
> 
> Given the fact that we count on settings API for data connection, I don't
> see how we could define the settings "value" in an interface. :( I mean, we
> can of course write the possible values down in a certain API but since we
> don't really use that API for data connection, it can't really help and
> doesn't make much sense IMO. 
> 
> Any other idea?

Or maybe... an acceptable place for the definitions could be nsIRadioInterface.idl, along with SetupDataCallByType, DeactivateDataCallByType, GetDataCallByType?

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> > (In reply to pgravel from comment #0)
> > > Just like it's done in bug 937485 I would like to request to use enums for
> > > data call settings coming from "ril.data.apnSettings"
> > > 
> > > These include:
> > > - APN type
> > > - Authentication type (None, Chap, PAP, PapOrChap)
> > > - Protocol/Roaming Protocol (IPv4, IPv6, IPv4v6)
> > 
> > Given the fact that we count on settings API for data connection, I don't
> > see how we could define the settings "value" in an interface. :( I mean, we
> > can of course write the possible values down in a certain API but since we
> > don't really use that API for data connection, it can't really help and
> > doesn't make much sense IMO. 
> > 

The current implementation is, DataConnectionManager listens to settings change to get apn settings from gaia. It obviously has nothing to do with any ril interface. After DataConnectionManager receives the settings, the module does what it needs/wants. It's all within the module implementation. And still, has nothing to do with any ril interface.

We could of course anyway write down those values on a certain .idl but given that .idl isn't even on a code path, it doesn't help and makes no sense for now.

We have a refactoring plan for v2.3, NetworkManager enhancement - bug 904514.
In that plan, we are going to move nsINetworkInterface out of RadioInterfaceLayer.js. Then CAF doesn't need to implement nsINetworkInterface on CAF side. At that moment, we will add another API to nsIRadioInterface.idl for NetworkInterface instances to call so that we could have a reasonable constant definition exposed to .idl. But for now, I don't think it's a good idea. After my second thought, I'd like to close this as WONTFIX based on my explanation comment 1 and above. Objection?


> > Any other idea?
> 
> Or maybe... an acceptable place for the definitions could be
> nsIRadioInterface.idl, along with SetupDataCallByType,
> DeactivateDataCallByType, GetDataCallByType?

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #3)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> > > (In reply to pgravel from comment #0)
> > > > Just like it's done in bug 937485 I would like to request to use enums for
> > > > data call settings coming from "ril.data.apnSettings"
> > > > 
> > > > These include:
> > > > - APN type
> > > > - Authentication type (None, Chap, PAP, PapOrChap)
> > > > - Protocol/Roaming Protocol (IPv4, IPv6, IPv4v6)
> > > 
> > > Given the fact that we count on settings API for data connection, I don't
> > > see how we could define the settings "value" in an interface. :( I mean, we
> > > can of course write the possible values down in a certain API but since we
> > > don't really use that API for data connection, it can't really help and
> > > doesn't make much sense IMO. 
> > > 
> 
> The current implementation is, DataConnectionManager listens to settings
> change to get apn settings from gaia. It obviously has nothing to do with
> any ril interface. After DataConnectionManager receives the settings, the
> module does what it needs/wants. It's all within the module implementation.
> And still, has nothing to do with any ril interface.
> 
> We could of course anyway write down those values on a certain .idl but
> given that .idl isn't even on a code path, it doesn't help and makes no
> sense for now.
> 
> We have a refactoring plan for v2.3, NetworkManager enhancement - bug 904514.
> In that plan, we are going to move nsINetworkInterface out of
> RadioInterfaceLayer.js. Then CAF doesn't need to implement
> nsINetworkInterface on CAF side. At that moment, we will add another API to
> nsIRadioInterface.idl for NetworkInterface instances to call so that we
> could have a reasonable constant definition exposed to .idl. 

Let me clarify a bit. Above is a very rough idea not firmed yet, but our goal is to keep the network manager structure and interface clearer.

> But for now, I
> don't think it's a good idea. After my second thought, I'd like to close
> this as WONTFIX based on my explanation comment 1 and above. Objection?
> 
> 
> > > Any other idea?
> > 
> > Or maybe... an acceptable place for the definitions could be
> > nsIRadioInterface.idl, along with SetupDataCallByType,
> > DeactivateDataCallByType, GetDataCallByType?

Comment 5

[Edgar Chen [:edgar]]

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #3)
> (In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> > (In reply to Hsin-Yi Tsai [:hsinyi] from comment #1)
> > > (In reply to pgravel from comment #0)
> > > > Just like it's done in bug 937485 I would like to request to use enums for
> > > > data call settings coming from "ril.data.apnSettings"
> > > > 
> > > > These include:
> > > > - APN type
> > > > - Authentication type (None, Chap, PAP, PapOrChap)
> > > > - Protocol/Roaming Protocol (IPv4, IPv6, IPv4v6)
> > > 
> > > Given the fact that we count on settings API for data connection, I don't
> > > see how we could define the settings "value" in an interface. :( I mean, we
> > > can of course write the possible values down in a certain API but since we
> > > don't really use that API for data connection, it can't really help and
> > > doesn't make much sense IMO. 
> > > 
> 
> The current implementation is, DataConnectionManager listens to settings
> change to get apn settings from gaia. It obviously has nothing to do with
> any ril interface. After DataConnectionManager receives the settings, the
> module does what it needs/wants. It's all within the module implementation.
> And still, has nothing to do with any ril interface.
> 
> We could of course anyway write down those values on a certain .idl but
> given that .idl isn't even on a code path, it doesn't help and makes no
> sense for now.
> 
> We have a refactoring plan for v2.3, NetworkManager enhancement - bug 904514.
> In that plan, we are going to move nsINetworkInterface out of
> RadioInterfaceLayer.js. Then CAF doesn't need to implement
> nsINetworkInterface on CAF side. At that moment, we will add another API to
> nsIRadioInterface.idl for NetworkInterface instances to call so that we
> could have a reasonable constant definition exposed to .idl. But for now, I
> don't think it's a good idea. After my second thought, I'd like to close
> this as WONTFIX based on my explanation comment 1 and above. Objection?

Agree with Hsinyi.
We get and use the apn configurations from settings api directly. Having these values defined in ril .idl doesn't help anything and makes no sense for now.

> 
> 
> > > Any other idea?
> > 
> > Or maybe... an acceptable place for the definitions could be
> > nsIRadioInterface.idl, along with SetupDataCallByType,
> > DeactivateDataCallByType, GetDataCallByType?

Comment 6

[pgravel]

(In reply to Hsin-Yi Tsai (OOO Sep. 8 ~ Sep. 10) [:hsinyi] from comment #3)
> The current implementation is, DataConnectionManager listens to settings
> change to get apn settings from gaia. It obviously has nothing to do with
> any ril interface. After DataConnectionManager receives the settings, the
> module does what it needs/wants. It's all within the module implementation.
> And still, has nothing to do with any ril interface.
> 
> We could of course anyway write down those values on a certain .idl but
> given that .idl isn't even on a code path, it doesn't help and makes no
> sense for now.
> 
> We have a refactoring plan for v2.3, NetworkManager enhancement - bug 904514.
> In that plan, we are going to move nsINetworkInterface out of
> RadioInterfaceLayer.js. Then CAF doesn't need to implement
> nsINetworkInterface on CAF side. At that moment, we will add another API to
> nsIRadioInterface.idl for NetworkInterface instances to call so that we
> could have a reasonable constant definition exposed to .idl. But for now, I
> don't think it's a good idea. After my second thought, I'd like to close
> this as WONTFIX based on my explanation comment 1 and above. Objection?
> 

No objections.

It would still be nice if both gaia and gecko could pull from a fixed list of definitions (e.g: "2" instead of "Pap") rather than using strings that much match on both sides (so that there is no risk of gaia suddenly starting to send "Pap" while gecko still uses "PAP")... but as was mentioned, the settings API doesn't really make this sort of handshaking really viable.

The 2.3 changes look very interesting, looking forward to it :)

Comment 7

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to pgravel from comment #6)
> (In reply to Hsin-Yi Tsai (OOO Sep. 8 ~ Sep. 10) [:hsinyi] from comment #3)
> > The current implementation is, DataConnectionManager listens to settings
> > change to get apn settings from gaia. It obviously has nothing to do with
> > any ril interface. After DataConnectionManager receives the settings, the
> > module does what it needs/wants. It's all within the module implementation.
> > And still, has nothing to do with any ril interface.
> > 
> > We could of course anyway write down those values on a certain .idl but
> > given that .idl isn't even on a code path, it doesn't help and makes no
> > sense for now.
> > 
> > We have a refactoring plan for v2.3, NetworkManager enhancement - bug 904514.
> > In that plan, we are going to move nsINetworkInterface out of
> > RadioInterfaceLayer.js. Then CAF doesn't need to implement
> > nsINetworkInterface on CAF side. At that moment, we will add another API to
> > nsIRadioInterface.idl for NetworkInterface instances to call so that we
> > could have a reasonable constant definition exposed to .idl. But for now, I
> > don't think it's a good idea. After my second thought, I'd like to close
> > this as WONTFIX based on my explanation comment 1 and above. Objection?
> > 
> 
> No objections.
> 
> It would still be nice if both gaia and gecko could pull from a fixed list
> of definitions (e.g: "2" instead of "Pap") rather than using strings that
> much match on both sides (so that there is no risk of gaia suddenly starting
> to send "Pap" while gecko still uses "PAP")... but as was mentioned, the
> settings API doesn't really make this sort of handshaking really viable.

Totally agree :(

I'll close this as WONTFIX based on above.

> 
> The 2.3 changes look very interesting, looking forward to it :)

Glad to hear that. Welcome to join our discussion there.