-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trace Logger Feature Review and Merge #2953
base: devel
Are you sure you want to change the base?
Changes from 43 commits
2a5dba3
2e67154
27a58bc
2156618
25ede54
8d30c55
3638982
6160ed2
d1c48ca
5d50140
9b1cd59
3a5a366
ccf88a9
10ef05b
3f0d2db
1a000c2
8c53be4
33dd922
11eabfe
9193a20
aaadbab
5d372d4
a4b5cb6
b548ebb
b0a9ba0
f99fc34
d598e4e
74eb8f8
0f0692a
ed63e2d
a94b8f7
3e28797
e70c568
8bbda39
d30244b
3e0b546
15cb971
44fc775
acf1982
6486bd3
64844f4
66a9cb8
2dbf993
b42e62c
9e10a49
ca35e21
7539954
7d4e9e4
f3479d1
6bc9b92
97af364
179127e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
#### | ||
# F prime CMakeLists.txt: | ||
# | ||
# SOURCE_FILES: combined list of source and autocoding files | ||
# MOD_DEPS: (optional) module dependencies | ||
# | ||
#### | ||
set(MOD_DEPS | ||
Fw/Com | ||
) | ||
set(SOURCE_FILES | ||
"${CMAKE_CURRENT_LIST_DIR}/TraceBuffer.cpp" | ||
"${CMAKE_CURRENT_LIST_DIR}/TraceEntity.cpp" | ||
"${CMAKE_CURRENT_LIST_DIR}/Trace.fpp" | ||
) | ||
register_fprime_module() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
module Fw { | ||
|
||
type TraceBuffer | ||
|
||
|
||
@ Port for sending Traces | ||
port Trace( | ||
$id: FwTraceIdType @< Trace ID | ||
ref timeTag: Fw.Time @< Time Tag | ||
$type: TraceCfg.TraceType @< The trace type argument | ||
ref args: TraceBuffer @< Buffer containing serialized trace entry | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
#include <Fw/Trace/TraceBuffer.hpp> | ||
#include <Fw/Types/Assert.hpp> | ||
#include <Fw/Log/TextLogString.hpp> | ||
#include <stdio.h> | ||
|
||
namespace Fw { | ||
|
||
TraceBuffer::TraceBuffer(const U8 *args, NATIVE_UINT_TYPE size) { | ||
Check notice Code scanning / CodeQL Use of basic integral type Note
size uses the basic integral type unsigned int rather than a typedef with size and signedness.
|
||
SerializeStatus stat = SerializeBufferBase::setBuff(args,size); | ||
Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter args has not been checked.
Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter size has not been checked.
|
||
FW_ASSERT(FW_SERIALIZE_OK == stat, static_cast<FwAssertArgType>(stat)); | ||
Shivaly-Reddy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
TraceBuffer::TraceBuffer() { | ||
} | ||
|
||
TraceBuffer::~TraceBuffer() { | ||
} | ||
|
||
TraceBuffer::TraceBuffer(const TraceBuffer& other) : Fw::SerializeBufferBase() { | ||
Check warning Code scanning / CppCheck Member variable 'TraceBuffer::m_bufferData' is not assigned in the copy constructor. Should it be copied? Warning
Member variable 'TraceBuffer::m_bufferData' is not assigned in the copy constructor. Should it be copied?
|
||
SerializeStatus stat = SerializeBufferBase::setBuff(other.m_bufferData,other.getBuffLength()); | ||
Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter other has not been checked.
|
||
FW_ASSERT(FW_SERIALIZE_OK == stat, static_cast<FwAssertArgType>(stat)); | ||
} | ||
|
||
TraceBuffer& TraceBuffer::operator=(const TraceBuffer& other) { | ||
if(this == &other) { | ||
return *this; | ||
} | ||
|
||
SerializeStatus stat = SerializeBufferBase::setBuff(other.m_bufferData,other.getBuffLength()); | ||
FW_ASSERT(FW_SERIALIZE_OK == stat, static_cast<FwAssertArgType>(stat)); | ||
return *this; | ||
} | ||
|
||
NATIVE_UINT_TYPE TraceBuffer::getBuffCapacity() const { | ||
Check notice Code scanning / CodeQL Use of basic integral type Note
getBuffCapacity uses the basic integral type unsigned int rather than a typedef with size and signedness.
|
||
return sizeof(this->m_bufferData); | ||
} | ||
|
||
const U8* TraceBuffer::getBuffAddr() const { | ||
return this->m_bufferData; | ||
} | ||
|
||
U8* TraceBuffer::getBuffAddr() { | ||
return this->m_bufferData; | ||
} | ||
|
||
void TraceBuffer::toString(std::string& text) const{ | ||
|
||
//FW_ASSERT(text != nullptr); | ||
FW_ASSERT(this->getBuffLength() < FW_TRACE_BUFFER_MAX_SIZE); | ||
|
||
std::string str_format(reinterpret_cast<const char*>(this->m_bufferData),this->getBuffLength()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this being used? It looks like there is commented out code that we shouldn't deliver. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was initially writing it as a string output to file to be able to read it. I've removed it since. I thought it would be nice to have for debugging purposes in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed commented out code |
||
text += str_format; | ||
Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter text has not been checked.
Check warning Code scanning / CodeQL Unchecked return value Warning
The return value of non-void function
operator+= Error loading related location Loading |
||
/*char temp_text[1]; | ||
for (NATIVE_UINT_TYPE i = 0; i < this->getBuffLength(); i++) { | ||
//snprintf(temp_text,sizeof(temp_text),"%d",this->m_bufferData[i]); | ||
temp_text[0] = static_cast<char>(this->m_bufferData[i]); | ||
text[i] = temp_text[0]; | ||
}*/ | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
/* | ||
* TraceBuffer.hpp | ||
* | ||
* Author: sreddy | ||
*/ | ||
|
||
/* | ||
* Description: | ||
* This object contains the TraceBuffer type, used for storing trace entries | ||
*/ | ||
#ifndef FW_TRACE_BUFFER_HPP | ||
#define FW_TRACE_BUFFER_HPP | ||
|
||
#include <FpConfig.hpp> | ||
#include <Fw/Types/Serializable.hpp> | ||
#include <Fw/Cfg/SerIds.hpp> | ||
#include <string> | ||
|
||
namespace Fw { | ||
|
||
class TraceBuffer : public SerializeBufferBase { | ||
public: | ||
|
||
enum { | ||
SERIALIZED_TYPE_ID = FW_TYPEID_TRACE_BUFF, | ||
SERIALIZED_SIZE = FW_TRACE_BUFFER_MAX_SIZE+ sizeof(FwBuffSizeType) | ||
}; | ||
|
||
TraceBuffer(const U8 *args, NATIVE_UINT_TYPE size); | ||
TraceBuffer(); | ||
TraceBuffer(const TraceBuffer& other); | ||
virtual ~TraceBuffer(); | ||
TraceBuffer& operator=(const TraceBuffer& other); | ||
|
||
NATIVE_UINT_TYPE getBuffCapacity() const; // !< returns capacity, not current size, of buffer | ||
U8* getBuffAddr(); | ||
const U8* getBuffAddr() const; | ||
//! Supports writing this buffer to a string representation | ||
void toString(std::string& text) const; | ||
|
||
private: | ||
U8 m_bufferData[FW_TRACE_BUFFER_MAX_SIZE]; // command argument buffer | ||
}; | ||
|
||
} | ||
|
||
#endif | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
/* | ||
* TraceEntity.cpp | ||
* | ||
*/ | ||
|
||
#include <Fw/Trace/TraceEntity.hpp> | ||
#include <Fw/Types/Assert.hpp> | ||
|
||
namespace Fw { | ||
|
||
TraceEntity::TraceEntity() : m_id(0) { | ||
this->m_type = FW_PACKET_TRACE; | ||
} | ||
|
||
TraceEntity::~TraceEntity() { | ||
} | ||
|
||
SerializeStatus TraceEntity::serialize(SerializeBufferBase& buffer) const { | ||
|
||
|
||
SerializeStatus stat = ComPacket::serializeBase(buffer); | ||
Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter buffer has not been checked.
|
||
if (stat != FW_SERIALIZE_OK) { | ||
return stat; | ||
} | ||
|
||
stat = buffer.serialize(this->m_id); | ||
if (stat != FW_SERIALIZE_OK) { | ||
return stat; | ||
} | ||
|
||
stat = buffer.serialize(this->m_timeTag); | ||
if (stat != FW_SERIALIZE_OK) { | ||
return stat; | ||
} | ||
|
||
// We want to add data but not size for the ground software | ||
return buffer.serialize(this->m_traceBuffer.getBuffAddr(),m_traceBuffer.getBuffLength(),true); | ||
|
||
} | ||
|
||
SerializeStatus TraceEntity::deserialize(SerializeBufferBase& buffer) { | ||
|
||
SerializeStatus stat = deserializeBase(buffer); | ||
Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter buffer has not been checked.
|
||
if (stat != FW_SERIALIZE_OK) { | ||
return stat; | ||
} | ||
|
||
stat = buffer.deserialize(this->m_id); | ||
if (stat != FW_SERIALIZE_OK) { | ||
return stat; | ||
} | ||
|
||
stat = buffer.deserialize(this->m_timeTag); | ||
if (stat != FW_SERIALIZE_OK) { | ||
return stat; | ||
} | ||
|
||
// remainder of buffer must be telemetry value | ||
NATIVE_UINT_TYPE size = buffer.getBuffLeft(); | ||
Check notice Code scanning / CodeQL Use of basic integral type Note
size uses the basic integral type unsigned int rather than a typedef with size and signedness.
|
||
stat = buffer.deserialize(this->m_traceBuffer.getBuffAddr(),size,true); | ||
if (stat == FW_SERIALIZE_OK) { | ||
// Shouldn't fail | ||
stat = this->m_traceBuffer.setBuffLen(size); | ||
FW_ASSERT(stat == FW_SERIALIZE_OK,static_cast<NATIVE_INT_TYPE>(stat)); | ||
} | ||
return stat; | ||
} | ||
|
||
void TraceEntity::setId(FwTraceIdType id) { | ||
this->m_id = id; | ||
Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter id has not been checked.
|
||
} | ||
|
||
void TraceEntity::setTraceBuffer(const TraceBuffer& buffer) { | ||
this->m_traceBuffer = buffer; | ||
Check warning Code scanning / CodeQL Unchecked return value Warning
The return value of non-void function
operator= Error loading related location Loading Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter buffer has not been checked.
|
||
} | ||
|
||
void TraceEntity::setTimeTag(const Fw::Time& timeTag) { | ||
this->m_timeTag = timeTag; | ||
Check warning Code scanning / CodeQL Unchecked return value Warning
The return value of non-void function
operator= Error loading related location Loading Check warning Code scanning / CodeQL Unchecked function argument Warning
This use of parameter timeTag has not been checked.
|
||
} | ||
|
||
FwTraceIdType TraceEntity::getId() { | ||
return this->m_id; | ||
} | ||
|
||
Fw::Time& TraceEntity::getTimeTag() { | ||
return this->m_timeTag; | ||
} | ||
|
||
TraceBuffer& TraceEntity::getTraceBuffer() { | ||
return this->m_traceBuffer; | ||
} | ||
|
||
|
||
} /* namespace Fw */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
/* | ||
* TraceEntity.hpp | ||
* | ||
* Author: sreddy | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Put in more comments describing what this class is for. Or, is it misspelled "TraceEntry"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's way better than TraceEntity. I'll change it. |
||
|
||
#ifndef TRACEENTITY_HPP_ | ||
#define TRACEENTITY_HPP_ | ||
|
||
#include <Fw/Com/ComPacket.hpp> | ||
#include <Fw/Trace/TraceBuffer.hpp> | ||
#include <Fw/Time/Time.hpp> | ||
|
||
namespace Fw { | ||
|
||
class TraceEntity : public ComPacket { | ||
public: | ||
|
||
TraceEntity(); | ||
virtual ~TraceEntity(); | ||
|
||
SerializeStatus serialize(SerializeBufferBase& buffer) const; //!< serialize contents | ||
SerializeStatus deserialize(SerializeBufferBase& buffer); | ||
|
||
void setId(FwTraceIdType id); | ||
void setTraceBuffer(const TraceBuffer& buffer); | ||
void setTimeTag(const Fw::Time& timeTag); | ||
|
||
FwTraceIdType getId(); | ||
Fw::Time& getTimeTag(); | ||
TraceBuffer& getTraceBuffer(); | ||
|
||
protected: | ||
FwTraceIdType m_id; // !< Channel id | ||
Fw::Time m_timeTag; // !< time tag | ||
TraceBuffer m_traceBuffer; // !< serialized argument data | ||
}; | ||
|
||
} /* namespace Fw */ | ||
|
||
#endif /* TRACEENTITY_HPP_ */ | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
\page FwTracePort Fw::Trace Port | ||
# Fw::Trace Port | ||
|
||
## 1. Introduction | ||
The `Fw::Trace` port is used to pass serialized trace log values with time tags to record diagnostic data that indicates the order of execution in software. | ||
|
||
## 2. Requirements | ||
Trace Requirements in FPrime: | ||
1. F’Prime shall provide a feature called Trace to log diagnostic information on various points of software execution. | ||
- It is a way to create time correlation events as a reaction for resetting software. | ||
2. Trace shall provide a port to log the following: | ||
- Event ID : A unique identifier recording a “Trace Event” | ||
- Time Tag : Captures a high precision timestamp | ||
- Client Data : A small sample of user arguments recorded into trace buffer | ||
3. Trace shall create trace points and log events in software automatically for thread switching(?) and ...TBD, ask Tim | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. :) Tim says, and port calls in and out, and message queue and dequeue. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lol thanks Tim :) |
||
4. Trace shall provide a port for other components or user defined points to log events during software execution. | ||
5. Trace shall record log events in an in-memory logger with data products to dump the buffers | ||
6. Trace shall store data on a per thread basis so there is only one write to the buffer, preventing a need for mutual exclusion. | ||
7. Trace shall maintain a double buffer, one for logs collected in the previous run before the latest reset and the other for current autopsy events. This way during init a data product can be generated for the trace events from prior run while actively collecting current trace logs. | ||
8. Trace logs shall be stored in recoverable memory so they can be preserved over resets | ||
9. Trace shall write the trace data (stored in buffers) to data products upon boot initialization, a reset and by ground command | ||
10. Trace shall provide a ground command to dump trace logs upon request. | ||
11. Trace shall provide a command to enable/disable Trace logging as it can consume significant code and processing | ||
12. Trace entries shall be put in a dictionary so ground systems can automatically decode trace data | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need a new packet type. Where is it used? The traces would come down as a data product.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was part of initial implementation . Removed it.