Skip to content

Conversation

J05HM0N5TER
Copy link

ADB List (ls) for both direct device communication and for communication though the ADB server.

@J05HM0N5TER
Copy link
Author

J05HM0N5TER commented Apr 5, 2025

I have tested this with both ADB server USB and direct USB communication. I have not tested it through TCP connection.

@cocool97
Copy link
Owner

cocool97 commented Apr 6, 2025

Hi @J05HM0N5TER , thanks for the PR ! Will have a look at it this week :)

#[error("unknown transport: {0}")]
UnknownTransport(String),
/// An unknown file mode was encountered in list
#[error("Unknown file type {0}")]
Copy link
Owner

Choose a reason for hiding this comment

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

This should rather be "Unknown file mode"

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I have fixed this.

fn push(&mut self, stream: &mut dyn Read, path: &dyn AsRef<str>) -> Result<()>;

/// List the items in a directory on the device
fn list(&mut self, path: &str) -> Result<Vec<ADBListItem>>;
Copy link
Owner

Choose a reason for hiding this comment

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

I think we should rather use dyn AsRef<str> here to be more flexible on allowed input types

Copy link
Author

Choose a reason for hiding this comment

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

I used &str here because the stat function above also used it, so I thought there was some reason to use it with file paths. Should I update the stat function as well?

Copy link
Owner

Choose a reason for hiding this comment

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

Just change yours for now, I'll soon release 2.2.0 and I'll add these API breaking changes inside :)

Copy link
Author

Choose a reason for hiding this comment

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

I have updated it, please have a look when you have time.

Comment on lines 56 to 59
connection.read_exact(&mut mode)?;
connection.read_exact(&mut size)?;
connection.read_exact(&mut time)?;
connection.read_exact(&mut name_len)?;
Copy link
Owner

Choose a reason for hiding this comment

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

Can't we use connection.read_u32<LittleEndian>() here ?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I didn't know that function existed. I have fixed it now.

@cocool97
Copy link
Owner

I rebased latest main changes on your branch

@cocool97
Copy link
Owner

Also removed duplicate list command that was causing cargo run --bin adb_cli -- local --help to fail

@J05HM0N5TER
Copy link
Author

J05HM0N5TER commented Apr 13, 2025

@cocool97 I think I might have messed up your rebase. I happened to be working on it as you did your stuff and I thought I messed up my branch though some local moving of get repos, and did a git push --force-with-lease.

Can you please re-push your local version to undo my undo of the rebase you did? I have the fixes for some of your comments locally, but I'll just stash them for now to not make the git history more complicated.

If it is too much of a mess, we can just squash merge this at the end and remove all the extra stuff.

@J05HM0N5TER
Copy link
Author

Also removed duplicate list command that was causing cargo run --bin adb_cli -- local --help to fail

Thanks for that. I was a bit confused on how there seemed to be two different places that the commands a defined in. I thought that the definitions of the commands that were against the ADB server were completely disconnected from the ones that interacted directly with the device.

J05HM0N5TER and others added 2 commits May 2, 2025 15:17
ADB List (ls) for both direct device communication and for communication though the adb server.
@J05HM0N5TER J05HM0N5TER force-pushed the adb-list branch 2 times, most recently from 9165bfe to d3a886d Compare May 4, 2025 13:33
@cocool97
Copy link
Owner

cocool97 commented May 4, 2025

Thanks for you recent changes, I'll review it asap :)

Can you fix the failing fmt job ?

@J05HM0N5TER
Copy link
Author

Thanks for you recent changes, I'll review it asap :)

Can you fix the failing fmt job ?

I just pushed the changes to fix the fmt, could you please rerun the workflow?

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