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

device/bluetooth/bluez/bluetooth_bluez_unittest.cc

 // Copyright 2013 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 <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <utility>
 
@@ skipping 2485 matching lines @@
   // we invalidate the RSSI of the device.
   TestBluetoothAdapterObserver observer(adapter_);
 
   // When discovery is over, the value should be invalidated.
   properties->rssi.set_valid(false);
   properties->NotifyPropertyChanged(properties->rssi.name());
 
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_EQ(devices[idx], observer.last_device());
 
-  int unknown_power = BluetoothDevice::kUnknownPower;
-  EXPECT_EQ(unknown_power, devices[idx]->GetInquiryRSSI());
+  EXPECT_TRUE(devices[idx]->GetInquiryRSSI() == BluetoothDevice::kUnknownRSSI);
+}
+
+TEST_F(BluetoothBlueZTest, DeviceInquiryRSSIOutOfBounds) {
+  // RSSI should be a int8 but luez returns a int16 because DBus doesn't

[scheib, 2016/05/04 01:18:37]

bluez
^

[ortuno, 2016/05/04 16:44:11]

Done.

+  // support int8 even though Bluez uses a int8 internally. This test
+  // makes sure we handle values outside of the int8 range.
+  GetAdapter();
+
+  BluetoothAdapter::DeviceList devices = adapter_->GetDevices();
+  ASSERT_EQ(2U, devices.size());
+
+  int idx = GetDeviceIndexByAddress(
+      devices, bluez::FakeBluetoothDeviceClient::kPairedDeviceAddress);
+  ASSERT_NE(-1, idx);
+
+  bluez::FakeBluetoothDeviceClient::Properties* properties =
+      fake_bluetooth_device_client_->GetProperties(dbus::ObjectPath(
+          bluez::FakeBluetoothDeviceClient::kPairedDevicePath));
+
+  properties->rssi.set_valid(true);
+
+  // Small invalid value
+  properties->rssi.ReplaceValue(-129);
+  EXPECT_TRUE(devices[idx]->GetInquiryRSSI() == BluetoothDevice::kUnknownRSSI);
+
+  // Small valid value
+  properties->rssi.ReplaceValue(-128);
+  EXPECT_EQ(-128, devices[idx]->GetInquiryRSSI());
+
+  // Large valid value. (127 is used for BluetoothDevice::kUnknownRSSI).
+  properties->rssi.ReplaceValue(126);
+  EXPECT_EQ(126, devices[idx]->GetInquiryRSSI());
+
+  // Large invalid value
+  properties->rssi.ReplaceValue(128);
+  EXPECT_TRUE(devices[idx]->GetInquiryRSSI() == BluetoothDevice::kUnknownRSSI);

[scheib, 2016/05/04 01:18:37]

EXPECT_EQ instead of _TRUE. (and elsewhere)

[ortuno, 2016/05/04 16:44:12]

That's what I tried to do the first time but I get a linking error :/

error: undefined reference to 'device::BluetoothDevice::kUnknownRSSI'

 }
 
 TEST_F(BluetoothBlueZTest, DeviceInquiryTxPowerInvalidated) {
   // Simulate invalidation of inquiry TxPower of a device, as it occurs
   // when discovery is finished.
   GetAdapter();
 
   BluetoothAdapter::DeviceList devices = adapter_->GetDevices();
   ASSERT_EQ(2U, devices.size());
 
@@ skipping 15 matching lines @@
   // we invalidate the tx_power of the device.
   TestBluetoothAdapterObserver observer(adapter_);
 
   // When discovery is over, the value should be invalidated.
   properties->tx_power.set_valid(false);
   properties->NotifyPropertyChanged(properties->tx_power.name());
 
   EXPECT_EQ(1, observer.device_changed_count());
   EXPECT_EQ(devices[idx], observer.last_device());
 
-  int unknown_power = BluetoothDevice::kUnknownPower;
-  EXPECT_EQ(unknown_power, devices[idx]->GetInquiryTxPower());
+  EXPECT_TRUE(devices[idx]->GetInquiryTxPower() ==
+              BluetoothDevice::kUnknownTxPower);
+}
+
+TEST_F(BluetoothBlueZTest, DeviceInquiryTxPowerOutOfBounds) {
+  // TxPower should be a int8 but Bluez returns a int16 because DBus doesn't
+  // support int8 even though Bluez uses a int8 internally. This test
+  // makes sure we handle values outside of the int8 range.
+  GetAdapter();
+
+  BluetoothAdapter::DeviceList devices = adapter_->GetDevices();
+  ASSERT_EQ(2U, devices.size());
+
+  int idx = GetDeviceIndexByAddress(
+      devices, bluez::FakeBluetoothDeviceClient::kPairedDeviceAddress);
+  ASSERT_NE(-1, idx);
+
+  bluez::FakeBluetoothDeviceClient::Properties* properties =
+      fake_bluetooth_device_client_->GetProperties(dbus::ObjectPath(
+          bluez::FakeBluetoothDeviceClient::kPairedDevicePath));
+
+  properties->tx_power.set_valid(true);
+
+  // Small invalid value
+  properties->tx_power.ReplaceValue(-129);
+  EXPECT_TRUE(devices[idx]->GetInquiryTxPower() ==
+              BluetoothDevice::kUnknownTxPower);
+
+  // Small valid value
+  properties->tx_power.ReplaceValue(-128);
+  EXPECT_EQ(-128, devices[idx]->GetInquiryTxPower());
+
+  // Large valid value. (127 is used for BluetoothDevice::kUnknownTxPower).

[scheib, 2016/05/04 01:18:37]

What does Bluez use for UnknownTxPower? This patch is changing chromium to use
-128. This comment, and previous block need updates?

[ortuno, 2016/05/04 16:44:12]

In bluez, if a property is not present property.is_valid() returns false.
Done.

+  properties->tx_power.ReplaceValue(126);
+  EXPECT_EQ(126, devices[idx]->GetInquiryTxPower());
+
+  // Large invalid value
+  properties->tx_power.ReplaceValue(128);
+  EXPECT_TRUE(devices[idx]->GetInquiryTxPower() ==
+              BluetoothDevice::kUnknownTxPower);
 }
 
 TEST_F(BluetoothBlueZTest, ForgetDevice) {
   GetAdapter();
 
   BluetoothAdapter::DeviceList devices = adapter_->GetDevices();
   ASSERT_EQ(2U, devices.size());
 
   int idx = GetDeviceIndexByAddress(
       devices, bluez::FakeBluetoothDeviceClient::kPairedDeviceAddress);
@@ skipping 1570 matching lines @@
   EXPECT_EQ(0, device->GetProductID());
   EXPECT_EQ(0, device->GetDeviceID());
 }
 
 TEST_F(BluetoothBlueZTest, GetConnectionInfoForDisconnectedDevice) {
   GetAdapter();
   BluetoothDevice* device = adapter_->GetDevice(
       bluez::FakeBluetoothDeviceClient::kPairedDeviceAddress);
 
   // Calling GetConnectionInfo for an unconnected device should return a result
-  // in which all fields are filled with BluetoothDevice::kUnknownPower.
+  // in which all fields are filled with BluetoothDevice::kUnknownRSSI and
+  // BluetoothDevice::kUnknownTxPower.
   BluetoothDevice::ConnectionInfo conn_info(0, 0, 0);
   device->GetConnectionInfo(base::Bind(&SaveConnectionInfo, &conn_info));
-  int unknown_power = BluetoothDevice::kUnknownPower;
-  EXPECT_NE(0, unknown_power);
-  EXPECT_EQ(unknown_power, conn_info.rssi);
-  EXPECT_EQ(unknown_power, conn_info.transmit_power);
-  EXPECT_EQ(unknown_power, conn_info.max_transmit_power);
+
+  EXPECT_TRUE(conn_info.rssi == BluetoothDevice::kUnknownRSSI);
+  EXPECT_TRUE(conn_info.transmit_power == BluetoothDevice::kUnknownTxPower);
+  EXPECT_TRUE(conn_info.max_transmit_power == BluetoothDevice::kUnknownTxPower);
 }
 
 TEST_F(BluetoothBlueZTest, GetConnectionInfoForConnectedDevice) {
   GetAdapter();
   BluetoothDevice* device = adapter_->GetDevice(
       bluez::FakeBluetoothDeviceClient::kPairedDeviceAddress);
 
   device->Connect(nullptr, GetCallback(),
                   base::Bind(&BluetoothBlueZTest::ConnectErrorCallback,
                              base::Unretained(this)));
@@ skipping 328 matching lines @@
   adapter_->Shutdown();
   adapter_bluez->OnStopDiscoveryError(GetDiscoveryErrorCallback(), "", "");
 
   // 1 error reported to RemoveDiscoverySession because of OnStopDiscoveryError,
   // and kNumberOfDiscoverySessions errors queued with AddDiscoverySession.
   EXPECT_EQ(0, callback_count_);
   EXPECT_EQ(1 + kNumberOfDiscoverySessions, error_callback_count_);
 }
 
 }  // namespace bluez