Add RELATIVE_ORIENTATION sensor implementation on macOS to //device/generic_sensor

device/generic_sensor/platform_sensor_fusion_relative_orientation.cc

+// Copyright 2017 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 "device/generic_sensor/platform_sensor_fusion_relative_orientation.h"
+
+#include <stdint.h>
+
+#include <cmath>
+
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/memory/ptr_util.h"
+#include "device/generic_sensor/generic_sensor_consts.h"
+#include "device/generic_sensor/platform_sensor_provider.h"
+#include "device/generic_sensor/public/cpp/sensor_shared_buffer_reader.h"
+#include "device/generic_sensor/relative_orientation_fusion_algorithm_using_accelerometer.h"
+
+namespace {
+
+constexpr double kQuaternionThreshold = 0.01;

[Mikhail, 2017/06/20 13:50:00]

where did this value come from?

[juncai, 2017/06/20 19:50:55]

The quaternion is a unit quaternion so I thought 0.01 is a reasonable threshold
for detecting any changes.

Any suggestion of better value for this parameter? Thanks!

[timvolodine, 2017/06/21 18:22:11]

There should be some rational explanation for this threshold. e.g. for Euler
angles we had a threshold of 0.1 degree (to filter out potential noise).

[juncai, 2017/07/20 00:22:16]

In the new CL:
https://chromium-review.googlesource.com/c/578253/
This threshold can be adjusted and documented for different sensor fusion
algorithms.

+
+bool IsSignificantlyDifferent(const device::SensorReading& reading1,
+                              const device::SensorReading& reading2) {
+  return (std::fabs(reading1.values[0] - reading2.values[0]) >=
+          kQuaternionThreshold) ||
+         (std::fabs(reading1.values[1] - reading2.values[1]) >=
+          kQuaternionThreshold) ||
+         (std::fabs(reading1.values[2] - reading2.values[2]) >=
+          kQuaternionThreshold);
+}
+
+}  // namespace
+
+namespace device {
+
+PlatformSensorFusionRelativeOrientation::
+    PlatformSensorFusionRelativeOrientation(
+        mojo::ScopedSharedBufferMapping mapping,
+        PlatformSensorProvider* provider,
+        const PlatformSensorProviderBase::CreateSensorCallback& callback)
+    : PlatformSensor(mojom::SensorType::RELATIVE_ORIENTATION,
+                     std::move(mapping),
+                     provider),
+      callback_(callback) {
+  provider->CreateSensor(

[Mikhail, 2017/06/20 13:50:00]

think it's worth first look for existing sensors using 'provider->GetSensor()'

[juncai, 2017/06/20 19:50:55]

Done.

+      mojom::SensorType::ACCELEROMETER,
+      base::Bind(
+          &PlatformSensorFusionRelativeOrientation::CreateAccelerometerCallback,
+          base::Unretained(this)));
+}
+
+PlatformSensorFusionRelativeOrientation::
+    ~PlatformSensorFusionRelativeOrientation() {
+  if (accelerometer_)
+    accelerometer_->RemoveClient(this);
+}
+
+mojom::ReportingMode
+PlatformSensorFusionRelativeOrientation::GetReportingMode() {
+  return accelerometer_->GetReportingMode();
+}
+
+PlatformSensorConfiguration
+PlatformSensorFusionRelativeOrientation::GetDefaultConfiguration() {
+  return PlatformSensorConfiguration(kDefaultAccelerometerFrequencyHz);

[Mikhail, 2017/06/20 13:50:00]

why not use accelerometer_->GetDefaultConfiguration() ?

[juncai, 2017/06/20 19:50:55]

Done.

+}
+
+bool PlatformSensorFusionRelativeOrientation::StartSensor(
+    const PlatformSensorConfiguration& configuration) {
+  if (!accelerometer_->StartSensor(configuration))
+    return false;
+
+  accelerometer_reader_ = base::MakeUnique<SensorSharedBufferReader>(

[Mikhail, 2017/06/20 13:50:00]

instead of introducing SensorSharedBufferReader I'd add "bool
PlatfromSensor::GetLatestReading(Reading*)" API, thus we do not have to expose
shared_buffer_mapping()

[juncai, 2017/06/20 19:50:55]

Done.

+      static_cast<const SensorReadingSharedBuffer*>(
+          accelerometer_->shared_buffer_mapping().get()));
+
+  return true;
+}
+
+void PlatformSensorFusionRelativeOrientation::StopSensor() {
+  accelerometer_->StopSensor();
+}
+
+void PlatformSensorFusionRelativeOrientation::OnSensorReadingChanged() {
+  SensorReading reading;
+  reading.timestamp = (base::TimeTicks::Now() - base::TimeTicks()).InSecondsF();
+
+  if (!accelerometer_reader_->GetSensorReading(&reading))
+    return;
+
+  fusion_algorithm_->set_acceleration(reading.values[0], reading.values[1],
+                                      reading.values[2]);
+
+  fusion_algorithm_->GetRelativeOrientationData(
+      &reading.values[0].value(), &reading.values[1].value(),
+      &reading.values[2].value(), &reading.values[3].value());
+
+  if (GetReportingMode() == mojom::ReportingMode::ON_CHANGE &&
+      !IsSignificantlyDifferent(reading_, reading)) {
+    return;
+  }
+
+  reading_ = reading;
+  UpdateSensorReading(reading_,
+                      GetReportingMode() == mojom::ReportingMode::ON_CHANGE);
+}
+
+void PlatformSensorFusionRelativeOrientation::OnSensorError() {
+  NotifySensorError();
+}
+
+bool PlatformSensorFusionRelativeOrientation::IsNotificationSuspended() {

[Mikhail, 2017/06/20 13:50:00]

think here the suspend status of its own clients should be checked, otherwise
the contained "accelerometer_" can never be stopped when we've gone to suspended
state

[juncai, 2017/06/20 19:50:55]

Done.

+  return false;
+}
+
+bool PlatformSensorFusionRelativeOrientation::CheckSensorConfiguration(
+    const PlatformSensorConfiguration& configuration) {
+  return configuration.frequency() > 0 &&

[Mikhail, 2017/06/20 13:50:00]

why not forward this call to the contained "accelerometer_" ?

[juncai, 2017/06/20 19:50:55]

Done.

+         configuration.frequency() <=
+             mojom::SensorConfiguration::kMaxAllowedFrequency;
+}
+
+void PlatformSensorFusionRelativeOrientation::CreateAccelerometerCallback(
+    scoped_refptr<PlatformSensor> accelerometer) {
+  accelerometer_ = accelerometer;
+  if (accelerometer_) {
+    accelerometer_->AddClient(this);
+    fusion_algorithm_ = base::MakeUnique<
+        RelativeOrientationFusionAlgorithmUsingAccelerometer>();
+    callback_.Run(scoped_refptr<PlatformSensor>(this));
+  } else {
+    callback_.Run(nullptr);
+  }
+}
+
+}  // namespace device