timmbogner / Farm-Data-Relay-System

A system that uses ESP-NOW, LoRa, and other protocols to transport sensor data in remote areas without relying on WiFi.
MIT License
485 stars 108 forks source link

Feature add: FDRS logging verbosity #194

Closed aviateur17 closed 4 months ago

aviateur17 commented 4 months ago

Per discussion #192 , add two additional levels of logging verbosity FDRS_DEBUG_1 and FDRS_DEBUG_2.

FDRS_DEBUG will only show basic output so that end users are not overloaded with information and to help speed up processing of code. Additional debug levels can be used to troubleshoot and for development.

// Choose none or one of the debug options below.
// DEBUG will show some information but for troubleshooting a good level would be FDRS_DEBUG_1
#define FDRS_DEBUG     // Enable USB-Serial debugging
// define FDRS_DEBUG_1 // Additional logging messages for troubleshooting purposes - commment out the other FDRS_DEBUG options
// define FDRS_DEBUG_2 // Show all logging messages for development purposes - comment out the other FDRS_DEBUG options
timmbogner commented 4 months ago

Looks cool! I'm working on CBOR now, and will be testing this as I do.

I'll probably change some wording slightly before it's official, but I'll stick to your descriptions of the levels.

aviateur17 commented 4 months ago

Yep, change wording to your liking. I think this should be complete. As we work on code we can modify the DBG statements to classify which log level they should be in but what I've submitted should be all the background stuff needed. I guess I haven't tried compiling it in Arduino so I should do that before the day is out.

aviateur17 commented 4 months ago

Confirmed successful compile in Arduino 1.8.19 for each of the debug #define statements individually and all of them commented out for ESP8266. I believe that should be okay with the other micro controllers as well.

timmbogner commented 4 months ago

I got an idea to simplify this:

We go back to only having #define FDRS_DEBUG, but we add #define DBG_LEVEL 0 (thru 2). This new config can fall back to a global/default, so it wouldn't have to be included in every sketch. I think it should be left in all the example configs, though, just commented out. This saves us some explanation in the config, plus would be handy for people using globals. I think it would all be possible, we will probably need to make use of preprocessor conditional statements. Do you have thoughts on this? I'm cool with putting it on the shelf if you just want to get this merged.

aviateur17 commented 4 months ago

Your direction sounds great, just want to make sure I understand.

So if FDRS_DEBUG is commented out then none of the DBG statements will print If FDRS_DEBUG is not commented out then would depend upon the DBG_LEVEL Level 0 would print DBG Level 1 would print DBG and DBG1 and Level 2 would print DBG and DBG1 and DBG2?

Default in Globals would be level 0?

timmbogner commented 4 months ago

Yep, right on the money.

aviateur17 commented 4 months ago

Okay, I think we should be good to go. Let me know if you see any issues but I compiled against Arduino 1.8.19 and didn't have issues.

timmbogner commented 4 months ago

I think you forgot to setup the case of #if (DBG_LEVEL == 0), but other than that it looks perfect (and really nifty).

I'm going to add a couple new debug messages and then merge this ASAP once that's patched up.

timmbogner commented 4 months ago

I figured that was something I could contribute, hope you don't mind.

From here I'm going to add a printout of the debug verbosity level at boot, then the required entries to the readme file. Instead of noting all of your contributions individually in the docs, I will probably add a spot for you in the main readme "thank you" section soon. When I do, would you prefer I stick to your username, or can I use your actual name? It doesn't get that much traffic, but I figured I'd double check in case of privacy concerns.

aviateur17 commented 4 months ago

I think you forgot to setup the case of #if (DBG_LEVEL == 0), but other than that it looks perfect (and really nifty).

Technically I don't think that's required because if FDRS_DEBUG is defined Level 0 is the minimum assumed, but it's probably better for readability to specifically put it in there.

aviateur17 commented 4 months ago

I figured that was something I could contribute, hope you don't mind.

Nope, not at all, I appreciate the help!

From here I'm going to add a printout of the debug verbosity level at boot,

I was thinking about that as well so that's a good idea.

then the required entries to the readme file. Instead of noting all of your contributions individually in the docs, I will probably add a spot for you in the main readme "thank you" section soon. When I do, would you prefer I stick to your username, or can I use your actual name? It doesn't get that much traffic, but I figured I'd double check in case of privacy concerns.

I think my name is listed in my github account so It doesn't matter to me either way. You can put Jeff Lehman or aviateur17, whichever is consistent with the rest of the document but definitely not required.

timmbogner commented 4 months ago

it's probably better for readability to specifically put it in there.

Without it, DBG1() & DBG2() were undefined.

I'm really excited for this. Thanks again, Jeff!