Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

EnableBtOnConnect #737

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

Conversation

lazarosfs
Copy link
Contributor

This enables bt on connect with user intervention (adding general preference)
It also tries to connect when starting if bt is enabled

This enables bt on connect with user intervention (adding general preference)
It also tries to connect when starting if bt is enabled
@cpfeiffer
Copy link
Contributor

The change itself looks good, but it breaks the DeviceCommunicationServiceTestCase. I'll try to fix it tomorrow unless someone beats me to it.

You can run the tests with ./gradlew test btw, or directly from within Android Studio (where you may need to adjust the execution directory).

@lazarosfs
Copy link
Contributor Author

I didn't understand where it breaks, couldn't find it in Android Studio that is why I didn't fix it. The changes work as they are except tests.

@danielegobbetti
Copy link
Contributor

While appreciating the effort, I honestly believe we should not toggle the Bluetooth adapter from within the app, as this could:

  • create complex and potentially bug-inducing dependencies on boot (depending on android version and on the number of apps the BOOT_COMPLETED message could come earlier than the BLUETOOTH_ENABLED one, but the BT adapter might be already powering on)
  • create misunderstanding (or even problems to the user) if the BT interface gets suddendly and unexpectedly powered on (think while being in a situation where the radios MUST be kept off).

For these reasons I - personally - am against merging this change.

@lazarosfs
Copy link
Contributor Author

lazarosfs commented Jul 28, 2017

There is no way of enabling btr without user intervention. And we added an option for it, in case someone does not want it.
In most apps I use if bt or location is used and it's off you get a prompt.
The point is to connect if bt is on when the app is started.
That way we don't have to open the app and hit connect after each time the phone reboots.
I think there is no point of starting the app in every boot automatically if it needs user intervention to work.

We should also honor autoreconnect option before we auto connect on startup. Pressing Connect button does not check autoreconnect, it just sets autoreconnect and connects.
@cpfeiffer
Copy link
Contributor

Thanks, I will have a look tomorrow!

@cpfeiffer
Copy link
Contributor

OK, I think we all agree that if GB is configured to launch on startup and to connect when BT is on, then it's really supposed to connect right after booting (scenario 1). Everything else would be a bug.

So without your change, this scenario didn't work?

A new scenario (2) is: Gadgetbridge is running, BT is off and you try to connect. That should pop up a confirmation box "Do you really want to turn on BT?". If confirmed, BT should be enabled and then a connection should be established.

Did I get this right? Anything I forgot?

@cpfeiffer
Copy link
Contributor

Note that BT enabling should NOT be triggered automatically, through any other means than manually connecting to a device. Right?

@danielegobbetti
Copy link
Contributor

Scenario 1) is fine by me. Scenario 2) is in my opinion dangerous because "BT off" could mean many things we have little control over (e.g. BT is already powering on, but slow; BT is scheduled to be activated, but it has not happened yet; the ROM has some quirks for BT access like extra security (think privacy guard)). So I believe that the current situation where a toast is shown "Bluetooth is disabled" is the right compromise in this case.

@danielegobbetti
Copy link
Contributor

Regarding this specific pull request, I have the problem that the start command automatically triggers the connect. Currently (current master) the code already calls start from connect to ensure service is running.

I am unaware of the reason because originally we had both start() and connect(), but with this change they would practically become a single status, and this should a change explicitly discussed, evaluated and approved.

@lazarosfs
Copy link
Contributor Author

lazarosfs commented Aug 5, 2017

cpfeifer I agree on both scenarios, that is exactly what I had in mind.
Right now no it does not autoconnect after boot.
Start and Connect are not the same, if bt is off only start() is ran,otherwise no attempts or toasts are shown

@Avamander
Copy link
Contributor

How would this pull request deal with Airplane Mode? Another option to respect it or to ignore it in the app's settings?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants