Map signal_strength from percentage to dBm per spec

chrome/browser/chromeos/policy/device_status_collector.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/policy/device_status_collector.h"
 
 #include <stdint.h>
 #include <cstdio>
 #include <limits>
 #include <sstream>
@@ skipping 618 matching lines @@
     if (position_.altitude_accuracy >= 0.)
       location->set_altitude_accuracy(position_.altitude_accuracy);
     if (position_.heading >= 0. && position_.heading <= 360)
       location->set_heading(position_.heading);
     if (position_.speed >= 0.)
       location->set_speed(position_.speed);
     location->set_error_code(em::DeviceLocation::ERROR_CODE_NONE);
   }
 }
 
+int DeviceStatusCollector::ConvertSignalStrength(int signal_strength) {
+  // Shill attempts to convert attempts to convert from its internal dBm to a

[Thiemo Nagel, 2015/04/16 15:16:33]

Please remove duplicate "attempts to convert".

[Andrew T Wilson (Slow), 2015/04/17 14:41:02]

Done.

+  // percentage range (from 1-100) by subtracting 120 from the raw dBm value,

[Thiemo Nagel, 2015/04/16 15:16:33]

s/1-100/0-100/
s/substracting 120 from/adding 120 to/

[Andrew T Wilson (Slow), 2015/04/17 14:41:02]

Done.

+  // and then clamping the result to the range 1-100.

[Thiemo Nagel, 2015/04/16 15:16:33]

s/1-100/0-100/

I'd suggest to include some kind of cross-reference to the shill function, eg.
its name "shill::WiFiService::SignalToStrength()".

[Andrew T Wilson (Slow), 2015/04/17 14:41:02]

Done.

+  //
+  // To convert back to dBm, we subtract 120 from the percentage value to yield
+  // a clamped dBm value in the range of -119 to -20dBm.
+  //
+  // TODO(atwilson): Tunnel the raw dBm signal strength from Shill instead of
+  // doing the conversion here so we can report non-clamped values
+  // (crbug.com/463334).
+  DCHECK_GT(signal_strength, 0);
+  DCHECK_LE(signal_strength, 100);
+  return signal_strength - 120;
+}
+
 void DeviceStatusCollector::GetNetworkInterfaces(
     em::DeviceStatusReportRequest* request) {
   // Maps shill device type strings to proto enum constants.
   static const struct {
     const char* type_string;
     em::NetworkInterface::NetworkDeviceType type_constant;
   } kDeviceTypeMap[] = {
     { shill::kTypeEthernet,  em::NetworkInterface::TYPE_ETHERNET,  },
     { shill::kTypeWifi,      em::NetworkInterface::TYPE_WIFI,      },
     { shill::kTypeWimax,     em::NetworkInterface::TYPE_WIMAX,     },
@@ skipping 52 matching lines @@
   }
 
   // Don't write any network state if we aren't in a kiosk session.
   if (!GetAutoLaunchedKioskSessionInfo())
     return;
 
   // Walk the various networks and store their state in the status report.
   chromeos::NetworkStateHandler::NetworkStateList state_list;
   network_state_handler->GetNetworkListByType(
       chromeos::NetworkTypePattern::Default(),
       true,  // configured_only

[Thiemo Nagel, 2015/04/16 15:16:33]

At that occasion: Please insert a blank in front of "// configured_only".

[Andrew T Wilson (Slow), 2015/04/17 14:41:01]

Done.

[Thiemo Nagel, 2015/04/17 15:04:49]

You fixed it in the other file, but not here.  :)

[Andrew T Wilson (Slow), 2015/04/17 15:37:51]

Gah, fixed here too.

       false,  // visible_only,
       0,      // no limit to number of results
       &state_list);
 
   for (const chromeos::NetworkState* state: state_list) {
     // Determine the connection state and signal strength for |state|.
     em::NetworkState::ConnectionState connection_state_enum =
         em::NetworkState::UNKNOWN;
     const std::string connection_state_string(state->connection_state());
     for (size_t i = 0; i < arraysize(kConnectionStateMap); ++i) {
       if (connection_state_string == kConnectionStateMap[i].state_string) {
         connection_state_enum = kConnectionStateMap[i].state_constant;
         break;
       }
     }
 
     // Copy fields from NetworkState into the status report.
     em::NetworkState* proto_state = request->add_network_state();
     proto_state->set_connection_state(connection_state_enum);
-    proto_state->set_signal_strength(state->signal_strength());
+    // If shill has provided a signal strength, convert it to dBm and store it

[Thiemo Nagel, 2015/04/16 15:16:33]

As far as I can see, this specific conversion should be limited to wifi
networks.  Also, it seems that zero signal strength implies "no measurement"
only for wifi (and wimax).

[Andrew T Wilson (Slow), 2015/04/17 14:41:02]

Only report this now for wifi networks.

+    // in the status report.
+    if (state->signal_strength()) {
+      proto_state->set_signal_strength(
+          ConvertSignalStrength(state->signal_strength()));
+    }
     if (!state->device_path().empty())
       proto_state->set_device_path(state->device_path());
 
     if (!state->ip_address().empty())
       proto_state->set_ip_address(state->ip_address());
 
     if (!state->gateway().empty())
       proto_state->set_gateway(state->gateway());
   }
 }
@@ skipping 185 matching lines @@
   ScheduleGeolocationUpdateRequest();
 }
 
 void DeviceStatusCollector::ReceiveVolumeInfo(
     const std::vector<em::VolumeInfo>& info) {
   if (report_hardware_status_)
     volume_info_ = info;
 }
 
 }  // namespace policy