chmorgan / libesphttpd

Libesphttpd - web server for ESP8266 / ESP32
Mozilla Public License 2.0
128 stars 45 forks source link

Re-comment noisy OTA upload log message #9

Closed dkonigsberg closed 6 years ago

dkonigsberg commented 6 years ago

This line is commented out in the latest revision of the upstream project. When it is left in, it generates an extremely large amount of log messages during OTA upload, slowing the whole upload process down. It should be commented (or otherwise disabled) unless one is debugging a low level of the upload process.

chmorgan commented 6 years ago

@dkonigsberg what do you think of a compile time conditional for verbose output like this? You could add a #undef VERBOSE_OUTPUT at the top of the file (below the includes) and then wrap this output with #ifdef VERBOSE_OUTPUT ...

endif

That would let someone enable/disable helpful output for debugging without having to touch a bunch of different lines to enable/disable debugging?

dkonigsberg commented 6 years ago

I would be fine with that. However, messages like this are orders of magnitude more noisy than all the rest. Most of the time, its useful to still have the rest of the log messages. Perhaps two levels of verbosity?

chmorgan commented 6 years ago

I'm actually thinking we should switch to using the built in logging functionality in esp-idf, http://esp-idf.readthedocs.io/en/latest/api-reference/system/log.html?highlight=logging

That would let someone reconfigure the logging levels at run-time and make the format align with the standard logging system.

esp8266 people would need some stubs but that shouldn't be too bad.

What do you think? We could leave the esp8266 stubs out for now to minimize the work and do those later.

dkonigsberg commented 6 years ago

I'd kinda like to get this one super-noisy (and generally useless) log message done away with now.

As a follow-on task, I agree that cleaning up the logging (as you suggest) is a good idea. I haven't actually done anything myself with the ESP32 (or related ESP-IDF) as of yet.

chmorgan commented 6 years ago

@dkonigsberg agreed. Merging.

chmorgan commented 6 years ago

@dkonigsberg btw I have a bit of a rewrite of the ota stuff in place for esp32.

are you on esp8266? That would actually be excellent because I can build it here but can't confirm it is working and it isn't super easy to rebuild each time.

dkonigsberg commented 6 years ago

Yes, I'm on the ESP8266 (ESP-12S, to be specific).

It would also be helpful if you would fork the upstream "esphttpd" example project, so we can have an integration sample that's kept up to date with your library changes.

chmorgan commented 6 years ago

@dkonigsberg I forked the freertos one. Do you mean the non-rtos one?

dkonigsberg commented 6 years ago

Yeah.

chmorgan commented 6 years ago

@dkonigsberg done!