Gamepad API support for chrome on android

content/browser/gamepad/gamepad_platform_data_fetcher_android.cc

+// Copyright (c) 2014 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 "base/strings/string_number_conversions.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "content/browser/gamepad/gamepad_platform_data_fetcher_android.h"
+#include "third_party/WebKit/public/platform/WebGamepads.h"
+
+#if defined(OS_ANDROID)

[scottmg, 2014/02/06 21:17:07]

Remove this, it shouldn't be compiled on other platforms.

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Done.

+
+namespace content {
+
+GamepadPlatformDataFetcherAndroid::GamepadPlatformDataFetcherAndroid() {
+}
+
+void GamepadPlatformDataFetcherAndroid::GetGamepadData(
+                                            blink::WebGamepads* pads, bool) {

[scottmg, 2014/02/06 21:17:07]

nit; this should be +4. git cl format or clang-format?

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Done.

+  GamepadsReader* m_gamepadsReader = GamepadsReader::GetInstance();

[scottmg, 2014/02/06 21:17:07]

No m_, and chromium C++ code should use hacker_style for locals (also
ifDeviceConnected below)

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Done.

+
+  pads->length = m_gamepadsReader->UpdateGamepadsCount();
+  if (pads->length == 0)
+    return;
+
+  for (unsigned int i = 0; i < WebGamepads::itemsLengthCap; i++) {
+    bool ifDeviceConnected = m_gamepadsReader->IsDeviceConnected(i);
+    if (ifDeviceConnected) {
+      // Set connected flag to true for all the gamepad devices that are
+      // attached.
+      pads->items[i].connected = true;
+
+      // Map the Gamepad DeviceName String to the WebGamepad Id and then
+      // convert the utf-8 id string to WebUChar.
+      // Ideally it should be mapped to vendor and product information
+      // but it is only available at kernel level and it can not be
+      // queried using class android.hardware.input.InputManager.

[scottmg, 2014/02/06 21:17:07]

Is it possible to correlate to a /dev/ or something? Would that generally be
readable?

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

/dev/ is not readable for android. We instead depend on framework classes
MotionRanges and KeyCharacterMap and we extract gamepad data from
dispatchKeyEvent and dispatchMotionEvent callbacks.
1. We extract the info of all the device connected and identify gamepad devices
out of these.
2. Get the number of axes by counting axes manually.
3. Get the number of buttons by quering each keycode and then the connected
device tells us if that keycode can be produced.

+
+      std::string string1 = m_gamepadsReader->GetDeviceName(i);
+      base::TruncateUTF8ToByteSize(string1, WebGamepad::idLengthCap - 1,

[scottmg, 2014/02/06 21:17:07]

Why truncate the UTF8 rather than the UTF16? Is this really UTF8 in the first
place or ASCII?

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Yes, it is UTF8 that gets returned from GamepadReader.
I have tried to keep it parallel to linux file.

+                                   &string1);
+      base::string16 tmp16 = base::UTF8ToUTF16(string1);
+      memset(pads->items[i].id, 0, sizeof(pads->items[i].id));
+      tmp16.copy(pads->items[i].id, arraysize(pads->items[i].id) - 1);
+
+      // timestamp is queried from GamepadReader.
+      pads->items[i].timestamp = m_gamepadsReader->GetDeviceTimestamp(i);

[scottmg, 2014/02/06 21:17:07]

Is this the only place this is used? If so, it could just be a monotonically
increasing int.

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

The timestamp is not monotonically increasing. It is the system time when the
last update has come from the connected gamepad device.
i.e. When a key press event of Motion update event has been recieved from the
device.

+
+      // A float vector is queried from GamepadReader that stores the
+      // state of axeses.

[scottmg, 2014/02/06 21:17:07]

nit; "axes"

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Done.

+      std::vector<float> axes = m_gamepadsReader->GetDeviceAxes(i);

[scottmg, 2014/02/06 21:17:07]

So many copies from kernel to content. :(

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

We are not reading it from the kernel, instead we are reading it from a java
object (GamepadList).
GamepadList has already maintained the state of connected GamepadDevices.
GamepadList is updated only when if there is any change in the state of
connected gamepad devices(KeyEvent or MotionEvent comes from one of the
connected devices).
It is the responsiblity of GamepadList to provide the gamepad data in Standard
gamepad format for the known devices.

+
+      // Set WebGamepad axeslength to total number of axes on the gamepad
+      // device.
+      pads->items[i].axesLength = axes.size();
+
+      // A float vector is queried from GamepadReader that stores the
+      // button's state.
+      std::vector<float> buttons = m_gamepadsReader->GetDeviceButtons(i);
+
+      // Set WebGamepad buttonslength to total numbers of buttons on the
+      // gamepad device.
+      pads->items[i].buttonsLength = buttons.size();
+
+      // Set WebGamepad connected flag to false if buttonslength or
+      // axeslength returned by
+      // JNI are larger than permitted value.

[scottmg, 2014/02/06 21:17:07]

Is this really what we want? It would probably be better just to return the
first axesLengthCap and buttonsLengthCap of them.

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Done.

+      if (pads->items[i].axesLength > WebGamepad::axesLengthCap ||
+            pads->items[i].buttonsLength > WebGamepad::buttonsLengthCap) {

[scottmg, 2014/02/06 21:17:07]

nit; indent wrong

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Done.

+        pads->items[i].connected = false;
+      } else {
+        // Copy axes state to the WebGamepad axes[].
+        for (unsigned int j = 0; j < pads->items[i].axesLength; j++)
+          pads->items[i].axes[j] = axes[j];
+
+        // Copy buttons state to the WebGamepad buttons[].
+        for (unsigned int j = 0; j < pads->items[i].buttonsLength; j++)
+          pads->items[i].buttons[j] = buttons[j];
+      }
+    } else {
+      // Set connected flag to flase for all the gamepad devices that are
+      // not attached.
+      pads->items[i].connected = false;
+    }
+  }
+}
+
+void GamepadPlatformDataFetcherAndroid::PauseHint(bool isaccesspaused) {
+  GamepadsReader::GetInstance()->NotifyForGamepadsAccess(isaccesspaused);
+}
+
+GamepadPlatformDataFetcherAndroid::~GamepadPlatformDataFetcherAndroid() {

[scottmg, 2014/02/06 21:17:07]

Move this up with the constructor to match the header order.

[SaurabhK, 2014/02/10 12:50:47]

On 2014/02/06 21:17:07, scottmg wrote:

Done.

+}
+
+}  // namespace content
+
+#endif