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

Adb parse device list add offline field improve #671

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

18506711722
Copy link
Contributor

Adb parse device list add offline field improve

@@ -610,6 +616,7 @@ export namespace AdbServerClient {
model?: string | undefined;
device?: string | undefined;
transportId: bigint;
offline?:boolean
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to use a single status field (and an enum) to replace offline and authenticating fields.

It might only contain Offline, Unauthorized and Online these three values initially, it can be extended later.

@@ -169,11 +169,17 @@ export class AdbServerClient {
break;
}
}

if(status === 'offline' && !product && !model && !device){
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? In which situation the device will (or won't) have these fields?

(these three fields are optional in the type definition)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

phone status online -> offline ,There will be no product,model and device fields

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this exclude all offline devices?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, he doesn't exclude all offline devices, just filter out unwanted offline push messages

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any concern?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter out unwanted offline push messages

Which ones are "wanted" and which ones are "unwanted"?

I'd like the method to return all devices, and let users decide what to do (they can filter by !product && !model && !device themselves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous versions will filter out offline information, which is not friendly to devices that need to listen in real time

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still don't understand what you mean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

 if (status !== "device" && status !== "unauthorized" ) {
continue;
}

offline push message Will be filtered

 if (status !== "device" && status !== "unauthorized"  && status !== "offline" ) {
continue;
}

Copy link
Contributor Author

@18506711722 18506711722 Sep 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

status === "offline" There will be multiple offline messages and not all of them will make sense

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