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 display options for wall-mounted tablet operation #829

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

schup011
Copy link

  • New options for screensaver in global settings (in addition to the existing ones) to keep screen on as long as music is played. This can be useful if the app is running on a wall-mounted tablet where somebody wants to see always what is currently playing.
    • Display clock when not playing
    • Display black screen when not playing, let then android system determine when the screen is completely off / locked.
  • New option in global settings to always switch to NowPlaying view after an item has been chosen in ListView. This also supports wall-mounted operation: User wants to choose some music and then walk away.

Please have a special look on the BlackScreenSaver class, I am not completely sure whether the approach to make the screen black is robust, also against future Android versions (tested successfully, but only on Android 10).

@schup011
Copy link
Author

Additional comment: You can differentiate in the code changes between the two new features by looking at the two first commits.

  • First commit handles the NowPlaying switching
  • Second commit handles the screensaver options.

@kaaholst
Copy link
Owner

kaaholst commented Jun 16, 2024

More contributions, thanks! Just keep them coming 😃

To handle screensaver options for not playing, I think subscribe to PlayStatusChanged in BaseActivity, is the cleaner solution, rather than relying on NowPlayingFragment being present in the activity.

In the event listener, set up the existing inactivity timer and handler according to preferences. We don't need two sets of timers and handlers.

Regarding the black screen saver, I don't think it's needed. Please Experiment with standard Android settings, e.g sleep timer and remove screen lock.

@schup011
Copy link
Author

More contributions, thanks! Just keep them coming 😃

To handle screensaver options for not playing, I think subscribe to PlayStatusChanged in BaseActivity, is the cleaner solution, rather than relying on NowPlayingFragment being present in the activity.

You mean something like this, just some other action instead of updatePlayPauseIcon?

  @MainThread
   @Subscribe(sticky = true, threadMode = ThreadMode.MAIN)
   public void onEventMainThread(PlayStatusChanged event) {
       if (event.player.equals(requireService().getActivePlayer())) {
           updatePlayPauseIcon(event.playStatus);
       }
   }

In the event listener, set up the existing inactivity timer and handler according to preferences. We don't need two sets of timers and handlers.

Regarding the black screen saver, I don't think it's needed. Please Experiment with standard Android settings, e.g sleep timer and remove screen lock.

OK, I will have a look.

@kaaholst
Copy link
Owner

You mean something like this, just some other action instead of updatePlayPauseIcon?

  @MainThread
   @Subscribe(sticky = true, threadMode = ThreadMode.MAIN)
   public void onEventMainThread(PlayStatusChanged event) {
       if (event.player.equals(requireService().getActivePlayer())) {
           updatePlayPauseIcon(event.playStatus);
       }
   }

Yes!

@schup011
Copy link
Author

schup011 commented Jun 18, 2024

In the event listener, set up the existing inactivity timer and handler according to preferences. We don't need two sets of timers and handlers.
Regarding the black screen saver, I don't think it's needed. Please Experiment with standard Android settings, e.g sleep timer and remove screen lock.

OK, of course I can make the Screensaver class configurable, then its constructor would get some property like clockEnable = true / false. But apart from that, the commands which I am using in BlackScreensaver (which are Android system settings) would remain the same, just be transferred to the configurable Screensaver class. Do you agree?

@kaaholst
Copy link
Owner

OK, of course I can make the Screensaver class configurable, then its constructor would get some property like clockEnable = true / false. But apart from that, the commands which I am using in BlackScreensaver (which are Android system settings) would remain the same, just be transferred to the configurable Screensaver class. Do you agree?

I think I misunderstood the intension. I thought the back screen saver was an alternative to the clock screen saver. This will not work as intended because the FLAG_KEEP_SCREEN_ON will prevent the screen to go off, and the device to go to sleep. Even if you fix this, I don't think we need the back screen saver. To achieve the desired features, implement keep screen on and the clock screen saver, when playing music, and let Android control screen on/off and device sleep modes. The user can set preferences for this using standard Android system options. In fact, screen on/off is not possible using public Android APIs. WindowManager.LayoutParams.screenBrightness will only adjust the brightness off the screen, not turn it off.

@schup011
Copy link
Author

It works as expected. What I wanted is that the screen starts becoming black exactly at the time that is predefined in the code (5 minutes). In BlackScreensaver, I am removing the flag KEEP_SCREEN_ON. This has the effect that after 5 minutes, screen turns black (yes, I know, NOT off), and then android system settings take over to really turn it off according to the android system settings.

The basic problem is that currently, as the existing three options for screensaver are designed, the clock appears after 5 minutes, but the screen off appears after the time that is set in the system. That inconsistency is what I wanted to correct.

To make it really consistent, there are two options:

  • Either both "screen off" options (1. after inactivity, 2. after nonplaying) are using my new BlackScrenSaver (or those lines integrated into ScreenSaver), which would mean that screen turns black after 5 minutes, then turns off according to system settings.

  • Or both "screen off" options are using just the removed FLAG_KEEP_SCREEN_ON, which would mean that the screen turns off after a time according to system settings.

First option is what I would prefer, but you are the boss :-).

@kaaholst
Copy link
Owner

I see it the other way around.
Off means use Android settings.
On means KEEP_SCREEN_ON. This can either just keep the screen on, or show a clock after inactivity.

I think this is consistent. There is no relation between the time before Android turns off the screen, and the inactivity timer.

But the addition of keeping the screen on when playing, can make sense. Optionally using the clock screensaver. Off cause I haven't heard anyone requesting this yet.

@schup011
Copy link
Author

Ok, accepted. I need some time to rework this one.

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