Skip to content
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.

Sign up for GitHub

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

[new-core-alignment] proto file cleanups part 2 (logging) #1361

Merged
merged 5 commits into from
Feb 13, 2024
Merged

[new-core-alignment] proto file cleanups part 2 (logging) #1361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eb6b5d6
monitoring.proto split into monitoring and logging part (fully compat…
rex-schilasky Feb 12, 2024
dc9e447
Merge branch 'master' into new-core-alignment/proto-file-cleanup2
rex-schilasky Feb 13, 2024
b1e85db
SetCoreTime/GetCoreTime .. removed again (merge conflict not solved c…
rex-schilasky Feb 13, 2024
26af684
m_core_time_start removed
rex-schilasky Feb 13, 2024
fdaabdb
m_core_time removed
rex-schilasky Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,6 @@ endif()
# --------------------------------------------------------
add_subdirectory(contrib/ecalproto)

# --------------------------------------------------------
# old ecal core protobuf interface (for compatibility)
# --------------------------------------------------------
add_subdirectory(ecal/pb)

# --------------------------------------------------------
# ecal core protobuf interface
# --------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion app/mon/mon_gui/src/widgets/log_widget/log_widget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ LogWidget::~LogWidget()

void LogWidget::getEcalLogs()
{
eCAL::pb::Logging logging;
eCAL::pb::LogMessageList logging;
std::string logging_string;

if (eCAL::Monitoring::GetLogging(logging_string))
Expand Down
6 changes: 3 additions & 3 deletions app/mon/mon_gui/src/widgets/models/log_model.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,9 @@ Qt::ItemFlags LogModel::flags(const QModelIndex &index) const
return QAbstractItemModel::flags(index);
}

void LogModel::insertLogs(const eCAL::pb::Logging& logging_pb)
void LogModel::insertLogs(const eCAL::pb::LogMessageList& logging_pb)
{
int inserted_row_count = logging_pb.logs().size();
int inserted_row_count = logging_pb.log_messages().size();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'inserted_row_count' of type 'int' can be declared 'const' [misc-const-correctness]

Suggested change
int inserted_row_count = logging_pb.log_messages().size();
{const

int size_before = logs_.size();

// Remove entries from the top
Expand Down Expand Up @@ -244,7 +244,7 @@ void LogModel::insertLogs(const eCAL::pb::Logging& logging_pb)
beginInsertRows(QModelIndex(), size_before, size_after - 1);

int counter = inserted_row_count;
for (auto& log_message_pb : logging_pb.logs())
for (auto& log_message_pb : logging_pb.log_messages())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'auto &log_message_pb' can be declared as 'const auto &log_message_pb' [readability-qualified-auto]

Suggested change
for (auto& log_message_pb : logging_pb.log_messages())
;const

{
if (counter <= max_entries_)
{
Expand Down
4 changes: 2 additions & 2 deletions app/mon/mon_gui/src/widgets/models/log_model.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#pragma warning(push)
#pragma warning(disable: 4100 4127 4146 4505 4800 4189 4592) // disable proto warnings
#endif
#include <ecal/core/pb/monitoring.pb.h>
#include <ecal/core/pb/logging.pb.h>
#ifdef _MSC_VER
#pragma warning(pop)
#endif
Expand Down Expand Up @@ -73,7 +73,7 @@ class LogModel : public QAbstractItemModel
QModelIndex parent(const QModelIndex &index) const override;
Qt::ItemFlags flags(const QModelIndex &index) const override;

void insertLogs(const eCAL::pb::Logging& logs);
void insertLogs(const eCAL::pb::LogMessageList& logs);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: function 'LogModel::insertLogs' has a definition with different parameter names [readability-inconsistent-declaration-parameter-name]

         ^
Additional context

app/mon/mon_gui/src/widgets/models/log_model.cpp:216: the definition seen here

                 ^

app/mon/mon_gui/src/widgets/models/log_model.h:75: differing parameters are named here: ('logs'), in definition: ('logging_pb')

         ^


void setParseTimeEnabled(bool enabled);
bool isParseTimeEnabled() const;
Expand Down
8 changes: 4 additions & 4 deletions app/mon/mon_tui/src/model/log.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#ifdef _MSC_VER
#pragma warning(push, 0) // disable proto warnings
#endif
#include "ecal/core/pb/monitoring.pb.h"
#include <ecal/core/pb/logging.pb.h>
#ifdef _MSC_VER
#pragma warning(pop)
#endif
Expand Down Expand Up @@ -53,7 +53,7 @@ class LogModel
bool is_polling;
int capacity = 500;

eCAL::pb::Logging logs;
eCAL::pb::LogMessageList logs;

std::mutex mtx;
std::thread update_thread;
Expand Down Expand Up @@ -113,7 +113,7 @@ class LogModel
eCAL::Monitoring::GetLogging(raw_data);
logs.ParseFromString(raw_data);

auto &pb_logs = logs.logs();
auto &pb_logs = logs.log_messages();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'auto &pb_logs' can be declared as 'const auto &pb_logs' [readability-qualified-auto]

Suggested change
auto &pb_logs = logs.log_messages();
const auto &pb_logs = logs.log_messages();

auto new_entries_count = pb_logs.size();
if(new_entries_count == 0)
{
Expand All @@ -126,7 +126,7 @@ class LogModel
{
data.erase(data.begin(), data.begin() + overflow_size);
}
for(auto &l: logs.logs()) data.push_back(ToLogEntry(l));
for(auto &l: logs.log_messages()) data.push_back(ToLogEntry(l));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: 'auto &l' can be declared as 'const auto &l' [readability-qualified-auto]

Suggested change
for(auto &l: logs.log_messages()) data.push_back(ToLogEntry(l));
for(const auto &l: logs.log_messages()) data.push_back(ToLogEntry(l));

}

NotifyUpdate();
Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/io/udp/ecal_udp_logging_receiver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
#ifdef _MSC_VER
#pragma warning(push, 0) // disable proto warnings
#endif
#include <ecal/core/pb/monitoring.pb.h>
#include <ecal/core/pb/logging.pb.h>
#ifdef _MSC_VER
#pragma warning(pop)
#endif
Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/io/udp/ecal_udp_logging_sender.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
#ifdef _MSC_VER
#pragma warning(push, 0) // disable proto warnings
#endif
#include <ecal/core/pb/monitoring.pb.h>
#include <ecal/core/pb/logging.pb.h>
#ifdef _MSC_VER
#pragma warning(pop)
#endif
Expand Down
5 changes: 2 additions & 3 deletions ecal/core/src/logging/ecal_log_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ namespace eCAL
m_filter_mask_file(log_level_info | log_level_warning | log_level_error | log_level_fatal | log_level_debug1 | log_level_debug2),
m_filter_mask_udp(log_level_info | log_level_warning | log_level_error | log_level_fatal | log_level_debug1 | log_level_debug2)
{
m_core_time = std::chrono::duration<double>(-1.0);
}

CLog::~CLog()
Expand Down Expand Up @@ -292,7 +291,7 @@ namespace eCAL
Log(m_level, msg_);
}

void CLog::GetLogging(eCAL::pb::Logging& logging_)
void CLog::GetLogging(eCAL::pb::LogMessageList& logging_)
{
// clear protobuf object
logging_.Clear();
Expand All @@ -304,7 +303,7 @@ namespace eCAL
while (siter != m_log_msglist.end())
{
// add log message
eCAL::pb::LogMessage* pMonLogMessage = logging_.add_logs();
eCAL::pb::LogMessage* pMonLogMessage = logging_.add_log_messages();

// copy content
pMonLogMessage->CopyFrom(*siter);
Expand Down
4 changes: 1 addition & 3 deletions ecal/core/src/logging/ecal_log_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ namespace eCAL
**/
void Log(const std::string& msg_);

void GetLogging(eCAL::pb::Logging& logging_);
void GetLogging(eCAL::pb::LogMessageList& logging_);

private:
void RegisterLogMessage(const eCAL::pb::LogMessage& log_msg_);
Expand Down Expand Up @@ -125,7 +125,5 @@ namespace eCAL
eCAL_Logging_Filter m_filter_mask_con;
eCAL_Logging_Filter m_filter_mask_file;
eCAL_Logging_Filter m_filter_mask_udp;

std::chrono::duration<double> m_core_time;
};
}
2 changes: 1 addition & 1 deletion ecal/core/src/monitoring/ecal_monitoring_def.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ namespace eCAL

int GetLogging(std::string& log_)
{
eCAL::pb::Logging logging;
eCAL::pb::LogMessageList logging;
if (g_log() != nullptr) g_log()->GetLogging(logging);

log_ = logging.SerializeAsString();
Expand Down
1 change: 1 addition & 0 deletions ecal/core_pb/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ set(ProtoFiles
${CMAKE_CURRENT_SOURCE_DIR}/src/ecal/core/pb/ecal.proto
${CMAKE_CURRENT_SOURCE_DIR}/src/ecal/core/pb/host.proto
${CMAKE_CURRENT_SOURCE_DIR}/src/ecal/core/pb/layer.proto
${CMAKE_CURRENT_SOURCE_DIR}/src/ecal/core/pb/logging.proto
${CMAKE_CURRENT_SOURCE_DIR}/src/ecal/core/pb/monitoring.proto
${CMAKE_CURRENT_SOURCE_DIR}/src/ecal/core/pb/process.proto
${CMAKE_CURRENT_SOURCE_DIR}/src/ecal/core/pb/service.proto
Expand Down
25 changes: 17 additions & 8 deletions ecal/pb/include/ecal/pb/mma/mma.pb.h → ecal/core_pb/src/ecal/core/pb/logging.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,22 @@
* ========================= eCAL LICENSE =================================
*/

#pragma once
syntax = "proto3";

#ifdef _MSC_VER
#pragma message("WARNING: This header file is deprecated. It will be removed in future eCAL versions. Please include <ecal/app/pb/mma/mma.pb.h> instead")
#endif /*_MSC_VER*/
#ifdef __GNUC__
#pragma message "WARNING: This header file is deprecated. It will be removed in future eCAL versions. Please include <ecal/app/pb/mma/mma.pb.h> instead"
#endif /* __GNUC__ */
package eCAL.pb;

#include <ecal/app/pb/mma/mma.pb.h>
message LogMessage // eCAL log message
{
int64 time = 1; // time
string hname = 2; // host name
int32 pid = 3; // process id
string pname = 4; // process name
string uname = 5; // unit name
int32 level = 6; // message level
string content = 7; // message content
}

message LogMessageList // eCAL log message list
{
repeated LogMessage log_messages = 1; // log messages
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need the renaming? It's a bit nicer but not sure if it's strictly necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But much nicer :-)

}
16 changes: 0 additions & 16 deletions ecal/core_pb/src/ecal/core/pb/monitoring.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,6 @@ import "ecal/core/pb/topic.proto";

package eCAL.pb;

message LogMessage // eCAL monitoring log message
{
int64 time = 1; // time
string hname = 2; // host name
int32 pid = 3; // process id
string pname = 4; // process name
string uname = 5; // unit name
int32 level = 6; // message level
string content = 7; // message content
}

message Monitoring // eCAL monitoring information
{
repeated Host hosts = 1; // hosts
Expand All @@ -45,8 +34,3 @@ message Monitoring // eCAL monitoring information
repeated Client clients = 5; // clients
repeated Topic topics = 4; // topics
}

message Logging // eCAL logging information
{
repeated LogMessage logs = 1; // log messages
}
89 changes: 0 additions & 89 deletions ecal/pb/CMakeLists.txt
View file
Open in desktop

This file was deleted.

29 changes: 0 additions & 29 deletions ecal/pb/include/ecal/pb/ecal.pb.h
View file
Open in desktop

This file was deleted.

29 changes: 0 additions & 29 deletions ecal/pb/include/ecal/pb/host.pb.h
View file
Open in desktop

This file was deleted.

Loading
Loading