Replace DEBUG_TRACE with LOG #4385
Conversation
src/main/io/serial.c
Outdated
| @@ -115,7 +115,7 @@ void pgResetFn_serialConfig(serialConfig_t *serialConfig) | |||
| serialConfig->portConfigs[i].peripheral_baudrateIndex = BAUD_115200; | |||
| } | |||
|
|
|||
| serialConfig->portConfigs[0].functionMask = FUNCTION_MSP | FUNCTION_DEBUG_TRACE; | |||
| serialConfig->portConfigs[0].functionMask = FUNCTION_MSP | FUNCTION_LOG; | |||
There was a problem hiding this comment.
Probably we should revert this and not set FUNCTION_LOG by default on any ports. The intention is to leave at least error messages always enabled and written to blackbox. Otherwise we might be sending LOG frames via MSP to callers that are not ready for it (specially MSPv1 only callers).
Since FEATURE_DEBUG_TRACE is now gone, enabling logging shouldn't be as painful as enabling debug trace was.
src/main/common/memory.c
Outdated
| @@ -50,11 +52,11 @@ void * memAllocate(size_t wantedSize, resourceOwner_e owner) | |||
| retPointer = &dynHeap[dynHeapFreeWord]; | |||
| dynHeapFreeWord += wantedWords; | |||
| dynHeapUsage[owner] += wantedWords * sizeof(uint32_t); | |||
| DEBUG_TRACE("Memory allocated. Free memory = %d", memGetAvailableBytes()); | |||
| LOG_E(SYSTEM, "Memory allocated. Free memory = %d", memGetAvailableBytes()); | |||
There was a problem hiding this comment.
This is DEBUG or INFO, not ERROR
src/main/io/serial.c
Outdated
| @@ -115,7 +115,7 @@ void pgResetFn_serialConfig(serialConfig_t *serialConfig) | |||
| serialConfig->portConfigs[i].peripheral_baudrateIndex = BAUD_115200; | |||
| } | |||
|
|
|||
| serialConfig->portConfigs[0].functionMask = FUNCTION_MSP | FUNCTION_DEBUG_TRACE; | |||
| serialConfig->portConfigs[0].functionMask = FUNCTION_MSP | FUNCTION_LOG; | |||
| } | ||
| } | ||
| } else if (mspLogPort) { | ||
| mspSerialPushPort(MSP_DEBUGMSG, (uint8_t*)buf, size, mspLogPort, MSP_V2_NATIVE); |
There was a problem hiding this comment.
This is actually dangerous. MSP on unconnected VCP will hang, could be very dangerous in flight.
There was a problem hiding this comment.
Ooops. Good catch! Thing is, we might also want sync messages when debugging to make the FC wait for the listener to connect. Maybe we can get rid of the _SYNC macros and add a setting to make all logging statements sync? In any case, I'll make the code path for synchronous logging check the arming state before blocking.
fiam
force-pushed
the
agh_log
branch
3 times, most recently
from
1de0f10 to
a4baca3
Compare
|
@digitalentity We need to discuss what to do with the SYNC macros as well as handling the "push-to-MSP" in the safer way possible. My vote for the first is to just drop them and use a setting to make all logging statements synchronous (and maybe make it block arming when it's enabled). For the latter, I think we should just skip logging over MSP when the craft is armed and change the VCP driver to avoid blocking if there's nothing connected (or add a timeout if there's no reliable way to detect a connection). |
|
Agreed. Let's just drop SYNC and make everything async. We need to consider that |
|
@fiam can you fix conflicts? |
fiam
force-pushed
the
agh_log
branch
4 times, most recently
from
637c4c8 to
c994e50
Compare
LOG system has multiple levels, selectable both at compile and run times. FEATURE_TRACE has been removed, since we now rely just on the log level/topic and the defined outputs for the log messages.
Make all logging asynchronous
LOG system has multiple levels, selectable both at compile and run
times. FEATURE_TRACE has been removed, since we now rely just on
the log level/topic and the defined outputs for the log messages.