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

esp32,esp8266: Fix ESP32 log levels, esp.osdebug() documentation #12900

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

projectgus
Copy link
Contributor

@projectgus projectgus commented Nov 7, 2023

As part of the first fix, the ESP32 binary file unfortunately gets 20KB larger as it was being built without INFO level logging functions. The DEBUG and VERBOSE levels still aren't compiled in by default, enabling these requires a custom build with different sdkconfig (have added documentation for this).

Binary sizes, ESP32_GENERIC board:

Max log level Size Compared to "Error"
Error 1642112
Info 1662208 +20,096 (101.2%)
Debug 1697808 +55,696 (103.4%)
Verbose 1706336 +64,224 (103.9%)

This work was funded through GitHub Sponsors.

@jimmo
Copy link
Member

jimmo commented Nov 7, 2023

Thanks @projectgus , looks good!

I had missed (when reporting #12815) that the docs were wrong for esp8266 also. Good fix!

@projectgus
Copy link
Contributor Author

projectgus commented Nov 7, 2023

I had missed (when reporting #12815) that the docs were wrong for esp8266 also. Good fix!

It might be one to look at breaking in MP 2.0, as it's really a different function on each of the ports: on esp8266 it's setting the destination UART for os_printf(), and on esp32 it's setting the minimum log level threshold.

(We could also implement runtime redirecting of ESP-IDF logs to another UART, but it's probably not a hugely important feature either... and it would still be a different API on esp8266.)

@@ -62,12 +62,18 @@ Functions

.. function:: flash_erase(sector_no)

.. function:: osdebug(level)
.. function:: osdebug(uart_no, [level])
Copy link
Member

Choose a reason for hiding this comment

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

The changes below are not fully correct.

On esp8266 the behaviour for osdebug(uart_no) is:

  • pass None to disable debug logging
  • pass an integer UART number to enable logging on that UART

For esp32 it's osdebug(uart_no, level=LOG_DEBUG) with:

  • uart_no is None to disable logging
  • otherwise logging is enabled and the optional level specifies the level, with default LOG_LOCAL_LEVEL which I think is LOG_DEBUG (?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dpgeorge This also wasn't quite right, for a simple function it has a lot of complexities!

Have pushed a new commit which I've re-tested to double check behaviour matches docs..

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

At some point the config changed such that no messages above Error level
were compiled into the final binary.

Fixes issue micropython#12815.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
The behaviour described in the docs was not correct for either port.

This work was funded through GitHub Sponsors.

Signed-off-by: Angus Gratton <angus@redyak.com.au>
@dpgeorge dpgeorge merged commit a800ed5 into micropython:master Nov 22, 2023
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

esp32: Can't change log level since IDF 5.0 update.
3 participants