search

micropython / micropython-esp32

Old port of MicroPython to the ESP32 -- new port is at https://github.com/micropython/micropython
MIT License
678 stars 218 forks source link

esp/modesp.c: Add osdebug() and oslog() #112

Closed ak15199 closed 6 years ago

ak15199 commented 7 years ago

Code lineage:

osdebug() is based loosely on the version in esp8266, but there didn't seem to be an obvious way of choosing a particular UART. The basic behavior is the same, though: provide None, and logging is disabled; provide an integer and logging is restored to the default level.

To build on that, and because the IDF provides more functionality, a second parameter has now been implemented which allows the active log level to be set:

esp.osdebug(uart[, level])

The module has a corresponding set of LOG_ values to set this accordingly. An additional level esp.LOG_CRITICAL, which maps to esp.LOG_ERROR has been included in order to provide better symetry with standard Python logging frameworks.

In order to test out correct behavior, it was straightforward to add a second function that calls the IDF to perform a logging operation:

esp.oslog(level, tag, message)

Because our intent to write anything in Python that can be in Python, the thought is that format and parameters will be transformed to a simple string in Python, to them be passed here for rendering.

ak15199 commented 7 years ago

I wanted to be able to switch off logging to the console, and was disappointed to see esp.osdebug() missing. I mean, esp32 is an evolving platform, so it's totally fair, but I still wanted it.

There are a couple of other things that I want to work on, but this one seemed like relatively low hanging fruit that I could use to learn the platform. Hopefully I haven't made too many rookie mistakes along the way, and that I've observed style guidelines and pull request convention, but let me know if I need to do anything differently in the future.

dpgeorge commented 7 years ago

I reviewed the code before reading the above comments, sorry!

An additional level esp.LOG_CRITICAL, which maps to esp.LOG_ERROR has been included in order to provide better symetry with standard Python logging frameworks.

IMO this is not necessary. The less constants in the esp module the better.

Because our intent to write anything in Python that can be in Python, the thought is that format and parameters will be transformed to a simple string in Python, to them be passed here for rendering.

With that philosophy (which is a good philosophy) one should use the Python logging module, not have something extra like oslog.

dpgeorge commented 7 years ago

I wanted to be able to switch off logging to the console, and was disappointed to see esp.osdebug() missing. I mean, esp32 is an evolving platform, so it's totally fair, but I still wanted it.

Yes, I agree, the info messages can get annoying (not to mention mess up scripts/tests that don't expect them). So the addition of esp.osdebug is certainly welcome, thanks!

ak15199 commented 7 years ago

I just commented on your original comments before I saw your latest messages. Unless I hear otherwise, I'll remove oslog from the pull request. Thanks for your attention, Damien!

dpgeorge commented 6 years ago

This PR was reworked a little to make esp.osdebug(None) always disable the log even if a second arg is given, and then merged in 84035f0f78b7584961fb7092612194f093188071

Thanks @ak15199 for the contribution.