Upcoming changes to Chromium OS change how accelerometers are presented.

Currently both base and lid accelerometers are presented as a single iio_device. AccelerometerReader would read this file system to get the needed information.

Now they are being separated into different iio_devices. There have been other changes to the underlying file system that needs to be addressed for correct initialization.

This change update AccelerometerReader to handle this new format. While preserving the legacy path for devices on older kernels.

TEST=Manual testing on devices. This was loaded on a device representing both kernel versions.
BUG=515949, chrome-os-partner:40177, 431391

Review URL: https://codereview.chromium.org/1306453003

Cr-Commit-Position: refs/heads/master@{#346148}

1 files changed, 218 insertions, 90 deletions

diff --git a/chromeos/accelerometer/accelerometer_reader.cc b/chromeos/accelerometer/accelerometer_reader.cc
index 04122ca0..400faac 100644
--- a/chromeos/accelerometer/accelerometer_reader.cc
+++ b/chromeos/accelerometer/accelerometer_reader.cc
@@ -5,8 +5,10 @@
 #include "chromeos/accelerometer/accelerometer_reader.h"
 
 #include <string>
+#include <vector>
 
 #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[] =
+const base::FilePath::CharType kLegacyScaleNameFormatString[] =
     "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[] =
+const char kLegacyAccelerometerScanIndexPathFormatString[] =
     "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 kAccelerometerScanIndexPathFormatString[] =
+    "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"};
+// The axes on each accelerometer. The order was changed on kernel 3.18+.
+const char kAccelerometerAxes[][2] = {"x", "y", "z"};
+const char kLegacyAccelerometerAxes[][2] = {"y", "x", "z"};
 
 // The length required to read uint values from configuration files.
 const size_t kMaxAsciiUintLength = 21;
@@ -63,14 +75,11 @@ const size_t kDataSize = 2;
 // The mean acceleration due to gravity on Earth in m/s^2.
 const float kMeanGravity = 9.80665f;
 
-// The number of accelerometers.
-const int kNumberOfAccelerometers = 2;
-
 // The number of axes for which there are acceleration readings.
 const int kNumberOfAxes = 3;
 
 // The size of data in one reading of the accelerometers.
-const int kSizeOfReading = kDataSize * kNumberOfAccelerometers * kNumberOfAxes;
+const int kSizeOfReading = kDataSize * kNumberOfAxes;
 
 // Reads |path| to the unsigned int pointed to by |value|. Returns true on
 // success or false on failure.
@@ -82,7 +91,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;
@@ -117,6 +143,15 @@ class AccelerometerFileReader
  private:
   friend class base::RefCountedThreadSafe<AccelerometerFileReader>;
 
+  // Represents necessary information in order to read an accelerometer device.
+  struct ReadingData {
+    // The full path to the accelerometer device to read.
+    base::FilePath path;
+
+    // The accelerometer sources which can be read from |path|.
+    std::vector<AccelerometerSource> sources;
+  };
+
   // Configuration structure for accelerometer device.
   struct ConfigurationData {
     ConfigurationData();
@@ -125,9 +160,6 @@ class AccelerometerFileReader
     // Number of accelerometers on device.
     size_t count;
 
-    // Length of accelerometer updates.
-    size_t length;
-
     // Which accelerometers are present on device.
     bool has[ACCELEROMETER_SOURCE_COUNT];
 
@@ -136,10 +168,28 @@ class AccelerometerFileReader
 
     // Index of each accelerometer axis in data stream.
     int index[ACCELEROMETER_SOURCE_COUNT][3];
+
+    // The information for each accelerometer device to be read. In kernel 3.18
+    // there is one per ACCELEROMETER_SOURCE_COUNT, on 3.14 there is only one.
+    std::vector<ReadingData> reading_data;
   };
 
   ~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 InitializeAccelerometer(const base::FilePath& iio_path,
+                               const base::FilePath& name,
+                               const std::string& location);
+
+  // TODO(jonross): Separate the initialization into separate files. Add a gyp
+  // rule to have them built for the appropriate kernels. (crbug.com/525658)
+  // When accelerometers are presented as a single iio_device this will perform
+  // the initialization for both of them.
+  bool InitializeLegacyAccelerometers(const base::FilePath& iio_path,
+                                      const base::FilePath& name);
+
   // Attempts to read the accelerometer data. Upon a success, converts the raw
   // reading to an AccelerometerUpdate and notifies observers.
   void ReadFileAndNotify();
@@ -179,8 +229,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 +242,32 @@ 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);
+  bool legacy_cross_accel = false;
+  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;
+    legacy_cross_accel = !base::ReadFileToString(
+        base::FilePath(iio_path).Append(kAccelerometerLocationFileName),
+        &location);
+    if (legacy_cross_accel) {
+      if (!InitializeLegacyAccelerometers(iio_path, name))
+        return;
+    } else {
+      base::TrimWhitespaceASCII(location, base::TRIM_ALL, &location);
+      if (!InitializeAccelerometer(iio_path, name, location))
+        return;
     }
-    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 +278,15 @@ void AccelerometerFileReader::Initialize(
       if (configuration_.index[i][j] < 0 ||
           configuration_.index[i][j] >=
               3 * static_cast<int>(configuration_.count)) {
+        const char* axis = legacy_cross_accel ? kLegacyAccelerometerAxes[j]
+                                              : kAccelerometerAxes[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 +316,108 @@ void AccelerometerFileReader::RemoveObserver(
   observers_->RemoveObserver(observer);
 }
 
+bool AccelerometerFileReader::InitializeAccelerometer(
+    const base::FilePath& iio_path,
+    const base::FilePath& name,
+    const std::string& location) {
+  size_t config_index = 0;
+  for (; config_index < arraysize(kAccelerometerNames); ++config_index) {
+    if (location == kAccelerometerNames[config_index])
+      break;
+  }
+
+  if (config_index >= arraysize(kAccelerometerNames)) {
+    LOG(ERROR) << "Unrecognized location: " << location << " for device "
+               << name.MaybeAsASCII() << "\n";
+    return false;
+  }
+
+  double scale;
+  if (!ReadFileToDouble(iio_path.Append(kAccelerometerScaleFileName), &scale))
+    return false;
+
+  const int kNumberAxes = arraysize(kAccelerometerAxes);
+  for (size_t i = 0; i < kNumberAxes; ++i) {
+    std::string accelerometer_index_path = base::StringPrintf(
+        kAccelerometerScanIndexPathFormatString, kAccelerometerAxes[i]);
+    if (!ReadFileToInt(iio_path.Append(accelerometer_index_path.c_str()),
+                       &(configuration_.index[config_index][i]))) {
+      LOG(ERROR) << "Index file " << accelerometer_index_path
+                 << " could not be parsed\n";
+      return false;
+    }
+    configuration_.scale[config_index][i] = scale;
+  }
+  configuration_.has[config_index] = true;
+  configuration_.count++;
+
+  ReadingData reading_data;
+  reading_data.path =
+      base::FilePath(kAccelerometerDevicePath).Append(name.BaseName());
+  reading_data.sources.push_back(
+      static_cast<AccelerometerSource>(config_index));
+
+  configuration_.reading_data.push_back(reading_data);
+
+  return true;
+}
+
+bool AccelerometerFileReader::InitializeLegacyAccelerometers(
+    const base::FilePath& iio_path,
+    const base::FilePath& name) {
+  ReadingData reading_data;
+  reading_data.path =
+      base::FilePath(kAccelerometerDevicePath).Append(name.BaseName());
+  // 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(
+        kLegacyScaleNameFormatString, kAccelerometerNames[i]);
+    // Read the scale for all axes.
+    int scale_divisor = 0;
+    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(kLegacyAccelerometerAxes); ++j) {
+      configuration_.scale[i][j] = kMeanGravity / scale_divisor;
+      std::string accelerometer_index_path = base::StringPrintf(
+          kLegacyAccelerometerScanIndexPathFormatString,
+          kLegacyAccelerometerAxes[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++;
+      reading_data.sources.push_back(static_cast<AccelerometerSource>(i));
+    }
+  }
+
+  // Adjust the directions of accelerometers to match the AccelerometerUpdate
+  // type specified in chromeos/accelerometer/accelerometer_types.h.
+  configuration_.scale[ACCELEROMETER_SOURCE_SCREEN][1] *= -1.0f;
+  configuration_.scale[ACCELEROMETER_SOURCE_SCREEN][2] *= -1.0f;
+
+  configuration_.reading_data.push_back(reading_data);
+  return true;
+}
+
 void AccelerometerFileReader::ReadFileAndNotify() {
   DCHECK(initialization_successful_);
-  char reading[kSizeOfReading];
 
   // Initiate the trigger to read accelerometers simultaneously
   int bytes_written = base::WriteFile(
@@ -302,26 +428,28 @@ 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;
-  }
-
   update_ = new AccelerometerUpdate();
-
-  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]);
+  for (auto reading_data : configuration_.reading_data) {
+    int reading_size = reading_data.sources.size() * kSizeOfReading;
+    DCHECK_GT(reading_size, 0);
+    char reading[reading_size];
+    int bytes_read = base::ReadFile(reading_data.path, reading, reading_size);
+    if (bytes_read < reading_size) {
+      LOG(ERROR) << "Accelerometer Read " << bytes_read << " byte(s), expected "
+                 << reading_size << " bytes from accelerometer "
+                 << reading_data.path.MaybeAsASCII();
+      return;
+    }
+    for (AccelerometerSource source : reading_data.sources) {
+      DCHECK(configuration_.has[source]);
+      int16* values = reinterpret_cast<int16*>(reading);
+      update_->Set(source, values[configuration_.index[source][0]] *
+                               configuration_.scale[source][0],
+                   values[configuration_.index[source][1]] *
+                       configuration_.scale[source][1],
+                   values[configuration_.index[source][2]] *
+                       configuration_.scale[source][2]);
+    }
   }
 
   observers_->Notify(FROM_HERE,