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

Orientation is locked if there is no sensor for it #9693

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

Redirion
Copy link
Member

@Redirion Redirion commented Jan 16, 2023

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

This #4478 (comment) in the ever lasting full screen button discussion is actually valid. If a device does not feature an accelerometer (STB, Terminal, TV), the screen cannot rotate. It is more some sort of "sanity" check, as "def_accelerometer_rotation" in the SettingsProvider in AOSP is "false", which means, that after a factory reset, auto rotation of the screen should be disabled anyway. So a device manufacturer must have deliberately broken it (actually a device bug, not a bug in NewPipe) or a user must have selected screen auto rotation, even though it doesn't work. So this probably affects only a handful of users of NewPipe.

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

@Redirion Redirion added GUI Issue is related to the graphical user interface device/software specific Issues that only happen on some devices or with some specific hardware/software labels Jan 16, 2023
@TobiGr TobiGr added the player Issues related to any player (main, popup and background) label Jan 16, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jan 16, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Lgtm. Let's see if it works for users :-)

return android.provider.Settings.System.getInt(
context.getContentResolver(), Settings.System.ACCELEROMETER_ROTATION, 0) == 0;
context.getContentResolver(), Settings.System.ACCELEROMETER_ROTATION, 0) == 0
Copy link
Member

Choose a reason for hiding this comment

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

I wonder what the getInt returns when there is no accelerometer for rotation... Shouldn't it already return 0, as it's the default value passed for when the stored value is not valid?

Copy link
Member Author

Choose a reason for hiding this comment

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

the setting exists in aosp. The manufacturer would have to deliberately remove the setting. So it most likely is there. And if not: all the guys with their realy odd hardware are happy, that the button is there for them

@Stypox Stypox merged commit 4c5c2a3 into TeamNewPipe:dev Feb 7, 2023
@Redirion Redirion deleted the accelerometerfix branch February 9, 2023 10:47
@Stypox Stypox mentioned this pull request Mar 1, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
device/software specific Issues that only happen on some devices or with some specific hardware/software GUI Issue is related to the graphical user interface player Issues related to any player (main, popup and background)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants