[logging] API change for ecal logging #1853

Peguen · 2024-12-11T09:36:22Z

Description

Removed setter and getter for loglevel. This means, changing of log level is only possible via configuration before initialization.
Removed eCAL::Logging::Log(const std::string&) signature. User now always has to specify the log level to write with.
Deprecated set_loglevel in Python bindings. Python will now always log in "log_level_all" until new logging and API will be introduced.

github-actions

clang-tidy made some suggestions

github-actions · 2024-12-11T09:49:45Z

ecal/core/include/ecal/cimpl/ecal_log_cimpl.h

  /**
   * @brief Log a message (with current log level). 
   *
   * @param msg_  The log message string.
  **/
-  ECALC_API void eCAL_Logging_Log(const char* const msg_);
+  ECALC_API void eCAL_Logging_Log(enum eCAL_Logging_eLogLevel level_, const char* const msg_);


warning: parameter 'msg_' is const-qualified in the function declaration; const-qualification of parameters only has an effect in function definitions [readability-avoid-const-params-in-decls]

Suggested change

ECALC_API void eCAL_Logging_Log(enum eCAL_Logging_eLogLevel level_, const char* const msg_);

ECALC_API void eCAL_Logging_Log(enum eCAL_Logging_eLogLevel level_, const char* msg_);

github-actions · 2024-12-11T09:49:46Z

ecal/core/src/cimpl/ecal_log_cimpl.cpp

@@ -29,19 +29,9 @@

 extern "C"
 {
-  ECALC_API void eCAL_Logging_SetLogLevel(enum eCAL_Logging_eLogLevel level_)
+  ECALC_API void eCAL_Logging_Log(enum eCAL_Logging_eLogLevel level_, const char* const msg_)


warning: no header providing "ECALC_API" is directly included [misc-include-cleaner]

ecal/core/src/cimpl/ecal_log_cimpl.cpp:27:

- #include "ecal_common_cimpl.h" + #include "ecal/ecal_os.h" + #include "ecal_common_cimpl.h"

github-actions · 2024-12-11T09:49:46Z

ecal/core/src/cimpl/ecal_log_cimpl.cpp

@@ -29,19 +29,9 @@

 extern "C"
 {
-  ECALC_API void eCAL_Logging_SetLogLevel(enum eCAL_Logging_eLogLevel level_)
+  ECALC_API void eCAL_Logging_Log(enum eCAL_Logging_eLogLevel level_, const char* const msg_)


warning: no header providing "eCAL_Logging_eLogLevel" is directly included [misc-include-cleaner]

ecal/core/src/cimpl/ecal_log_cimpl.cpp:27:

- #include "ecal_common_cimpl.h" + #include "ecal/ecal_log_level.h" + #include "ecal_common_cimpl.h"

github-actions · 2024-12-11T09:49:46Z

ecal/core/src/cimpl/ecal_log_cimpl.cpp

-  ECALC_API void eCAL_Logging_Log(const char* const msg_)
-  {
-    eCAL::Logging::Log(msg_);
+    eCAL::Logging::Log(level_, msg_);


warning: no header providing "eCAL::Logging::Log" is directly included [misc-include-cleaner]

ecal/core/src/cimpl/ecal_log_cimpl.cpp:27:

- #include "ecal_common_cimpl.h" + #include "ecal/ecal_log.h" + #include "ecal_common_cimpl.h"

github-actions · 2024-12-11T09:49:46Z

ecal/core/src/logging/ecal_log.cpp

    /**
     * @brief Log a message.
     *
     * @param msg_   The log message string.
    **/
-    void Log(const std::string& msg_)
+    void Log(eCAL_Logging_eLogLevel level_, const std::string& msg_)


warning: no header providing "eCAL_Logging_eLogLevel" is directly included [misc-include-cleaner]

ecal/core/src/logging/ecal_log.cpp:26:

- #include "ecal_log_provider.h" + #include "ecal/ecal_log_level.h" + #include "ecal_log_provider.h"

github-actions · 2024-12-11T09:49:46Z

ecal/samples/cpp/benchmarks/counter_snd/src/counter_snd.cpp

@@ -78,7 +78,7 @@
        msgs  = 0;
        bytes = 0;

-        eCAL::Logging::Log(out.str());
+        eCAL::Logging::Log(log_level_info, out.str());


warning: no header providing "log_level_info" is directly included [misc-include-cleaner]

ecal/samples/cpp/benchmarks/counter_snd/src/counter_snd.cpp:19:

- #include <ecal/ecal.h> + #include "ecal/ecal_log_level.h" + #include <ecal/ecal.h>

github-actions · 2024-12-11T09:49:46Z

ecal/samples/cpp/benchmarks/performance_snd/src/performance_snd.cpp

@@ -98,7 +98,7 @@ int main(int argc, char **argv)
        out << "Messages/s:            " << (unsigned int)(msgs  /                            diff_time.count()) << std::endl;
        out << "Latency (us):          " << (diff_time.count() * 1e6) / (double)msgs                             << std::endl;
        std::cout << out.str() << std::endl;
-        eCAL::Logging::Log(out.str());
+        eCAL::Logging::Log(log_level_info, out.str());


warning: no header providing "eCAL::Logging::Log" is directly included [misc-include-cleaner]

ecal/samples/cpp/benchmarks/performance_snd/src/performance_snd.cpp:27:

+ #include "ecal/ecal_log.h"

github-actions · 2024-12-11T09:49:47Z

ecal/samples/cpp/benchmarks/performance_snd/src/performance_snd.cpp

@@ -98,7 +98,7 @@
        out << "Messages/s:            " << (unsigned int)(msgs  /                            diff_time.count()) << std::endl;
        out << "Latency (us):          " << (diff_time.count() * 1e6) / (double)msgs                             << std::endl;
        std::cout << out.str() << std::endl;
-        eCAL::Logging::Log(out.str());
+        eCAL::Logging::Log(log_level_info, out.str());


warning: no header providing "log_level_info" is directly included [misc-include-cleaner]

ecal/samples/cpp/benchmarks/performance_snd/src/performance_snd.cpp:27:

+ #include "ecal/ecal_log_level.h"

github-actions · 2024-12-11T09:49:47Z

lang/python/core/src/ecal_clang.cpp

 {
-  eCAL::Logging::Log(message_);
+  eCAL::Logging::Log(eCAL_Logging_eLogLevel(level_), message_);


warning: no header providing "eCAL::Logging::Log" is directly included [misc-include-cleaner]

lang/python/core/src/ecal_clang.cpp:27:

+ #include "ecal/ecal_log.h"

github-actions · 2024-12-11T09:49:47Z

lang/python/core/src/ecal_clang.cpp

 {
-  eCAL::Logging::Log(message_);
+  eCAL::Logging::Log(eCAL_Logging_eLogLevel(level_), message_);


warning: no header providing "eCAL_Logging_eLogLevel" is directly included [misc-include-cleaner]

lang/python/core/src/ecal_clang.cpp:27:

+ #include "ecal/ecal_log_level.h"

rex-schilasky · 2024-12-11T13:32:44Z

lang/python/core/ecal/core/core.py

@@ -148,7 +148,8 @@ def log_setlevel(level):
  :type level: int

  """
-  return _ecal.log_setlevel(level)
+  print("[WARNING] 'log_setlevel(int)' is deprecated. All logs will have the log level 'all' until the new API is introduced.")


Implement set log level function using global level state variable in ecal python c++ wrapper as intermediate solution.

rex-schilasky

Finally :-)

github-actions

clang-tidy made some suggestions

github-actions · 2024-12-12T10:15:05Z

lang/python/core/src/ecal_clang.cpp

 /****************************************/
 /*      log_message                     */
 /****************************************/
-void log_message(const char* message_)
+void log_message(const eCAL_Logging_eLogLevel& log_level_, const char* message_)


warning: no header providing "eCAL_Logging_eLogLevel" is directly included [misc-include-cleaner]

lang/python/core/src/ecal_clang.cpp:27:

+ #include "ecal/ecal_log_level.h"

github-actions · 2024-12-12T10:15:06Z

lang/python/core/src/ecal_clang.cpp

 {
-  eCAL::Logging::Log(message_);
+  eCAL::Logging::Log(log_level_, message_);


warning: no header providing "eCAL::Logging::Log" is directly included [misc-include-cleaner]

lang/python/core/src/ecal_clang.cpp:27:

+ #include "ecal/ecal_log.h"

github-actions · 2024-12-12T10:15:06Z

lang/python/core/src/ecal_clang.h

 /**
 * @brief Log a message (with current log level).
 *
 * @param message_  The log message string.
 **/
-void log_message(const char* message_);
+void log_message(const eCAL_Logging_eLogLevel& level_, const char* message_);


warning: no header providing "eCAL_Logging_eLogLevel" is directly included [misc-include-cleaner]

lang/python/core/src/ecal_clang.h:26:

- #include <ecal/ecalc.h> + #include "ecal/ecal_log_level.h" + #include <ecal/ecalc.h>

github-actions · 2024-12-12T10:15:06Z

lang/python/core/src/ecal_wrap.cxx

@@ -64,6 +64,8 @@ PySubscriberCallbackMapT    g_subscriber_pycallback_map;
 PyServerMethodCallbackMapT  g_server_method_pycallback_map;
 PyClientCallbackMapT        g_client_pycallback_map;

+eCAL_Logging_eLogLevel      g_python_log_level = log_level_info;


warning: no header providing "eCAL_Logging_eLogLevel" is directly included [misc-include-cleaner]

lang/python/core/src/ecal_wrap.cxx:22:

- #define PY_SSIZE_T_CLEAN + #include "ecal/ecal_log_level.h" + #define PY_SSIZE_T_CLEAN

github-actions · 2024-12-12T10:15:06Z

lang/python/core/src/ecal_wrap.cxx

@@ -64,6 +64,8 @@
 PyServerMethodCallbackMapT  g_server_method_pycallback_map;
 PyClientCallbackMapT        g_client_pycallback_map;

+eCAL_Logging_eLogLevel      g_python_log_level = log_level_info;


warning: no header providing "log_level_info" is directly included [misc-include-cleaner]

eCAL_Logging_eLogLevel g_python_log_level = log_level_info; ^

github-actions · 2024-12-12T10:15:06Z

lang/python/core/src/ecal_wrap.cxx

@@ -64,6 +64,8 @@
 PyServerMethodCallbackMapT  g_server_method_pycallback_map;
 PyClientCallbackMapT        g_client_pycallback_map;

+eCAL_Logging_eLogLevel      g_python_log_level = log_level_info;


warning: variable 'g_python_log_level' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

eCAL_Logging_eLogLevel g_python_log_level = log_level_info; ^

Peguen added 3 commits December 11, 2024 08:53

Removed get/set of loglevel and Log(const std::string&) implementation.

7d38709

Changed Log implementations accordingly.

8fe56f5

Changed language binding accordingly.

2af39ae

Peguen requested review from rex-schilasky and hannemn December 11, 2024 09:36

Peguen added the cherry-pick-to-NONE Don't cherry-pick these changes label Dec 11, 2024

github-actions bot reviewed Dec 11, 2024

View reviewed changes

Peguen added 3 commits December 11, 2024 12:54

Removed SetLogFilter in csharp binding.

e2d0cab

Restored logger comment.

4b1bc68

Removed undefined functions in ecal_log_provider.h

d07d20d

rex-schilasky requested changes Dec 11, 2024

View reviewed changes

Python return log level with a global variable.

4a64f0b

rex-schilasky approved these changes Dec 12, 2024

View reviewed changes

github-actions bot reviewed Dec 12, 2024

View reviewed changes

Peguen merged commit d7a868b into master Dec 12, 2024
20 checks passed

Peguen deleted the feature/ecal_api_ecal_log_h branch December 12, 2024 12:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[logging] API change for ecal logging #1853

[logging] API change for ecal logging #1853

Peguen commented Dec 11, 2024 •

edited

Loading

github-actions bot left a comment

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

github-actions bot Dec 11, 2024

rex-schilasky Dec 11, 2024

rex-schilasky left a comment

github-actions bot left a comment

github-actions bot Dec 12, 2024

github-actions bot Dec 12, 2024

github-actions bot Dec 12, 2024

github-actions bot Dec 12, 2024

github-actions bot Dec 12, 2024

github-actions bot Dec 12, 2024

	ECALC_API void eCAL_Logging_Log(enum eCAL_Logging_eLogLevel level_, const char* const msg_);
	ECALC_API void eCAL_Logging_Log(enum eCAL_Logging_eLogLevel level_, const char* msg_);

[logging] API change for ecal logging #1853

[logging] API change for ecal logging #1853

Conversation

Peguen commented Dec 11, 2024 • edited Loading

Description

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot Dec 11, 2024

Choose a reason for hiding this comment

rex-schilasky Dec 11, 2024

Choose a reason for hiding this comment

rex-schilasky left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot Dec 12, 2024

Choose a reason for hiding this comment

github-actions bot Dec 12, 2024

Choose a reason for hiding this comment

github-actions bot Dec 12, 2024

Choose a reason for hiding this comment

github-actions bot Dec 12, 2024

Choose a reason for hiding this comment

github-actions bot Dec 12, 2024

Choose a reason for hiding this comment

github-actions bot Dec 12, 2024

Choose a reason for hiding this comment

Peguen commented Dec 11, 2024 •

edited

Loading