Add Wimax to ONC.

Add Wimax to ONC.

This adds the object "WiMAX" to ONC, which contains the basic properties
 AutoConnect (read/write)
 EAP (write-only)
 SignalStrength (read-only).
Also all general network properties like Name or ConnectionState are supported.

BUG=414417
Committed: https://crrev.com/a901ffa9729db84945b44831ce2aacf48dea5bec
Cr-Commit-Position: refs/heads/master@{#295467}

[pneubeck (no reviews), 2014-09-17 16:28:37]

pneubeck@chromium.org changed reviewers:
+ armansito@chromium.org, stevenjb@chromium.org

[pneubeck (no reviews), 2014-09-17 16:28:37]

Since Steven has not much time this week,
Arman, please take a look.

[pneubeck (no reviews), 2014-09-17 16:29:31]

Ah, I can't test this myself with a real wimax AP as I don't have such around.
I can try with the fake dbus clients tomorrow.

[stevenjb, 2014-09-17 16:49:04]

lgtm

https://codereview.chromium.org/578823003/diff/1/chromeos/network/network_sta...
File chromeos/network/network_state_handler.cc (right):

https://codereview.chromium.org/578823003/diff/1/chromeos/network/network_sta...
chromeos/network/network_state_handler.cc:706: // Handle WiMAX in the same way
as WiFi.
nit: I think we should probably handle Wimax like Cellular, you might want to
check with ben on that.

[armansito, 2014-09-17 18:47:05]

armansito@chromium.org changed reviewers:
+ benchan@chromium.org

[armansito, 2014-09-17 18:47:06]

Added benchan@ to review. How will ONC handle WiMax credentials? I remember
there being username+password settings for WiMax (similar to the Cellular APN).
How will these be handled API wise? Will there be policy management for them?

[pneubeck (no reviews), 2014-09-17 20:10:47]

https://codereview.chromium.org/578823003/diff/1/chromeos/network/network_sta...
File chromeos/network/network_state_handler.cc (right):

https://codereview.chromium.org/578823003/diff/1/chromeos/network/network_sta...
chromeos/network/network_state_handler.cc:706: // Handle WiMAX in the same way
as WiFi.
On 2014/09/17 16:49:04, stevenjb wrote:
> nit: I think we should probably handle Wimax like Cellular, you might want to
> check with ben on that.

Sorry, this is a leftover from me debugging the API test.
I think the change in this function should, if at all, happen in a separate CL.

[pneubeck (no reviews), 2014-09-17 20:13:01]

On 2014/09/17 18:47:06, armansito wrote:
> Added benchan@ to review. How will ONC handle WiMax credentials? I remember
> there being username+password settings for WiMax (similar to the Cellular
APN).
According to wimax_config_view.cc:155 and :157, Wimax is configured with (a
subset of) the same EAP settings as Wifi and Ethernet EAP, see
onc_translation_tables.cc:26 and :32.

> How will these be handled API wise?
Note that this CL doesn't change how the Wimax service itself is handled.
(I removed the change in network_state_handler.cc)

> Will there be policy management for them?
Not to my knowledge, but Ben might have plans. EAP settings should then be
handled identically however.

[armansito, 2014-09-17 20:21:37]

lgtm ONC-wise, but Ben must approve the WiMax specifics.

[Ben Chan, 2014-09-17 22:51:38]

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
File chromeos/network/network_state.cc (right):

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
chromeos/network/network_state.cc:168: if ((type() == shill::kTypeWifi || type()
== shill::kTypeWimax) &&
TODO: cellular has signal strength as well.

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/onc/onc...
File chromeos/network/onc/onc_translation_tables.cc (right):

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/onc/onc...
chromeos/network/onc/onc_translation_tables.cc:124: {
::onc::wimax::kSignalStrength, shill::kSignalStrengthProperty},
what fields should be included here?

wimax has the following properties:
- network ID
- EAP anonymous identity
- EAP user identity
- EAP use password
- Save credentials
- Auto connect

https://codereview.chromium.org/578823003/diff/20001/components/onc/onc_const...
File components/onc/onc_constants.cc (right):

https://codereview.chromium.org/578823003/diff/20001/components/onc/onc_const...
components/onc/onc_constants.cc:83: const char kWimax[] = "WiMAX";
perhaps update the variable name as well?

[pneubeck (no reviews), 2014-09-18 08:50:11]

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
File chromeos/network/network_state.cc (right):

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
chromeos/network/network_state.cc:168: if ((type() == shill::kTypeWifi || type()
== shill::kTypeWimax) &&
On 2014/09/17 22:51:38, Ben Chan wrote:
> TODO: cellular has signal strength as well.

Done.

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/onc/onc...
File chromeos/network/onc/onc_translation_tables.cc (right):

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/onc/onc...
chromeos/network/onc/onc_translation_tables.cc:124: {
::onc::wimax::kSignalStrength, shill::kSignalStrengthProperty},
On 2014/09/17 22:51:38, Ben Chan wrote:
> what fields should be included here?
> 
> wimax has the following properties:

> - network ID
network ID was not exposed before. but I could add it, if it serves any purpose.

> - Auto connect
This was used so far, so I added it.

The EAP.Identity was shown in the old UI, which would require Shill->ONC
translation.
This CL only adds support to configure (no read support yet) all EAP settings in
the same way as for Wifi and Ethernet, which is a superset of:
> - EAP anonymous identity
> - EAP user identity
> - EAP use password
See wimax_fields in onc_signature.cc

> - Save credentials
This is part of EAP in ONC, so also supported write-only with this CL.


For example the test file wimax.onc in this CL is translated to
shill_wimax.json.

https://codereview.chromium.org/578823003/diff/20001/components/onc/onc_const...
File components/onc/onc_constants.cc (right):

https://codereview.chromium.org/578823003/diff/20001/components/onc/onc_const...
components/onc/onc_constants.cc:83: const char kWimax[] = "WiMAX";
On 2014/09/17 22:51:38, Ben Chan wrote:
> perhaps update the variable name as well?

Yes, I will do that in a follow up, globally replacing Wimax by WiMAX.
I don't want to do this in this CL as there are too many occurrences.

[pneubeck (no reviews), 2014-09-18 09:22:46]

Patchset #3 (id:40001) has been deleted

[pneubeck (no reviews), 2014-09-18 09:23:01]

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
File chromeos/network/network_state.cc (right):

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
chromeos/network/network_state.cc:168: if ((type() == shill::kTypeWifi || type()
== shill::kTypeWimax) &&
On 2014/09/18 08:50:11, pneubeck wrote:
> On 2014/09/17 22:51:38, Ben Chan wrote:
> > TODO: cellular has signal strength as well.
> 
> Done.

Actually, not done.
I checked Shill's side, and 'visible' of Cellular is always true. So cellular
would need a bit of special consideration: maybe set visible=false if
signal_strength <= 0?

But I'd like to keep that out of this CL, as it's unrelated.

[Ben Chan, 2014-09-18 14:57:06]

lgtm

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
File chromeos/network/network_state.cc (right):

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/network...
chromeos/network/network_state.cc:168: if ((type() == shill::kTypeWifi || type()
== shill::kTypeWimax) &&
On 2014/09/18 09:23:01, pneubeck wrote:
> On 2014/09/18 08:50:11, pneubeck wrote:
> > On 2014/09/17 22:51:38, Ben Chan wrote:
> > > TODO: cellular has signal strength as well.
> > 
> > Done.
> 
> Actually, not done.
> I checked Shill's side, and 'visible' of Cellular is always true. So cellular
> would need a bit of special consideration: maybe set visible=false if
> signal_strength <= 0?
> 
> But I'd like to keep that out of this CL, as it's unrelated.

I wouldn't suggest adding cellular stuff in this CL.

I'm not sure if there has to be a relation between SignalStrength > 0 and
visible=true. shill guarantees that SignalStrength is within [0, 100]. I still
don't see why we'd specially handle WiFi/WiMAX but not cellular. But I'm ok if
we leave this as a TODO to further investigate.

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/onc/onc...
File chromeos/network/onc/onc_translation_tables.cc (right):

https://codereview.chromium.org/578823003/diff/20001/chromeos/network/onc/onc...
chromeos/network/onc/onc_translation_tables.cc:124: {
::onc::wimax::kSignalStrength, shill::kSignalStrengthProperty},
On 2014/09/18 08:50:11, pneubeck wrote:
> On 2014/09/17 22:51:38, Ben Chan wrote:
> > what fields should be included here?
> > 
> > wimax has the following properties:
> 
> > - network ID
> network ID was not exposed before. but I could add it, if it serves any
purpose.
> 
> > - Auto connect
> This was used so far, so I added it.
> 
> The EAP.Identity was shown in the old UI, which would require Shill->ONC
> translation.
> This CL only adds support to configure (no read support yet) all EAP settings
in
> the same way as for Wifi and Ethernet, which is a superset of:
> > - EAP anonymous identity
> > - EAP user identity
> > - EAP use password
> See wimax_fields in onc_signature.cc
> 
> > - Save credentials
> This is part of EAP in ONC, so also supported write-only with this CL.
> 
> 
> For example the test file wimax.onc in this CL is translated to
> shill_wimax.json.

let's leave "network ID" out for now as it's mostly informational.

[pneubeck (no reviews), 2014-09-18 15:06:48]

The CQ bit was checked by pneubeck@chromium.org

[commit-bot: I haz the power, 2014-09-18 15:07:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/578823003/20001

[commit-bot: I haz the power, 2014-09-18 15:31:50]

Committed patchset #2 (id:20001) as 5e360f831c11a6e93335126591f8229fb91ef742

[commit-bot: I haz the power, 2014-09-18 15:32:25]

Patchset 2 (id:??) landed as
https://crrev.com/a901ffa9729db84945b44831ce2aacf48dea5bec
Cr-Commit-Position: refs/heads/master@{#295467}