Skip to content

Commit

Permalink
Merge pull request #256 from adamfowleruk/feature-253
Browse files Browse the repository at this point in the history 
Fixes #253. Android Bluetooth permissions and status improvements
  • Loading branch information
@adamfowleruk
adamfowleruk authored Feb 2, 2023
2 parents 12e3e18 + 8994a29 commit 1c15f3e
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 61 deletions.
2 changes: 1 addition & 1 deletion app/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ android {
minSdkVersion 21
targetSdkVersion 31
versionCode 2
versionName "2.1.0"
versionName "2.1.1"

testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
}
Expand Down
114 changes: 61 additions & 53 deletions app/src/main/java/io/heraldprox/herald/app/MainActivity.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,56 +157,7 @@ protected void onCreate(Bundle savedInstanceState) {
// REQUIRED : Ensure app has all required permissions
requestPermissions();

// Test UI specific process to gather data from sensor for presentation
final Sensor sensor = AppDelegate.getAppDelegate().sensor();
sensor.add(this);
sensor.add(socialMixingScore);
((TextView) findViewById(R.id.device)).setText(SensorArray.deviceDescription);
PayloadData payloadData = ((SensorArray) AppDelegate.getAppDelegate().sensor()).payloadData();
((TextView) findViewById(R.id.payload)).setText("PAYLOAD : " + payloadData.shortName());

if (BLESensorConfiguration.gpdmpEnabled) {
// We generate a static identified and use this for the TEST payload, and for the channel UUID
// DO NOT DO THIS IN PRODUCTION - this is JUST for THIS demo app
mySenderRecipientId = UUID.fromString("aaaaaaaa-aaaa-aaaa-aaaa-aa" + payloadData.hexEncodedString().substring(10));
Log.d(tag, "Setting my own GPDMP channel ID (channelID=" + mySenderRecipientId.toString() + ")");
// Listen for any and all data sent on the default GPDMP channel
((SensorArray) sensor).getGPDMPMessageListenerManager().addMessageListener(defaultChannelId, this);
}

targetListAdapter = new TargetListAdapter(this, targets);
final ListView targetsListView = ((ListView) findViewById(R.id.targets));
targetsListView.setAdapter(targetListAdapter);
targetsListView.setOnItemClickListener(this);
// - Temporal histogram model configuration
((TemporalHistogramModelView) findViewById(R.id.temporalHistogram)).model(temporalHistogramModel);
((TemporalHistogramModelView) findViewById(R.id.temporalHistogram)).bin(3);

// Test programmatic control of sensor on/off
final Switch onOffSwitch = findViewById(R.id.sensorOnOffSwitch);
onOffSwitch.setChecked(false);
onOffSwitch.setOnCheckedChangeListener(new CompoundButton.OnCheckedChangeListener() {
@Override
public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
if (isChecked) {
sensor.start();
} else {
sensor.stop();
}
}
});

// Sensor is on by default, unless automated test has been enabled,
// in which case, sensor is off by default and controlled by test
// server remote commands.
final AutomatedTestClient automatedTestClient = AppDelegate.getAppDelegate().automatedTestClient;
if (null == automatedTestClient) {
sensor.stop();
sensor.start(); // called again (first called by AppDelegate constructor) to ensure permissions are forced
} else {
sensor.stop();
automatedTestClient.add(this);
}
// All Herald advert/scan start code now moved to onRequestPermissionsResult()
}

/**
Expand All @@ -216,6 +167,12 @@ private void requestPermissions() {
// Check and request permissions
final List<String> requiredPermissions = new ArrayList<>();

// See https://developer.android.com/guide/topics/connectivity/bluetooth/permissions
// These two are 'legacy' permissions, but in testing are still needed on Android v12+
requiredPermissions.add(Manifest.permission.BLUETOOTH);
// This is required for power management (I.e. the turn Bluetooth on/off button on the UI)
requiredPermissions.add(Manifest.permission.BLUETOOTH_ADMIN);

// The next two ARE ***BOTH*** STILL NEEDED for Android 12 in order for didMeasure and didRead to work!
requiredPermissions.add(Manifest.permission.ACCESS_COARSE_LOCATION);
requiredPermissions.add(Manifest.permission.ACCESS_FINE_LOCATION);
Expand All @@ -233,9 +190,7 @@ private void requestPermissions() {
requiredPermissions.add(Manifest.permission.BLUETOOTH_ADVERTISE);
} else {
// Legacy permissions
// See https://developer.android.com/guide/topics/connectivity/bluetooth/permissions
requiredPermissions.add(Manifest.permission.BLUETOOTH);
requiredPermissions.add(Manifest.permission.BLUETOOTH_ADMIN);
// Note: There are no permissions here as Android documentation for v12+ is incorrect!
}
final String[] requiredPermissionsArray = requiredPermissions.toArray(new String[0]);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
Expand Down Expand Up @@ -269,6 +224,59 @@ public void onRequestPermissionsResult(final int requestCode, @NonNull final Str
if (!permissionsGranted) {
Log.e(tag, "Application does not have all required permissions to start (permissions=" + Arrays.asList(permissions) + ")");
}

// Herald sensor initialisation code moved here to prevent mistiming between the advert starting and permissions being granted

// Test UI specific process to gather data from sensor for presentation
final Sensor sensor = AppDelegate.getAppDelegate().sensor();
sensor.add(this);
sensor.add(socialMixingScore);
((TextView) findViewById(R.id.device)).setText(SensorArray.deviceDescription);
PayloadData payloadData = ((SensorArray) AppDelegate.getAppDelegate().sensor()).payloadData();
((TextView) findViewById(R.id.payload)).setText("PAYLOAD : " + payloadData.shortName());

if (BLESensorConfiguration.gpdmpEnabled) {
// We generate a static identified and use this for the TEST payload, and for the channel UUID
// DO NOT DO THIS IN PRODUCTION - this is JUST for THIS demo app
mySenderRecipientId = UUID.fromString("aaaaaaaa-aaaa-aaaa-aaaa-aa" + payloadData.hexEncodedString().substring(10));
Log.d(tag, "Setting my own GPDMP channel ID (channelID=" + mySenderRecipientId.toString() + ")");
// Listen for any and all data sent on the default GPDMP channel
((SensorArray) sensor).getGPDMPMessageListenerManager().addMessageListener(defaultChannelId, this);
}

targetListAdapter = new TargetListAdapter(this, targets);
final ListView targetsListView = ((ListView) findViewById(R.id.targets));
targetsListView.setAdapter(targetListAdapter);
targetsListView.setOnItemClickListener(this);
// - Temporal histogram model configuration
((TemporalHistogramModelView) findViewById(R.id.temporalHistogram)).model(temporalHistogramModel);
((TemporalHistogramModelView) findViewById(R.id.temporalHistogram)).bin(3);

// Test programmatic control of sensor on/off
final Switch onOffSwitch = findViewById(R.id.sensorOnOffSwitch);
onOffSwitch.setChecked(false);
onOffSwitch.setOnCheckedChangeListener(new CompoundButton.OnCheckedChangeListener() {
@Override
public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
if (isChecked) {
sensor.start();
} else {
sensor.stop();
}
}
});

// Sensor is on by default, unless automated test has been enabled,
// in which case, sensor is off by default and controlled by test
// server remote commands.
final AutomatedTestClient automatedTestClient = AppDelegate.getAppDelegate().automatedTestClient;
if (null == automatedTestClient) {
sensor.stop();
sensor.start(); // called again (first called by AppDelegate constructor) to ensure permissions are forced
} else {
sensor.stop();
automatedTestClient.add(this);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions herald/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ plugins {
}

group = 'io.heraldprox'
version = '2.1.0'
version = '2.1.1'

apply plugin: 'com.android.library'

Expand All @@ -17,7 +17,7 @@ android {
minSdkVersion 21
targetSdkVersion 31
versionCode 2
versionName "2.1.0"
versionName "2.1.1"

testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner"
consumerProguardFiles "consumer-rules.pro"
Expand Down
45 changes: 40 additions & 5 deletions herald/src/main/java/io/heraldprox/herald/sensor/ble/ConcreteBLETransmitter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,8 @@ public void start() {
// TODO do we need to do something here in case Sensor.init() is called? E.g. stop and restart?
// Override state to starting anyway to ensure we try to restart
myLoopTask.doStart();
myLoopTask.healthCheck();
// Immediately calling a healthcheck can cause advertising to be stopped before it has had chance to start
// myLoopTask.healthCheck();
}

@Override
Expand All @@ -139,6 +140,8 @@ public void stop() {
} else {
logger.fault("stop, transmitter already disabled");
}
// Clean out this reference (forced)
bluetoothGattServer = null;
}


Expand Down Expand Up @@ -207,6 +210,8 @@ private void healthCheck() {
if (advertLoopState != AdvertLoopState.stopped) {
logger.debug("advertLoopTask, healthCheck, stopping advert following bluetooth state change (isSupported={},bluetoothPowerOff={})", isSupported(), bluetoothStateManager.state() == BluetoothState.poweredOff);
doStop(now,true);
} else {
logger.debug("advertLoopTask, healthCheck, transmitter is disabled, not supported, or Bluetooth is powered off");
}
return;
}
Expand Down Expand Up @@ -339,7 +344,13 @@ private void disableGattServer() {
logger.fault("disableGattServer, no BLUETOOTH_CONNECT permission to clear and stop GATT server");
} else {
try {
bluetoothGattServer.clearServices();
// bluetoothGattServer.clearServices(); // Clears non-Herald GATT services (E.g. fetch device name, model, etc.)
BluetoothGattService ourService = bluetoothGattServer.getService(BLESensorConfiguration.linuxFoundationServiceUUID);
if (null != ourService) {
// Service is already advertised, so remove it
logger.debug("disableGattServer clearing single service for Herald (NOT calling clearServices())");
bluetoothGattServer.removeService(ourService);
}
bluetoothGattServer.close();
logger.debug("disableGattServer, GATT server stopped successfully");
} catch (Throwable e) {
Expand Down Expand Up @@ -519,6 +530,10 @@ public boolean isSupported() {
@Override
public void bluetoothStateManager(@NonNull final BluetoothState didUpdateState) {
logger.debug("didUpdateState (state={},transmitterEnabled={})", didUpdateState, transmitterEnabled.get());
if (BluetoothState.poweredOff == didUpdateState) {
// Clean out this reference (forced)
bluetoothGattServer = null;
}
myLoopTask.healthCheck();
}

Expand Down Expand Up @@ -620,6 +635,14 @@ private void removeData(@NonNull final BluetoothDevice device) {
}
}

@Override
public void onServiceAdded(int status, @NonNull BluetoothGattService service) {
logger.debug("BluetoothGattServerCallback, onServiceAdded (successfully={},uuid={}",
(BluetoothGatt.GATT_SUCCESS == status),
service.getUuid().toString()
);
}

@Override
public void onConnectionStateChange(@NonNull final BluetoothDevice bluetoothDevice, final int status, final int newState) {
lastInteraction = new Date();
Expand All @@ -635,6 +658,7 @@ public void onConnectionStateChange(@NonNull final BluetoothDevice bluetoothDevi
myLoopTask.healthCheck();
}


@Override
public void onCharacteristicWriteRequest(@NonNull final BluetoothDevice device, final int requestId, @NonNull final BluetoothGattCharacteristic characteristic, final boolean preparedWrite, final boolean responseNeeded, final int offset, @Nullable final byte[] value) {
lastInteraction = new Date();
Expand Down Expand Up @@ -816,13 +840,20 @@ private static void setGattService(@NonNull final SensorLogger logger, @NonNull
logger.debug("setGattService, serviceList, - with characteristic: {}",chr.getUuid());
}
}
// TODO decide if we need to not clear other services (I.e. can we interfere with other apps? Worrying if we can...)
bluetoothGattServer.clearServices();
// Only modify our own service (This should be the only service on GATT we can see anyway)
// bluetoothGattServer.clearServices();
BluetoothGattService ourService = bluetoothGattServer.getService(BLESensorConfiguration.linuxFoundationServiceUUID);
if (null != ourService) {
// Service is already advertised, so remove it
logger.debug("setGattService clearing single service for Herald (NOT calling clearServices())");
boolean success = bluetoothGattServer.removeService(ourService);
logger.debug("setGattService clearing single service result (successful={})",success);
}

// Logic check - ensure there are now no Gatt Services
List<BluetoothGattService> services = bluetoothGattServer.getServices();
for (final BluetoothGattService svc : services) {
logger.fault("setGattService device clearServices() call did not correctly clear service (service={})",svc.getUuid());
logger.debug("setGattService is advertising (non-Herald) service (service={})",svc.getUuid());
}

final BluetoothGattService service = new BluetoothGattService(BLESensorConfiguration.linuxFoundationServiceUUID, BluetoothGattService.SERVICE_TYPE_PRIMARY);
Expand Down Expand Up @@ -858,6 +889,10 @@ private static void setGattService(@NonNull final SensorLogger logger, @NonNull
if (count > 1) {
logger.fault("setGattService device incorrectly sharing multiple Herald services (count={})", count);
}
if (0 == count) {
// Note that the service will probably not be listed yet - updating the advert is asynchronous
logger.fault("setGattService couldn't list Herald services after setting! Should be advertising now (or soon...).");
}

logger.debug("setGattService successful (service={},signalCharacteristic={},payloadCharacteristic={})",
service.getUuid(), signalCharacteristic.getUuid(), payloadCharacteristic.getUuid());
Expand Down

0 comments on commit 1c15f3e

Please sign in to comment.