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

feat(android)!: rewrite native code to replace TYPE_ORIENTATION w/ TYPE_ACCELEROMETER & TYPE_MAGNETIC_FIELD #78

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

mgurzixo
Copy link

Platforms affected

Android

Motivation and Context

On Android, the plugin used a Sensor.TYPE_ORIENTATION which is deprecated. This sensor was more and more #64 in new devices. So now the plugin uses Sensor.TYPE_ACCELEROMETER and Sensor.TYPE_MAGNETIC_FIELD which are available everywhere. They are fused together as instructed here, except that the SensorManager.getOrientation() has a bug that will not be fixed. Stochastically found a solution that works like a charm and is used here.

Increasing reports of compass not working on some android devices

Description

rewrote src/android/CompassListener.java

Testing

tested on poco X3 NFC, OnePlus2, OnePlus6 & Samsung SM-T590.

Checklist

src/android/CompassListener.java Outdated Show resolved Hide resolved
src/android/CompassListener.java Outdated Show resolved Hide resolved
src/android/CompassListener.java Outdated Show resolved Hide resolved
@erisu erisu changed the title Rewrite Android native plugin feat(android)!: rewrite native code to replace TYPE_ORIENTATION w/ TYPE_ACCELEROMETER & TYPE_MAGNETIC_FIELD Aug 29, 2022
@mgurzixo
Copy link
Author

Hi!

Sounds good, this is my first PR on Github. I fully agree with all the remarks; maybe put an uppercase on "Use ...".

Is there anything I have to do?

@breautek
Copy link

Welcome to pull requests!

Is there anything I have to do?

Github will keep track of all commits done to your branch that initiated the PR (in this case, your master branch). So if you make more commits, it will appear here allowing reviewers to see and respond to updates. So all you need to do (if you're willing of course) is to make a new commit to your branch that includes the recommended changes.

If you have any questions or concerns about a particular recommendation, feel free to ask by commenting.

maybe put an uppercase on "Use ...".

I'm sure Erisu will accept that, so feel free to capitalise on the first letter for all sentences in comments ;)

In future pull requests, I'd recommend creating a branch for your changes to make a pull request from, but don't worry that for this pull request. You'll just have to make sure you don't commit anything to your master branch if it's not intended to be included in this pull request.

mgurzixo and others added 3 commits August 29, 2022 16:52
Co-authored-by: エリス <erisu@users.noreply.github.com>
Co-authored-by: エリス <erisu@users.noreply.github.com>
Co-authored-by: エリス <erisu@users.noreply.github.com>
@mgurzixo
Copy link
Author

Ok, got it, thanks!

@mgurzixo mgurzixo closed this Aug 29, 2022
@breautek breautek reopened this Aug 29, 2022
@breautek
Copy link

breautek commented Aug 29, 2022

We'll keep the PR open because changes will have to be reviewed again and eventually merged in by a maintainer, which I believe is still the intention. Do correct me if I'm wrong.

@mgurzixo
Copy link
Author

Sorry, my mistake!

@j2l
Copy link

j2l commented Nov 26, 2022

Hello!
Do you know how long we have to wait for device orientation to work?
Ticket was May 2021, PR is August 2022, we're in November 2022, PR is not reviewed.
Meanwhile: npm i https://github.com/mgurzixo/cordova-plugin-device-orientation

@mgurzixo
Copy link
Author

Hello @j2l , I am using Quasar, so I don´t know about native cordova, but I suspect that your solution works perfectly :).
May I suggest you to clone it on github, so that it becomes "your" code that you can patch at will?

@j2l
Copy link

j2l commented Nov 26, 2022

Thanks @mgurzixo
Can't make it work, so I'll try to switch to CapacitorJS which updated its motion lib.

@mgurzixo
Copy link
Author

mgurzixo commented Nov 26, 2022

I was not able to make Capacitor plugin working (it is horribly complicated...), so I ended up switching to Cordova plugin, which was a LOT simpler and easier to modify.
In the worst case, just install the official plugin, and replace the original compassListener.java with mine everywhere ( node_modules and android)
Very dirty, but a good stop-gap solution ;)

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.

4 participants