Ctrl+n+n for settings panel #2260

brunoais · 2021-04-17T16:10:35Z

Show the settings panel if pressing CTRL+n and then, without releasing CTRL, press n again

Still not done/updated:
README:

New shortcut
Rename what CTRL+SHIFT+n does

Closes: #2264

brunoais · 2021-04-17T16:14:21Z

If all looks good, I'll update documentation

brunoais · 2021-04-17T16:20:52Z

server/src/main/java/com/genymobile/scrcpy/ControlMessage.java

-    public static final int TYPE_COLLAPSE_NOTIFICATION_PANEL = 6;
-    public static final int TYPE_GET_CLIPBOARD = 7;
-    public static final int TYPE_SET_CLIPBOARD = 8;
-    public static final int TYPE_SET_SCREEN_POWER_MODE = 9;
-    public static final int TYPE_ROTATE_DEVICE = 10;
+    public static final int TYPE_EXPAND_SETTINGS_PANEL = 6;
+    public static final int TYPE_COLLAPSE_PANELS = 7;
+    public static final int TYPE_GET_CLIPBOARD = 8;
+    public static final int TYPE_SET_CLIPBOARD = 9;
+    public static final int TYPE_SET_SCREEN_POWER_MODE = 10;
+    public static final int TYPE_ROTATE_DEVICE = 11;


Not fully sure about changing the value of the constants as I did (and related) as it will make the previous server incompatible.

On one hand, as I did,

breaks backwards compatibility.

making this change allows cleaner code on both ends.

On the other hand, by just adding at the end

Keeps backwards compatibility

The code is less readable and maintainable.

I ended up going with 1, but I don't mind changing to 2 if you prefer.
Maybe doing as 2 and change to 1 soon, though, later.

Not fully sure about changing the value of the constants as I did (and related) as it will make the previous server incompatible.

That's totally acceptable, servers are not expected to be compatible at all with any other scrcpy client version. This would add annoying constraints for absolutely no benefit for scrcpy.

rom1v

On the principle, probably ok.

However, on my Android 6 device, I don't understand what it should open, it does nothing more than if I press Alt+n only once.

On my Android 10:

[server] ERROR: Could not invoke method
java.lang.NoSuchMethodException: com.android.internal.statusbar.IStatusBarService$Stub$Proxy.expandSettingsPanel []
	at java.lang.Class.getMethod(Class.java:2072)
	at java.lang.Class.getMethod(Class.java:1693)
	at com.genymobile.scrcpy.wrappers.StatusBarManager.getExpandSettingsPanel(StatusBarManager.java:30)
	at com.genymobile.scrcpy.wrappers.StatusBarManager.expandSettingsPanel(StatusBarManager.java:52)
	at com.genymobile.scrcpy.Device.expandSettingsPanel(Device.java:231)
	at com.genymobile.scrcpy.Controller.handleEvent(Controller.java:111)
	at com.genymobile.scrcpy.Controller.control(Controller.java:71)
	at com.genymobile.scrcpy.Server$2.run(Server.java:100)
	at java.lang.Thread.run(Thread.java:919)

Also, it doesn't build/link tests on debug mode:

error

[2/10] Compiling C object app/test_control_msg_serialize.p/tests_test_control_msg_serialize.c.o
../app/tests/test_control_msg_serialize.c: In function ‘main’:
../app/tests/test_control_msg_serialize.c:292:5: warning: implicit declaration of function ‘test_serialize_collapse_notification_panel’; did you mean ‘test_serialize_expand_notification_panel’? [-Wimplicit-function-declaration]
  292 |     test_serialize_collapse_notification_panel();
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |     test_serialize_expand_notification_panel
At top level:
../app/tests/test_control_msg_serialize.c:195:13: warning: ‘test_serialize_collapse_panels’ defined but not used [-Wunused-function]
  195 | static void test_serialize_collapse_panels(void) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../app/tests/test_control_msg_serialize.c:180:13: warning: ‘test_serialize_expand_settings_panel’ defined but not used [-Wunused-function]
  180 | static void test_serialize_expand_settings_panel(void) {
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[7/10] Linking target app/test_control_msg_serialize
FAILED: app/test_control_msg_serialize 
cc  -o app/test_control_msg_serialize app/test_control_msg_serialize.p/tests_test_control_msg_serialize.c.o app/test_control_msg_serialize.p/src_control_msg.c.o app/test_control_msg_serialize.p/src_util_str_util.c.o -fsanitize=address,undefined -Wl,--as-needed -Wl,--no-undefined -Wl,--start-group /usr/lib/x86_64-linux-gnu/libavformat.so /usr/lib/x86_64-linux-gnu/libavcodec.so /usr/lib/x86_64-linux-gnu/libavutil.so /usr/lib/x86_64-linux-gnu/libSDL2.so -Wl,--end-group
/usr/bin/ld: app/test_control_msg_serialize.p/tests_test_control_msg_serialize.c.o: in function `main':
../app/tests/test_control_msg_serialize.c:292: undefined reference to `test_serialize_collapse_notification_panel'
collect2: error: ld returned 1 exit status
[8/10] Compiling C object app/scrcpy.p/src_input_manager.c.o
ninja: build stopped: subcommand failed.

rom1v · 2021-04-17T16:17:52Z

app/src/input_manager.c

-                        collapse_notification_panel(controller);
-                    } else {
+                        collapse_panels(controller);
+                    } else if(repeatCount == 0) {


It could also probably be done on double-click on mouse button 5 now 😉

Exactly! That PR is nearly ready ^^

app/src/input_manager.c

brunoais · 2021-04-17T16:42:53Z

I'll see those settings problems. I only own an android 6 phone. I will try to see why that exception happens with an emulator.

brunoais · 2021-04-17T17:24:05Z

@rom1v Please confirm if it works now on your Android 10.

brunoais · 2021-04-17T17:25:08Z

However, on my Android 6 device, I don't understand what it should open

It's the same as scrolling the panel down a 2nd time. You get the buttons to turn on/off wifi, autorotate, etc...

Edit:
I tried with a friend's phone now (OnePlus 3 with Android 9). With the panel he has, if I take more than 1.5s to press the 2nd time, the settings panel doesn't show. However, if you touch the screen, it suddenly shows (I think it's a ~~bug~~feature)

rom1v · 2021-04-18T09:50:27Z

Please confirm if it works now on your Android 10.

Yes, it works 👍

Could you please rebase your commits (with git rebase -i) to get a clean history. Each commit should be "correct" the first time (for example, fix the previous commits directly instead of a separate comit to "fix syntax to follow the rules"). In general, I do it, but if you have several PR, that will help 😉 Thank you.

brunoais · 2021-04-18T10:08:33Z

@rom1v Fine to me.
I'll do that when all looks correct to you. Does all look correct now?

rom1v · 2021-04-20T17:34:55Z

Thank you, this shortcut is very practical. 👍

I rebased/changed some minor things:

I moved the static local variables to track the key repeat count to input_manager fields
I fixed some build and unused tests issues (you should build in debug mode and test using ninja -Cx test)
I added a commit to expand using MOD+n+n (it was left unused in your latest version, so the shortcut didn't work)
I refactored the way the legacy expandSettingsPanel() method is handled (mainly for consistency with the other implementations, like in wrappers/ActivityManager.java)
I merged the commit for newer Android versions with the main implementation
I modified commit messages, to rephrase and/or to respect the Git 50/72 rule

Here is a branch: settings_panel.
It includes #2264 too.

Please test/review if it's ok for you.

The collapsing action collapses any panels. By the way, the Android method is named collapsePanels(). PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

This will allow shortcuts such as MOD+n+n to open the settings panel. PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

brunoais · 2021-04-20T18:32:25Z

Thank you, this shortcut is very practical. +1

I rebased/changed some minor things:

...

OK. I'll work on taking these into account for next times.

Here is a branch: settings_panel.
It includes #2264 too.

Please test/review if it's ok for you.

Overall looks good. I only have nitpicks:

StatusBarManager.java: expandSettingsPanelMethodLegacy, I would make it a boolean for the new version instead of a boolean for the legacy version (expandSettingsPanelMethodNewVersion) and default it to true. This is also for the bottom "if" to not have the negation. For readability purposes.

Is it OK if I rebase this branch on the one you did and make the changes to the README?

Refs Genymobile#2260 <Genymobile#2260 (comment)>

The collapsing action collapses any panels. By the way, the Android method is named collapsePanels(). PR Genymobile#2260 <Genymobile#2260> Signed-off-by: Romain Vimont <[email protected]>

This will allow shortcuts such as MOD+n+n to open the settings panel. PR Genymobile#2260 <Genymobile#2260> Signed-off-by: Romain Vimont <[email protected]>

PR Genymobile#2260 <Genymobile#2260> Signed-off-by: Romain Vimont <[email protected]>

Double-click on extra mouse button to open the settings panel (a single-click opens the notification panel). This is consistent with the keyboard shortcut MOD+n+n. PR Genymobile#2264 <Genymobile#2264> Signed-off-by: Romain Vimont <[email protected]>

rom1v · 2021-04-20T19:05:05Z

I would make it a boolean for the new version instead of a boolean for the legacy version

OK, I first made the change for the existing code: ba47b33
Then I changed in this patchet.

New branch: settings_panel.2.

Is it OK if I rebase this branch on the one you did and make the changes to the README?

Yes, just push force settings_panel.2 to your ctrl_n_n-forSettingsPanel, this will update the PR, then you could add a commit for the README.

brunoais · 2021-04-22T21:00:33Z

What do you think of the README like that?

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

Refs #2260 <#2260 (comment)>

The collapsing action collapses any panels. By the way, the Android method is named collapsePanels(). PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

This will allow shortcuts such as MOD+n+n to open the settings panel. PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v · 2021-04-25T13:02:57Z

👌 (I just changed left-double-click to double-left-click, and the same for double-5th-click).

Merged into dev.

brunoais changed the base branch from master to dev April 17, 2021 16:10

brunoais force-pushed the ctrl_n_n-forSettingsPanel branch from 7ba0fde to a4aa0e6 Compare April 17, 2021 16:12

brunoais commented Apr 17, 2021

View reviewed changes

rom1v force-pushed the dev branch from b28adab to c4f3a76 Compare April 17, 2021 16:25

rom1v reviewed Apr 17, 2021

View reviewed changes

brunoais force-pushed the ctrl_n_n-forSettingsPanel branch from 89ad79d to 7b4679b Compare April 17, 2021 16:52

brunoais force-pushed the ctrl_n_n-forSettingsPanel branch from a070911 to 764975e Compare April 17, 2021 18:08

brunoais force-pushed the ctrl_n_n-forSettingsPanel branch 3 times, most recently from 02efbf9 to aa60476 Compare April 18, 2021 18:14

brunoais mentioned this pull request Apr 18, 2021

Press 2x extra mouse button to open settings #2264

Closed

rom1v force-pushed the dev branch from c4f3a76 to b9c3f65 Compare April 19, 2021 18:17

brunoais force-pushed the ctrl_n_n-forSettingsPanel branch from aa60476 to cd533a9 Compare April 19, 2021 18:55

rom1v pushed a commit that referenced this pull request Apr 20, 2021

Rename control message type to COLLAPSE_PANELS

f06dbd7

The collapsing action collapses any panels. By the way, the Android method is named collapsePanels(). PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Apr 20, 2021

Count repeated identical key events

b8f3064

This will allow shortcuts such as MOD+n+n to open the settings panel. PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Apr 20, 2021

Add control message to expand settings panel

66b6170

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Apr 20, 2021

Add keyboard shortcut to expand settings panel

21b8579

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v and others added 5 commits April 20, 2021 20:59

Reverse boolean logic for readability

ba47b33

Refs Genymobile#2260 <Genymobile#2260 (comment)>

Rename control message type to COLLAPSE_PANELS

3eb46ad

The collapsing action collapses any panels. By the way, the Android method is named collapsePanels(). PR Genymobile#2260 <Genymobile#2260> Signed-off-by: Romain Vimont <[email protected]>

Count repeated identical key events

a704417

This will allow shortcuts such as MOD+n+n to open the settings panel. PR Genymobile#2260 <Genymobile#2260> Signed-off-by: Romain Vimont <[email protected]>

Add control message to expand settings panel

e081b5a

PR Genymobile#2260 <Genymobile#2260> Signed-off-by: Romain Vimont <[email protected]>

Add keyboard shortcut to expand settings panel

70f7e07

PR Genymobile#2260 <Genymobile#2260> Signed-off-by: Romain Vimont <[email protected]>

Document 4th+5th + 2xn shortcuts

01f424e

brunoais force-pushed the ctrl_n_n-forSettingsPanel branch from cd533a9 to 01f424e Compare April 22, 2021 20:57

brunoais requested a review from rom1v April 22, 2021 21:01

rom1v pushed a commit that referenced this pull request Apr 25, 2021

Document 4th+5th + 2xn shortcuts

7a6608a

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v added a commit that referenced this pull request Apr 25, 2021

Reverse boolean logic for readability

bb4614d

Refs #2260 <#2260 (comment)>

rom1v pushed a commit that referenced this pull request Apr 25, 2021

Rename control message type to COLLAPSE_PANELS

66c5818

The collapsing action collapses any panels. By the way, the Android method is named collapsePanels(). PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Apr 25, 2021

Count repeated identical key events

9576283

This will allow shortcuts such as MOD+n+n to open the settings panel. PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Apr 25, 2021

Add control message to expand settings panel

50eecda

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Apr 25, 2021

Add keyboard shortcut to expand settings panel

6fa63cf

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v pushed a commit that referenced this pull request Apr 25, 2021

Document 4th+5th + 2xn shortcuts

d7e6589

PR #2260 <#2260> Signed-off-by: Romain Vimont <[email protected]>

rom1v closed this Apr 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ctrl+n+n for settings panel #2260

Ctrl+n+n for settings panel #2260

brunoais commented Apr 17, 2021 •

edited

Loading

brunoais commented Apr 17, 2021

brunoais Apr 17, 2021 •

edited

Loading

rom1v Apr 17, 2021

rom1v left a comment

rom1v Apr 17, 2021

brunoais Apr 17, 2021

brunoais commented Apr 17, 2021

brunoais commented Apr 17, 2021

brunoais commented Apr 17, 2021 •

edited

Loading

rom1v commented Apr 18, 2021

brunoais commented Apr 18, 2021

rom1v commented Apr 20, 2021

brunoais commented Apr 20, 2021 •

edited

Loading

rom1v commented Apr 20, 2021

brunoais commented Apr 22, 2021

rom1v commented Apr 25, 2021

Ctrl+n+n for settings panel #2260

Ctrl+n+n for settings panel #2260

Conversation

brunoais commented Apr 17, 2021 • edited Loading

brunoais commented Apr 17, 2021

brunoais Apr 17, 2021 • edited Loading

Choose a reason for hiding this comment

rom1v Apr 17, 2021

Choose a reason for hiding this comment

rom1v left a comment

Choose a reason for hiding this comment

rom1v Apr 17, 2021

Choose a reason for hiding this comment

brunoais Apr 17, 2021

Choose a reason for hiding this comment

brunoais commented Apr 17, 2021

brunoais commented Apr 17, 2021

brunoais commented Apr 17, 2021 • edited Loading

rom1v commented Apr 18, 2021

brunoais commented Apr 18, 2021

rom1v commented Apr 20, 2021

brunoais commented Apr 20, 2021 • edited Loading

rom1v commented Apr 20, 2021

brunoais commented Apr 22, 2021

rom1v commented Apr 25, 2021

brunoais commented Apr 17, 2021 •

edited

Loading

brunoais Apr 17, 2021 •

edited

Loading

brunoais commented Apr 17, 2021 •

edited

Loading

brunoais commented Apr 20, 2021 •

edited

Loading