CB-12667 android: Added logic for searching sensors from Samsung vendor

apache / cordova-plugin-device-orientation

Apache Cordova Device Orientation Plugin

https://cordova.apache.org/

Apache License 2.0

58 stars 83 forks source link

CB-12667 android: Added logic for searching sensors from Samsung vendor #34

Closed matrosov-nikita closed 7 years ago

matrosov-nikita commented 7 years ago

Platforms affected

Android

What does this PR do?

This PR adds opportunity for searching orientation sensors which have specific orientation type (it's relevant for Samsung devices).

Checklist

[x] Reported an issue in the JIRA database
[x] Commit message follows the format: "CB-3232: (android) Fix bug with resolving file paths", where CB-xxxx is the JIRA ID & "android" is the platform affected.
[x] Added automated test coverage as appropriate for this change.

cordova-qa commented 7 years ago

Cordova CI Build has completed successfully.

Commit - Link Dashboard - Link

66 tests run, 9 skipped, 0 failed.

infil00p commented 7 years ago

@matrosov-nikita Which Samsung devices are affected? All of them? Only the Samsung Galaxy S3/4/5? If we could get more info on that, we can test for this and try and make there there's no regressions.

matrosov-nikita commented 7 years ago

@infil00p, sorry for late reply, I've tested a plugin with two Samsung devices: 1) Samsung Galaxy Core Prime VE Duos SM-G361H (Android 5.1) It has only 'Screen Orientation Sensor' with orientation type 65558 2) Samsung Galaxy S5 (Android 5.1) samsung It has two orientation sensors - 'Screen Orientation Sensor'(type=65558) and 'Orientation Sensor'(type=3)

For Samsung Galaxy S5 plugin works perfect. So, not all devices are affected. It depends on what orientation sensors the device has.

filmaj commented 7 years ago

A quick Google search reveals that the Note 2 and Note 4 also have this sensor type:

http://www.mattcurry.com/projects-2/the-n-a-o-m-i-project/note4-sensor-output/ https://github.com/hbibel/TapToUnlock/wiki/Sensor-Dumps-of-Different-Devices

Interestingly, the only device I was able to dig up that has the Samsung-specific orientation sensor but not the "normal" orientation sensor is the SM-G361H model @matrosov-nikita points out.

@matrosov-nikita are you able to confirm that this device's Samsung Orientation Sensor works and provides the same kind of data as the regular android Orientation sensor?

matrosov-nikita commented 7 years ago

@filmaj, I can confirm that auto and manual tests work w/ my fix.

petrot commented 7 years ago

This commit will fix the compass problem on samsung devices? e.g. Samsung Galaxy J5 (2016)

matrosov-nikita commented 7 years ago

@petrot, what do you mean 'compass problem'. Do you have any particular cases which don't work for you? I could test them with this fix.

petrot commented 7 years ago

One of my user has a Galaxy J5, but as I recognized yesterday, this phone does not have a sensor. So my problem has been solved.. :)

matrosov-nikita commented 7 years ago

@filmaj, @infil00p, do I need to do something else on this PR?

filmaj commented 7 years ago

@matrosov-nikita as long as you tested this on one of the real devices affected by the issue, and confirmed the fix, I am good to have this merged.

filmaj commented 7 years ago

Just realized you don't have committer rights - I will merge this in.

filmaj commented 7 years ago

@matrosov-nikita can you rebase with latest master first, please?

matrosov-nikita commented 7 years ago

@filmaj, done

cordova-qa commented 7 years ago

Cordova CI Build has completed successfully.

Commit - Link Dashboard - Link

66 tests run, 9 skipped, 0 failed.