Skip to content

Conversation

@ekuleshov
Copy link
Collaborator

@ekuleshov ekuleshov commented Jun 8, 2025

A relatively large change, but mostly about tidying Android platform code:

It may use some additional testing. These changes touch some functionality I don't use...

private void waitIfBonding() {
int counter = 0;
if (mBondingDevices.isEmpty() == false) {
while (mBondingDevices.isEmpty() == false) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chipweinberger this seem to be a bug. I assumed here should be a loop

for (BluetoothGatt gatt : mConnectedDevices.values()) {

if (func == "adapterTurnOff") {
if ("adapterTurnOff".equals(func)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chipweinberger seem to be another string comparison bug

@chipweinberger
Copy link
Owner

if you feel confident in it, feel free to merge and release it

@ekuleshov
Copy link
Collaborator Author

if you feel confident in it, feel free to merge and release it

I'll try to do some more testing before making a release.

@chipweinberger did you see my notes in the waitIfBonding() method? Does that change look right to you? I'm not sure how to test it.

@chipweinberger
Copy link
Owner

yes it looks right. a little surprising that we've gone so long with it "broken"

it's a "best practice" to not do anything else while bonding

# Conflicts:
#	packages/flutter_blue_plus_android/android/src/main/java/com/lib/flutter_blue_plus/FlutterBluePlusPlugin.java
@Gaby-Torres
Copy link

Just checking in on this PR, my team is experiencing bonding latency and are curious as to whether this has shown improvement in bonding performance?

@chipweinberger
Copy link
Owner

chipweinberger commented Aug 1, 2025

no it will have no effect

@chipweinberger
Copy link
Owner

chipweinberger commented Aug 2, 2025

we should probably just remove waitIfBonding all together

the code works as is. we shouldn't change functionality thats working.

@chipweinberger chipweinberger force-pushed the master branch 2 times, most recently from a86d5ad to 5a236d2 Compare September 6, 2025 08:33
@chipweinberger
Copy link
Owner

just closing to tidy things up

given the merge conflicts, you'd probably want to start fresh.

@ekuleshov
Copy link
Collaborator Author

just closing to tidy things up
given the merge conflicts, you'd probably want to start fresh.

@chipweinberger I could have still merged the conflicts...

BTW, the last release missing the change logs and it is also fail to build on Android.
Issue introduced in this change

.../.pub-cache/hosted/pub.dev/flutter_blue_plus_android-5.0.1/android/src/main/java/com/lib/flutter_blue_plus/FlutterBluePlusPlugin.java:970: error: incompatible types: bad type in conditional expression
                    String inst = (instanceId != null ? instanceId : "noinst");
                                                        ^
    Integer cannot be converted to String
.../.pub-cache/hosted/pub.dev/flutter_blue_plus_android-5.0.1/android/src/main/java/com/lib/flutter_blue_plus/FlutterBluePlusPlugin.java:1104: error: incompatible types: bad type in conditional expression
                    String inst = (instanceId != null ? instanceId : "noinst");
                                                        ^
    Integer cannot be converted to String
.../.pub-cache/hosted/pub.dev/flutter_blue_plus_android-5.0.1/android/src/main/java/com/lib/flutter_blue_plus/FlutterBluePlusPlugin.java:1219: error: incompatible types: bad type in conditional expression
                    String inst = (instanceId != null ? instanceId : "noinst");
                                                        ^
    Integer cannot be converted to String
3 errors

@chipweinberger chipweinberger reopened this Sep 6, 2025
@chipweinberger
Copy link
Owner

ok. reopened. please continue work on it!

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 this pull request may close these issues.

4 participants