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

Add methods to disable logger completely (#708) #861

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bobatsar
Copy link

As described in #708 the string interpolation can use quite a lot of CPU time, even if the logger is set to LogLevel.none.

This PR allows to set the logger to null which then removes the string interpolations.

@@ -433,9 +433,19 @@ class FlutterReactiveBle {
///
/// Use [LogLevel.verbose] for full debug output. Make sure to run this only for debugging purposes.
/// Use [LogLevel.none] to disable logging. This is also the default.
Copy link
Collaborator

Choose a reason for hiding this comment

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

if you set logLevel to LogLevel.none you will disable logs

Copy link
Author

Choose a reason for hiding this comment

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

Even when setting the logLevel to none, the ressource hungry string interpolation happens. Please reopen!

/// Sets the logger.
///
/// Set to null to disable logging.
set logger(Logger? logger) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you please updated example app?

@Taym95
Copy link
Collaborator

Taym95 commented Jun 5, 2024

Thanks a lot for contributing, and sorry for closing it. I double-checked, and indeed it uses quite a lot of CPU time. Please update the example app and let's merge this one. It will be available in the next version.

@bobatsar
Copy link
Author

bobatsar commented Jun 5, 2024

Ok, I will have a look at the example app.

How should the behavior be in the example app?
Logging completely disabled or how should I change it?

The example app still works as it is with the old behavior (_debugLogger is initialized to DebugLogger).
To disable the logger (my PR) one simply has to call e.g.

final _ble = FlutterReactiveBle();
_ble.logger = null;

@bobatsar
Copy link
Author

@Taym95
In my opinion the disabled logger should be the default (to make sure no performance is wasted in the logger) and only when enabling it manually it should be started.

Otherwise all users have to disable it to get the best performance.

This woul mean some more changes in the code and the example.

What is your preferred solution?

@Taym95
Copy link
Collaborator

Taym95 commented Jun 10, 2024

@Taym95 In my opinion the disabled logger should be the default (to make sure no performance is wasted in the logger) and only when enabling it manually it should be started.

Otherwise all users have to disable it to get the best performance.

This woul mean some more changes in the code and the example.

What is your preferred solution?

It makes sense, but some people are already using this. Functionality will stop working for them when the update happens

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