rmtmckenzie / flutter_native_device_orientation

Native device orientation plugin for flutter.
MIT License
76 stars 63 forks source link

Remove verbose log #61

Closed itsJoKr closed 9 months ago

itsJoKr commented 9 months ago

This plugin makes some very verbose logs listening using window listener when using your other plugin for qr code scanning. This causes way too much spam and makes the console unusable.

An easy solution is to remove these logs. Do you think they are essential info for developers?

Other solutions might be to add LogLevel info, or to log this only once (with a flag). But I have a feeling that just removing it is fine. What do you think?

rmtmckenzie commented 9 months ago

Hmm. I would have expected that to only be called when the orientation listener is started, at least that was the intent. I don't see any reason why it needs to be logged, but I'm actually more concerned why it would be creating that much spam.

rmtmckenzie commented 9 months ago

I won't be able to take a good look into for this for a few days so if you could just do a quick bit of testing that'd be a huge help once I do get to looking at it. What kind of behavior is triggering the logging - when the orientation changes, when the camera is started/stopped, or is it a continuous stream or something (which would be much more of a concern)?

itsJoKr commented 9 months ago

Yes, this log is continuously logging, maybe once each frame.

I've tested qr scanner package, and it happens on Android emulator, and Android device (Pixel 7), but not on iOS device.

rmtmckenzie commented 9 months ago

I'm assuming you're seeing this in your app? Can you test it with the example app for the qr_mobile_vision package - I tested with that and was not able to reproduce this. I'm only seeing the logging when it starts and when the orientation changes.

I've also pushed a new version out with a lot of changes, and will be pushing out a new version of qr_mobile_vision soon, so that might help as well.

itsJoKr commented 9 months ago

I got the same thing with example. One thing I didn't note is that this only happens when the detection scans for the barcode.

And digging down, I noticed that the cause might be entirely in example, when setting state in qrCodeCallback. If I comment that out I don't see verbose logs (but the example doesn't work).

So it's like the rebuild of QrCamera widget somehow calls this device orientation again.

https://github.com/rmtmckenzie/flutter_native_device_orientation/assets/11093480/0ba0e6bc-6762-4c70-b807-9647f53770b4

rmtmckenzie commented 9 months ago

Ah okay that's making more sense then, I probably didn't build the orientation listener to persist between state changes. I think that should be relatively easy to fix, I'll try to take a look later today.

On Fri, Jan 26, 2024, 03:23 Josip Krnjic @.***> wrote:

I got the same thing with example. One thing I didn't note is that this only happens when the detection scans for the barcode.

And digging down, I noticed that the cause might be entirely in example, when setting state in qrCodeCallback. If I comment that out I don't see verbose logs (but the example doesn't work).

So it's like the rebuild of QrCamera widget somehow calls this device orientation again.

https://github.com/rmtmckenzie/flutter_native_device_orientation/assets/11093480/0ba0e6bc-6762-4c70-b807-9647f53770b4

— Reply to this email directly, view it on GitHub https://github.com/rmtmckenzie/flutter_native_device_orientation/pull/61#issuecomment-1911919252, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXVV7BI6EH3MABJZQFBLTDYQOGZXAVCNFSM6AAAAABB6J2K6SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMJRHEYTSMRVGI . You are receiving this because you commented.Message ID: @.*** com>

rmtmckenzie commented 9 months ago

Okay, I've published a new version that fixes that; the logging should only be done once when the app starts listening for orientation changes. That helped find an actually decently bad bug; the stream of orientation events was being created every time the NativeDeviceOrientationReader was built, which is obviously not ideal.

I'll let you close this if you're happy with the new version.

itsJoKr commented 9 months ago

Great. I will close this.