diff --git a/common/output-model.cpp b/common/output-model.cpp index 8011df5bef..8554f82cd7 100644 --- a/common/output-model.cpp +++ b/common/output-model.cpp @@ -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() ); } } diff --git a/src/fw-logs/fw-logs-formatting-options.cpp b/src/fw-logs/fw-logs-formatting-options.cpp index 7f05499754..c7472f0714 100644 --- a/src/fw-logs/fw-logs-formatting-options.cpp +++ b/src/fw-logs/fw-logs-formatting-options.cpp @@ -5,6 +5,7 @@ #include #include +#include #include @@ -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 diff --git a/src/fw-logs/fw-logs-parser.cpp b/src/fw-logs/fw-logs-parser.cpp index 73618dfce4..314afcd891 100644 --- a/src/fw-logs/fw-logs-parser.cpp +++ b/src/fw-logs/fw-logs-parser.cpp @@ -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; @@ -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]; @@ -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 ); diff --git a/src/fw-logs/fw-string-formatter.cpp b/src/fw-logs/fw-string-formatter.cpp index aa79557b6f..23bfb4fca3 100644 --- a/src/fw-logs/fw-string-formatter.cpp +++ b/src/fw-logs/fw-string-formatter.cpp @@ -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 @@ -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 " diff --git a/tools/fw-logger/rs-fw-logger.cpp b/tools/fw-logger/rs-fw-logger.cpp index 42eccb09e5..2745df77ce 100644 --- a/tools/fw-logger/rs-fw-logger.cpp +++ b/tools/fw-logger/rs-fw-logger.cpp @@ -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(); @@ -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()); } @@ -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) {