BrainiumLLC / cargo-mobile

Rust on mobile made easy!
Apache License 2.0
1.23k stars 52 forks source link

Android logcat level #28

Closed davidkern closed 3 years ago

davidkern commented 3 years ago

Thanks for creating this tool - it is generally pretty ergonomic and I appreciate all the thought that has gone into it.

I did get stuck a bit with android logging, so I wanted to bring the issue up for discussion, and then I'd be happy to put up a PR if changes are desirable.

I had added a few debug and trace logs to my project and was confused for a bit why I couldn't get anything to display on the logcat started by cargo android run. I eventually figured out that I could add -vv to get more logging, but that wasn't clear at first. If I had any info level logs I may have figured it out sooner, but regardless, it wasn't clear that -vv was a thing until I read through the source.

So now I can at least see logs from the device. But with how the noise levels work currently I can only choose between warn, info or everything. I'd very much like to be able to limit this according to the standard android log levels. In particular, normally I would be interested in debug but not trace. While I do include trace logs in my debug builds, they are there for the rare case when that much detail is needed to figure out a bug. Typically I use debug for the troubleshoot-by-print dance as well as to emit periodic diagnostic data. But these get overwhelmed by the trace logs if they are enabled.

There are two ways I could think to add the ability to get debug but not trace:

Just add another noise level, so -vv gets you debug output but not trace, and -vvv gets everything. Pro: it is a small change, Con: it changes existing behavior

Add an additional option specifically for the device log level, e.g. --debug or --log=debug. Pro: doesn't change existing behavior, Con: bigger change and what should happen if verbosity and this new option are both provided.

What do you think?

zeerooth commented 3 years ago

Hi, I'm happy that you've found this tool useful!

First of all, I think we should definitely put the information about debugging and different verbosity levels in README so that future users can find this information quickly.

About different verbosity levels: It's definitely a nice option to include --log parameter because there are 6 logging levels in Android and every developer might be interested in different one on different stages of debugging/testing their applications. I also think that this option should override -v and -vv because these are less verbose in nature. So, in the end we would end up with 2 different options to change the logging verbosity:

  1. Through -v and -vv which is a tad quicker and predefined
  2. With --log=error/warn/info/... which overrides levels predefined by -v and -vv and gives greater flexibility for developers

Would these changes be alright?

davidkern commented 3 years ago

I think your proposal makes logical sense. On implementation, filter seemed a more precise name to emphasize this does not change what the app logs, only what is displayed.

I need to test the above PR in a workflow - but it feels like this would cover the use case.

Edit: I've now tested this against a mobile project and looks like this works out ok.