astarte-platform / astarte-device-sdk-qt5

Astarte Qt5 Device SDK
http://astarte-platform.org/
Apache License 2.0
11 stars 12 forks source link

Change Logging level for libmosquittopp logs #51

Open rickvargas opened 2 years ago

rickvargas commented 2 years ago

Description:

The logs from libmosquittopp are all handled as qWarning from the SDK. They should be handled with the right logging level instead of statically warning.

Environment:

Will happen on all versions.

Steps:

  1. Start your application with qWarning logging level
  2. See Debug (int level 16) messages from libmosquittopp are logged as qWarning

Current Results:

  1. See Debug (int level 16) messages from libmosquittopp are logged as qWarning

Expected Results (Suggestions):

  1. See Debug (int level 16) messages from libmosquittopp are not logged as qWarning but as qDebug

Additional Information:

Libmosquittopp has a public Enum of the logging levels.

harlem88 commented 2 years ago

Hello @rickvargas, maybe you refer to master branch because in the release-1.0 branch we already merged this PR #47, that it should be something like your issue. Take a look here release-1.0.

rickvargas commented 2 years ago

Ok

rickvargas commented 2 years ago

I took a look and it seems it has space for enhancement. I had done this way in my system, feel free to use or ignore it:

diff --git a/transports/hyperdrivemqttclientwrapper.cpp b/transports/hyperdrivemqttclientwrapper.cpp
index aa1b037..9bac6d0 100644
--- a/transports/hyperdrivemqttclientwrapper.cpp
+++ b/transports/hyperdrivemqttclientwrapper.cpp
@@ -118,7 +118,26 @@ void MQTTClientWrapperPrivate::on_unsubscribe(int mid)

 void MQTTClientWrapperPrivate::on_log(int level, const char *str)
 {
-    qWarning() << "MOSQUITTO LOG!" << level << str;
+    switch(level){
+        case MOSQ_LOG_INFO:
+           qCInfo(mqttWrapperDC) << "MOSQUITTO INFO!" << level << str;
+           break;
+        case MOSQ_LOG_NOTICE:
+           qCInfo(mqttWrapperDC) << "MOSQUITTO NOTICE!" << level << str;
+           break;
+        case MOSQ_LOG_WARNING:
+           qCWarning(mqttWrapperDC) << "MOSQUITTO WARNING!" << level << str;
+           break;
+        case MOSQ_LOG_ERR:
+           qCritical(mqttWrapperDC) << "MOSQUITTO ERROR!" << level << str;
+           break;
+        case MOSQ_LOG_DEBUG:
+           qCDebug(mqttWrapperDC) << "MOSQUITTO DEBUG!" << level << str;
+           break;
+        default:
+           qCWarning(mqttWrapperDC) << "MOSQUITTO LOG!" << level << str;
+           break;
+    }
 }

 void MQTTClientWrapperPrivate::on_error()
bettio commented 2 years ago

Thanks for reporting this issue, there are indeed some good idea here that we should implement. For instance we should indeed start using logging categories for sure also in that code.

Just an additional note, I think a good library should never use qCritical since it's behavior might change:

It exits if the environment variable QT_FATAL_CRITICALS is not empty.