Skip to content

Fix/flashlight intensity issue#3733

Open
AshishDhyani131 wants to merge 2 commits into
kylecorry31:mainfrom
AshishDhyani131:fix/flashlight-intensity-issue
Open

Fix/flashlight intensity issue#3733
AshishDhyani131 wants to merge 2 commits into
kylecorry31:mainfrom
AshishDhyani131:fix/flashlight-intensity-issue

Conversation

@AshishDhyani131
Copy link
Copy Markdown
Contributor

Description

Fixed the issue of flashlight selector to currently active

Related Issue

3680

Checklist

  • I have reviewed the CONTRIBUTING.md guide and confirm that I am following it
  • I have tested my changes on an Android device or emulator
  • I have added/updated tests where appropriate
  • I have updated documentation where appropriate

Screenshots

Copy link
Copy Markdown
Owner

@kylecorry31 kylecorry31 left a comment

Choose a reason for hiding this comment

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

Thank you, looks to be on the right track but there are some changes I would like and a bug that was introduced.

Comment on lines +136 to +138
if (mode != FlashlightMode.Off) {
selectedMode = mode
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Is it possible to move this to within the on method so that it is wrapped by the synchronized block?

Moving it there would also allow it to be removed on line 182 as well


private fun turnOn() {
flashlight.set(selectedMode)
flashlight.set(flashlight.selectedMode)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I think this should maintain a local selectedMode state (like it did before), and sync that with the flashlight subsystem's mode on resume.

This technique is causing a bug in which when I change the mode and then turn off the flashlight it switches back to 0 without me leaving the page. I would like it to only default back to 0 when I come back to the page and the flashlight is off.

Potentially just the code you have in onResume will do the job, but that will need to be verified.

FlashlightMode.Torch
}
turnOn()
flashlight.set(mode)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I would like both of these flashlight.set lines in this function to be restored to turnOn - that way there's a single point where it is handling that. I think my other comment about the selectedMode bug and bringing back the selectedMode state will allow this to be function to be restored while retaining the behavior we want

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.

2 participants