kenjdavidson / react-native-bluetooth-classic

⚛ Bluetooth classic Android(Bluetooth)/IOS(ExternalAccessory) module for serial communication
https://kenjdavidson.github.io/react-native-bluetooth-classic
MIT License
250 stars 93 forks source link

event handler listener type incorrect #332

Open ShaneZhengNZ opened 4 months ago

ShaneZhengNZ commented 4 months ago

Mobile Device Environment Provide a list of operating systems on which this issue is relevant.

Application Environment Provide information about your development environment:

Describe the bug As far as I am concerned, the event listener should receive event parameter with BluetoothDeviceEvent type, for example,

onDeviceDisconnected(
    listener: BluetoothEventListener<BluetoothDeviceEvent>
  ): BluetoothEventSubscription {
    return this.createBluetoothEventSubscription(BluetoothEventType.DEVICE_DISCONNECTED, listener);
  }

, but what I saw, my listener receives the device object (type BluetoothNativeDevice).

therefore, when I use 'event.device', it is undefined, I have to do something like const device = event as unknown as BluetoothNativeDevice;, so that I can use device.name, otherwise, typescript is not happy for me to use event.name

Expected behavior type definition for the event handler should be

onDeviceDisconnected(
    listener: BluetoothEventListener<BluetoothNativeDevice>
  ): BluetoothEventSubscription {
    return this.createBluetoothEventSubscription(BluetoothEventType.DEVICE_DISCONNECTED, listener);
  }

or change the native code to actually return BluetoothDeviceEvent (not BluetoothNativeDevice).

if you agree with me, I am happy to provide a PR depends on which option you think is most appropriate.

kenjdavidson commented 4 months ago

Sure, PR it up and we'll see. Without looking at the code what you're saying sounds correct, I'm just shocked something like this has gone so long without issue.

manhdev0901 commented 4 months ago

Hello @kenjdavidson,

Sorry for the inconvenience, but I am also facing an issue related to listening for any events returned from the Bluetooth device.

The issue I'm encountering is as follows: I have configured Android settings and successfully connected to the desired Bluetooth device using the connect() method. I have also been able to send data using the write() method (which is great). However, the problem is that I cannot listen to any events or receive any information from the Bluetooth device, even though I have set up the following methods to listen: onStateChanged(), read(), onStateChanged(). Despite trying each method individually and all of them together, and setting up the Bluetooth device to send data to test if the mobile receives it, the result is still nothing.

Therefore, if you notice any missing configurations or know the issue causing this, please provide feedback to help me resolve this problem. Initially, I thought it was a device issue, so I spent more than a week researching it but still couldn't find a solution, which is why I am writing here for your and everyone's understanding.

This message is a bit long, so I hope you can take the time to read and understand it. I look forward to your prompt response.

brianwachira commented 4 months ago

Hello @kenjdavidson,

Sorry for the inconvenience, but I am also facing an issue related to listening for any events returned from the Bluetooth device.

The issue I'm encountering is as follows: I have configured Android settings and successfully connected to the desired Bluetooth device using the connect() method. I have also been able to send data using the write() method (which is great). However, the problem is that I cannot listen to any events or receive any information from the Bluetooth device, even though I have set up the following methods to listen: onStateChanged(), read(), onStateChanged(). Despite trying each method individually and all of them together, and setting up the Bluetooth device to send data to test if the mobile receives it, the result is still nothing.

Therefore, if you notice any missing configurations or know the issue causing this, please provide feedback to help me resolve this problem. Initially, I thought it was a device issue, so I spent more than a week researching it but still couldn't find a solution, which is why I am writing here for your and everyone's understanding.

This message is a bit long, so I hope you can take the time to read and understand it. I look forward to your prompt response.

I think I am experiencing a similar issue. I have even created an issue -> https://github.com/kenjdavidson/react-native-bluetooth-classic/issues/333

You can check it out.

kenjdavidson commented 4 months ago

@ShaneZhengNZ I believe the appropriate fix is to actually return the correct BluetoothDeviceEvent from the native side, rather than change the type definition to be BluetoothNativeDevice. I'm still looking for the PR if you have time to complete it. I agree, you should get the correct event object and be able to call event.device.

ShaneZhengNZ commented 3 months ago

@kenjdavidson I do have some time to provide the proper fix. lol. However, I am not very familiar with the native languages. It will take a bit longer, but I am keen to try.

larlin commented 1 week ago

I just want to add that the same is true for onDeviceDiscovered as well. I didn't get around to report it but this is how I wrote in my code:

let discoveryListener = async (device:BluetoothNativeDevice) =>{/* application code */}  
// The API is a lie, the callback is not called with a BluetoothDeviceEvent but instead with a BluetoothNativeDevice.
subscription = RNBluetoothClassic.onDeviceDiscovered(discoveryListener as unknown as (event:BluetoothDeviceEvent) => null)

I guess others have done similar things so fixing this will break things. Which is fine as it is in line with what the API should be but maybe bump the version code a bit more to indicate it.

kenjdavidson commented 1 week ago

Lol @ the vicious comment.

Makes sense to bump the version. But I'm hoping someone opens a pr for the native module changes and will include this. Not sure bumping the version for just this when the workaround is fairly straight forward makes sense.

Or maybe it is. Feel free to open a pr for it and I can release a fix.