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..7887b6e0ea2df8e6005cca8705f1c66e690f1345 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.

[flackr, 2015/08/27 22:32:11]

nit: Add to comment that the order of axes was flipped on kernel 3.18+ so that
it's obvious it's not a typo :-).

[jonross, 2015/08/27 22:59:06]

Done.

-const char kAccelerometerAxes[][2] = {"y", "x", "z"};
+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 separate_devices = false;

[flackr, 2015/08/27 22:32:11]

nit: s/separate_devices/legacy_cros_accel

[jonross, 2015/08/27 22:59:06]

Done.

+  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;
+    separate_devices = base::ReadFileToString(
+        base::FilePath(iio_path).Append(kAccelerometerLocationFileName),
+        &location);
+    if (separate_devices) {
+      base::TrimWhitespaceASCII(location, base::TRIM_ALL, &location);
+      if (!InitializeAccelerometer(iio_path, name, location))
+        return;
+    } else {
+      if (!InitializeLegacyAccelerometers(iio_path, name))
+        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 = separate_devices ? kAccelerometerAxes[j]
+                                            : kLegacyAccelerometerAxes[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 the scale for all axes.
+  int scale_divisor = 0;

[flackr, 2015/08/27 22:32:11]

nit: Move declaration to for loop scope right before read of scale file.

[jonross, 2015/08/27 22:59:06]

Done.

+  // 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]);
+    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,29 @@ 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) {
+      if (!configuration_.has[source])
+        continue;
+      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,