bluetooth: Return int8_t and use -128 for unknown tx power.

components/proximity_auth/proximity_monitor_impl_unittest.cc

 // Copyright 2015 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 "components/proximity_auth/proximity_monitor_impl.h"
 
 #include <memory>
 #include <utility>
 
 #include "base/macros.h"
@@ skipping 218 matching lines @@
   EXPECT_TRUE(monitor_.IsUnlockAllowed());
   EXPECT_TRUE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        ProximityState_CheckRssi_UnknownRssi) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_RSSI);
   monitor_.Start();
 
   ProvideConnectionInfo({0, 0, 4});
-  ProvideConnectionInfo({BluetoothDevice::kUnknownPower, 0, 4});
+  ProvideConnectionInfo({BluetoothDevice::kUnknownRSSI, 0, 4});
 
   EXPECT_FALSE(monitor_.IsUnlockAllowed());
   EXPECT_FALSE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        ProximityState_CheckRssi_UnknownTxPower) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_RSSI);
   monitor_.Start();
 
   ProvideConnectionInfo({0, 0, 4});
-  ProvideConnectionInfo({0, BluetoothDevice::kUnknownPower, 4});
+  ProvideConnectionInfo({0, BluetoothDevice::kUnknownTxPower, 4});
 
   EXPECT_FALSE(monitor_.IsUnlockAllowed());
   EXPECT_FALSE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        ProximityState_CheckRssi_UnknownMaxTxPower) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_RSSI);
   monitor_.Start();
 
   ProvideConnectionInfo({0, 0, 4});
-  ProvideConnectionInfo({0, 0, BluetoothDevice::kUnknownPower});
+  ProvideConnectionInfo({0, 0, BluetoothDevice::kUnknownTxPower});
 
   EXPECT_FALSE(monitor_.IsUnlockAllowed());
   EXPECT_FALSE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        ProximityState_CheckTxPower_RssiIndicatesProximity_TxPowerDoesNot) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_TRANSMIT_POWER);
   monitor_.Start();
 
@@ skipping 35 matching lines @@
   EXPECT_TRUE(monitor_.IsUnlockAllowed());
   EXPECT_TRUE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        ProximityState_CheckTxPower_UnknownRssi) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_TRANSMIT_POWER);
   monitor_.Start();
 
   ProvideConnectionInfo({0, 0, 4});
-  ProvideConnectionInfo({BluetoothDevice::kUnknownPower, 0, 4});
+  ProvideConnectionInfo({BluetoothDevice::kUnknownRSSI, 0, 4});
 
   EXPECT_FALSE(monitor_.IsUnlockAllowed());
   EXPECT_FALSE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        ProximityState_CheckTxPower_UnknownTxPower) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_TRANSMIT_POWER);
   monitor_.Start();
 
   ProvideConnectionInfo({0, 0, 4});
-  ProvideConnectionInfo({0, BluetoothDevice::kUnknownPower, 4});
+  ProvideConnectionInfo({0, BluetoothDevice::kUnknownTxPower, 4});
 
   EXPECT_FALSE(monitor_.IsUnlockAllowed());
   EXPECT_FALSE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        ProximityState_CheckTxPower_UnknownMaxTxPower) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_TRANSMIT_POWER);
   monitor_.Start();
 
   ProvideConnectionInfo({0, 0, 4});
-  ProvideConnectionInfo({0, 0, BluetoothDevice::kUnknownPower});
+  ProvideConnectionInfo({0, 0, BluetoothDevice::kUnknownTxPower});
 
   EXPECT_FALSE(monitor_.IsUnlockAllowed());
   EXPECT_FALSE(monitor_.IsInRssiRange());
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest, ProximityState_StartThenStop) {
   monitor_.SetStrategy(ProximityMonitor::Strategy::CHECK_RSSI);
 
   monitor_.Start();
   ProvideConnectionInfo({0, 0, 4});
@@ skipping 163 matching lines @@
   histogram_tester.ExpectUniqueSample(
       "EasyUnlock.AuthProximity.TransmitPowerDelta", -1, 1);
   histogram_tester.ExpectUniqueSample(
       "EasyUnlock.AuthProximity.TimeSinceLastZeroRssi", 304, 1);
   histogram_tester.ExpectUniqueSample(
       "EasyUnlock.AuthProximity.RemoteDeviceModelHash",
       1881443083 /* hash of "LGE Nexus 5" */, 1);
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
-       RecordProximityMetricsOnAuthSuccess_ClampedValues) {
+       RecordProximityMetricsOnAuthSuccess_ClampedValues_LowerThreshold) {
+  // Tests that we clamp values lower than -100 before histograming.
+  int8_t rssi = -128;
+  int8_t tx_power = -127;
+  int8_t max_tx_power = 0;
+  int8_t tx_power_delta = tx_power - max_tx_power;
+
   monitor_.Start();

[Ilya Sherman, 2016/05/04 23:01:44]

nit: I'd keep this line right above ProvideConnectionInfo

[ortuno, 2016/05/07 00:32:09]

Done.

-  ProvideConnectionInfo({-99999, 99999, 12345});
+
+  ASSERT_NE(BluetoothDevice::kUnknownRSSI, rssi);
+  ASSERT_NE(BluetoothDevice::kUnknownTxPower, tx_power);
+  ASSERT_NE(BluetoothDevice::kUnknownTxPower, max_tx_power);
+  ASSERT_NE(BluetoothDevice::kUnknownTxPower, tx_power_delta);
+
+  ProvideConnectionInfo({rssi, tx_power, max_tx_power});
 
   base::HistogramTester histogram_tester;
   monitor_.RecordProximityMetricsOnAuthSuccess();
   histogram_tester.ExpectUniqueSample("EasyUnlock.AuthProximity.RollingRssi",
                                       -100, 1);
   histogram_tester.ExpectUniqueSample(
+      "EasyUnlock.AuthProximity.TransmitPowerDelta", -100, 1);
+}
+
+TEST_F(ProximityAuthProximityMonitorImplTest,
+       RecordProximityMetricsOnAuthSuccess_ClampedValues_UpperThreshold) {
+  // Tests that we clamp values larger than 50 before histograming.
+  int8_t rssi = 126;
+  int8_t tx_power = 127;
+  int8_t max_tx_power = 0;
+  int8_t tx_power_delta = tx_power - max_tx_power;
+
+  monitor_.Start();
+
+  ASSERT_NE(BluetoothDevice::kUnknownRSSI, rssi);
+  ASSERT_NE(BluetoothDevice::kUnknownTxPower, tx_power);
+  ASSERT_NE(BluetoothDevice::kUnknownTxPower, max_tx_power);
+  ASSERT_NE(BluetoothDevice::kUnknownTxPower, tx_power_delta);
+
+  ProvideConnectionInfo({rssi, tx_power, max_tx_power});
+
+  base::HistogramTester histogram_tester;
+  monitor_.RecordProximityMetricsOnAuthSuccess();
+  histogram_tester.ExpectUniqueSample("EasyUnlock.AuthProximity.RollingRssi",
+                                      50, 1);
+  histogram_tester.ExpectUniqueSample(
       "EasyUnlock.AuthProximity.TransmitPowerDelta", 50, 1);
 }
 
 TEST_F(ProximityAuthProximityMonitorImplTest,
        RecordProximityMetricsOnAuthSuccess_UnknownValues) {
   // Note: A device without a recorded name will have its Bluetooth address as
   // its name.
   RemoteDevice unnamed_remote_device(
       kRemoteDeviceUserId, kBluetoothAddress, kRemoteDevicePublicKey,
       RemoteDevice::BLUETOOTH_CLASSIC, kBluetoothAddress,
       kPersistentSymmetricKey, std::string());
 
   std::unique_ptr<base::TickClock> clock(new base::SimpleTestTickClock());
   ProximityMonitorImpl monitor(unnamed_remote_device, std::move(clock));
   monitor.AddObserver(&observer_);
   monitor.Start();
-  ProvideConnectionInfo({127, 127, 127});
+  ProvideConnectionInfo({BluetoothDevice::kUnknownRSSI,
+                         BluetoothDevice::kUnknownTxPower,
+                         BluetoothDevice::kUnknownTxPower});
 
   base::HistogramTester histogram_tester;
   monitor.RecordProximityMetricsOnAuthSuccess();
   histogram_tester.ExpectUniqueSample("EasyUnlock.AuthProximity.RollingRssi",
-                                      127, 1);
+                                      BluetoothDevice::kUnknownRSSI, 1);

[Ilya Sherman, 2016/05/04 23:01:44]

Hmm, the reported RSSI shouldn't change, even if the backing constant happens to
have changed.  This test expectation should remain the concrete number 127.

[ortuno, 2016/05/07 00:32:09]

Ah. I misunderstood the purpose of the test. Fixed.

   histogram_tester.ExpectUniqueSample(
-      "EasyUnlock.AuthProximity.TransmitPowerDelta", 127, 1);
+      "EasyUnlock.AuthProximity.TransmitPowerDelta",
+      BluetoothDevice::kUnknownTxPower, 1);

[Ilya Sherman, 2016/05/04 23:01:44]

Ditto.

[ortuno, 2016/05/07 00:32:09]

Done.

   histogram_tester.ExpectUniqueSample(
       "EasyUnlock.AuthProximity.TimeSinceLastZeroRssi",
       base::TimeDelta::FromSeconds(10).InMilliseconds(), 1);
   histogram_tester.ExpectUniqueSample(
       "EasyUnlock.AuthProximity.RemoteDeviceModelHash",
       -1808066424 /* hash of "Unknown" */, 1);
 }
 
 }  // namespace proximity_auth