bluetooth: web: Chooser RSSI indicator now based on percentile buckets

content/browser/bluetooth/bluetooth_device_chooser_controller.cc

 // Copyright 2016 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 "content/browser/bluetooth/bluetooth_device_chooser_controller.h"
 
 #include <set>
 #include <string>
 #include <unordered_set>
 
@@ skipping 12 matching lines @@
 #include "content/public/browser/web_contents_delegate.h"
 #include "device/bluetooth/bluetooth_adapter.h"
 #include "device/bluetooth/bluetooth_common.h"
 #include "device/bluetooth/bluetooth_discovery_session.h"
 
 using device::BluetoothUUID;
 using UUIDSet = device::BluetoothDevice::UUIDSet;
 
 namespace {
 
-// Anything worse than or equal to this will show 0 bars.
-const int kMinRSSI = -100;
-// Anything better than or equal to this will show the maximum bars.
-const int kMaxRSSI = -55;
-// Number of RSSI levels used in the signal strength image.
-const int kNumSignalStrengthLevels = 5;
-
-const content::UMARSSISignalStrengthLevel kRSSISignalStrengthEnumTable[] = {
-    content::UMARSSISignalStrengthLevel::LEVEL_0,
-    content::UMARSSISignalStrengthLevel::LEVEL_1,
-    content::UMARSSISignalStrengthLevel::LEVEL_2,
-    content::UMARSSISignalStrengthLevel::LEVEL_3,
-    content::UMARSSISignalStrengthLevel::LEVEL_4};
+// Signal Strength Display Notes:
+//
+// RSSI values are displayed by the chooser to empower a user to differentiate
+// between multiple devices with the same name, comparing devices with different
+// names is a secondary goal. It is important that a user be able to move closer

[juncai, 2016/12/09 23:31:40]

s/be/is

[scheib, 2016/12/10 00:04:04]

Done.

+// and farther away from a device and have it transition between two different
+// signal strength levels, thus we want to spread RSSI values out accross
+// displayed levels.
+//
+// RSSI values from UMA in RecordRSSISignalStrength are charted here:
+// https://goo.gl/photos/pCoAkF7mPyza9B1k7 (2016-12-08)
+// with a copy-paste of table data at every 5dBm:
+//  dBm   CDF* Histogram Bucket Quantity (hand drawn estimate)
+// -100 00.0%  -
+//  -95 00.4%  --
+//  -90 01.9%  ---
+//  -85 05.1%  ---
+//  -80 09.2%  ----
+//  -75 14.9%  -----
+//  -70 22.0%  ------
+//  -65 32.4%  --------
+//  -60 47.9%  ---------
+//  -55 60.4%  --------
+//  -50 72.8%  ---------
+//  -45 85.5%  -------
+//  -40 94.5%  -----
+//  -35 97.4%  ---
+//  -30 99.0%  --
+//  -25 99.7%  -
+//
+// CDF: Cumulative Distribution Function:
+// https://en.wikipedia.org/wiki/Cumulative_distribution_function
+//
+// Conversion to signal strengths is done by selecting 4 threshold points
+// equally spaced through the CDF.
+const int k20thPercentileRSSI = -71;
+const int k40thPercentileRSSI = -63;
+const int k60thPercentileRSSI = -55;
+const int k80thPercentileRSSI = -47;
 
 }  // namespace
 
 namespace content {
 
 bool BluetoothDeviceChooserController::use_test_scan_duration_ = false;
 
 namespace {
 constexpr size_t kMaxLengthForDeviceName =
     29;  // max length of device name in filter.
@@ skipping 373 matching lines @@
 
   if (!powered) {
     discovery_session_timer_.Stop();
   }
 }
 
 int BluetoothDeviceChooserController::CalculateSignalStrengthLevel(
     int8_t rssi) {
   RecordRSSISignalStrength(rssi);
 
-  if (rssi <= kMinRSSI) {
-    RecordRSSISignalStrengthLevel(
-        UMARSSISignalStrengthLevel::LESS_THAN_OR_EQUAL_TO_MIN_RSSI);

[juncai, 2016/12/09 23:31:40]

"LESS_THAN_OR_EQUAL_TO_MIN_RSSI" is not used, maybe can remove it from:
//content/browser/bluetooth/bluetooth_metrics.h

[scheib, 2016/12/10 00:04:04]

We can't remove values for histograms. We could remove the name from C++, but I
don't think there's much need or value to do so. We would have to keep the other
enum values constant including COUNT, which just seems to be awkward. I think we
should just leave the enums unused.

+  if (rssi < k20thPercentileRSSI) {
+    RecordRSSISignalStrengthLevel(content::UMARSSISignalStrengthLevel::LEVEL_0);
     return 0;
+  } else if (rssi < k40thPercentileRSSI) {
+    RecordRSSISignalStrengthLevel(content::UMARSSISignalStrengthLevel::LEVEL_1);
+    return 1;
+  } else if (rssi < k60thPercentileRSSI) {
+    RecordRSSISignalStrengthLevel(content::UMARSSISignalStrengthLevel::LEVEL_2);
+    return 2;
+  } else if (rssi < k80thPercentileRSSI) {
+    RecordRSSISignalStrengthLevel(content::UMARSSISignalStrengthLevel::LEVEL_3);
+    return 3;
+  } else {
+    RecordRSSISignalStrengthLevel(content::UMARSSISignalStrengthLevel::LEVEL_4);
+    return 4;
   }
-
-  if (rssi >= kMaxRSSI) {
-    RecordRSSISignalStrengthLevel(
-        UMARSSISignalStrengthLevel::GREATER_THAN_OR_EQUAL_TO_MAX_RSSI);

[juncai, 2016/12/09 23:31:40]

"GREATER_THAN_OR_EQUAL_TO_MAX_RSSI" is not used, maybe can remove it from:
//content/browser/bluetooth/bluetooth_metrics.h

[scheib, 2016/12/10 00:04:04]

ditto

-    return kNumSignalStrengthLevels - 1;
-  }
-
-  double input_range = kMaxRSSI - kMinRSSI;
-  double output_range = kNumSignalStrengthLevels - 1;
-  int level = static_cast<int>((rssi - kMinRSSI) * output_range / input_range);
-  DCHECK(kNumSignalStrengthLevels == arraysize(kRSSISignalStrengthEnumTable));
-  RecordRSSISignalStrengthLevel(kRSSISignalStrengthEnumTable[level]);
-  return level;
 }
 
 void BluetoothDeviceChooserController::SetTestScanDurationForTesting() {
   BluetoothDeviceChooserController::use_test_scan_duration_ = true;
 }
 
 void BluetoothDeviceChooserController::PopulateConnectedDevices() {
   for (const device::BluetoothDevice* device : adapter_->GetDevices()) {
     if (device->IsGattConnected()) {
       AddFilteredDevice(*device);
@@ skipping 118 matching lines @@
 
 void BluetoothDeviceChooserController::PostErrorCallback(
     blink::mojom::WebBluetoothResult error) {
   if (!base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(error_callback_, error))) {
     LOG(WARNING) << "No TaskRunner.";
   }
 }
 
 }  // namespace content