Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature: Generic Sensor-based motion sensors #4204

Merged
merged 1 commit into from
Apr 23, 2018

Conversation

anssiko
Copy link
Contributor

@anssiko anssiko commented Apr 18, 2018

  • Add Accelerometer
  • Add Gyroscope
  • Add Magnetometer
  • Add Orientation Sensor

Fix #2778
Fix #2777
Fix #2776

@anssiko
Copy link
Contributor Author

anssiko commented Apr 18, 2018

@Fyrd, I'm a bit puzzled by this error:

Error: [accelerometer:and_chr:64] Browser version missing: and_chr 64

If I add a key for 64 node validator/validate-jsons.js reports:

Error: [accelerometer:and_chr] Extra key found in and_chr: "67"

Something obvious I've overlooked?

I see other features use a similar structure, i.e. only define one key for and_chr.

@cvrebert
Copy link
Contributor

Something obvious I've overlooked?

Adding new browser versions via JSON isn't supported:
https://github.com/Fyrd/caniuse/blob/master/CONTRIBUTING.md#unsupported-changes
As far as caniuse JSON is concerned, Android Chrome's only possible version is currently 64.
Fyrd doesn't track historical compatibility data for mobile browsers for...some reason.

@Fyrd
Copy link
Owner

Fyrd commented Apr 19, 2018

@cvrebert is correct. As for the reason, see #3518. I am in the process of finding a solution for that, though.

- Add Accelerometer
- Add Gyroscope
- Add Magnetometer
- Add Orientation Sensor
@anssiko
Copy link
Contributor Author

anssiko commented Apr 19, 2018

@Fyrd, thanks for explaining the rationale, and thanks @cvrebert for pointing out the fix.

I pushed a fix that includes and_chr version 64 data only and this PR now passes Travis CI.

@Fyrd, feel free to merge if the PR looks good otherwise.

@Fyrd Fyrd merged commit 668c00b into Fyrd:master Apr 23, 2018
@Fyrd
Copy link
Owner

Fyrd commented Apr 23, 2018

Thanks!

@anssiko
Copy link
Contributor Author

anssiko commented May 8, 2018

@Fyrd, I notice this PR was merged (thanks!), and the site content was updated in the following data update: 4fa442c

In this data update the following files were added:

features-json/orientation-sensor.json
features-json/accelerometer.json
features-json/gyroscope.json
features-json/magnetometer.json

I noticed orientation-sensor is already "shown":true:

https://caniuse.com/#feat=orientation-sensor

Here is further data that should help you flip the shown flag to true also for accelerometer, gyroscope and magnetometer (per your guidance):

Cross-browser test suites for all these features:

Respective test results on Chrome 67 for Android (CA) and Chrome 67 for Desktop (CD):

In addition, for manual testing we have developed a tool that complements the automated cross-browser test suite:
https://intel.github.io/generic-sensor-demos/sensor-tester/build/bundled/

Please let me know if you have any questions or if I can be of further help.

@Fyrd
Copy link
Owner

Fyrd commented May 9, 2018

Hi @anssiko, thanks for all the links and information. I notice when I run those tests on either of my Mac laptops they mostly fail, is that expected (due to missing sensors in the device)?

Also even on Chrome 67 for Android I notice a simple test of 'Magnetometer' in window returns false. Am I missing something?

@anssiko
Copy link
Contributor Author

anssiko commented May 9, 2018

@Fyrd, correct, your Mac does not have the supporting sensor hardware. Some (older!) models do.

Magnetometer is available by enabling the ”Generic Sensor Extra Classes” experimental flag in about:flags as noted. The rest of the sensors are enabled by default in Chrome 67.

@Fyrd
Copy link
Owner

Fyrd commented May 9, 2018

Ah, that explains things, thanks. Should all upcoming versions of Chrome not also use n d #1 for Magnetometer then? (I'll go ahead and fix that)

@Fyrd
Copy link
Owner

Fyrd commented May 9, 2018

Okay, all features are now on the site. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants