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

feat(rt-thread): fix a bug of log #2811

Merged
merged 1 commit into from
Nov 26, 2021
Merged

feat(rt-thread): fix a bug of log #2811

merged 1 commit into from
Nov 26, 2021

Conversation

mysterywolf
Copy link
Contributor

Description of the feature or fix

fix a bug of log

Checkpoints

@kisvegabor
Copy link
Member

I think it makes sense to keep both options in parallel. E.g. the user might use printf and log to file simultaneously.
Although once the user has a custom print function to log into a file he can add a printf there too.

Anyway, I think we should keep the API the same as much as possible regardless of the config changes unless the changes are very clear and have a huge impact. (E.g. disable a widget).

What do you think?

@mysterywolf
Copy link
Contributor Author

mysterywolf commented Nov 23, 2021

I think we should keep the API the same as much as possible regardless of the config changes unless the changes are very clear and have a huge impact. (E.g. disable a widget).

There is a problem if the API is regardless of the configuration. Users cannot easily distinguish which APIs can be used under the current config and which APIs cannot be used under the current config.
For example, in #2810 , in theory, lv_log_register_print_cb only can be used effectively if LV_LOG_PRINTF is disabled (not defined); however, if we don't add any macros to limit, lv_log_register_print_cb also can be "used" even if LV_LOG_PRINTF is enabled. Users begin to confuse: "why I registered a printf callback function, but it doesn't work?" Especially for beginners who are not very familiar with the internal source code of LVGL, this (regardless of the config changes) will mislead them.

Maybe the document has a clear explanation, but I think we still need to remind users clearly and directly, because users cannot recite every sentence in the document.

On the other hand, memory wasted.

@kisvegabor
Copy link
Member

"why I registered a printf callback function, but it doesn't work?"

I think the root of the confusion comes from here:

void lv_log(const char * buf)
{
#if LV_LOG_PRINTF
    puts(buf);
#else
    if(custom_print_cb) custom_print_cb(buf);
#endif
}

It should be

void lv_log(const char * buf)
{
#if LV_LOG_PRINTF
    puts(buf);
#endif
    if(custom_print_cb) custom_print_cb(buf);
}

So allow printf and custom_print_cb to work together. IMO it's the cleanest solution.

@mysterywolf
Copy link
Contributor Author

mysterywolf commented Nov 26, 2021

OK I got it😅

@kisvegabor
Copy link
Member

Merged, thank you!

@mysterywolf mysterywolf deleted the rtt-dev branch November 26, 2021 13:50
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.

None yet

2 participants