Skip to content

Add turbo modules support - #1285

Merged
marcosinigaglia merged 60 commits into
innoveit:masterfrom
lucaswitch:master
Dec 9, 2024
Merged

marcosinigaglia merged 60 commits into
innoveit:masterfrom
lucaswitch:master

Conversation

@lucaswitch

Copy link
Copy Markdown
Contributor

This PR is still on development process so things can be broken.

Things are being done using the official docs and keeping backwards compat in mind.
New arch support can be tested using the examplenewarch app and old arch using the default example app.
Reference: https://github.com/reactwg/react-native-new-architecture/blob/main/docs/appendix.md

@marcosinigaglia

Copy link
Copy Markdown
Member

Hi, I migrated the Android side but the swift side is not so simple. The documentation and the examples are not correct (https://github.com/react-native-community/RNNewArchitectureLibraries/tree/feat/swift-event-emitter) so I have to find out other examples.

@lucaswitch

Copy link
Copy Markdown
Contributor Author

Hi, I migrated the Android side but the swift side is not so simple. The documentation and the examples are not correct (https://github.com/react-native-community/RNNewArchitectureLibraries/tree/feat/swift-event-emitter) so I have to find out other examples.

I guess we can still use the same RctEventEmitter or am i wrong about that?
Because most of libraries that i looked into was still using the old "sendEvent" and still works on 0.76.

…spec to support both architechtures, fix imports on ios.
@lucaswitch

Copy link
Copy Markdown
Contributor Author

Hi, I migrated the Android side but the swift side is not so simple. The documentation and the examples are not correct (https://github.com/react-native-community/RNNewArchitectureLibraries/tree/feat/swift-event-emitter) so I have to find out other examples.

I guess we can still use the same RctEventEmitter or am i wrong about that? Because most of libraries that i looked into was still using the old "sendEvent" and still works on 0.76.

Actually i'm wrong about that, look: react-native-community/RNNewArchitectureLibraries#19 (comment)

@lucaswitch

Copy link
Copy Markdown
Contributor Author

Screenshot 2024-11-13 at 16 05 10
I'm still trying to figure it out how to solve that for RCT_EXTERN_MODULE macro. @marcosinigaglia can you help with that?

Also i guess we will probably have to add typesafety in all events to be able to send events to codegen.

@GabrieleMazzola

Copy link
Copy Markdown

Hey @marcosinigaglia, just like @Marvedog I am also going to test this out over the next days for a new arch app we're building.

Will let you know if anything strange pops up! Thanks for the great work!

@lucaswitch

Copy link
Copy Markdown
Contributor Author

I think now is working, the new structure doesn't make me crazy but the only way I found is to keep a reference between objects in object-c and swift. Implementing the new classes in swift was not enough because the methods that handle events were giving error, if they change in the future you can go back to a structure only in swift. @lucaswitch if you can try to do some testing too. I will remove the new example folder shortly because with expo you can test the new architecture simply without having to have a specific example. For now we don't have time to support the old architecture as well so it will be a breaking change.

Ok, i'll check on the devices that i have here.

@marcosinigaglia

Copy link
Copy Markdown
Member

I've tested with a real app ios/android and after few fixes it (12.0.0-rc.2) it seems working. I am waiting for a few more tests to merge.

@GabrieleMazzola

Copy link
Copy Markdown

Hey, I'm just going to write here things as I test, hoping it helps.

iOS 18
iPhone 14 Pro

I tried to use the lib in my app with the new architecture, and the app was crashing when calling the BleManager.start method, without showing any useful info in the logs.
I then tried to run the iOS app with xcode, and I got the following:
image

Since I am using expo 52 (with prebuild), I've now added this to my app.json:

"infoPlist": {
  "NSBluetoothAlwaysUsageDescription": "This app requires access ..."
}

After launching prebuild and re-building the app, now I can scan successfully.

Will post more updates as I test, but perhaps this error can be better notified when running the app not natively with xcode.

@weese

weese commented Nov 25, 2024

Copy link
Copy Markdown
Contributor

I got an error in Xcode with version 12.0.0-rc.2. It is caused from a wrong filename of the Swift->Objective-C bridge header. I opened the PR in the source repo of this PR:

https://github.com/lucaswitch/react-native-ble-manager/pull/1

@lucaswitch

Copy link
Copy Markdown
Contributor Author

Hey, I'm just going to write here things as I test, hoping it helps.

iOS 18 iPhone 14 Pro

I tried to use the lib in my app with the new architecture, and the app was crashing when calling the BleManager.start method, without showing any useful info in the logs. I then tried to run the iOS app with xcode, and I got the following: image

Since I am using expo 52 (with prebuild), I've now added this to my app.json:

"infoPlist": {
  "NSBluetoothAlwaysUsageDescription": "This app requires access ..."
}

After launching prebuild and re-building the app, now I can scan successfully.

Will post more updates as I test, but perhaps this error can be better notified when running the app not natively with xcode.

Thats ok, every app that uses bluetooth needs that description and more.
But it needs to be set on every app as the usage can vary from user to user.

NativeEventEmitter is not necessary with the new event management
Added the rejection of the start promise to prevent crash
@marcosinigaglia

Copy link
Copy Markdown
Member

Hey, I'm just going to write here things as I test, hoping it helps.
iOS 18 iPhone 14 Pro
I tried to use the lib in my app with the new architecture, and the app was crashing when calling the BleManager.start method, without showing any useful info in the logs. I then tried to run the iOS app with xcode, and I got the following: image
Since I am using expo 52 (with prebuild), I've now added this to my app.json:

"infoPlist": {
  "NSBluetoothAlwaysUsageDescription": "This app requires access ..."
}

After launching prebuild and re-building the app, now I can scan successfully.
Will post more updates as I test, but perhaps this error can be better notified when running the app not natively with xcode.

Thats ok, every app that uses bluetooth needs that description and more. But it needs to be set on every app as the usage can vary from user to user.

I added a check to prevent the app from crashing.

@marcosinigaglia

Copy link
Copy Markdown
Member

I got an error in Xcode with version 12.0.0-rc.2. It is caused from a wrong filename of the Swift->Objective-C bridge header. I opened the PR in the source repo of this PR:

lucaswitch#1

Your fix is working only if you use use_frameworks!, I will check how to fix it and commit to solve the problem.

@Krupskis

Krupskis commented Dec 4, 2024

Copy link
Copy Markdown

Excited to test this as well!

@matt-42

matt-42 commented Dec 7, 2024

Copy link
Copy Markdown
Contributor

Hi! Thanks a lot for the work. I did a try and I had this error when testing rc3 and using bleManagerEmitter.addListener("BleManagerDidUpdateState", ...)

did the API to listen to ble events has changed ?


 android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0(LoadedApk.java:1852)
  android.app.LoadedApk$ReceiverDispatcher$Args.$r8$lambda$gDuJqgxY6Zb-ifyeubKeivTLAwk(Unknown Source:0)
  android.app.LoadedApk$ReceiverDispatcher$Args$$ExternalSyntheticLambda0.run(Unknown Source:2)
  android.os.Handler.handleCallback(Handler.java:958)
  android.os.Handler.dispatchMessage(Handler.java:99)
  android.os.Looper.loopOnce(Looper.java:224)
  android.os.Looper.loop(Looper.java:318)
  android.app.ActivityThread.main(ActivityThread.java:8777)
  java.lang.reflect.Method.invoke(Native Method)
  com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:561)
  com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1013)
Caused by java.lang.RuntimeException: __next_prime overflow
  com.facebook.react.bridge.CxxCallbackImpl.nativeInvoke(Native Method)
  com.facebook.react.bridge.CxxCallbackImpl.invoke(CxxCallbackImpl.java:27)
  it.innove.NativeBleManagerSpec.emitOnDidUpdateState(NativeBleManagerSpec.java:46)
  it.innove.BleManager$MyBroadcastReceiver.onReceive(BleManager.java:142)
  android.app.LoadedApk$ReceiverDispatcher$Args.lambda$getRunnable$0(LoadedApk.java:1842)
  android.app.LoadedApk$ReceiverDispatcher$Args.$r8$lambda$gDuJqgxY6Zb-ifyeubKeivTLAwk(Unknown Source:0)
  android.app.LoadedApk$ReceiverDispatcher$Args$$ExternalSyntheticLambda0.run(Unknown Source:2)
  android.os.Handler.handleCallback(Handler.java:958)
  android.os.Handler.dispatchMessage(Handler.java:99)
  android.os.Looper.loopOnce(Looper.java:224)
  android.os.Looper.loop(Looper.java:318)
  android.app.ActivityThread.main(ActivityThread.java:8777)
  java.lang.reflect.Method.invoke(Native Method)
  com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:561)
  com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1013)

@thehale

thehale commented Dec 8, 2024 •

Copy link
Copy Markdown

Jumping in to report that 12.0.0-rc.3 is working great in my speedcuber-timer app on RN 0.76 (scanning, connecting, notifications, disconnecting) across Android and iOS. Thanks so much for the hard work on this!


did the API to listen to ble events has changed ?

@matt-42, looks like the answer is "yes" -- see https://github.com/innoveit/react-native-ble-manager/pull/1285/files#diff-0a0e6737f721e95ba0e323dbf727fa099aa28bf25c147fd3fdd7ea36f8dc7e0a. Thanks for the tip!

Previously I had to instantiate a NativeEventEmitter and listen to specific documented events.

// Before ( <= v11.5.7)
import { NativeEventEmitter, NativeModules } from 'react-native';
import BleManager from 'react-native-ble-manager';

const BleManagerModule = NativeModules.BleManager || {};
const BleEvents = new NativeEventEmitter(BleManagerModule);

BleManager.start();

const subscription = BleEvents.addListener(
    'BleManagerDiscoverPeripheral',
    (device) => { /* do stuff */ },
);

Now, there are easy-to-use methods directly available on the BleManager

// After ( >= v12.0.0)
import BleManager from 'react-native-ble-manager';

BleManager.start();

const subscription = BleManager.onDiscoverPeripheral(
  (device) => { /* do stuff */ }
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants