-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Only set the log level *if* bluetooth is available. #288
Conversation
…evel throws a PlatformException.
Please accept the CLA, so that someone can merge this. Or let us know if you don't want to, so I would create the PR. |
My understanding is some other folks (Meidi Tönisson & co) made a version of this CL and submitted it to the repo for me already. this PR can probably be closed. |
It doesn't seem to be in the master yet, so if there's a PR open for it already, please link it and ask @pauldemarco to close this PR. |
ah, hm. Okay. Let me get the CLA okay-ed by Google's lawyers since this change was done on company time. I'll get back with you shortly I hope! |
Hi @andretietz I got the approval from Google lawyers. Can you take another look at this PR? |
Looks good so far, I personally would remove the version change, since this is up to @pauldemarco to decide. If he merges something else before and increasing the version number, he will change this anyways. |
made the requested changes |
@efortuna Did you check that it doesn't crash, if there's a log before setting the loglevel? |
Hi @efortuna @andretietz, Sorry for the delay on this, I have some questions. It was my intention that setting the log level should succeed whether or not Bluetooth is available. On future platforms, we may need to set verbosity during times when Bluetooth is unavailable. It looks like this will only error out if FlutterBlue is being used on an unsupported platform... something other than Android/iOS. Was this the case? |
It's been a long time since I encountered this issue so I could be wrong, but if memory serves, this happens if you run it on an Android or iOS simulator? IIRC you have to construct a FlutterBlue object to be able to check the "platform is supported" call (it's not a static method) so you have to construct an object to do that check. Hence this constructor dealing with the resulting crash of setting logging on an "unsupported platform" (aka a simulator). |
It's basically crashing when (for some reason) running on a device that doesn't support bluetooth. Usually this cannot happen, since the play store is not showing apps to devices without bluetooth if bluetooth is required, but in cases of manually installing an apk and the emulator, this just crashes. |
This happens every time you run on a simulator. Which is a very common occurrence if you're developing an app. And this plugin shouldn't crash just because a user has run it on some "unsupported device" (perhaps they have bluetooth off?). |
Without this change,
setLogLevel
insideFlutterBlue.instance
throws a PlatformException