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

support mdns command through adb server with server-status #55

Merged
merged 25 commits into from
Dec 1, 2024

Conversation

JinkeJ
Copy link
Contributor

@JinkeJ JinkeJ commented Nov 12, 2024

No description provided.

@JinkeJ JinkeJ mentioned this pull request Nov 12, 2024
@cocool97
Copy link
Owner

Thanks for the PR, will have a look at it asap !

@cocool97 cocool97 changed the base branch from main to mdns November 13, 2024 19:13
@cocool97
Copy link
Owner

cocool97 commented Nov 14, 2024

I had a look at the code and it looks good to me, I just made a few changes to it. Can you try and validate that nothing broke ? I removed the connect_with_envs() method and added a start_server() method that can take environment variable if needed.

For the varint parsing, couldn't we use a crate like this one and therefore remove this parser from adb_client ?

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Nov 14, 2024

The structure of server status is protobuf. You can replace the function with a crate.

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Nov 15, 2024

@cocool97
Copy link
Owner

Is everything still working as expected with my modifications ?

I can't test it as my server does not support mdns and server-status..

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Nov 16, 2024

It seems that "start_server" runs twice if we run mdns_force_openscreen_backend.
Furthermore, ServerStatus fmt function should keep output same as adb, that is only writeln when usb_backend_forced or mdns_backend_forced is true.

* chore: clarify licensing

* fix: pub export AdbStatResponse
* feat: rework USB auth

* ci: run clippy with `--all-features`
* feat: add `framebuffer_bytes` method
@JinkeJ
Copy link
Contributor Author

JinkeJ commented Nov 27, 2024

Any update?

* feat: massive internal refactoring

* feat: fix doc + add 'doc' github action

* feat: improve code; add TLS

---------

Co-authored-by: LIAUD Corentin <[email protected]>
@cocool97
Copy link
Owner

Hey, sorry for the delay !

I looked at your code, the only thing missing is adding a mdns check subcommand to adb_cli in order to provide the same functionalities as the official binary.

Can you handle it ? I'll merge it afterwards

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Nov 29, 2024

running start_server twice causes the stuck of starting new process, because when the first process haven't lifted up, the second process will try to start the server again.

cli-s1n and others added 2 commits November 30, 2024 10:21
This allows to be more flexible on USB device we want to connect to,
without making public API to complex
* feat: add benches
@cocool97
Copy link
Owner

Would you mind having a look at it ? I'm afraid I won't have time in the next few days..

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Nov 30, 2024

I will have a try, but might need to define a new function or corrupt current existing function

@cocool97
Copy link
Owner

cocool97 commented Nov 30, 2024

No problem, I'm planning on releasing a v2.1 with mdns

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Dec 1, 2024

@cocool97 Please resolve the conflict with main branch because I didn't pull the mdns branch to my local.

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Dec 1, 2024

mdns check, mdns devices, reconnect, reconnect offline, tcpip, usb, forwardremoveall, reverseremoveall, server-status

Those features are added in these commits.

@cocool97
Copy link
Owner

cocool97 commented Dec 1, 2024

Thanks, I'll rebase it asap

@cocool97
Copy link
Owner

cocool97 commented Dec 1, 2024

Double start_server is now fixed ?

@JinkeJ
Copy link
Contributor Author

JinkeJ commented Dec 1, 2024

Yes, I removed start_server, because connect will start too.

@cocool97 cocool97 merged commit e8d0108 into cocool97:mdns Dec 1, 2024
5 checks passed
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.

4 participants