Skip to content
This repository was archived by the owner on Sep 6, 2023. It is now read-only.

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

Closed
wants to merge 2 commits into from
Closed

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
773828f
esp/modesp.c: Add osdebug() and oslog()
ak15199 Jun 15, 2017
e614378
esp32/modesp.c updates for logging pull request per dpgeorge
ak15199 Jun 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions esp32/modesp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#include <stdio.h>

#include "esp_log.h"
#include "esp_spi_flash.h"

#include "py/runtime.h"
Expand All @@ -37,6 +38,25 @@
#include "drivers/dht/dht.h"
#include "modesp.h"

STATIC mp_obj_t esp_osdebug(size_t n_args, const mp_obj_t *args) {
switch (n_args) {
case 1:
if (args[0] == mp_const_none) {
esp_log_level_set("*", ESP_LOG_ERROR);
} else {
esp_log_level_set("*", LOG_LOCAL_LEVEL);
}
break;

case 2:
esp_log_level_set("*", mp_obj_get_int(args[1]));
break;
}

return mp_const_none;
}
STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(esp_osdebug_obj, 1, 2, esp_osdebug);
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to have this function just take 1 arg, esp.osdebug(level), same as esp8266, and just have level=None mean disable, and any other value is an integer that is passed as the second arg to esp_log_level_set?

Copy link
Member

Choose a reason for hiding this comment

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

In the case of the ESP32 it doesn't really allow you to redirect errors to different UARTs (?) so I'd say it's fine to repurpose the single argument to this function to be the log level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe? You're the maintainer, so get the final say.

The reasoning for the additional parameter is twofold: that repurposing the existing argument would break backwards compatibility for anyone migrating code from ESP8266 in a minor but unpredictable way; the other is that although I can't see how to do it yet, the ESP32 does have two UARTs, and the hardware allows developers to change pin assignments for how UARTs are routed... it may be that at some point this would become possible.

But would appreciate your thoughts.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, points taken. But I think the implementation should be adjusted so that passing None as the first arg always disables logging, even if there are 2 args passed (eg esp.osdebug(None, 123)).


STATIC mp_obj_t esp_flash_read(mp_obj_t offset_in, mp_obj_t buf_in) {
mp_int_t offset = mp_obj_get_int(offset_in);
mp_buffer_info_t bufinfo;
Expand Down Expand Up @@ -93,6 +113,8 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_3(esp_neopixel_write_obj, esp_neopixel_write_);
STATIC const mp_rom_map_elem_t esp_module_globals_table[] = {
{ MP_ROM_QSTR(MP_QSTR___name__), MP_ROM_QSTR(MP_QSTR_esp) },

{ MP_ROM_QSTR(MP_QSTR_osdebug), MP_ROM_PTR(&esp_osdebug_obj) },

{ MP_ROM_QSTR(MP_QSTR_flash_read), MP_ROM_PTR(&esp_flash_read_obj) },
{ MP_ROM_QSTR(MP_QSTR_flash_write), MP_ROM_PTR(&esp_flash_write_obj) },
{ MP_ROM_QSTR(MP_QSTR_flash_erase), MP_ROM_PTR(&esp_flash_erase_obj) },
Expand All @@ -101,6 +123,14 @@ STATIC const mp_rom_map_elem_t esp_module_globals_table[] = {

{ MP_ROM_QSTR(MP_QSTR_neopixel_write), MP_ROM_PTR(&esp_neopixel_write_obj) },
{ MP_ROM_QSTR(MP_QSTR_dht_readinto), MP_ROM_PTR(&dht_readinto_obj) },

{ MP_ROM_QSTR(MP_QSTR_LOG_NONE), MP_ROM_INT((mp_uint_t)ESP_LOG_NONE)},
{ MP_ROM_QSTR(MP_QSTR_LOG_ERROR), MP_ROM_INT((mp_uint_t)ESP_LOG_ERROR)},
{ MP_ROM_QSTR(MP_QSTR_LOG_WARNING), MP_ROM_INT((mp_uint_t)ESP_LOG_WARN)},
{ MP_ROM_QSTR(MP_QSTR_LOG_INFO), MP_ROM_INT((mp_uint_t)ESP_LOG_INFO)},
{ MP_ROM_QSTR(MP_QSTR_LOG_DEBUG), MP_ROM_INT((mp_uint_t)ESP_LOG_DEBUG)},
{ MP_ROM_QSTR(MP_QSTR_LOG_VERBOSE), MP_ROM_INT((mp_uint_t)ESP_LOG_VERBOSE)},
{ MP_ROM_QSTR(MP_QSTR_LOG_DEFAULT), MP_ROM_INT((mp_uint_t)LOG_LOCAL_LEVEL)},
};

STATIC MP_DEFINE_CONST_DICT(esp_module_globals, esp_module_globals_table);
Expand Down