Fix Bluetooth state change after lock screen

[jeremypw]

Fixes #23
Related to elementary/switchboard-plug-bluetooth#21

This PR does not allow greeter to change Bluetooth settings - in greeter the wingpanel just displays the current global state.

There are alternative PRs on this issue #131 & #143 but there seems no logical reason why the greeter should be able to change the Bluetooth state when that is a user setting.

An alternative approach would be to discontinue the user setting and let the Bluetooth be changed at anytime for all users.

Note that there can be problems if two users are logged in simultaneously with different Bluetooth settings but they are present in master and not related to this PR.

[vjr]

@jeremypw this PR does not work on my hardware which are a desktop and a laptop with the intel AX200 and AX201 chips respectively - "does not work" meaning the BT turns back on after resuming from sleep.

I had tried this issue out on my older laptop with an older intel wifi+BT chip (model AC7625 I think) and could not repro the issue off master branch - cant test on it anymore because I sold this laptop off.

Note that on my AX20x chips when resuming from sleep the on_interface_added () callback in src/Services/Manager.vala was being invoked with the iface being an Adapter which my PR #143 fixed for my case - this callback was not being invoked on my older laptop with the different BT chipset - which I'm not sure if it is due to differences in the device driver implementations.

Would it make sense for you to include the changes in PR #143 into this PR and have people test it to see if it resolves the issue for them?

[jeremypw]

@vjr Thanks for trying this out! It seems it only fixes part of the problem then. I'll have another look at it and at your PR and try to merge them as you suggest.

[jeremypw]

@vjr I note that your PR refers to the settings object in set_last_state () while in the greeter, which my PR explicitly prevents because it was this that was causing the problem for me. When you say this PR does not work, does the bluetooth remain on even after logging into the user who turned it off?

Are you saying that in your case the adapter is being re-powered when resuming from suspend by the device driver?

[jeremypw]

@vjr It seems that the changes you made stop the indicator updating after middle-clicking on it to power it off. The indicator still appears active even though Bluetooth settings plug indicator Bluetooth is off.

[vjr]

@jeremypw

When you say this PR does not work, does the bluetooth remain on even after logging into the user who turned it off?

I had not tried this scenario previously but if I'm understanding your steps and repeating them correctly here, then yes, the BT turns back on after logging into the user who turned it off

Are you saying that in your case the adapter is being re-powered when resuming from suspend by the device driver?

Yes in my case the adapter appears to be re-powered or re-initialised (if these are the right terms) when resuming from suspend.

[vjr]

@vjr It seems that the changes you made stop the indicator updating after middle-clicking on it to power it off. The indicator still appears active even though Bluetooth settings plug indicator Bluetooth is off.

Doh - I think I did not try this scenario (these steps) in my testing of the issue. Perhaps it would help to take a step back to list out a test plan (steps to follow to reproduce the issue and to verify any attempted fix) so that there is better coverage here?

[jeremypw]

That's odd because the "last-state" should be restored after the user logs in so it should not matter if an adapter was re-powered by the device driver while in the greeter - that should not be recorded in the settings (I'll check it isn't). The problem with your approach is that it does not distinguish between an unwanted power change and one initiated by the user.

It may be an idea to simply not show the bluetooth indicator in the greeter if it only shows the correct state while in a user session.

[jeremypw]

@vjr

I suggest these tests:

(1)

• Start a user session with BT on.
• Switch off BT by middle-clicking on the indicator (This should work)
• Lock the screen with <Super>L
• Reactivate the screen to show the greeter - is BT indicator active or inactive?
• Log back in as the same user - is BT indicator active or inactive? (Should be inactive).

(2) Same as (1) except log out user

(3) Same as (1) except restart computer.

[jeremypw]

I had not tried this scenario previously but if I'm understanding your steps and repeating them correctly here, then yes, the BT turns back on after logging into the user who turned it off

To clarify - is the BT showing as "on" in the greeter? Or does it turn back on after logging into the user who turned it off? (Using my PR of course).

[vjr]

That's odd because the "last-state" should be restored after the user logs in so it should not matter if an adapter was re-powered by the device driver while in the greeter - that should not be recorded in the settings (I'll check it isn't). The problem with your approach is that it does not distinguish between an unwanted power change and one initiated by the user.

It may be an idea to simply not show the bluetooth indicator in the greeter if it only shows the correct state while in a user session.

It's just my opinion (maybe the UX folks should chime in with definitive guidance on how this should behave) but I say we allow the BT state to be toggled in the greeter and simply record and restore the last state changed either by whichever user session or via the greeter - which I think is what the current master branch intent.

Probably just focus on reducing scope of any fix to simply prevent from the state changing on its own - whether after a restart, or after resuming from sleep, or after logout/login.

[vjr]

I had not tried this scenario previously but if I'm understanding your steps and repeating them correctly here, then yes, the BT turns back on after logging into the user who turned it off

To clarify - is the BT showing as "on" in the greeter? Or does it turn back on after logging into the user who turned it off? (Using my PR of course).

Yes, the BT is showing up as ON in the greeter and remains on after logging back into the user who turned it off - and this is after resuming from sleep in my case.

I don't see the problem occur if I restart or only logout/login, it happens after resume from sleep.

(yes using just your PR without any other modifications)

[jeremypw]

Just checking - you need to kill and restart the wingpanel after installing my PR ;-) sudo killall io.elementary.wingpanel

[vjr]

@vjr

I suggest these tests:

(1)

* Start a user session with BT on.

* Switch off BT by middle-clicking on the indicator (This should work)

* Lock the screen with `<Super>L`

* Reactivate the screen to show the greeter - is BT indicator active or inactive?

* Log back in as the same user - is BT indicator active or inactive? (Should be inactive).

(2) Same as (1) except log out user

(3) Same as (1) except restart computer.

@jeremypw I tried these three use cases and the BT indicator remains inactive in the greeter and after logging back in for all three use cases with both master and this PR - meaning the issue does not manifest itself.

It's only when I suspend/resume does the BT indicator become active in the greeter and after logging back in, with both master and this PR.

[vjr]

Just checking - you need to kill and restart the wingpanel after installing my PR ;-) sudo killall io.elementary.wingpanel

Yep :-) I do this but without sudo...

[jeremypw]

Oh yes I left out test (4) - as (1) but suspend the computer.

Its odd because I get the problem with master when locking the screen (and this was the issue reported).

[vjr]

Oh yes I left out test (4) - as (1) but suspend the computer.

Its odd because I get the problem with master when locking the screen (and this was the issue reported).

@jeremypw how about only making the BT popover toggle button insensitive while in greeter and remove the is_in_session flag check and return in the other places like set_global_state () so that the BT state is saved and restored from the setting? Would that be a reasonable approach to combine this PR with the changes in mine?

[vjr]

Oh I see you updated the PR, let me test the latest code again...

[jeremypw]

Hm, I am, like you,now getting the problem with suspend/resume - at least I can now investigate it. Might have to revert those last changes.

[jeremypw]

There seems to be some subtle race condition going on - it will need a bit of time to fix.

[vjr]

Hm, I am, like you,now getting the problem with suspend/resume - at least I can now investigate it. Might have to revert those last changes.

@jeremypw I'll re-test your upcoming changes later on - perhaps tomorrow my timezone - but wanted to see if you've considered my earlier comment #156 (comment) and what you think about only using the is_in_session flag to only make the BT toggle button insensitive in the greeter but remove the check-and-return from the rest of the code so that the state is restored in the greeter too? I mean this change along with incorporating the changes in my PR #143 ?

[jeremypw]

@vjr OK, I'll try your suggestion. At the moment I have got it working OK with lock and suspend but its not working with restart. In the latter case, the BT indicator is active in greeter and starts off inactive (as expected) in the user session but switches to active after a perceptible fraction of a second. So there is a different race when restarting :-(

[jeremypw]

Closed in favour of @vjr 's PR, which is simpler.