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

Different exit codes for failure to connect and disconnect #3085

Closed
wants to merge 1 commit into from

Conversation

madduck
Copy link
Contributor

@madduck madduck commented Mar 5, 2022

Modify the return logic such that exit code 1 is used when the initial
connection fails, but if a session is established, and then the device
disconnects, exit code 2 is emitted.

Closes: Github:#3083
Signed-off-by: martin f. krafft [email protected]

Modify the return logic such that exit code 1 is used when the initial
connection fails, but if a session is established, and then the device
disconnects, exit code 2 is emitted.

Closes: Github:Genymobile#3083
Signed-off-by: martin f. krafft <[email protected]>
rom1v added a commit that referenced this pull request Mar 6, 2022
We must distinguish 3 cases for await_for_server():
 - an error occurred
 - no error occurred, the device is connected
 - no error occurred, the device is not connected (user requested to
   quit)

For this purpose, use an additional output parameter to indicate if the
device is connected (only set when no error occurs).

Refs #3085 <#3085>
rom1v pushed a commit that referenced this pull request Mar 6, 2022
Modify the return logic such that exit code 1 is used when the initial
connection fails, but if a session is established, and then the device
disconnects, exit code 2 is emitted.

Fixes #3083 <#3083>
PR #3085 <#3085>
Signed-off-by: martin f. krafft <[email protected]>
Signed-off-by: Romain Vimont <[email protected]>
@rom1v
Copy link
Collaborator

rom1v commented Mar 6, 2022

Hi,

Thank you for your PR.

I added a preliminary commit to report the 3 possible states in await_for_server() (this simplifies the implementation of this feature): 1f4c801

Then I made minor changes to your commit:

  • use a dedicated enum scrcpy_exit_code (to avoid exposing SDL constants resulting of implementation details)
  • (nitpick) EXIT CODESEXIT STATUS in manpage (for consistency with other manpages)

The resulting branch is here: exit_code

Please review/test :)

@madduck
Copy link
Contributor Author

madduck commented Mar 8, 2022

@rom1v I've been running scrcpy from your exit-code branch, and it works just fine. Thank you!

@madduck
Copy link
Contributor Author

madduck commented Mar 8, 2022

Do you need me to migrate your branch over to this PR or should we just close this and make a new one on the basis of your branch?

rom1v added a commit that referenced this pull request Mar 10, 2022
@rom1v
Copy link
Collaborator

rom1v commented Mar 10, 2022

Merged into dev.

I also added a commit to document the exit statis in scrcpy --help: b1dbc30

Exit status:

      0  Normal program termination
      1  Start failure
      2  Device disconnected while running

@rom1v rom1v closed this Mar 10, 2022
rom1v added a commit that referenced this pull request Jun 18, 2023
Audio did not work on Honor devices.

Two workarounds are necessary:
 - a system context must be set as a base context of FakeContext (so
   that a PackageManager is available),
 - the same workaround as for Meizu phones must also be applied (so that
   ActivityThread.currentApplication() return a valid instance).

These workarounds must not be applied for all devices, they cause other
issues.

Fixes #4015 <#4015>
Refs #3085 <#3805>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Jun 19, 2023
Audio did not work on Honor devices.

To make it work, a system context must be set as a base context of
FakeContext (so that a PackageManager is available), and a current
ActivityThread must be initialized.

These workarounds must not be applied for all devices, because they
might cause other issues.

Fixes #4015 <#4015>
Refs #3085 <#3805>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Jun 19, 2023
Audio did not work on Honor devices.

To make it work, a system context must be set as a base context of
FakeContext (so that a PackageManager is available), and a current
ActivityThread must be initialized.

These workarounds must not be applied for all devices, because they
might cause other issues.

Fixes #4015 <#4015>
Refs #3085 <#3805>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Jun 19, 2023
Audio did not work on Honor devices.

To make it work, a system context must be set as a base context of
FakeContext (so that a PackageManager is available), and a current
Application must be initialized.

These workarounds must not be applied for all devices, because they
might cause other issues.

Fixes #4015 <#4015>
Refs #3085 <#3805>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Jun 19, 2023
Audio did not work on Honor devices.

To make it work, a system context must be set as a base context of
FakeContext (so that a PackageManager is available), and a current
Application must be initialized.

These workarounds must not be applied for all devices, because they
might cause other issues.

Fixes #4015 <#4015>
Refs #3085 <#3805>

Co-authored-by: Simon Chan <[email protected]>
rom1v added a commit that referenced this pull request Jun 19, 2023
Audio did not work on Honor devices.

To make it work, a system context must be set as a base context of
FakeContext (so that a PackageManager is available), and a current
Application and ActivityThread must be set.

These workarounds must not be applied for all devices, because they
might cause other issues.

Fixes #4015 <#4015>
Refs #3085 <#3805>

Co-authored-by: Simon Chan <[email protected]>
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