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

Fix Bluetooth connectivity problems #3167

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

simelis
Copy link

@simelis simelis commented Nov 5, 2023

Remove use_auto_connect as it's value never changes.
Reset always_scan after successful connection or manual collector restart.
Restart bluetooth once BleDisconnectedException with status 133 is caught.
Restart bluetooth more frequently on CONNECT_NOW failures.
Add aggressive_bluetooth_restart advanced setting.

Related discussion here: #3165

Donatas Šimelis and others added 3 commits November 4, 2023 23:45
Reset always_scan after successful connection or manual collector restart.
Restart bluetooth once BleDisconnectedException with status 133 is caught.
Restart bluetooth more frequently on CONNECT_NOW failures.
@Navid200
Copy link
Collaborator

Navid200 commented Nov 5, 2023

xDrip already has too many settings.
New settings should only be added if/when absolutely necessary in my opinion.

The Bluetooth watchdog is only a last resort. When it triggers, it could disconnect anything and everything. I have experienced my Android Auto GPS stopping while driving when it triggered.

@Navid200
Copy link
Collaborator

Navid200 commented Nov 5, 2023

When you say the auto connect value never changes, are you sure this is not related to the trust auto connect on the Bluetooth settings page under less common settings?

@simelis
Copy link
Author

simelis commented Nov 5, 2023

xDrip already has too many settings. New settings should only be added if/when absolutely necessary in my opinion.

The Bluetooth watchdog is only a last resort. When it triggers, it could disconnect anything and everything. I have experienced my Android Auto GPS stopping while driving when it triggered.

I'm aware and perfectly fine by Bluetooth disconnecting everything, if that is needed to keep the connection with the sensor alive. This is child's phone which doesn't run any critical programs or services other than related to diabetes management, xDrip being the most critical of them all. I know there are people who have a separate phone dedicated for xDrip/AAPS, and I'm sure that on time BG data is also a big priority for them.

I agree about xDrip having too much settings, especially when a lot of them are only relevant for older sensors like g5, which is not very clear for new users. Should we put some effort in regrouping settings or hiding those, which are not relevant for the current data source?

Regarding this PR, would you suggest that the same Bluetooth watchdog setting could be used for this case?

@Navid200
Copy link
Collaborator

Navid200 commented Nov 5, 2023

@simelis I am one of the volunteers. I am not the owner of this repository who is the lead developer.

There are phones that have trouble. Xiaomi is one. Samsung sometimes is one.
The challenge for us is to try to make as many people as we can happy.

I have removed some of the old settings that were not needed. I will continue to do that.
Just because a setting includes G5 in the title does not mean that is not needed for G6 or Dexcom One.

If we change a setting in xDrip to help Nokia, we have to make sure it will not hurt Samsung or Pixel or Motorola or ...

Please wait for @jamorham for answers to your questions.

@simelis
Copy link
Author

simelis commented Nov 5, 2023

When you say the auto connect value never changes, are you sure this is not related to the trust auto connect on the Bluetooth settings page under less common settings?

It's a private variable, which has a default value of false and is not set anywhere, the app compiles just fine after I removed that variable and its usages in handleWakeup(). I don't see any way it could be changed anywhere else. The trust auto connect setting has a separate getTrustAutoConnect() method, which is used only when specialPairingWorkaround() is used. So no, as far as I understand, it is not related to this setting, it's hard for me to find out what is the purpose of that variable at all.

@simelis
Copy link
Author

simelis commented Nov 5, 2023

Just because a setting includes G5 in the title does not mean that is not needed for G6 or Dexcom One.

Sure, I don't trust the names, I always look it up in code. The settings inside Bluetooth section is quite a mess though.

g5_bluetooth_watchdog setting, for example, is not used together with generic bluetooth_watchdog anywhere in code, so they could be merged.

bluetooth_watchdog_timer, close_gatt_on_ble_disconnect, bluetooth_use_scan, bluetooth_excessive_wakelocks, bluetooth_frequent_reset, use_transmiter_pl_bluetooth, use_rfduino_bluetooth, pref_dex_collection_polling, always_discover_services, blukon_unbonding are used only in DexCollectionService and are not relevant for all other implementations.

bluetooth_allow_background_scans is used only by JamBaseBluetoothSequencer, that is, BlueJay, InPen, LeFun and MiBand services.

It's not clear to me where Companion Bluetooth setting is actually used, I can trace it to UiBasedCollector, that is, manual entry? Not sure how it's related.

It would make much sense to hide these settings when they are not relevant.

@Navid200
Copy link
Collaborator

Navid200 commented Nov 5, 2023

Some settings are used for Libre as well and not just Dexcom.
UI based is companion app: https://navid200.github.io/xDrip/docs/Follow/CompanionApp.html

@jamorham
Copy link
Collaborator

jamorham commented Nov 7, 2023

Which handsets and android versions has this been tested with?

@simelis
Copy link
Author

simelis commented Nov 7, 2023

Which handsets and android versions has this been tested with?

@jamorham I'm testing this using Nokia G21 on android 13 for about 4 days now. The aggressive Bluetooth restart kicks in about 4-5 times a day for me, but the data loss is getting fixed in 10 to 15 minutes without any user action, and that makes me quite happy with the results ;)

@Navid200
Copy link
Collaborator

Considering this is for Nokia, I suggest we make sure that existing solutions don't work first.

Do you mind having a look at the following and giving the suggestions a try?
https://dontkillmyapp.com/hmd-global

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.

None yet

3 participants