ble: fix DeviceCommunicationService disposes DeviceSupport

When a device goes out of range and then comes back into range DeviceCommunicationService disposes DeviceSupport while trying to re-establish a connection.
This causes race conditions as there are multiple for path for reconnections that can be executed in parallel.
The DeviceService.ACTION_CONNECT path kills everything abruptly potential leaving the gadget with a broken connection.

partial backport from PR #4916
This commit is contained in:
Thomas Kuehne
2025-06-24 09:53:50 +02:00
committed by José Rebelo
parent 7fcc879280
commit ac56df52ac
9 changed files with 131 additions and 89 deletions
@@ -621,6 +621,7 @@ public class DeviceSettingsPreferenceConst {
public static final String PREFS_KEY_DEVICE_BLE_API_CHARACTERISTIC = "prefs_device_ble_api_filter_char";
public static final String PREFS_KEY_DEVICE_BLE_API_PACKAGE = "prefs_device_ble_api_package";
public static final String PREFS_DEVICE_SUPPORT_CAN_RECONNECT = "prefs_device_support_can_reconnect";
public static final String PREFS_DEVICE_GATT_SYNCHRONOUS_WRITES = "prefs_device_gatt_synchronous_writes";
public static final String PREF_BATTERY_DISCHARGE_INTERVAL1_WATT = "battery_discharge_interval1_watt";
public static final String PREF_BATTERY_DISCHARGE_INTERVAL2_WATT = "battery_discharge_interval2_watt";
@@ -1500,6 +1500,11 @@ public class DeviceSpecificSettingsFragment extends AbstractPreferenceFragment i
);
}
deviceSpecificSettings.addRootScreen(
DeviceSpecificSettingsScreen.DEVELOPER,
R.xml.devicesettings_device_support_can_reconnect
);
deviceSpecificSettings.addRootScreen(
DeviceSpecificSettingsScreen.DEVELOPER,
R.xml.devicesettings_header_intent_api,
@@ -601,4 +601,15 @@ public abstract class AbstractDeviceSupport implements DeviceSupport {
@Override
public void onMusicOperation(int operation, int playlistIndex, String playlistName, ArrayList<Integer> musicIds) {}
@Override
public boolean canReconnect() {
final boolean defaultValue = true;
DevicePrefs prefs = getDevicePrefs();
if (prefs != null) {
return prefs.getBoolean(DeviceSettingsPreferenceConst.PREFS_DEVICE_SUPPORT_CAN_RECONNECT,
defaultValue);
}
return defaultValue;
}
}
@@ -1,10 +1,11 @@
/* Copyright (C) 2015-2024 Andreas Böhler, Andreas Shimokawa, Arjan
/* Copyright (C) 2015-2025 Andreas Böhler, Andreas Shimokawa, Arjan
Schrijver, Avamander, Carsten Pfeiffer, Daniel Dakhno, Daniele Gobbetti,
Daniel Hauck, Davis Mosenkovs, Dikay900, Dmitriy Bogdanov, Frank Slezak,
Gabriele Monaco, Gordon Williams, ivanovlev, João Paulo Barraca, José
Rebelo, Julien Pivotto, Kasha, keeshii, Martin, Matthieu Baerts, mvn23,
NekoBox, Nephiel, Petr Vaněk, Sebastian Kranz, Sergey Trofimov, Steffen
Liebergeld, Taavi Eomäe, TylerWilliamson, Uwe Hermann, Yoran Vulker
Liebergeld, Taavi Eomäe, TylerWilliamson, Uwe Hermann, Yoran Vulker,
Thomas Kuehne
This file is part of Gadgetbridge.
@@ -67,7 +68,6 @@ import nodomain.freeyourgadget.gadgetbridge.capabilities.loyaltycards.LoyaltyCar
import nodomain.freeyourgadget.gadgetbridge.deviceevents.GBDeviceEventCameraRemote;
import nodomain.freeyourgadget.gadgetbridge.devices.DeviceCoordinator;
import nodomain.freeyourgadget.gadgetbridge.externalevents.AlarmClockReceiver;
import nodomain.freeyourgadget.gadgetbridge.externalevents.AlarmReceiver;
import nodomain.freeyourgadget.gadgetbridge.externalevents.BluetoothConnectReceiver;
import nodomain.freeyourgadget.gadgetbridge.externalevents.BluetoothPairingRequestReceiver;
import nodomain.freeyourgadget.gadgetbridge.externalevents.CMWeatherReceiver;
@@ -263,7 +263,6 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
private VolumeChangeReceiver mVolumeChangeReceiver = null;
private AlarmReceiver mAlarmReceiver = null;
private final List<CalendarReceiver> mCalendarReceiver = new ArrayList<>();
private CMWeatherReceiver mCMWeatherReceiver = null;
private LineageOsWeatherReceiver mLineageOsWeatherReceiver = null;
@@ -475,7 +474,7 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
return;
}
connectToDevice(target);
connectToDevice(target, false);
}
}
};
@@ -590,25 +589,17 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
return registeredStruct;
}
private void connectToDevice(GBDevice device){
connectToDevice(device, false);
}
private void connectToDevice(@Nullable final GBDevice device, final boolean firstTime) {
final List<GBDevice> gbDevs = new ArrayList<>(2);
final boolean fromExtra;
private void connectToDevice(@Nullable final GBDevice device, boolean firstTime) {
final List<GBDevice> gbDevs = new ArrayList<>();
boolean fromExtra = false;
final Prefs prefs = getPrefs();
final GBPrefs prefs = getPrefs();
if (device != null) {
if (!device.getDeviceCoordinator().isConnectable()) {
GB.toast("Cannot connect to Scannable Device", Toast.LENGTH_SHORT, GB.INFO);
return;
}
gbDevs.add(device);
fromExtra = true;
} else {
fromExtra = false;
List<GBDevice> gbAllDevs = GBApplication.app().getDeviceManager().getDevices();
if (gbAllDevs != null && !gbAllDevs.isEmpty()) {
@@ -633,67 +624,105 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
return;
}
for (GBDevice gbDevice : gbDevs) {
for (final GBDevice gbDevice : gbDevs) {
final String deviceAddress = gbDevice.getAddress();
if (!gbDevice.getDeviceCoordinator().isConnectable()) {
// we cannot connect to beacons, skip this device
LOG.debug("connectToDevice - {} isn't connectable", deviceAddress);
if (fromExtra) {
GB.toast("Cannot connect to Scannable Device", Toast.LENGTH_SHORT, GB.INFO);
}
continue;
}
String btDeviceAddress = gbDevice.getAddress();
boolean autoReconnect = GBPrefs.AUTO_RECONNECT_DEFAULT;
final boolean autoReconnect;
if (prefs != null && prefs.getPreferences() != null) {
autoReconnect = getPrefs().getAutoReconnect(gbDevice);
autoReconnect = prefs.getAutoReconnect(gbDevice);
if (!fromExtra && !autoReconnect) {
LOG.debug("connectToDevice - {} neither from extra nor auto reconnect (1)",
deviceAddress);
continue;
}
final Set<String> lastDeviceAddresses = new HashSet<>(prefs.getStringSet(GBPrefs.LAST_DEVICE_ADDRESSES, Collections.emptySet()));
if (!lastDeviceAddresses.contains(btDeviceAddress)) {
lastDeviceAddresses.add(btDeviceAddress);
if (!lastDeviceAddresses.contains(deviceAddress)) {
lastDeviceAddresses.add(deviceAddress);
prefs.getPreferences().edit().putStringSet(GBPrefs.LAST_DEVICE_ADDRESSES, lastDeviceAddresses).apply();
}
} else {
autoReconnect = GBPrefs.AUTO_RECONNECT_DEFAULT;
}
if (!fromExtra && !autoReconnect) {
LOG.debug("connectToDevice - {} neither from extra nor auto reconnect (2)",
deviceAddress);
continue;
}
DeviceStruct registeredStruct = getDeviceStructOrNull(gbDevice);
if (registeredStruct == null) {
LOG.debug("connectToDevice - {} create new device struct", deviceAddress);
registeredStruct = createDeviceStruct(gbDevice);
} else {
final GBDevice deviceFromStruct = registeredStruct.getDevice();
if (isDeviceConnecting(deviceFromStruct) || isDeviceConnected(deviceFromStruct)) {
continue;
}
try {
removeDeviceSupport(gbDevice);
} catch (final DeviceNotFoundException e) {
LOG.error("connectToDevice(): Failed to remove device support: {}", e, e);
}
}
try {
final DeviceSupport deviceSupport = mFactory.createDeviceSupport(gbDevice);
DeviceSupport deviceSupport = registeredStruct.getDeviceSupport();
if (deviceSupport != null) {
setDeviceSupport(gbDevice, deviceSupport);
if (deviceSupport.isConnected()) {
LOG.debug("connectToDevice - {} device support is already connected",
deviceAddress);
continue;
}
GBDevice supportDevice = deviceSupport.getDevice();
if (supportDevice != null && supportDevice.isConnected()) {
LOG.debug("connectToDevice - {} device is already connected",
deviceAddress);
continue;
}
if (supportDevice != null && supportDevice.isConnecting()) {
LOG.debug("connectToDevice - {} device is already connecting",
deviceAddress);
continue;
}
}
if (deviceSupport != null && !deviceSupport.canReconnect()) {
try {
LOG.debug("connectToDevice - {} dispose device support", deviceAddress);
deviceSupport.dispose();
} catch (final Exception e) {
LOG.error("connectToDevice - {} failed to dispose device support",
deviceAddress, e);
}
deviceSupport = null;
}
final boolean createSupport = (deviceSupport == null);
if (createSupport) {
LOG.debug("connectToDevice - {} create new device support", deviceAddress);
deviceSupport = mFactory.createDeviceSupport(gbDevice);
registeredStruct.setDeviceSupport(deviceSupport);
}
if (deviceSupport != null) {
final boolean connected;
if (firstTime) {
deviceSupport.connectFirstTime();
connected = deviceSupport.connectFirstTime();
} else {
deviceSupport.setAutoReconnect(autoReconnect);
deviceSupport.setScanReconnect(reconnectViaScan);
deviceSupport.connect();
connected = deviceSupport.connect();
}
LOG.debug("connectToDevice - {} connected:{} firstTime:{}", deviceAddress,
connected, firstTime);
} else {
GB.toast(this, getString(R.string.cannot_connect, "Can't create device support"), Toast.LENGTH_SHORT, GB.ERROR);
}
} catch (Exception e) {
LOG.warn("exception in connectToDevice for {}", deviceAddress, e);
GB.toast(this, getString(R.string.cannot_connect, e.getMessage()), Toast.LENGTH_SHORT, GB.ERROR, e);
}
@@ -1157,25 +1186,11 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
}
}
/**
* Disposes the current DeviceSupport instance (if any) and sets a new device support instance
* (if not null).
*
* @param deviceSupport deviceSupport to reokace/add
*/
private void setDeviceSupport(GBDevice device, DeviceSupport deviceSupport) throws DeviceNotFoundException {
DeviceStruct deviceStruct = getDeviceStruct(device);
DeviceSupport cachedDeviceSupport = deviceStruct.getDeviceSupport();
if (deviceSupport != cachedDeviceSupport && cachedDeviceSupport != null) {
cachedDeviceSupport.dispose();
}
deviceStruct.setDeviceSupport(deviceSupport);
}
private void removeDeviceSupport(GBDevice device) throws DeviceNotFoundException {
DeviceStruct struct = getDeviceStruct(device);
if(struct.getDeviceSupport() != null){
struct.getDeviceSupport().dispose();
DeviceSupport support = struct.getDeviceSupport();
if(support != null){
support.dispose();
}
struct.setDeviceSupport(null);
}
@@ -1230,10 +1245,11 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
}
for(DeviceStruct struct : deviceStructs){
if(struct.getDevice().equals(device)){
if(struct.getDeviceSupport() == null)
DeviceSupport support = struct.getDeviceSupport();
if(support == null)
throw new DeviceNotFoundException(device);
return struct.getDeviceSupport();
return support;
}
}
throw new DeviceNotFoundException(device);
@@ -1252,10 +1268,6 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
}
}
private boolean isDeviceConnected(GBDevice device) {
return isDeviceConnected(device.getAddress());
}
private boolean isDeviceConnected(String deviceAddress) {
for(DeviceStruct struct : deviceStructs){
if(struct.getDevice().getAddress().compareToIgnoreCase(deviceAddress) == 0){
@@ -1265,19 +1277,6 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
return false;
}
private boolean isDeviceConnecting(GBDevice device) {
return isDeviceConnecting(device.getAddress());
}
private boolean isDeviceConnecting(String deviceAddress) {
for(DeviceStruct struct : deviceStructs){
if(struct.getDevice().getAddress().compareToIgnoreCase(deviceAddress) == 0){
return struct.getDevice().isConnecting();
}
}
return false;
}
private boolean isDeviceInitialized(GBDevice device) {
return isDeviceInitialized(device.getAddress());
}
@@ -1291,17 +1290,6 @@ public class DeviceCommunicationService extends Service implements SharedPrefere
return false;
}
private boolean deviceStateEquals(GBDevice device, GBDevice.State... states){
if((device = getDeviceByAddressOrNull(device.getAddress())) != null){
for(GBDevice.State possibleState : states){
if(device.getState() == possibleState){
return true;
}
}
}
return false;
}
private boolean isDeviceReconnecting(GBDevice device) {
if((device = getDeviceByAddressOrNull(device.getAddress())) != null){
return device.getState().equalsOrHigherThan(GBDevice.State.NOT_CONNECTED);
@@ -142,4 +142,12 @@ public interface DeviceSupport extends EventHandler {
* converts String in a device specific way, e.g. re-map characters for a custom font
*/
String customStringFilter(String inputString);
/**
* can this DeviceSupport instance's {@link #connect} be used to re-establish a
* lost connection or must {@link DeviceCommunicationService#connectToDevice} call
* {@link #dispose}, {@link DeviceSupportFactory#createDeviceSupport} and only
* then {@link #connect}
*/
boolean canReconnect();
}
@@ -34,7 +34,6 @@ import java.util.UUID;
import nodomain.freeyourgadget.gadgetbridge.capabilities.loyaltycards.LoyaltyCard;
import nodomain.freeyourgadget.gadgetbridge.deviceevents.GBDeviceEventCameraRemote;
import nodomain.freeyourgadget.gadgetbridge.impl.GBDevice;
import nodomain.freeyourgadget.gadgetbridge.impl.GBDeviceMusic;
import nodomain.freeyourgadget.gadgetbridge.model.Alarm;
import nodomain.freeyourgadget.gadgetbridge.model.CalendarEventSpec;
import nodomain.freeyourgadget.gadgetbridge.model.CallSpec;
@@ -541,4 +540,9 @@ public class ServiceDeviceSupport implements DeviceSupport {
}
delegate.onMusicOperation(operation, playlistIndex, playlistName, musicIds);
}
@Override
public boolean canReconnect() {
return delegate.canReconnect();
}
}
+10
View File
@@ -0,0 +1,10 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="960"
android:viewportHeight="960"
android:tint="?attr/colorControlNormal">
<path
android:fillColor="@android:color/white"
android:pathData="M314,845Q210,797 145,700Q80,603 80,481Q80,455 82.5,430Q85,405 91,381L45,408L5,339L196,229L306,419L236,459L182,365Q171,392 165.5,421Q160,450 160,481Q160,578 213,657.5Q266,737 354,775L314,845ZM620,360L620,280L729,280Q683,223 618,191.5Q553,160 480,160Q425,160 376,177Q327,194 286,225L246,155Q296,120 355,100Q414,80 480,80Q559,80 631,109.5Q703,139 760,195L760,140L840,140L840,360L620,360ZM594,960L403,850L513,660L582,700L525,798Q643,781 721.5,691Q800,601 800,480Q800,469 799.5,459.5Q799,450 797,440L878,440Q879,450 879.5,459.5Q880,469 880,480Q880,615 799.5,721.5Q719,828 590,865L634,891L594,960Z"/>
</vector>
+3
View File
@@ -631,6 +631,9 @@
<string name="prefs_title_gatt_settings">Generic Attribute Profile</string>
<string name="gatt_synchronous_writes_title">Synchronous GATT writes</string>
<string name="gatt_synchronous_writes_summary">Prefer synchronous GATT writes over asynchronous writes if a characteristic supports both.</string>
<string name="prefs_device_support_settings">Device Support</string>
<string name="device_support_can_reconnect_title">Reuse device support</string>
<string name="device_support_can_reconnect_summary">Reuse DeviceSupport instance when reconnecting the gadget.</string>
<string name="pref_disable_new_ble_scanning">Disable new BLE scanning</string>
<string name="pref_summary_disable_new_ble_scanning">Check this option if your device cannot be found during discovery</string>
<string name="not_connected">Not connected</string>
@@ -0,0 +1,12 @@
<?xml version="1.0" encoding="utf-8"?>
<androidx.preference.PreferenceScreen xmlns:android="http://schemas.android.com/apk/res/android">
<PreferenceCategory android:title="@string/prefs_device_support_settings" />
<SwitchPreferenceCompat
android:defaultValue="true"
android:icon="@drawable/ic_cycle"
android:key="prefs_device_support_can_reconnect"
android:layout="@layout/preference_checkbox"
android:summary="@string/device_support_can_reconnect_summary"
android:title="@string/device_support_can_reconnect_title" />
</androidx.preference.PreferenceScreen>