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

event handler listener type incorrect #332

Open
ShaneZhengNZ opened this issue Jul 18, 2024 · 7 comments
Open

event handler listener type incorrect #332

ShaneZhengNZ opened this issue Jul 18, 2024 · 7 comments

Comments

@ShaneZhengNZ
Copy link

ShaneZhengNZ commented Jul 18, 2024

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

  • Device: Any Device
  • OS: Any OS

Application Environment
Provide information about your development environment:

  • React Native version: v0.73.0
  • RN Bluetooth Classic version: v1.73.0-rc.11

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
Copy link
Owner

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
Copy link

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
Copy link

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 -> #333

You can check it out.

@kenjdavidson
Copy link
Owner

@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
Copy link
Author

@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
Copy link

larlin commented Nov 18, 2024

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
Copy link
Owner

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.

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 a pull request may close this issue.

5 participants