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

Debug refactor #4050

Open
wants to merge 8 commits into
base: 0_15
Choose a base branch
from
Open

Debug refactor #4050

wants to merge 8 commits into from

Conversation

blazoncek
Copy link
Collaborator

Separate different debug components]

  • generic (WLED_DEBUG)
  • FX (WLED_DEBUG_FX)
  • bus manager (WLED_DEBUG_BUS)
  • pin manager (WLED_DEBUG_PINMANAGER)
  • usermods (WLED_DEBUG_USERMODS)
  • math (WLED_DEBUG_MATH)
  • all (WLED_DEBUG_ALL)

In an attempt to reduce flash usage while still getting a few debug messages.

It would still be preferable to implement #3396 instead or alongside.

- generic (WLED_DEBUG)
- FX (WLED_DEBUG_FX)
- bus manager (WLED_DEBUG_BUS)
- pin manager (WLED_DEBUG_PINMANAGER)
- usermods (WLED_DEBUG_USERMODS)
- math (WLED_DEBUG_MATH)
- all (WLED_DEBUG_ALL)

In an attempt to reduce flash usage while still getting a few debug messages.
@softhack007 softhack007 self-requested a review July 22, 2024 09:26
@netmindz
Copy link
Collaborator

From a brief look - QuickDebug has more features

With this MR I think it would be all to eat to use the wrong value going forward with new development, then lost time with people not seeing the logs they expect etc. It's one thing to do a find and replace, it's quite another to ensure correct usage going forward

@netmindz
Copy link
Collaborator

Out of interest, how much smaller is that the image without the logs?

@blazoncek
Copy link
Collaborator Author

From a brief look - QuickDebug has more features

True, but it also uses more RAM and flash and is not a drop-in replacement. I would still prefer to use it, though, if we can manage to change entire codebase.

It's one thing to do a find and replace, it's quite another to ensure correct usage going forward

The debug options are divided into separate groups which makes debug output even less crowded. I.e. if you want to debug segment and effect handling then you'd use WLED_DEBUG_FX and your output would not be overwhelmed by other unnecessary debug information. Or, if you are working on enhancing bus manager you would only want to see WLED_DEBUG_BUS output.
There are no restrictions to keep using WLED_DEBUG (or add WLED_DEBUG_ALL) for anything an uneducated user wants.

Out of interest, how much smaller is that the image without the logs?

Instead of receiving 100.1% to 105% flash usage on my development builds I can use debug I need and still be around 98.9% of flash usage. Non debug builds yield about 97%.

@netmindz
Copy link
Collaborator

From a brief look - QuickDebug has more features

True, but it also uses more RAM and flash and is not a drop-in replacement. I would still prefer to use it, though, if we can manage to change entire codebase.

Hopefully not so much that it's a blocker

It's one thing to do a find and replace, it's quite another to ensure correct usage going forward

The debug options are divided into separate groups which makes debug output even less crowded.

Yes I understand what you are trying to achieve, it's just the code discipline and chance for user error when coding to use the wrong DEBUGXX_PRINTLN by mistake. That's where QuickDebug has a big advantage

Out of interest, how much smaller is that the image without the logs?

Instead of receiving 100.1% to 105% flash usage on my development builds I can use debug I need and still be around 98.9% of flash usage. Non debug builds yield about 97%.

A fair chunk of space then. I think QuickDebug will produce smaller builds based on the log level, I'm not sure if changing the log level for specific tags would also reduce the filesize

@softhack007 softhack007 added discussion in progress This pull request is currently being reviewed and, if necessary, modified labels Sep 28, 2024
@softhack007 softhack007 added this to the 0.15.1 candidate milestone Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion in progress This pull request is currently being reviewed and, if necessary, modified
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants