From b91776048d86f8de3ecf9cbc2db6da0cda3102ad Mon Sep 17 00:00:00 2001 From: Raphael Kubo da Costa Date: Wed, 24 Jan 2024 08:27:39 +0000 Subject: [PATCH] sensors: Read Euler angles instead of quaternions from WebDriver As a result of https://github.com/w3c/deviceorientation/pull/124 and https://github.com/w3c/orientation-sensor/pull/83. The reading format is alpha-beta-gamma as described in the spec. They are measured in degrees and must fall within certain ranges. Internally, however, we always perform the Euler angles to quaternions conversion at the edges (i.e. in ChromeDriver and the Internals implementation used by content_shell), so that the the CDP and //services layers remain unchanged and continue to support only quaternions and the {ABSOLUTE,RELATIVE}_ORIENTATION_QUATERNION types for simplicity. The code to convert Euler angles to quaternions was copied from SensorInspectorAgent in Blink and is available for use by any callers that need to validate Euler angles and convert them. The original code remains in place because the entirety of the SensorInspectorAgent will be removed soon due to the work on bug 1506995. The test values for the orientation-sensor web tests had to be adapted: we now provide the inputs as Euler angles. The expected values have changed slightly as we had to find Euler _and_ quaternion values that were easy enough to read. Written in collaboration with Juha Vainio Bug: 1506995, 1520912, 1520919 Validate-Test-Flakiness: skip Change-Id: I047f41f172f0bbcf30c7462926cec7ae0a66d4e5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5229177 Commit-Queue: Raphael Kubo Da Costa Reviewed-by: danakj Reviewed-by: Vladimir Nechaev Reviewed-by: Reilly Grant Cr-Commit-Position: refs/heads/main@{#1251287} --- BUILD.gn | 1 + session_commands.cc | 43 ++++++++++++++++++++-------------- test/run_py_tests.py | 56 ++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 78 insertions(+), 22 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 41234964..b0adb2d5 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -360,6 +360,7 @@ source_set("lib") { "//net", "//net/server:http_server", "//net/traffic_annotation:test_support", + "//services/device/public/cpp/generic_sensor", "//services/network:network_service", "//services/network/public/cpp", "//services/network/public/mojom", diff --git a/session_commands.cc b/session_commands.cc index 85b30da6..4b36413f 100644 --- a/session_commands.cc +++ b/session_commands.cc @@ -44,6 +44,7 @@ #include "chrome/test/chromedriver/logging.h" #include "chrome/test/chromedriver/session.h" #include "chrome/test/chromedriver/util.h" +#include "services/device/public/cpp/generic_sensor/orientation_util.h" #include "services/network/public/mojom/url_loader_factory.mojom.h" namespace { @@ -1168,30 +1169,31 @@ bool ParseXYZValue(const base::Value::Dict& params, return true; } -bool ParseOrientationQuaternion(const base::Value::Dict& params, - base::Value::Dict* out_params) { - constexpr size_t kQuaternionListSize = 4; // x, y, z, w - - const base::Value::List* value_list = params.FindList("quaternion"); - if (!value_list || value_list->size() != kQuaternionListSize) { +bool ParseOrientationEuler(const base::Value::Dict& params, + base::Value::Dict* out_params) { + if (!params.contains("alpha") || !params.contains("beta") || + !params.contains("gamma")) { return false; } - std::optional x = (*value_list)[0].GetIfDouble(); - if (!x.has_value()) { + + std::optional alpha = params.FindDouble("alpha"); + if (!alpha.has_value()) { return false; } - std::optional y = (*value_list)[1].GetIfDouble(); - if (!y.has_value()) { + std::optional beta = params.FindDouble("beta"); + if (!beta.has_value()) { return false; } - std::optional z = (*value_list)[2].GetIfDouble(); - if (!z.has_value()) { + std::optional gamma = params.FindDouble("gamma"); + if (!gamma.has_value()) { return false; } - std::optional w = (*value_list)[3].GetIfDouble(); - if (!w.has_value()) { + device::SensorReading quaternion_readings; + if (!device::ComputeQuaternionFromEulerAngles(*alpha, *beta, *gamma, + &quaternion_readings)) { return false; } + // Construct a dict that looks like this: // { // quaternion: { @@ -1201,9 +1203,13 @@ bool ParseOrientationQuaternion(const base::Value::Dict& params, // w: VAL4 // } // } + const double x = quaternion_readings.orientation_quat.x; + const double y = quaternion_readings.orientation_quat.y; + const double z = quaternion_readings.orientation_quat.z; + const double w = quaternion_readings.orientation_quat.w; out_params->Set( "quaternion", - base::Value::Dict().Set("x", *x).Set("y", *y).Set("z", *z).Set("w", *w)); + base::Value::Dict().Set("x", x).Set("y", y).Set("z", z).Set("w", w)); return true; } @@ -1239,9 +1245,10 @@ base::expected ParseSensorUpdateParams( } } else if (*type == "absolute-orientation" || *type == "relative-orientation") { - if (!ParseOrientationQuaternion(*reading_dict, &reading)) { - return base::unexpected( - Status(kInvalidArgument, "Could not parse quaternion")); + if (!ParseOrientationEuler(*reading_dict, &reading)) { + return base::unexpected(Status( + kInvalidArgument, "Could not parse " + *type + + " readings. Invalid alpha/beta/gamma values")); } } else { return base::unexpected(Status( diff --git a/test/run_py_tests.py b/test/run_py_tests.py index baf12eeb..8971b652 100755 --- a/test/run_py_tests.py +++ b/test/run_py_tests.py @@ -4454,8 +4454,10 @@ def testUpdateVirtualSensorWithoutXYZValues(self): }) def testUpdateVirtualSensorWithoutXYZWValues(self): + expected_error = ("invalid argument: Could not parse absolute-orientation " + "readings. Invalid alpha/beta/gamma values") self.assertRaisesRegex(chromedriver.InvalidArgument, - "invalid argument: Could not parse quaternion", + expected_error, self._driver.UpdateVirtualSensor, 'absolute-orientation', { 'y': 2.0, @@ -4463,7 +4465,7 @@ def testUpdateVirtualSensorWithoutXYZWValues(self): 'w': 4.0 }) self.assertRaisesRegex(chromedriver.InvalidArgument, - "invalid argument: Could not parse quaternion", + expected_error, self._driver.UpdateVirtualSensor, 'absolute-orientation', { 'x': 1.0, @@ -4471,7 +4473,7 @@ def testUpdateVirtualSensorWithoutXYZWValues(self): 'w': 4.0 }) self.assertRaisesRegex(chromedriver.InvalidArgument, - "invalid argument: Could not parse quaternion", + expected_error, self._driver.UpdateVirtualSensor, 'absolute-orientation', { 'x': 1.0, @@ -4479,7 +4481,7 @@ def testUpdateVirtualSensorWithoutXYZWValues(self): 'w': 4.0 }) self.assertRaisesRegex(chromedriver.InvalidArgument, - "invalid argument: Could not parse quaternion", + expected_error, self._driver.UpdateVirtualSensor, 'absolute-orientation', { 'x': 1.0, @@ -4487,6 +4489,52 @@ def testUpdateVirtualSensorWithoutXYZWValues(self): 'z': 3.0 }) + def testUpdateVirtualSensorWithoutAlphaBetaGammaValues(self): + expected_error = ("invalid argument: Could not parse relative-orientation " + "readings. Invalid alpha/beta/gamma values") + self.assertRaisesRegex(chromedriver.InvalidArgument, + expected_error, + self._driver.UpdateVirtualSensor, + 'relative-orientation', { + 'beta': 2.0, + 'gamma': 3.0 + }) + self.assertRaisesRegex(chromedriver.InvalidArgument, + expected_error, + self._driver.UpdateVirtualSensor, + 'relative-orientation', { + 'alpha': 1.0, + 'gamma': 3.0 + }) + self.assertRaisesRegex(chromedriver.InvalidArgument, + expected_error, + self._driver.UpdateVirtualSensor, + 'relative-orientation', { + 'alpha': 1.0, + 'beta': 2.0 + }) + + def testUpdateVirtualSensorOutOfRangeEulerAngles(self): + # Alpha, beta and gamma must be within the ranges defined by the Device + # Orientation API. + test_inputs = ( + # Alpha range: [0, 360). + [-1, 2, 3], [361, 2, 3], + # Beta range: [-180, 180). + [1, -181, 3], [1, 180, 3], + # Gamma range: [-90, 90). + [1, 2, -91], [1, 2, 90] + ) + expected_error = ("invalid argument: Could not parse relative-orientation " + "readings. Invalid alpha/beta/gamma values") + for test_input in test_inputs: + alpha, beta, gamma = test_input + self.assertRaisesRegex(chromedriver.InvalidArgument, + expected_error, + self._driver.UpdateVirtualSensor, + 'relative-orientation', + { 'alpha': alpha, 'beta': beta, 'gamma': gamma }) + def testRemoveVirtualSensorWithInvalidSensorName(self): self.assertRaisesRegex( chromedriver.InvalidArgument,