[sensors] Introduce Generic Sensor API interfaces

device/sensors/public/interfaces/sensor_factory.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]

No nested namespaces please (we have to fix some existing cases like
device.usb). The only exception is new mojom should nest an inner mojom
namespace to disambiguate from non-mojom symbols.

Please change this to

module device.mojom;

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

Done.

+
+import "sensor.mojom";
+
+// Interface that allows its clients to create instances of Sensor inerface.
+//
+// @param type - SensorType, type of the sensor that should be created.

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

This argument documentation syntax seems arbitrary (it kind of mimics other
styles but not quite) and doesn't match anything we use elsewhere.

We don't have a documentation syntax for mojom, just like we don't have one for
chromium C++ code, and I don't think we need to adopt one. Can you please just
document this like you would any other C++ class method?

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

Done.

+// @param sensor_request - Sensor interface that is initialized by the factory.
+// @return result - Result of the request, SUCCESS on success, FAILURE otherwise.
+// @return shmem  - SharedBuffer handle that is used by the client to fetch
+//                  sensor reading, or null if the request has failed.
+// @return offset - The offset at which SharedBuffer must be mapped by client.
+// @return size   - The size of a sensor reading.
+// @return mode   - The ReportingMode that is supported by the sensor.
+interface SensorFactory {

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

nit: Maybe SensorProvider? It's not really a factory, it doesn't really create
sensors so much as it provides sensor interfaces.

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

Done.

+  CreateSensor(SensorType type, Sensor& sensor_request) => (
+      Result result,
+      handle<shared_buffer>? shmem,
+      uint64 offset,
+      uint64 size,
+      ReportingMode mode);
+};

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

If the only thing you can do is "Create" Sensor, and the only thing you can do
with a Sensor interface is Start or Stop or SetClient, maybe we can simply
everything a bit? Here's my strawman proposal:

// Combine the shm details in a struct for clarity.
// (documentation explaining buffer layout)
struct SensorReadBuffer {
  handle<shared_buffer> memory;
  uint64 offset;
  uint64 size;
};

// One Sensor interface per monitoring instance. Then it only needs Stop().
interface Sensor {
  Stop();
};

interface SensorClient {
  RaiseError();
  UpdateReading();
};

interface SensorProvider {
  // |read_buffer| is null on failure. If the sensor supports async
  // notifications rather than continuous updates, |client_request| will
  // be non-null and should be bound by the client.
  MonitorSensor(SensorType type, double frequency, Sensor& sensor_request)
      => (SensorReadBuffer? read_buffer, SensorClient&? client_request);
};


This covers a lot of things: it removes uncertainty about what seem like
potentially fragile interactions between Start/Stop/SetClient, it eliminates
Result, eliminates ReportingMode, eliminates a need for Stop() to return a
result, and eliminates the need for a "Configuration" struct since the
configuration is only specified in one place, the MonitorSensor args.

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

Unfortunately, it is a more complex.

On browser side, we can have multiple Sensor interface instances
"Sensor(SensorType)" (one per frame). Each frame can have multiple users (JS
objects) of this interface and each JS instance has
it's own configuraion, which is added / removed when JS instance calls start /
stop method.

[Ken Rockot(use gerrit already), 2016/06/29 15:03:48]

OK but why not just have each JS instance get its own Sensor interface?