Skip to content

Commit

Permalink
Outputting module name. After integration, removed all temporary hacks
Browse files Browse the repository at this point in the history
  • Loading branch information
@OhadMeir
OhadMeir committed Mar 28, 2024
1 parent 7605192 commit 68747ad
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 40 deletions.
8 changes: 6 additions & 2 deletions common/output-model.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,15 @@ void output_model::thread_loop()
{
parsed_ok = true;

std::string module_print = "[" + parsed.module_name() + "]";
if( module_print == "[Unknown]" )
module_print.clear(); // Some devices don't support FW log modules

add_log( message.get_severity(),
parsed.file_name(),
parsed.line(),
rsutils::string::from()
<< "FW-LOG [" << parsed.thread_name() << "] " << parsed.message() );
rsutils::string::from() << "FW-LOG [" << parsed.thread_name() << "]"
<< module_print << parsed.message() );
}
}

Expand Down
5 changes: 2 additions & 3 deletions src/fw-logs/fw-logs-formatting-options.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <src/fw-logs/fw-logs-xml-helper.h>

#include <src/librealsense-exception.h>
#include <rsutils/string/from.h>

#include <sstream>

Expand All @@ -26,9 +27,7 @@ namespace librealsense
return event_it->second;
}

std::stringstream ss;
ss << "*** Unrecognized Log Id: " << id << "! P1 = 0x{0:x}, P2 = 0x{1:x}, P3 = 0x{2:x}";
return { 3, ss.str() }; // Default to legacy format - 3 params
throw librealsense::invalid_value_exception( rsutils::string::from() << "Unrecognized Log Id: " << id );
}

std::string fw_logs_formatting_options::get_file_name( int id ) const
Expand Down
14 changes: 4 additions & 10 deletions src/fw-logs/fw-logs-parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,9 @@ namespace librealsense
size_t extended_fw_logs_parser::get_log_size( const uint8_t * log ) const
{
const auto extended = reinterpret_cast< const extended_fw_log_binary * >( log );
// If there are parameters total_params_size_bytes includes the information

//return sizeof( extended_fw_log_binary ) + extended->total_params_size_bytes;
// TODO - HACK - The right size should be the commented line above, but currently there is a bug in HKR FW
// that they don't pack the parameter info and blob to the main struct. There are 4 padding bytes.
return sizeof( extended_fw_log_binary ) + extended->total_params_size_bytes + 4;
// If there are parameters total_params_size_bytes includes the information
return sizeof( extended_fw_log_binary ) + extended->total_params_size_bytes;
}

constexpr const uint32_t keep_settings = 0;
Expand Down Expand Up @@ -272,9 +269,6 @@ namespace librealsense
#pragma pack( push, 1 )
struct extended_fw_log_params_binary : public extended_fw_log_binary
{
// TODO - HACK - This uint32_t field should be deleted, but currently there is a bug in HKR FW
// that they don't pack the parameter info and blob to the main struct. There are 4 padding bytes.
uint32_t padding;
// param_info array size namber_of_params, 1 is just a placeholder. Need to use an array not a
// pointer because pointer can be 8 bytes of size and than we won't parse the data correctly
param_info info[1];
Expand All @@ -290,8 +284,8 @@ namespace librealsense
// Raw message offset is start of message, structured data offset is start of blob
size_t blob_offset = blob_start - raw->logs_buffer.data();
param_info adjusted_info = { static_cast< uint16_t >( with_params->info[i].offset - blob_offset ),
with_params->info[i].type,
with_params->info[i].size };
with_params->info[i].type,
with_params->info[i].size };
structured->params_info.push_back( adjusted_info );
}
structured->params_blob.assign( blob_start, blob_start + blob_size );
Expand Down
53 changes: 31 additions & 22 deletions src/fw-logs/fw-string-formatter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,22 @@ namespace librealsense
exp_replace_map[ss_regular_exp[1].str()] = ss_replacement[1].str();

// Legacy format - parse parameter as 4 raw bytes of float. Parameter can be uint16_t or uint32_t.
ss_regular_exp[2] << "\\{\\b(" << i << "):f\\}";
uint32_t as_int32 = 0;
memcpy( &as_int32, params_blob.data() + params_info[i].offset, params_info[i].size );
float as_float = *reinterpret_cast< const float * >( &as_int32 );
if( std::isfinite( as_float ) )
ss_replacement[2] << as_float;
else
if( params_info[i].size <= sizeof( uint32_t ) )
{
// Values for other regular expresions can be NaN when converted to float.
// Prepare replacement as hex value (will probably not be used because format is not with :f).
ss_replacement[2] << "0x" << hex << setw( 2 ) << setfill( '0' ) << as_int32;
ss_regular_exp[2] << "\\{\\b(" << i << "):f\\}";
uint32_t as_int32 = 0;
memcpy( &as_int32, params_blob.data() + params_info[i].offset, params_info[i].size );
float as_float = *reinterpret_cast< const float * >( &as_int32 );
if( std::isfinite( as_float ) )
ss_replacement[2] << as_float;
else
{
// Values for other regular expresions can be NaN when converted to float.
// Prepare replacement as hex value (will probably not be used because format is not with :f).
ss_replacement[2] << "0x" << hex << setw( 2 ) << setfill( '0' ) << as_int32;
}
exp_replace_map[ss_regular_exp[2].str()] = ss_replacement[2].str();
}
exp_replace_map[ss_regular_exp[2].str()] = ss_replacement[2].str();

// enum values are mapped by int (kvp) but we don't know if this parameter is related to enum, using
// unsigned long long because unrelated arguments can overflow int
Expand Down Expand Up @@ -143,27 +146,33 @@ namespace librealsense
switch( info.type )
{
case param_type::STRING:
return std::string( reinterpret_cast< const char * >( param_start ), info.size );
{
// Using stringstream to remove the terminating null character. We use this as regex replacement string,
// and having '\0' here will cause a terminating character in the middle of the result string.
stringstream str;
str << param_start;
return str.str();
}
case param_type::UINT8:
return rsutils::string::from() << *reinterpret_cast< const uint8_t * >( param_start );
return std::to_string( *reinterpret_cast< const uint8_t * >( param_start ) );
case param_type::SINT8:
return rsutils::string::from() << *reinterpret_cast< const int8_t * >( param_start );
return std::to_string( *reinterpret_cast< const int8_t * >( param_start ) );
case param_type::UINT16:
return rsutils::string::from() << *reinterpret_cast< const uint16_t * >( param_start );
return std::to_string( *reinterpret_cast< const uint16_t * >( param_start ) );
case param_type::SINT16:
return rsutils::string::from() << *reinterpret_cast< const int16_t * >( param_start );
return std::to_string( *reinterpret_cast< const int16_t * >( param_start ) );
case param_type::SINT32:
return rsutils::string::from() << *reinterpret_cast< const int32_t * >( param_start );
return std::to_string( *reinterpret_cast< const int32_t * >( param_start ) );
case param_type::UINT32:
return rsutils::string::from() << *reinterpret_cast< const uint32_t * >( param_start );
return std::to_string( *reinterpret_cast< const uint32_t * >( param_start ) );
case param_type::SINT64:
return rsutils::string::from() << *reinterpret_cast< const int64_t * >( param_start );
return std::to_string( *reinterpret_cast< const int64_t * >( param_start ) );
case param_type::UINT64:
return rsutils::string::from() << *reinterpret_cast< const uint64_t * >( param_start );
return std::to_string( *reinterpret_cast< const uint64_t * >( param_start ) );
case param_type::FLOAT:
return rsutils::string::from() << *reinterpret_cast< const float * >( param_start );
return std::to_string( *reinterpret_cast< const float * >( param_start ) );
case param_type::DOUBLE:
return rsutils::string::from() << *reinterpret_cast< const double * >( param_start );
return std::to_string( *reinterpret_cast< const double * >( param_start ) );
default:
throw librealsense::invalid_value_exception( rsutils::string::from()
<< "Unsupported parameter type "
Expand Down
13 changes: 10 additions & 3 deletions tools/fw-logger/rs-fw-logger.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ int main(int argc, char* argv[])
}
}

fw_log_device.start_collecting();

bool are_there_remaining_flash_logs_to_pull = true;
auto time_of_previous_polling_ms = std::chrono::high_resolution_clock::now();

Expand Down Expand Up @@ -141,11 +143,14 @@ int main(int argc, char* argv[])
auto parsed_log = fw_log_device.create_parsed_message();
bool parsing_result = fw_log_device.parse_log(log_message, parsed_log);

std::string module_print = parsed_log.module_name() + " ";
if( module_print == "Unknown " )
module_print.clear(); // Some devices don't support FW log modules

stringstream sstr;
sstr << datetime_string() << " " << parsed_log.timestamp() << " " << parsed_log.sequence_id()
<< " " << parsed_log.severity() << " " << parsed_log.thread_name()
<< " " << parsed_log.file_name() << " " << parsed_log.line()
<< " " << parsed_log.message();
<< " " << parsed_log.severity() << " " << parsed_log.thread_name() << " " << module_print
<< parsed_log.file_name() << " " << parsed_log.line() << " " << parsed_log.message();

fw_log_lines.push_back(sstr.str());
}
Expand Down Expand Up @@ -182,6 +187,8 @@ int main(int argc, char* argv[])
time_of_previous_polling_ms = std::chrono::high_resolution_clock::now();
}
}

fw_log_device.stop_collecting();
}
catch (const error & e)
{
Expand Down

0 comments on commit 68747ad

Please sign in to comment.