[sensors] [win] Implement ambient light sensor for Windows platform

device/generic_sensor/platform_sensor_reader_win.cc

+// Copyright 2016 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_reader_win.h"
+
+#include <Sensors.h>
+
+#include "base/callback.h"
+#include "base/time/time.h"
+#include "base/win/iunknown_impl.h"
+#include "device/generic_sensor/public/cpp/platform_sensor_configuration.h"
+#include "device/generic_sensor/public/cpp/sensor_reading.h"
+
+namespace device {
+
+// Init params for the PlatformSensorReaderWin.
+struct ReaderInitParams {
+  // ISensorDataReport::GetSensorValue is not const, therefore, report
+  // cannot be passed as const ref.
+  // ISensorDataReport& report - report that contains new sensor data.
+  // SensorReading& reading    - out parameter that must be populated.
+  // Returns HRESULT           - S_OK on success, otherwise error code.
+  using ReaderFunctor = base::Callback<HRESULT(ISensorDataReport& report,
+                                               SensorReading& reading)>;
+  SENSOR_TYPE_ID sensor_type_id;
+  mojom::ReportingMode reporting_mode;
+  ReaderFunctor reader_func;
+  unsigned long min_reporting_interval_ms = 0;
+};
+
+namespace {
+
+// Gets value from  the report for provided key.
+bool GetReadingValueForProperty(REFPROPERTYKEY key,
+                                ISensorDataReport& report,
+                                double* value) {
+  DCHECK(value);
+  PROPVARIANT variant_value = {};
+  if (SUCCEEDED(report.GetSensorValue(key, &variant_value))) {
+    if (variant_value.vt == VT_R8)
+      *value = variant_value.dblVal;
+    else if (variant_value.vt == VT_R4)
+      *value = variant_value.fltVal;
+    else
+      return false;
+    return true;
+  }
+
+  *value = 0;
+  return false;
+}
+
+// Creates ReaderInitParams params structure. To implement support for new
+// sensor types, new switch case should be added and appropriate fields must
+// be set:
+// sensor_type_id - GUID of the sensor supported by Windows.
+// reporting_mode - mode of reporting (ON_CHANGE | CONTINUOUS).
+// reader_func    - Functor that is responsible to populate SensorReading from
+//                  ISensorDataReport data.
+std::unique_ptr<ReaderInitParams> CreateReaderInitParamsForSensor(
+    mojom::SensorType type) {
+  auto params = std::make_unique<ReaderInitParams>();
+  switch (type) {
+    case mojom::SensorType::AMBIENT_LIGHT: {
+      params->sensor_type_id = SENSOR_TYPE_AMBIENT_LIGHT;
+      params->reporting_mode = mojom::ReportingMode::ON_CHANGE;
+      params->reader_func =
+          base::Bind([](ISensorDataReport& report, SensorReading& reading) {
+            double lux = 0.0;
+            if (!GetReadingValueForProperty(SENSOR_DATA_TYPE_LIGHT_LEVEL_LUX,
+                                            report, &lux)) {
+              return E_FAIL;
+            }
+            reading.values[0] = lux;
+            return S_OK;
+          });
+      return params;
+    }
+    default:
+      NOTIMPLEMENTED();
+      return nullptr;
+  }
+}
+}  // namespace
+
+// Class that implements ISensorEvents and IUnknown interfaces and used
+// by ISensor interface to dispatch state and data change events.
+class EventListener : public ISensorEvents, public base::win::IUnknownImpl {
+ public:
+  explicit EventListener(PlatformSensorReaderWin* platform_sensor_reader)
+      : platform_sensor_reader_(platform_sensor_reader) {
+    DCHECK(platform_sensor_reader_);
+  }
+
+  // IUnknown interface
+  ULONG STDMETHODCALLTYPE AddRef() override { return IUnknownImpl::AddRef(); }
+  ULONG STDMETHODCALLTYPE Release() override { return IUnknownImpl::Release(); }
+
+  STDMETHODIMP QueryInterface(REFIID riid, void** ppv) override {
+    if (riid == __uuidof(ISensorEvents)) {
+      *ppv = static_cast<ISensorEvents*>(this);
+      AddRef();
+      return S_OK;
+    }
+    return IUnknownImpl::QueryInterface(riid, ppv);
+  }
+
+ protected:
+  ~EventListener() override = default;
+
+  // ISensorEvents interface
+  STDMETHODIMP OnEvent(ISensor*, REFGUID, IPortableDeviceValues*) override {
+    return S_OK;
+  }
+
+  STDMETHODIMP OnLeave(REFSENSOR_ID sensor_id) override {
+    // If event listener is active and sensor is disconnected, notify client
+    // about the error.
+    platform_sensor_reader_->SensorError();
+    platform_sensor_reader_->StopSensor();
+    return S_OK;
+  }
+
+  STDMETHODIMP OnStateChanged(ISensor* sensor, SensorState state) override {
+    if (sensor == nullptr)
+      return E_INVALIDARG;
+
+    if (state != SensorState::SENSOR_STATE_READY &&
+        state != SensorState::SENSOR_STATE_INITIALIZING) {
+      platform_sensor_reader_->SensorError();
+      platform_sensor_reader_->StopSensor();
+    }
+    return S_OK;
+  }
+
+  STDMETHODIMP OnDataUpdated(ISensor* sensor,
+                             ISensorDataReport* report) override {
+    if (sensor == nullptr || report == nullptr)
+      return E_INVALIDARG;
+
+    // To get precise timestamp, we need to get delta between timestamp
+    // provided in the report and current system time. Then the delta in
+    // milliseconds is substracted from current high resolution timestamp.
+    SYSTEMTIME report_time;
+    HRESULT hr = report->GetTimestamp(&report_time);
+    if (FAILED(hr))
+      return hr;
+
+    base::TimeTicks ticks_now = base::TimeTicks::Now();

[miu, 2016/10/27 20:01:02]

1. How precise a measurement do you need? When sampling the two clocks, it's
possible for things like thread-context switching to cause 50+ ms to elapse
between each sampling (we hit this issue in the unit tests for base::Time, and
had to do special things to mitigate). Is this amount of error acceptable to
your use cases?

2. How often is OnDataUpdated() called? FWICT, this would probably be no more
than a few times in a second, so what you're doing here should be okay. If it's
more frequent, you may need to consider a "fancier" solution here. ;-)

+    base::Time time_now = base::Time::NowFromSystemTime();
+
+    base::Time::Exploded exploded;
+    exploded.year = report_time.wYear;
+    exploded.month = report_time.wMonth;
+    exploded.day_of_week = report_time.wDayOfWeek;
+    exploded.day_of_month = report_time.wDay;
+    exploded.hour = report_time.wHour;
+    exploded.minute = report_time.wMinute;
+    exploded.second = report_time.wSecond;
+    exploded.millisecond = report_time.wMilliseconds;
+
+    base::Time timestamp;
+    if (!base::Time::FromUTCExploded(exploded, &timestamp))
+      return E_FAIL;
+
+    base::TimeDelta delta = time_now - timestamp;
+
+    SensorReading reading;
+    reading.timestamp = (ticks_now - (delta + base::TimeTicks())).InSecondsF();

[miu, 2016/10/27 20:01:01]

1. Semantically, it seems the parentheses are in the wrong place here; although,
the math itself does the right thing. I think this statement should be:

  reading.timestamp = ((ticks_now - delta) - base::TimeTicks()).InSecondsF();

2. IMHO, the SensorReading::timestamp field should be of type base::TimeTicks
(this is allowed in Blink code), and the conversion to Blink's performance time
data type should occur at the last possible moment in
blink::SensorReading::timestamp(). This has the advantage that *all* time data
type/value translation is in one code module (right now it is split between here
and the other file). In general, in Chromium code, try to keep time values in
the base::TimeXXX types; and only map to other data types when necessary.

3. base::Time is not a monotonically increasing clock, and it may have jumped
around in some unpredictable way (between when the report's timestamp was
sampled and when time_now was sampled). Therefore, it's possible that the
TimeTicks values you're computing (based on them) will not be monotonically
increasing. You should consider adding some code to check that. Something like:

  if (last_sensor_reading_.timestamp > reading.timestamp) {
    reading.timestamp = last_sensor_reading_.timestamp;
    // Or, maybe it would be better to not report this new reading? Depends on
your intended use cases...
  }

+
+    return platform_sensor_reader_->SensorReadingChanged(*report, reading);
+  }
+
+ private:
+  PlatformSensorReaderWin* platform_sensor_reader_;
+
+  DISALLOW_COPY_AND_ASSIGN(EventListener);
+};
+
+// static
+std::unique_ptr<PlatformSensorReaderWin> PlatformSensorReaderWin::Create(
+    mojom::SensorType type,
+    base::win::ScopedComPtr<ISensorManager> sensor_manager) {
+  DCHECK(sensor_manager);
+
+  auto params = CreateReaderInitParamsForSensor(type);
+  if (!params)
+    return nullptr;
+
+  auto sensor = GetSensorForType(params->sensor_type_id, sensor_manager);
+  if (!sensor)
+    return nullptr;
+
+  PROPVARIANT variant = {};
+  if (SUCCEEDED(
+          sensor->GetProperty(SENSOR_PROPERTY_MIN_REPORT_INTERVAL, &variant))) {
+    if (variant.vt == VT_UI4)
+      params->min_reporting_interval_ms = variant.ulVal;
+  }
+
+  GUID interests[] = {SENSOR_EVENT_STATE_CHANGED, SENSOR_EVENT_DATA_UPDATED};
+  HRESULT hr = sensor->SetEventInterest(interests, arraysize(interests));
+  if (FAILED(hr))
+    return nullptr;
+
+  return base::WrapUnique(
+      new PlatformSensorReaderWin(sensor, std::move(params)));
+}
+
+// static
+base::win::ScopedComPtr<ISensor> PlatformSensorReaderWin::GetSensorForType(
+    REFSENSOR_TYPE_ID sensor_type,
+    base::win::ScopedComPtr<ISensorManager> sensor_manager) {
+  base::win::ScopedComPtr<ISensor> sensor;
+  base::win::ScopedComPtr<ISensorCollection> sensor_collection;
+  HRESULT hr = sensor_manager->GetSensorsByType(sensor_type,
+                                                sensor_collection.Receive());
+  if (FAILED(hr) || !sensor_collection)
+    return sensor;
+
+  ULONG count = 0;
+  hr = sensor_collection->GetCount(&count);
+  if (FAILED(hr) || count == 0)
+    return sensor;
+
+  sensor_collection->GetAt(0, sensor.Receive());
+  return sensor;
+}
+
+PlatformSensorReaderWin::PlatformSensorReaderWin(
+    base::win::ScopedComPtr<ISensor> sensor,
+    std::unique_ptr<ReaderInitParams> params)
+    : init_params_(std::move(params)),
+      task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      sensor_active_(false),
+      client_(nullptr),
+      sensor_(sensor),
+      event_listener_(new EventListener(this)) {
+  DCHECK(init_params_);
+  DCHECK(!init_params_->reader_func.is_null());
+  DCHECK(sensor_);
+}
+
+void PlatformSensorReaderWin::SetClient(Client* client) {
+  base::AutoLock autolock(lock_);
+  // Can be null.
+  client_ = client;
+}
+
+void PlatformSensorReaderWin::StopSensor() {
+  base::AutoLock autolock(lock_);
+  if (sensor_active_) {
+    sensor_->SetEventSink(nullptr);
+    sensor_active_ = false;
+  }
+}
+
+PlatformSensorReaderWin::~PlatformSensorReaderWin() {
+  DCHECK(task_runner_->BelongsToCurrentThread());
+}
+
+bool PlatformSensorReaderWin::StartSensor(
+    const PlatformSensorConfiguration& configuration) {
+  base::AutoLock autolock(lock_);
+
+  if (!SetReportingInterval(configuration))
+    return false;
+
+  // Set event listener.
+  if (!sensor_active_) {
+    base::win::ScopedComPtr<ISensorEvents> sensor_events;
+    HRESULT hr = event_listener_->QueryInterface(__uuidof(ISensorEvents),
+                                                 sensor_events.ReceiveVoid());
+
+    if (FAILED(hr) || !sensor_events)
+      return false;
+
+    if (FAILED(sensor_->SetEventSink(sensor_events.get())))
+      return false;
+
+    sensor_active_ = true;
+  }
+
+  return true;
+}
+
+bool PlatformSensorReaderWin::SetReportingInterval(
+    const PlatformSensorConfiguration& configuration) {
+  base::win::ScopedComPtr<IPortableDeviceValues> props;
+  if (SUCCEEDED(props.CreateInstance(CLSID_PortableDeviceValues))) {
+    unsigned interval =
+        (1 / configuration.frequency()) * base::Time::kMillisecondsPerSecond;
+    if (SUCCEEDED(props->SetUnsignedIntegerValue(
+            SENSOR_PROPERTY_CURRENT_REPORT_INTERVAL, interval))) {
+      base::win::ScopedComPtr<IPortableDeviceValues> return_props;
+      sensor_->SetProperties(props.get(), return_props.Receive());
+      return true;
+    }
+  }
+  return false;
+}
+
+HRESULT PlatformSensorReaderWin::SensorReadingChanged(
+    ISensorDataReport& report,
+    SensorReading& reading) const {
+  if (!client_)
+    return E_FAIL;
+
+  HRESULT hr = init_params_->reader_func.Run(report, reading);
+  if (SUCCEEDED(hr))
+    client_->OnReadingUpdated(reading);
+  return hr;
+}
+
+void PlatformSensorReaderWin::SensorError() {
+  if (client_)
+    client_->OnSensorError();
+}
+
+unsigned long PlatformSensorReaderWin::GetMinimalReportingIntervalMs() const {
+  return init_params_->min_reporting_interval_ms;
+}
+
+mojom::ReportingMode PlatformSensorReaderWin::GetReportingMode() const {
+  return init_params_->reporting_mode;
+}
+
+}  // namespace device