Update AccelerometerReader to support separate iio devices

chromeos/accelerometer/accelerometer_reader.cc

Index: chromeos/accelerometer/accelerometer_reader.cc
diff --git a/chromeos/accelerometer/accelerometer_reader.cc b/chromeos/accelerometer/accelerometer_reader.cc
index 04122ca09b90f18974d914382fba2a87a5721ce9..8db2c53afdbe7a78ede5ce1ac2be09907edb853b 100644
--- a/chromeos/accelerometer/accelerometer_reader.cc
+++ b/chromeos/accelerometer/accelerometer_reader.cc
@@ -4,9 +4,11 @@
 
 #include "chromeos/accelerometer/accelerometer_reader.h"
 
+#include <algorithm>
 #include <string>
 
 #include "base/bind.h"
+#include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/memory/singleton.h"
@@ -32,27 +34,37 @@ const base::FilePath::CharType kAccelerometerDevicePath[] =
 const base::FilePath::CharType kAccelerometerIioBasePath[] =
     FILE_PATH_LITERAL("/sys/bus/iio/devices/");
 
-// File within the device in kAccelerometerIioBasePath containing the scale of
-// the accelerometers.
-const base::FilePath::CharType kScaleFileName[] = "in_accel_scale";
-
 // This is the per source scale file in use on kernels older than 3.18. We
 // should remove this when all devices having accelerometers are on kernel 3.18
 // or later or have been patched to use new format: http://crbug.com/510831
 const base::FilePath::CharType kSourceScaleNameFormatString[] =
     "in_accel_%s_scale";
 
+// File within kAccelerometerDevicePath/device* which denotes a single scale to
+// be used across all axes.
+const base::FilePath::CharType kAccelerometerScaleFileName[] = "scale";
+
+// File within kAccelerometerDevicePath/device* which denotes the
+// AccelerometerSource for the accelerometer.
+const base::FilePath::CharType kAccelerometerLocationFileName[] = "location";
+
 // The filename giving the path to read the scan index of each accelerometer
 // axis.
 const char kAccelerometerScanIndexPath[] =
     "scan_elements/in_accel_%s_%s_index";
 
+// The filename giving the path to read the scan index of each accelerometer
+// when they are separate device paths.
+const char kSeparateAccelerometerScanIndexPath[] =
+    "scan_elements/in_accel_%s_index";
+
 // The names of the accelerometers. Matches up with the enum AccelerometerSource
 // in chromeos/accelerometer/accelerometer_types.h.
 const char kAccelerometerNames[ACCELEROMETER_SOURCE_COUNT][5] = {"lid", "base"};
 
 // The axes on each accelerometer.
-const char kAccelerometerAxes[][2] = {"y", "x", "z"};
+const char kSeparateAccelerometerAxes[][2] = {"x", "y", "z"};
+const char kUnifiedAccelerometerAxes[][2] = {"y", "x", "z"};
 
 // The length required to read uint values from configuration files.
 const size_t kMaxAsciiUintLength = 21;
@@ -60,6 +72,10 @@ const size_t kMaxAsciiUintLength = 21;
 // The size of individual values.
 const size_t kDataSize = 2;
 
+// The size of reading values for an entire accelerometer, when they are
+// separate device paths.
+const size_t kDataSizeForSeparateDevices = 6;
+
 // The mean acceleration due to gravity on Earth in m/s^2.
 const float kMeanGravity = 9.80665f;
 
@@ -82,7 +98,24 @@ bool ReadFileToInt(const base::FilePath& path, int* value) {
   }
   base::TrimWhitespaceASCII(s, base::TRIM_ALL, &s);
   if (!base::StringToInt(s, value)) {
-    LOG(ERROR) << "Failed to parse \"" << s << "\" from " << path.value();
+    LOG(ERROR) << "Failed to parse int \"" << s << "\" from " << path.value();
+    return false;
+  }
+  return true;
+}
+
+// Reads |path| to the double pointed to by |value|. Returns true on success or
+// false on failure.
+bool ReadFileToDouble(const base::FilePath& path, double* value) {
+  std::string s;
+  DCHECK(value);
+  if (!base::ReadFileToString(path, &s)) {
+    return false;
+  }
+  base::TrimWhitespaceASCII(s, base::TRIM_ALL, &s);
+  if (!base::StringToDouble(s, value)) {
+    LOG(ERROR) << "Failed to parse double \"" << s << "\" from "
+               << path.value();
     return false;
   }
   return true;
@@ -136,14 +169,43 @@ class AccelerometerFileReader
 
     // Index of each accelerometer axis in data stream.
     int index[ACCELEROMETER_SOURCE_COUNT][3];
+
+    // If true the accelerometer devices must be read from distinct files.
+    bool read_device_separately;
+
+    // Length of reading per accelerometer
+    size_t device_length[ACCELEROMETER_SOURCE_COUNT];
+
+    // The path to append to |kAccelerometerDevicePath| where the accelerometer
+    // device resides.
+    base::FilePath device_path[ACCELEROMETER_SOURCE_COUNT];
   };
 
   ~AccelerometerFileReader() {}
 
+  // When accelerometers are presented as separate iio_devices this will perform
+  // the initialize for one of the devices, at the given |iio_path| and the
+  // symbolic link |name|. |location| is defined by AccelerometerSoure.
+  bool InitializeDevice(const base::FilePath& iio_path,
+                        const base::FilePath& name,
+                        const std::string& location);
+
+  // When accelerometers are presented as a single iio_device this will perform
+  // the initialization for both of them.
+  bool InitializeBothAccelerometers(const base::FilePath& iio_path);
+
   // Attempts to read the accelerometer data. Upon a success, converts the raw
   // reading to an AccelerometerUpdate and notifies observers.
   void ReadFileAndNotify();
 
+  // When accelerometers are presented as separate iio_devices this will perform
+  // the read of both devices, from their separate locations.
+  bool ReadSeparateDevices();
+
+  // When accelerometers are presented as a single iio_device this will perform
+  // the read of the device.
+  bool ReadUnifiedDevice();
+
   // True if Initialize completed successfully, and there is an accelerometer
   // file to read.
   bool initialization_successful_;
@@ -179,8 +241,10 @@ void AccelerometerFileReader::Initialize(
   // Check for accelerometer symlink which will be created by the udev rules
   // file on detecting the device.
   base::FilePath device;
-  if (!base::ReadSymbolicLink(base::FilePath(kAccelerometerDevicePath),
-                              &device)) {
+
+  if (base::IsDirectoryEmpty(base::FilePath(kAccelerometerDevicePath))) {
+    LOG(ERROR) << "Accelerometer device directory is empty at "
+               << kAccelerometerDevicePath;
     return;
   }
 
@@ -190,59 +254,31 @@ void AccelerometerFileReader::Initialize(
     return;
   }
 
-  base::FilePath iio_path(base::FilePath(kAccelerometerIioBasePath).Append(
-      device));
-
-  // Read the scale for all axes.
-  int scale_divisor = 0;
-  bool per_source_scale =
-      !ReadFileToInt(iio_path.Append(kScaleFileName), &scale_divisor);
-  if (!per_source_scale && scale_divisor == 0) {
-    LOG(ERROR) << "Accelerometer " << kScaleFileName
-               << "has scale of 0 and will not be used.";
-    return;
-  }
-
-  // Read configuration of each accelerometer axis from each accelerometer from
-  // /sys/bus/iio/devices/iio:deviceX/.
-  for (size_t i = 0; i < arraysize(kAccelerometerNames); ++i) {
-    if (per_source_scale) {
-      configuration_.has[i] = false;
-      // Read scale of accelerometer.
-      std::string accelerometer_scale_path = base::StringPrintf(
-          kSourceScaleNameFormatString, kAccelerometerNames[i]);
-      if (!ReadFileToInt(iio_path.Append(accelerometer_scale_path.c_str()),
-                         &scale_divisor)) {
-        continue;
-      }
-      if (scale_divisor == 0) {
-        LOG(ERROR) << "Accelerometer " << accelerometer_scale_path
-                   << "has scale of 0 and will not be used.";
-        continue;
-      }
+  base::FileEnumerator symlink_dir(base::FilePath(kAccelerometerDevicePath),
+                                   false, base::FileEnumerator::FILES);
+  for (base::FilePath name = symlink_dir.Next(); !name.empty();
+       name = symlink_dir.Next()) {
+    base::FilePath iio_device;
+    if (!base::ReadSymbolicLink(name, &iio_device)) {
+      LOG(ERROR) << "Failed to read symbolic link " << kAccelerometerDevicePath
+                 << "/" << name.MaybeAsASCII() << "\n";
+      return;
     }
 
-    configuration_.has[i] = true;
-    for (size_t j = 0; j < arraysize(kAccelerometerAxes); ++j) {
-      configuration_.scale[i][j] = kMeanGravity / scale_divisor;
-      std::string accelerometer_index_path = base::StringPrintf(
-          kAccelerometerScanIndexPath, kAccelerometerAxes[j],
-          kAccelerometerNames[i]);
-      if (!ReadFileToInt(iio_path.Append(accelerometer_index_path.c_str()),
-                         &(configuration_.index[i][j]))) {
-        configuration_.has[i] = false;
-        break;
-      }
+    base::FilePath iio_path(base::FilePath(kAccelerometerIioBasePath)
+                                .Append(iio_device.BaseName()));
+    std::string location;
+    if (base::ReadFileToString(
+            base::FilePath(iio_path).Append(kAccelerometerLocationFileName),
+            &location)) {
+      base::TrimWhitespaceASCII(location, base::TRIM_ALL, &location);
+      if (!InitializeDevice(iio_path, name, location))
+        return;
+    } else {
+      if (!InitializeBothAccelerometers(iio_path))
+        return;
+      configuration_.device_path[ACCELEROMETER_SOURCE_SCREEN] = name.BaseName();
     }
-    if (configuration_.has[i])
-      configuration_.count++;
-  }
-
-  // Adjust the directions of accelerometers to match the AccelerometerUpdate
-  // type specified in chromeos/accelerometer/accelerometer_types.h.
-  configuration_.scale[ACCELEROMETER_SOURCE_SCREEN][0] *= -1.0f;
-  for (int i = 0; i < 3; ++i) {
-    configuration_.scale[ACCELEROMETER_SOURCE_ATTACHED_KEYBOARD][i] *= -1.0f;
   }
 
   // Verify indices are within bounds.
@@ -253,13 +289,16 @@ void AccelerometerFileReader::Initialize(
       if (configuration_.index[i][j] < 0 ||
           configuration_.index[i][j] >=
               3 * static_cast<int>(configuration_.count)) {
+        const char* axis = configuration_.read_device_separately
+                               ? kSeparateAccelerometerAxes[j]
+                               : kUnifiedAccelerometerAxes[j];
         LOG(ERROR) << "Field index for " << kAccelerometerNames[i] << " "
-                   << kAccelerometerAxes[j] << " axis out of bounds.";
+                   << axis << " axis out of bounds.";
         return;
       }
     }
   }
-  configuration_.length = kDataSize * 3 * configuration_.count;
+
   initialization_successful_ = true;
   Read();
 }
@@ -289,9 +328,93 @@ void AccelerometerFileReader::RemoveObserver(
   observers_->RemoveObserver(observer);
 }
 
+bool AccelerometerFileReader::InitializeDevice(const base::FilePath& iio_path,
+                                               const base::FilePath& name,
+                                               const std::string& location) {
+  configuration_.read_device_separately = true;
+  int config_index = std::find(std::begin(kAccelerometerNames),
+                               std::end(kAccelerometerNames), location) -
+                     std::begin(kAccelerometerNames);
+  if (config_index == -1) {
+    LOG(ERROR) << "Unrecognized location: " << location << " for device "
+               << name.MaybeAsASCII() << "\n";
+    return false;
+  }
+  configuration_.device_path[config_index] = name.BaseName();
+  configuration_.has[config_index] = true;
+
+  double scale;
+  if (!ReadFileToDouble(iio_path.Append(kAccelerometerScaleFileName), &scale))
+    return false;
+
+  for (size_t j = 0; j < arraysize(kSeparateAccelerometerAxes); ++j) {
+    std::string accelerometer_index_path = base::StringPrintf(
+        kSeparateAccelerometerScanIndexPath, kSeparateAccelerometerAxes[j]);
+    if (!ReadFileToInt(iio_path.Append(accelerometer_index_path.c_str()),
+                       &(configuration_.index[config_index][j]))) {
+      configuration_.has[config_index] = false;
+      LOG(ERROR) << "Index file " << accelerometer_index_path
+                 << " could not be parsed\n";
+      return false;
+    }
+    configuration_.scale[config_index][j] = scale;
+  }
+  configuration_.device_length[config_index] = kDataSizeForSeparateDevices;
+  if (configuration_.has[config_index])
+    configuration_.count++;
+  return true;
+}
+
+bool AccelerometerFileReader::InitializeBothAccelerometers(
+    const base::FilePath& iio_path) {
+  // Read the scale for all axes.
+  int scale_divisor = 0;
+  // Read configuration of each accelerometer axis from each accelerometer from
+  // /sys/bus/iio/devices/iio:deviceX/.
+  for (size_t i = 0; i < arraysize(kAccelerometerNames); ++i) {
+    configuration_.has[i] = false;
+    // Read scale of accelerometer.
+    std::string accelerometer_scale_path = base::StringPrintf(
+        kSourceScaleNameFormatString, kAccelerometerNames[i]);
+    if (!ReadFileToInt(iio_path.Append(accelerometer_scale_path.c_str()),
+                       &scale_divisor)) {
+      continue;
+    }
+    if (scale_divisor == 0) {
+      LOG(ERROR) << "Accelerometer " << accelerometer_scale_path
+                 << "has scale of 0 and will not be used.";
+      continue;
+    }
+
+    configuration_.has[i] = true;
+    for (size_t j = 0; j < arraysize(kUnifiedAccelerometerAxes); ++j) {
+      configuration_.scale[i][j] = kMeanGravity / scale_divisor;
+      std::string accelerometer_index_path = base::StringPrintf(
+          kAccelerometerScanIndexPath, kUnifiedAccelerometerAxes[j],
+          kAccelerometerNames[i]);
+      if (!ReadFileToInt(iio_path.Append(accelerometer_index_path.c_str()),
+                         &(configuration_.index[i][j]))) {
+        configuration_.has[i] = false;
+        LOG(ERROR) << "Index file " << accelerometer_index_path
+                   << " could not be parsed\n";
+        return false;
+      }
+    }
+    if (configuration_.has[i])
+      configuration_.count++;
+  }
+
+  // Adjust the directions of accelerometers to match the AccelerometerUpdate
+  // type specified in chromeos/accelerometer/accelerometer_types.h.

[jonross, 2015/08/24 18:30:35]

With the latest change to the kernel, the axes and signs of accelerometer values
match the accelerometer_types.h, as well as the html5 spec.

ScreenOrientationController was updates to work with the correct axes.
SensorManagerChromeOs removes the remapping of values it had.

The adjustments here bring the 3.14 kernel values inline with those of 3.18,
while supporting the fixes to ScreenOrientationController and
SensorManagerChromeOs.

+  configuration_.scale[ACCELEROMETER_SOURCE_SCREEN][1] *= -1.0f;
+  configuration_.scale[ACCELEROMETER_SOURCE_SCREEN][2] *= -1.0f;
+
+  configuration_.length = kDataSize * 3 * configuration_.count;
+  return true;
+}
+
 void AccelerometerFileReader::ReadFileAndNotify() {
   DCHECK(initialization_successful_);
-  char reading[kSizeOfReading];
 
   // Initiate the trigger to read accelerometers simultaneously
   int bytes_written = base::WriteFile(
@@ -302,19 +425,37 @@ void AccelerometerFileReader::ReadFileAndNotify() {
   }
 
   // Read resulting sample from /dev/cros-ec-accel.
-  int bytes_read = base::ReadFile(base::FilePath(kAccelerometerDevicePath),
-                                  reading, configuration_.length);
-  if (bytes_read < static_cast<int>(configuration_.length)) {
-    LOG(ERROR) << "Read " << bytes_read << " byte(s), expected "
-               << configuration_.length << " bytes from accelerometer";
-    return;
+  bool result;
+  update_ = new AccelerometerUpdate();
+  if (configuration_.read_device_separately) {
+    result = ReadSeparateDevices();
+  } else {
+    result = ReadUnifiedDevice();
   }
+  if (!result)
+    return;
 
-  update_ = new AccelerometerUpdate();
+  observers_->Notify(FROM_HERE,
+                     &AccelerometerReader::Observer::OnAccelerometerUpdated,
+                     update_);
+}
 
+bool AccelerometerFileReader::ReadSeparateDevices() {
+  char reading[kSizeOfReading];
+  int bytes_read;
   for (int i = 0; i < ACCELEROMETER_SOURCE_COUNT; ++i) {
     if (!configuration_.has[i])
       continue;
+    bytes_read = base::ReadFile(base::FilePath(kAccelerometerDevicePath)
+                                    .Append(configuration_.device_path[i]),
+                                reading, configuration_.device_length[i]);
+    if (bytes_read < static_cast<int>(configuration_.device_length[i])) {
+      LOG(ERROR) << "Accelerometer Read " << bytes_read << " byte(s), expected "
+                 << configuration_.device_length[i]
+                 << " bytes from accelerometer "
+                 << configuration_.device_path[i].MaybeAsASCII();
+      return false;
+    }
 
     int16* values = reinterpret_cast<int16*>(reading);
     update_->Set(
@@ -323,13 +464,36 @@ void AccelerometerFileReader::ReadFileAndNotify() {
         values[configuration_.index[i][1]] * configuration_.scale[i][1],
         values[configuration_.index[i][2]] * configuration_.scale[i][2]);
   }
+  return true;
+}
 
-  observers_->Notify(FROM_HERE,
-                     &AccelerometerReader::Observer::OnAccelerometerUpdated,
-                     update_);
+bool AccelerometerFileReader::ReadUnifiedDevice() {
+  char reading[kSizeOfReading];
+  int bytes_read = base::ReadFile(
+      base::FilePath(kAccelerometerDevicePath)
+          .Append(configuration_.device_path[ACCELEROMETER_SOURCE_SCREEN]),
+      reading, configuration_.length);
+  if (bytes_read < static_cast<int>(configuration_.length)) {
+    LOG(ERROR) << "Accelerometer Read " << bytes_read << " byte(s), expected "
+               << configuration_.length << " bytes from accelerometer";
+    return false;
+  }
+
+  for (int i = 0; i < ACCELEROMETER_SOURCE_COUNT; ++i) {
+    if (!configuration_.has[i])
+      continue;
+    int16* values = reinterpret_cast<int16*>(reading);
+    update_->Set(
+        static_cast<AccelerometerSource>(i),
+        values[configuration_.index[i][0]] * configuration_.scale[i][0],
+        values[configuration_.index[i][1]] * configuration_.scale[i][1],
+        values[configuration_.index[i][2]] * configuration_.scale[i][2]);
+  }
+  return true;
 }
 
-AccelerometerFileReader::ConfigurationData::ConfigurationData() : count(0) {
+AccelerometerFileReader::ConfigurationData::ConfigurationData()
+    : count(0), read_device_separately(false) {
   for (int i = 0; i < ACCELEROMETER_SOURCE_COUNT; ++i) {
     has[i] = false;
     for (int j = 0; j < 3; ++j) {