-
-
Notifications
You must be signed in to change notification settings - Fork 84
NimBLELog support logging without IDF logging, allow custom colors. #256
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
Conversation
thekurtovic
commented
Dec 5, 2024
- This will allow NimBLE logging to be enabled without requiring IDF logging to be enabled.
- Additionally this will allow for color codes to be customized for each log level.
- Cleaned up preprocessor indentation style.
|
WIP The kconfig part is a bit messy, but I'm not sure how else to go about it. I wanted to make it so that the log colors macros could be selected without requiring the if/elif color checks for each level. Let me know if there's a better way to do it. |
|
At first glance I think it's an interesting idea. I was thinking about overhauling the logging as well, I've removed quite a bit of it but better filtering is needed and the custom color idea is great. I'll look at this more in the next days but I have quite a bit on my agenda and want to publish a version 2 release before Christmas, so this might have to come after that. |
|
@thekurtovic Unfortunately this breaks the log filtering capability. |
|
This was meant to be an option which doesn't rely on esp logging, the loss of filtering would be a trade-off. Also wasn't aware you could even do that. |
|
Unfortunately to change the colors we need to bypass the log filtering and that wouldn't be great. Here is some more info though, https://docs.espressif.com/projects/esp-idf/en/stable/esp32/api-reference/system/log.html. |
|
The color part can still work with filtering like this. |
|
@thekurtovic That looks good, have you tested it? |
|
Yes, tested on esp32-s3. |
|
Perfect, please open a new PR or reopen this one and I will look to test/merge it after this release is done. Thanks for all the contributions! |