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

Proper resources disposal on "reload" #15

Open
wmaca opened this issue Dec 5, 2016 · 11 comments
Open

Proper resources disposal on "reload" #15

wmaca opened this issue Dec 5, 2016 · 11 comments

Comments

@wmaca
Copy link

wmaca commented Dec 5, 2016

When I press "Reload" or the application reloads due to Live Reload being enabled, the onDestroy method of the module is not called. Because of that, the sockets (and streams) are no correctly disposed.

When the reload is finished, I can no longer open a bluetooth socket. It requires me to disable and enable bluetooth, or to restart the application.

Is there ant callback or method I could implement that would correctly dispose these sockets when I reload my application?

@rusel1989
Copy link
Owner

@waltermacambira hey, yep this is bothering me too, there should definately be some way how to do it, I will look into it

@wmaca
Copy link
Author

wmaca commented Dec 7, 2016

@rusel1989 I am not familiar with Android development, but if you need any help on creating a PR for this, please let me know. I could not find in 'react-native' docs anywhere saying where to add a callback to make this kind of clean up. One way around it, would be to detect the socket is in this state, cleaning it if so.

@wmaca
Copy link
Author

wmaca commented Dec 13, 2016

@rusel1989 Solved at least for Android. I made the service variable static, so I could properly release the resources (service.stop()) when creating a new one.

@rusel1989
Copy link
Owner

@waltermacambira Cool, so can you please submit pull request with your fixes ?

@wmaca
Copy link
Author

wmaca commented Dec 15, 2016

@rusel1989 even if it is android only?

@timscott
Copy link

timscott commented Apr 3, 2017

@waltermacambira - I would appreciate an android only fix. Thanks.

@timscott
Copy link

timscott commented Apr 3, 2017

@waltermacambira - Is the fix you mean change this line to:

private static RCTBluetoothSerialService mBluetoothService;

I need to get this working, and I will make my own fork if necessary.

By the way, this is not just a development headache. CodePush deployments corrupt the connection. Any user that is connected at the time a CodePush update is applied will never be able to connect again until he manually unpairs via device Settings.

Also by the way, I attempted to implement a workaround using Bluetooth.unpairDevice. However, it seems that function is missing for some reason. I'm not sure why, because it seems to be implemented here. Anyone have a clue about that?

@timscott
Copy link

timscott commented Apr 3, 2017

I tried changing this line to:

private static RCTBluetoothSerialService mBluetoothService;

Did not fix the problem.

By the way, looking at output from adb logcat I never see the message logged at this line. In fact I never see any of the messages that begin with "Host ". Could it be the problem is that for some reason these lifecycle methods are never being called?

CORRECTION: This does fix the problem. Yay. (I made the dumb error of forgetting that native updates require a new APK because they are not applied by CodePush.)

@wmaca
Copy link
Author

wmaca commented Apr 4, 2017

Yes, the fix can be as simple as that. A better disposal of the streams would be nice too, but I did not see it as necessary.

@timscott
Copy link

timscott commented Apr 5, 2017

Alas, this fix is not working for me. While it appears to "fix" the connect error (I can successfully connect) the connection is not functional.

In my application I have implemented a request-response pattern. I write an AT command, then recursively dead until I reach either "OK" or "ERROR" or timeout. After the re-connect, the reads never return anything.

The problem is in fact worse because to achieve a good state I need not only to un-pair the device but to also restart my application.

@timscott
Copy link

timscott commented Apr 5, 2017

My problem seems to be solved by upgrading to v1.0.0-rc1.

I neglected to mention before that I have been using v0.1.6, which was the current NPM version when I started using this module, and it's still the most recent tag. That's the version I applied @waltermacambira's suggested change to.

Yesterday I tried v1.0.0-rc1, which I guess it was recently made the current version on NPM, but it completely failed. Today I diagnosed the problem as due to a breaking change: the read function was renamed readFromDevice. Since neither function was ever mentioned in the documentation of this module, it took a lot of digging to diagnose the problem.

I plan to make a PR with two things:

  1. Add a function named read in index.js. This will bring semantic consistency with the write function, thus reducing surprise.
  2. Add documentation for read and other undocumented functions to the README, which will potentially save people lots of time using this module.

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

No branches or pull requests

3 participants