Accessibility Service Overlay MVP #329
Conversation
app/src/main/java/com/jmstudios/redmoon/fragment/SettingsFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/res/xml/settings.xml
Outdated
| <Preference | ||
| android:key="@string/pref_key_use_accessibility_service" | ||
| android:title="@string/pref_title_use_accessibility_service" | ||
| android:summary="@string/pref_summary_use_accessibility_service" | ||
| > | ||
| <intent | ||
| android:action="Settings.ACTION_ACCESSIBILITY_SETTINGS" /> | ||
| </Preference> |
There was a problem hiding this comment.
I have mixed feelings about adding a setting here, as opposed to making it like our current approach for the overlay permission, where we just request it when the filter is first enabled.
On one hand, I'd prefer to avoid unnecessary preferences, especially since I can't really imagine anyone using Red Moon an Android 12+ and not enabling the accessibility service. (Maybe it should default on, with the "prompt when filter is turned on" flow if the setting is enabled?).
On the other hand, it has always annoyed me that you can't even open Night Screen if your device isn't rooted, so for the longest time I couldn't look at it to compare features. And I suppose some people might want to use Red Moon but not trust it with accessibility permissions.
Overall I lean towards leaving the setting out and just requiring the accessibility service on Android 12+, but I won't insist on it and am happy to be convinced otherwise.
There was a problem hiding this comment.
I agree that we should probably make this part of the onboarding process in Android 12+, but I do think there is some utility to the versions between 8 and 12 in being able to choose the method of filtering; This will cover up /everything/, including emergency controls, and I've actually been surprised by how strong a 90% darkness filter can be over the system stuff, even when your eyes are adjusted to full darkness.
This was just the most integrated/not-super-high effort way I could think of to make it discoverable; there's no app-level state involved. It's just a link to the settings page which has a toggle by necessity.
| } else if (isAccessibilityServiceOn(applicationContext)) { | ||
| EventBus.post(accessibilityServiceCommand(Command.getCommand(intent))) | ||
| mCurrentAppMonitor.monitoring = Command.getCommand(intent).turnOn && Config.secureSuspend | ||
| Log.i("$filterIsOn") | ||
| Command.getCommand(intent).onAnimationStart(this@FilterService) | ||
| Command.getCommand(intent).onAnimationEnd(this@FilterService) |
There was a problem hiding this comment.
My first reaction here was, why proxy commands through the regular filter service, when you could modify Command.toggle() to send the event directly (and thus, avoid running two separate services).
But then I thought, now that we have 3 different ways of filtering (normal overlay, accessibility, root), it might be good to keep the shared logic in FilterService and pull out the logic which differs. But I'm not sure — it might be ideal to make them each their own service (or, well, I'm not sure root mode needs a service running at all times at all).
I guess the bottom line is, it would be good to put some thought into what the multiple-filters architecture should actually look like, and I haven't done that yet. But I do worry that we're creating an even messier ball of code to work with in the future (and I kind of regret my getting rid of the Presenter for the filter service back in the day— that architecture was much easier to work in; I was new to coding and just didn't understand it at the time, and got rid of it in a misguided attempt to "simplify" the app by reducing lines of code. Here's what it looked like way back before I touched anything: https://github.com/LibreShift/red-moon/blob/333bf352336c792acbca8237fe33c7d4f19cf679/app/src/main/java/com/jmstudios/redmoon/presenter/ScreenFilterPresenter.java
I've toyed with the idea of moving back in that direction, in as part of fixing #208 but mostly if I were going to spend that amount of time coding, I'd put it into other projects.
There was a problem hiding this comment.
My phone isn't rooted, so I'm actually a bit curious as to what the root overlay offers over the accessibility one. I understand that some people have experienced the services getting killed, but I'm pretty sure the accessibility ones are the very last in line (and as such are effectively never killed). Perhaps we could go back down to one true way of filtering once Android 8~11 become fully obsolete.
I took this approach largely because it was the least likely to introduce bugs; I personally really didn't want a -90% night filter coming on while my phone is sitting on the dash of my car with a map pulled up, so I decided to be as conservative as possible with the architecture. I also have some ideas on refactors/improvements I'd like to see done (I was a big fan of Lux back when it used to work, which gave you a unified screen brightness bar from -90% to 100%) but I also have limited bandwidth.
app/src/main/java/com/jmstudios/redmoon/service/AccessibilityFilterService.kt
Outdated
Show resolved
Hide resolved
|
Maybe you could merge it now and release as beta only on Github? |
|
I've been using this for the past year or so on my phone and it's worked well. If there's no major blocking issues then I think approving and merging this would be a net benefit. |
|
Could you add the apk version with your changes here? Then people could install and test it. I tried to compile it myself, but Android Studio throws errors :/ |
|
I'm pretty sure the (rightfully-added) anti-scam measures on newer android versions would prevent you from actually testing the accessibility service if I gave you an APK. I did just push a change which fixes the failing APK build, though, which might fix your issue. |
|
I managed to compile. It works perfectly, thanks a lot! This should definitely be merged. |
Actually, it is still possible to use accessibility services from an APK. It will stop you from doing it the normal way, but if you know where to look, in the app details menu, in a kebab menu in the top right, you can disable this safeguard. So an APK actually would be useful. Assuming you're trustworthy ;D |
I'm not going to pretend that this is the finest code I've written, but this should be daily-usable and I want to prevent as much eye-searing as possible among our Android 12 users. This should be in a reasonable state to build upon as well.
It adds a link in the settings to the accessibility services page, and will always use the accessibility service instead of a standard overlay when said service is enabled. It all goes through the existing FilterService, so it should work with all the possible ways one can toggle and set filters.
Good news:
Bad news:
Resolves #309
Resolves #253
Resolves #252
Resolves #312
Resolves #236 (repro'd + confirmed fixed on Android 11)
Probably Resolves #234
Probably Resolves #277