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

[sensors][android] Fix crash when using DeviceMotion module #28839

Merged
merged 1 commit into from
May 14, 2024

Conversation

behenate
Copy link
Member

Why

Currently DeviceMotion module crashes with

Tried to obtain display from a Context not associated with  one.

Fixes #28820

How

The orientation is now obtained from the current activity instead of reactContext.applicationContext

Test Plan

Tested in BareExpo

@expo-bot expo-bot added the bot: suggestions ExpoBot has some suggestions label May 14, 2024
@expo-bot
Copy link
Collaborator

expo-bot commented May 14, 2024

The Pull Request introduced fingerprint changes against the base commit: abf6b83

Fingerprint diff
[
  {
    "type": "dir",
    "filePath": "../../packages/expo-sensors/android",
    "reasons": [
      "expoAutolinkingAndroid"
    ],
    "hash": "0f86ac256489e345fffd780ea1f873e4db8c17ca"
  }
]

Generated by PR labeler 🤖

@behenate behenate force-pushed the @behenate/sdk51/sensors-crash-fix branch from 868a643 to 7e51859 Compare May 14, 2024 14:51
@expo-bot expo-bot added bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels May 14, 2024
@behenate behenate marked this pull request as ready for review May 14, 2024 14:52
@behenate behenate requested a review from lukmccall as a code owner May 14, 2024 14:52
@brentvatne brentvatne merged commit 6158f5d into main May 14, 2024
20 checks passed
@brentvatne brentvatne deleted the @behenate/sdk51/sensors-crash-fix branch May 14, 2024 23:54
@brentvatne brentvatne added the published Changes from the PR have been published to npm label May 15, 2024
@cjoshmarshall
Copy link

Still getting the issue with 13.0.9

1000069665
Screenshot 2024-07-30 090511

@gani2009
Copy link

gani2009 commented Aug 5, 2024

this seems to only happen on emulator expo go, while physical device expo go/build is working with no error. expo-sensors version is 13.0.9 too

@cjoshmarshall
Copy link

cjoshmarshall commented Aug 6, 2024

gani2009 Not really. Expo app throws error in physical device too.

@behenate
Copy link
Member Author

behenate commented Aug 8, 2024

@gani2009 @cjoshmarshall Are you both using expo-go? Updating the package version in package.json won't usually work for expo-go (unless you only want changes from the js side of the package), since the native code is embedded into the app, which was last updated on 2nd of May, therefore the fix won't be there.

@cjoshmarshall
Copy link

@gani2009 @cjoshmarshall Are you both using expo-go? Updating the package version in package.json won't usually work for expo-go (unless you only want changes from the js side of the package), since the native code is embedded into the app, which was last updated on 2nd of May, therefore the fix won't be there.

@behenate Yup, using expo-go.

@behenate
Copy link
Member Author

behenate commented Aug 8, 2024

@cjoshmarshall Hm, in that case I'd recommend creating a development build. The quickest way is probably just running npx expo run:android or npx expo run:ios to create a local build. You can read more about this here, even more here and how to create development builds with eas here

For local builds you will need Android Studio and/or Xcode depending on what platform you want to develop for

@cjoshmarshall
Copy link

@behenate Apparently it seems like it does work in local builds. But still dont quite understand the reason why it wont work in expo-go.

@behenate
Copy link
Member Author

@cjoshmarshall The fix for the issue is in the native code of the package, but the native code for some selected, most popular packages is shipped with Expo Go and it's "frozen" at a specific version, because it's impossible to update it after compiling Expo Go. It's a little outdated, because we don't update Expo Go that often.

Expo Go doesn't actually use the native code from the node-modules folder (it's impossible as the app is already compiled), only the js code. So even if you have the newest version of the package in node-modules Expo Go will still be using the native code for the package from the time when it was compiled.

The current version of Expo Go doesn't contain this fix, so it still crashes.

For local builds you are compiling the code from node-modules into an app, so if you have the newest version of the package there, you can be sure you are also running the native code with all fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint changed bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SDK 51] expo-sensors 13.0.6 crash
6 participants