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

Not all Android versions do now log #66

Closed
peterdk opened this issue Feb 28, 2023 · 11 comments · Fixed by #67
Closed

Not all Android versions do now log #66

peterdk opened this issue Feb 28, 2023 · 11 comments · Fixed by #67
Labels
k::ux UX (user experience) releated regression
Milestone

Comments

@peterdk
Copy link

peterdk commented Feb 28, 2023

I upgraded from 0.12.0 to 0.13.0, and while logging still works at API 33, I get with indentical apk, no output in API 26 for example.
I don't know why? Could it be not selecting proper output log buffer? I basically do this:

 android_logger::init_once(Config::default().with_max_level(LevelFilter::Trace));
@peterdk
Copy link
Author

peterdk commented Feb 28, 2023

Thanks for the library by the way!

@tyranron
Copy link
Contributor

tyranron commented Mar 1, 2023

@peterdk maybe... did you try to specify log buffer explicitly?

@peterdk
Copy link
Author

peterdk commented Mar 1, 2023

When I specify

.with_log_buffer(LogId::System)

It immediately starts working again on Android 9. When I dont' specify it, all logs are silent.

I would expect that the LogId is set to System by default?

@tyranron
Copy link
Contributor

tyranron commented Mar 1, 2023

@peterdk

I would expect that the LogId is set to System by default?

No, of course not. By default, system defaults are used. I guess that it differs depending on Android version.

Prior 0.13, we've used __android_log_write(), and now we use __android_log_buf_write(). Its documentation says:

Apps should use __android_log_write() instead.

So, I can conclude that __android_log_write() is hardcoded to use LOG_ID_MAIN buffer.

I guess you should be totally OK with .with_log_buffer(LogId::Main).

@peterdk
Copy link
Author

peterdk commented Mar 1, 2023

Ah ok I understand. Just noting that lots of users probably now get their logs not visible by default. Maybe update documentation for this?

@tyranron
Copy link
Contributor

tyranron commented Mar 1, 2023

@peterdk could you bisect starting from which Android API version system defaults don't work? Is this API 26 and down and API 27 is OK?

@tyranron
Copy link
Contributor

tyranron commented Mar 1, 2023

Ideally, it would be nice to default to LOG_ID_DEFAULT on APIs supporting it and LOG_ID_MAIN otherwise. However, I cannot figure out how to detect API level programmatically.

@peterdk could you advise something here? Or maybe you can even craft a PR doing that?

@peterdk
Copy link
Author

peterdk commented Mar 1, 2023

What is your reasoning behind not just setting it to default main? I haven't come across any other logcat log in my day to day Android development? And in case some platform Android Rust code uses this library, they can set it to the preferred stream? As you wrote, normal apps only use Main. I didn't even know there were others.

@tyranron
Copy link
Contributor

tyranron commented Mar 1, 2023

rfc @rsglobal

@tyranron
Copy link
Contributor

tyranron commented Mar 1, 2023

@peterdk

As you wrote, normal apps only use Main.

If that was the case, the Android developers could have not introduce LOG_ID_DEFAULT at all. But it's there for some reason. Complying with their assumptions rather than introducing our own logic seems reasonable and future-proof.

rsglobal referenced this issue in rsglobal/android_logger-rs Mar 3, 2023
As per user report at least SDKv26 does not resolve LOG_ID_DEFAULT
to LOG_ID_MAIN internally, which forces users to set LOG_ID_MAIN
explicitly.

Rework android_log funcion to accept optional buf_id and use
__android_log_write ffi function if no buf_id specified.
This will return previous behavior for users that doesn't
need to override buf_id.

Closes: https://github.com/Nercury/android_logger-rs/issues/66
Fixes: fee1bea ("Support selecting target buffer of Android logging system")
Signed-off-by: Roman Stratiienko <[email protected]>
rsglobal referenced this issue in rsglobal/android_logger-rs Mar 3, 2023
As per user report at least SDKv26 does not resolve LOG_ID_DEFAULT
to LOG_ID_MAIN internally, which forces users to set LOG_ID_MAIN
explicitly.

Rework android_log funcion to accept optional buf_id and use
__android_log_write FFI function if no buf_id specified.
This will return previous behavior for users that do not
need to override buf_id.

Closes: https://github.com/Nercury/android_logger-rs/issues/66
Fixes: fee1bea ("Support selecting target buffer of Android logging system")
Signed-off-by: Roman Stratiienko <[email protected]>
rsglobal referenced this issue in rsglobal/android_logger-rs Mar 3, 2023
As per user report at least SDKv26 does not resolve LOG_ID_DEFAULT
to LOG_ID_MAIN internally, which forces users to set LOG_ID_MAIN
explicitly.

Rework android_log function to accept optional buf_id and use
__android_log_write FFI function if no buf_id specified.
This will return previous behavior for users that do not
need to override buf_id.

Closes: https://github.com/Nercury/android_logger-rs/issues/66
Fixes: fee1bea ("Support selecting target buffer of Android logging system")
Signed-off-by: Roman Stratiienko <[email protected]>
@tyranron tyranron added regression k::ux UX (user experience) releated labels Mar 7, 2023
@tyranron tyranron added this to the 0.13.1 milestone Mar 7, 2023
tyranron added a commit that referenced this issue Mar 7, 2023
Signed-off-by: Roman Stratiienko <[email protected]>
Co-authored-by: Kai Ren <[email protected]>
@tyranron
Copy link
Contributor

tyranron commented Mar 7, 2023

@peterdk fixed in #67, released in 0.13.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
k::ux UX (user experience) releated regression
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants