[sensors] Introduce Generic Sensor API interfaces

device/sensors/public/interfaces/sensor.mojom

+// 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.
+
+module device.sensors;

[Ken Rockot(use gerrit already), 2016/06/28 16:18:29]

See note in other mojom file about module names

[shalamov, 2016/06/29 15:02:17]

Done.

+
+// Types of supported sensors
+enum SensorType {
+  FIRST = 1,
+  AMBIENT_LIGHT = FIRST,
+  PROXIMITY,
+  LINEAR_ACCELERATION,
+  GYROSCOPE,
+  PRESSURE,
+  LAST = PRESSURE // Note: LAST is also equal to the types count.
+};
+
+// Reporting mode supported by the Sensor.
+// ON_CHANGE  - client will be notified through OnSensorReadingChanged() signal
+//              whenever sensor reading is changed.
+// CONTINUOUS - sensor will continuously update its reading with frequency
+//              specified in SensorConfiguration.frequency.
+//              OnSensorReadingChanged() signal is not sent to the client for
+//              sensors with CONTINUOUS reporting mode.
+enum ReportingMode {
+  ON_CHANGE,
+  CONTINUOUS
+};
+
+enum Result {
+  SUCCESS,
+  FAILURE
+};
+
+// Sensor reading buffer is putting fields of 64-bit floating point type in the

[Ken Rockot(use gerrit already), 2016/06/28 16:18:29]

This description could be clearer and doesn't really make sense floating in the
middle of the mojom here. I would include this information near the interface
method which provides the shared buffer, and I would be explicit about field
sizes and layout. i.e., note that they are tightly packed 64-bit floating point
values, the full buffer has a fixed size of 8+n*8 bytes for n fields, etc.
Anything to clarify.

[shalamov, 2016/06/29 15:02:17]

Done.

+// following order:
+// 1) timestamp
+// 2) sensor reading fields
+
+// Total count of fields in sensor reading (timestamp, values[3])
+const uint8 kSensorReadingFieldsCount = 4;
+
+// Size of a single sensor reading field in bytes.
+const uint8 kSensorReadingFieldSize = 8;
+
+struct SensorConfiguration {
+  // Frequency in Hz
+  double frequency;
+  // TODO(shalamov): Add map<string, union> for sensor specific configuration.
+};
+
+// Interface for controlling the Sensor.
+interface Sensor {
+  // Sets SensorClient interface that is used to notify client when sensor
+  // reading is changed (only for sensors with ReportingMode::ON_CHANGE), or
+  // when error is raised by Sensor interface implementation.
+  SetClient(SensorClient client);
+
+  // Requests sensor to start reading sensor data with specified
+  // SensorConfiguration. Returns SUCCESS on success, FAILURE otherwise.
+  Start(SensorConfiguration configuration) => (Result result);
+
+  // Requests sensor to stop reading sensor data for specified
+  // SensorConfiguration. Returns SUCCESS on success, FAILURE otherwiseS.
+  Stop(SensorConfiguration configuration) => (Result result);

[Ken Rockot(use gerrit already), 2016/06/28 16:18:29]

How is an implementation supposed to disambiguate sensor monitoring instances
(i.e. calls to Start) by |configuration| only? Seems dubious to simply compare
configuration fields by value or something like that.

And is it even valid to call Start() (and thus Stop()) more than once on a
single Sensor interface? Should it not be?

[shalamov, 2016/06/29 15:02:17]

We don't need to know anything about monitoring instances. Everytime new JS
instance is created, controller on blink side will add configuration that is
specific
for that instance (WIP: https://codereview.chromium.org/2051083002). On browser
side, optimal configuration is selected everytime new configuration added /
removed.

+};
+
+// Interface that client of the Sensor interface must implement to observe
+// sensor reading changes and error conditions.
+interface SensorClient {
+  // Signals SensorClient when there is an error.
+  OnSensorError();

[Ken Rockot(use gerrit already), 2016/06/28 16:18:29]

I think this pattern ("On" method naming) has been adopted in many places for
client interfaces, but it seems redundant to me if every client interface has a
bunch of On* methods. How about just

  RaiseError()
  UpdateReading()

? Then the client interface methods are imperative verb phrases like other
interface methods and they make plenty of sense in the context of "SensorClient"

[shalamov, 2016/06/29 15:02:17]

Done.

+
+  // Signals SensorClient when reading has been changed (only for sensors with
+  // ReportingMode::ON_CHANGE).
+  OnSensorReadingChanged();
+};