Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add torch brightness level support for Android 13+ #174

Merged
merged 5 commits into from Nov 15, 2022

Conversation

KryptKode
Copy link
Contributor

Notes

  • Add torch brightness level support for Android 13+
TorchBrightness.mp4

@tibbi
Copy link
Member

tibbi commented Oct 24, 2022

nice, lets use the brighest option as default though, not the darkest

@KryptKode
Copy link
Contributor Author

Hi @tibbi. This is fixed now from the commit

@tibbi
Copy link
Member

tibbi commented Oct 26, 2022

everytime I turn the flashlight on now, it blinks with max brightness first, then goes to the real selected brightness. It is the easiest to see with some really low setting. Lets make it to the low brightness from the beginning, do not blink with max brightness first.

@@ -14,7 +15,7 @@ import com.simplemobiletools.flashlight.extensions.updateWidgets
import com.simplemobiletools.flashlight.models.Events
import org.greenrobot.eventbus.EventBus

class MyCameraImpl(val context: Context) {
class MyCameraImpl private constructor(val context: Context, private val cameraTorchListener: CameraTorchListener?) {
Copy link
Contributor Author

@KryptKode KryptKode Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @tibbi. Just a suggestion.
The implementation in this class is a bit confusing because we have two implementations for Nougat+ and PreNougat devices. Would it be worth it to refactor this class to share the same API on both implementations (create a common Interface for both) ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if it works as it is, lets leave it for now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I had to separate the implementations into different files LollipopFlash and MashmallowPlusFlash for simplicity. It became hard to manage states without having a centralized place.

@tibbi
Copy link
Member

tibbi commented Nov 9, 2022

it still blinks at full power at turning on first, then it resets to the proper value. It is the easiest to see by setting the brightness to the lowest

@tibbi
Copy link
Member

tibbi commented Nov 9, 2022

If I turn on the flashlight, exit the app with Back button and start it up again, the brightness seekbar wont show up even if I turn on the flashlight repeatedly

- fix issues with brightness control
- add separate classes for Android 5 and Android 6+ for simplicity
- properly cleanup camera in onDestory and also when stroboscope/flashlight mode are disabled
@KryptKode
Copy link
Contributor Author

Hi @tibbi .
I have addressed the issues now and tested them on Android 13 and Android 12.

import android.graphics.SurfaceTexture
import android.hardware.Camera

class LollipopCameraFlash : CameraFlash {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't test this on Android 5. I ensured that it is consistent with what was on the MyCameraImpl class

@tibbi
Copy link
Member

tibbi commented Nov 15, 2022

I can make both seekbars show up at once, like this. 1. turn on morse code, 2. exit app wth back button, 3. launch app with its icon, 4. turn on flashlight, 5. turn on morse code. Both seekbars will show up. If I then turn off morse code, I have the flashlight icon disabled too, but the brightness seekbar is visible.

seekbar

@KryptKode
Copy link
Contributor Author

Hi, I fixed this now

@tibbi
Copy link
Member

tibbi commented Nov 15, 2022

I can still reproduce it the same way

@KryptKode KryptKode requested a review from tibbi November 15, 2022 11:57
- this prevents the two sliders for brightness and stroboscope from showing up at the same time
@@ -49,7 +49,6 @@ class MyCameraImpl private constructor(val context: Context, private var cameraT

fun toggleStroboscope(): Boolean {
handleCameraSetup()
cameraFlash!!.unregisterListeners()
Copy link
Contributor Author

@KryptKode KryptKode Nov 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, the reason why the issue where the two sliders were showing did not appear fixed is because this listener call was left in the previous push. While reviewing the code before committing, I must have undone the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We needed to move unregistering the listeners after disabling the flashlight so that it can callback on the UI to hide the brightness slider

@tibbi
Copy link
Member

tibbi commented Nov 15, 2022

alright, seems to work just fine now, thanks

@tibbi tibbi merged commit 858533e into SimpleMobileTools:master Nov 15, 2022
@KryptKode KryptKode deleted the feat/brightness-control branch November 15, 2022 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants