kaaholst / android-squeezer

Remote control for your Logitech Media Server ("Squeezeserver" etc) and players.
Apache License 2.0
75 stars 15 forks source link

Add display options for wall-mounted tablet operation #829

Open schup011 opened 2 weeks ago

schup011 commented 2 weeks ago

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 commented 2 weeks ago

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

kaaholst commented 1 week ago

More contributions, thanks! Just keep them coming :smiley:

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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:

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

kaaholst commented 5 days ago

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 commented 4 days ago

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