libopsdp: Update to new logger API

utils/logger.h was providing some really messy API for the logger. This patch integrates the new reworked API into libosdp. This is an experimental change with a lot of potential change/enhancements in future. Signed-off-by: Siddharth Chandrasekaran <sidcha.dev@gmail.com>
goToMain · Sep 3, 2023 · c9f3f98 · c9f3f98 · rm5248 · Oct 9, 2023
1 parent 48d6b3a
commit c9f3f98
Show file tree

Hide file tree

Showing 21 changed files with 57 additions and 49 deletions.
diff --git a/doc/libosdp/debugging.rst b/doc/libosdp/debugging.rst
@@ -32,7 +32,7 @@ as,
 
 .. code:: c
 
-    osdp_logger_init(LOG_DEBUG, uart_puts);
+    osdp_logger_init("osdp::cp", LOG_DEBUG, uart_puts);
 
 Packet Trace Builds
 -------------------

diff --git a/include/osdp.h b/include/osdp.h
@@ -967,6 +967,7 @@ typedef int (*osdp_log_puts_fn_t)(const char *msg);
 /**
  * @brief Configure OSDP Logging.
  *
+ * @param name A soft name for this module; will appear in all the log lines.
  * @param log_level OSDP log levels of type `enum osdp_log_level_e`. Default is
  *                  LOG_INFO.
  * @param puts_fn A puts() like function that will be invoked to write the log
@@ -975,7 +976,8 @@ typedef int (*osdp_log_puts_fn_t)(const char *msg);
  *                definition to see the behavioral expectations. When this is
  *                set to NULL, LibOSDP will log to stderr.
  */
-void osdp_logger_init(int log_level, osdp_log_puts_fn_t puts_fn);
+void osdp_logger_init(const char *name, int log_level,
+		      osdp_log_puts_fn_t puts_fn);
 
 /**
  * @brief Get LibOSDP version as a `const char *`. Used in diagnostics.

diff --git a/include/osdp.hpp b/include/osdp.hpp
@@ -16,9 +16,9 @@ class Common {
 public:
 	Common() {}
 
-	void logger_init(int log_level, osdp_log_puts_fn_t puts_fn)
+	void logger_init(const char *name, int log_level, osdp_log_puts_fn_t puts_fn)
 	{
-		osdp_logger_init(log_level, puts_fn);
+		osdp_logger_init(name, log_level, puts_fn);
 	}
 
 	const char *get_version()

diff --git a/osdpctl/cmd_start.c b/osdpctl/cmd_start.c
@@ -317,7 +317,7 @@ int cmd_handler_start(int argc, char *argv[], void *data)
 		info->scbk = NULL;
 	}
 
-	osdp_logger_init(c->log_level, NULL);
+	osdp_logger_init("osdp::cp", c->log_level, NULL);
 
 	if (c->mode == CONFIG_MODE_CP) {
 		c->cp_ctx = osdp_cp_setup2(c->num_pd, info_arr);

diff --git a/python/pyosdp_base.c b/python/pyosdp_base.c
@@ -214,7 +214,7 @@ static PyObject *pyosdp_set_loglevel(pyosdp_base_t *self, PyObject *args)
 		return NULL;
 	}
 
-	osdp_logger_init(log_level, NULL);
+	osdp_logger_init("pyosdp", log_level, NULL);
 
 	Py_RETURN_NONE;
 }
@@ -263,7 +263,7 @@ static int pyosdp_base_tp_init(pyosdp_base_t *self, PyObject *args, PyObject *kw
 
 	channel_manager_init(&self->channel_manager);
 
-	osdp_logger_init(OSDP_LOG_INFO, NULL);
+	osdp_logger_init("pyosdp", OSDP_LOG_INFO, NULL);
 
 	return 0;
 }

diff --git a/samples/c/cp_app.c b/samples/c/cp_app.c
@@ -53,7 +53,7 @@ int main()
 {
 	osdp_t *ctx;
 
-	osdp_logger_init(OSDP_LOG_DEBUG, NULL);
+	osdp_logger_init("osdp::cp", OSDP_LOG_DEBUG, NULL);
 
 	ctx = osdp_cp_setup2(1, pd_info);
 	if (ctx == NULL) {

diff --git a/samples/c/pd_app.c b/samples/c/pd_app.c
@@ -85,7 +85,7 @@ int main()
 {
 	osdp_t *ctx;
 
-	osdp_logger_init(OSDP_LOG_DEBUG, NULL);
+	osdp_logger_init("osdp::pd", OSDP_LOG_DEBUG, NULL);
 
 	ctx = osdp_pd_setup(&info_pd);
 	if (ctx == NULL) {

diff --git a/samples/cpp/cp_app.cpp b/samples/cpp/cp_app.cpp
@@ -52,7 +52,7 @@ int main()
 {
 	OSDP::ControlPanel cp;
 
-	cp.logger_init(OSDP_LOG_DEBUG, NULL);
+	cp.logger_init("osdp::cp", OSDP_LOG_DEBUG, NULL);
 
 	cp.setup(1, pd_info);
 

diff --git a/samples/cpp/pd_app.cpp b/samples/cpp/pd_app.cpp
@@ -76,7 +76,7 @@ int main()
 {
 	OSDP::PeripheralDevice pd;
 
-	pd.logger_init(OSDP_LOG_DEBUG, NULL);
+	pd.logger_init("osdp::pd", OSDP_LOG_DEBUG, NULL);
 
 	pd.setup(&info_pd);
 

diff --git a/src/osdp_common.c b/src/osdp_common.c
@@ -15,12 +15,6 @@
 
 #include "osdp_common.h"
 
-#ifdef CONFIG_DISABLE_PRETTY_LOGGING
-LOGGER_DEFINE(osdp, LOG_INFO, LOGGER_FLAG_NO_COLORS);
-#else
-LOGGER_DEFINE(osdp, LOG_INFO, LOGGER_FLAG_NONE);
-#endif
-
 uint16_t crc16_itu_t(uint16_t seed, const uint8_t *src, size_t len)
 {
 	for (; len > 0; len--) {
@@ -198,18 +192,21 @@ int osdp_rb_pop_buf(struct osdp_rb *p, uint8_t *buf, int max_len)
 /* --- Exported Methods --- */
 
 OSDP_EXPORT
-void osdp_logger_init(int log_level, osdp_log_puts_fn_t log_fn)
+void osdp_logger_init(const char *name, int log_level,
+		      osdp_log_puts_fn_t log_fn)
 {
-	logger_t *ctx = get_log_ctx(osdp);
+	logger_t ctx;
+	FILE *file = NULL;
+	int flags = LOGGER_FLAG_NONE;
 
-	logger_set_log_level(ctx, log_level);
-	if (log_fn) {
-		logger_set_put_fn(ctx, log_fn);
-		logger_set_file(ctx, NULL);
-	} else {
-		logger_set_put_fn(ctx, NULL);
-		logger_set_file(ctx, stderr);
-	}
+#ifdef CONFIG_DISABLE_PRETTY_LOGGING
+	flags |= LOGGER_FLAG_NO_COLORS;
+#endif
+	if (!log_fn)
+		file = stderr;
+
+	logger_init(&ctx, log_level, name, REPO_ROOT, log_fn, file, flags);
+	logger_set_default(&ctx); /* Mark this config as logging default */
 }
 
 OSDP_EXPORT

diff --git a/src/osdp_common.h b/src/osdp_common.h
@@ -16,9 +16,11 @@
 #include <utils/utils.h>
 #include <utils/queue.h>
 #include <utils/slab.h>
-#include <utils/logger.h>
 #include <utils/assert.h>
 
+#define USE_CUSTOM_LOGGER
+#include <utils/logger.h>
+
 #include <osdp.h>
 #include "osdp_config.h" /* generated */
 #include "osdp_export.h" /* generated */
@@ -31,6 +33,15 @@
 
 #define ARG_UNUSED(x) (void)(x)
 
+#define LOG_EM(...)    __logger_log(&pd->logger, LOG_EMERG,  __FILE__, __LINE__, __VA_ARGS__)
+#define LOG_ALERT(...) __logger_log(&pd->logger, LOG_ALERT,  __FILE__, __LINE__, __VA_ARGS__)
+#define LOG_CRIT(...)  __logger_log(&pd->logger, LOG_CRIT,   __FILE__, __LINE__, __VA_ARGS__)
+#define LOG_ERR(...)   __logger_log(&pd->logger, LOG_ERR,    __FILE__, __LINE__, __VA_ARGS__)
+#define LOG_INF(...)   __logger_log(&pd->logger, LOG_INFO,   __FILE__, __LINE__, __VA_ARGS__)
+#define LOG_WRN(...)   __logger_log(&pd->logger, LOG_WARNING,__FILE__, __LINE__, __VA_ARGS__)
+#define LOG_NOT(...)   __logger_log(&pd->logger, LOG_NOTICE, __FILE__, __LINE__, __VA_ARGS__)
+#define LOG_DBG(...)   __logger_log(&pd->logger, LOG_DEBUG,  __FILE__, __LINE__, __VA_ARGS__)
+
 #define ISSET_FLAG(p, f) (((p)->flags & (f)) == (f))
 #define SET_FLAG(p, f)	 ((p)->flags |= (f))
 #define CLEAR_FLAG(p, f) ((p)->flags &= ~(f))
@@ -84,7 +95,7 @@ union osdp_ephemeral_data {
 	assert(TO_OSDP(ctx)->_magic == OSDP_CTX_MAGIC);
 #define input_check_pd_offset(ctx, pd)                                         \
 	if (pd < 0 || pd >= NUM_PD(ctx)) {                                     \
-		LOG_ERR("Invalid PD number %d", pd);                           \
+		LOG_PRINT("Invalid PD number %d", pd);                         \
 		return -1;                                                     \
 	}
 #define input_check2(_1, _2)                                                   \
@@ -326,6 +337,8 @@ struct osdp_pd {
 	/* PD command callback to app with opaque arg pointer as passed by app */
 	void *command_callback_arg;
 	pd_command_callback_t command_callback;
+
+	logger_t logger;       /* logger context (from utils/logger.h) */
 };
 
 struct osdp {
@@ -371,9 +384,6 @@ int osdp_phy_send_packet(struct osdp_pd *pd, uint8_t *buf,
 __weak int64_t osdp_millis_now(void);
 int64_t osdp_millis_since(int64_t last);
 uint16_t osdp_compute_crc16(const uint8_t *buf, size_t len);
-void osdp_log(int log_level, const char *fmt, ...);
-void osdp_log_ctx_reset();
-void osdp_log_ctx_restore();
 
 const char *osdp_cmd_name(int cmd_id);
 const char *osdp_reply_name(int reply_id);

diff --git a/src/osdp_config.h.in b/src/osdp_config.h.in
@@ -17,6 +17,7 @@
 #define GIT_REV                        "@GIT_REV@"
 #define GIT_TAG                        "@GIT_TAG@"
 #define GIT_DIFF                       "@GIT_DIFF@"
+#define REPO_ROOT                      "@REPO_ROOT@"
 
 /**
  * @brief Other OSDP constants

diff --git a/src/osdp_cp.c b/src/osdp_cp.c
@@ -11,8 +11,6 @@
 #include "osdp_common.h"
 #include "osdp_file.h"
 
-LOGGER_DECLARE(osdp, "CP");
-
 #define CMD_POLL_LEN                   1
 #define CMD_LSTAT_LEN                  1
 #define CMD_ISTAT_LEN                  1
@@ -1227,6 +1225,7 @@ static struct osdp *__cp_setup(int num_pd, osdp_pd_info_t *info_list,
 	struct osdp_pd *pd = NULL;
 	struct osdp *ctx;
 	osdp_pd_info_t *info;
+	char name[24] = {0};
 
 	ctx = calloc(1, sizeof(struct osdp));
 	if (ctx == NULL) {
@@ -1271,6 +1270,10 @@ static struct osdp *__cp_setup(int num_pd, osdp_pd_info_t *info_list,
 		if (IS_ENABLED(CONFIG_OSDP_SKIP_MARK_BYTE)) {
 			SET_FLAG(pd, PD_FLAG_PKT_SKIP_MARK);
 		}
+
+		logger_get_default(&pd->logger);
+		snprintf(name, sizeof(name), "OSDP: CP: PD-%d", pd->address);
+		logger_set_name(&pd->logger, name);
 	}
 
 	if (scbk_count != num_pd) {
@@ -1342,7 +1345,6 @@ void osdp_cp_refresh(osdp_t *ctx)
 
 	do {
 		pd = GET_CURRENT_PD(ctx);
-		LOG_SET_PREFIX(pd->name);
 
 		if (cp_refresh(pd) < 0)
 			break;

diff --git a/src/osdp_file.c b/src/osdp_file.c
@@ -8,8 +8,6 @@
 
 #include "osdp_file.h"
 
-LOGGER_DECLARE(osdp, "FOP");
-
 static inline void file_state_reset(struct osdp_file *f)
 {
 	f->flags = 0;

diff --git a/src/osdp_pd.c b/src/osdp_pd.c
@@ -11,8 +11,6 @@
 #include <stdlib.h>
 #endif
 
-LOGGER_DECLARE(osdp, "PD");
-
 #define CMD_POLL_DATA_LEN              0
 #define CMD_LSTAT_DATA_LEN             0
 #define CMD_ISTAT_DATA_LEN             0
@@ -1059,11 +1057,10 @@ osdp_t *osdp_pd_setup(osdp_pd_info_t *info)
 {
 	struct osdp_pd *pd;
 	struct osdp *ctx;
+	char name[16] = {0};
 
 	assert(info);
 
-	LOG_SET_PREFIX(info->name);
-
 #ifndef CONFIG_OSDP_STATIC_PD
 	ctx = calloc(1, sizeof(struct osdp));
 	if (ctx == NULL) {
@@ -1099,6 +1096,10 @@ osdp_t *osdp_pd_setup(osdp_pd_info_t *info)
 	pd->seq_number = -1;
 	memcpy(&pd->channel, &info->channel, sizeof(struct osdp_channel));
 
+	logger_get_default(&pd->logger);
+	snprintf(name, sizeof(name), "OSDP: PD-%d", pd->address);
+	logger_set_name(&pd->logger, name);
+
 	if (pd_event_queue_init(pd)) {
 		goto error;
 	}
@@ -1148,7 +1149,6 @@ void osdp_pd_refresh(osdp_t *ctx)
 	input_check(ctx);
 	struct osdp_pd *pd = GET_CURRENT_PD(ctx);
 
-	LOG_SET_PREFIX(pd->name);
 	osdp_pd_update(pd);
 }
 

diff --git a/src/osdp_phy.c b/src/osdp_phy.c
@@ -6,8 +6,6 @@
 
 #include "osdp_common.h"
 
-LOGGER_DECLARE(osdp, "PHY");
-
 #define OSDP_PKT_MARK                  0xFF
 #define OSDP_PKT_SOM                   0x53
 #define PKT_CONTROL_SQN                0x03

diff --git a/tests/unit-tests/test-cp-fsm.c b/tests/unit-tests/test-cp-fsm.c
@@ -94,7 +94,7 @@ int test_cp_fsm_setup(struct test *t)
 		.channel.flush = NULL,
 		.scbk = NULL,
 	};
-	osdp_logger_init(t->loglevel, NULL);
+	osdp_logger_init("osdp::cp", t->loglevel, NULL);
 	struct osdp *ctx = (struct osdp *)osdp_cp_setup2(1, &info);
 	if (ctx == NULL) {
 		printf("   init failed!\n");

diff --git a/tests/unit-tests/test-cp-phy-fsm.c b/tests/unit-tests/test-cp-phy-fsm.c
@@ -97,7 +97,7 @@ int test_cp_phy_fsm_setup(struct test *t)
 		.channel.flush = NULL,
 		.scbk = NULL,
 	};
-	osdp_logger_init(t->loglevel, NULL);
+	osdp_logger_init("osdp::cp", t->loglevel, NULL);
 	struct osdp *ctx = (struct osdp *)osdp_cp_setup2(1, &info);
 	if (ctx == NULL) {
 		printf("   init failed!\n");

diff --git a/tests/unit-tests/test-cp-phy.c b/tests/unit-tests/test-cp-phy.c
@@ -147,7 +147,7 @@ int test_cp_phy_setup(struct test *t)
 		.channel.flush = NULL,
 		.scbk = NULL,
 	};
-	osdp_logger_init(t->loglevel, NULL);
+	osdp_logger_init("osdp::cp", t->loglevel, NULL);
 	struct osdp *ctx = (struct osdp *)osdp_cp_setup2(1, &info);
 	if (ctx == NULL) {
 		printf(SUB_1 "init failed!\n");

diff --git a/tests/unit-tests/test.c b/tests/unit-tests/test.c
@@ -152,7 +152,7 @@ int test_mock_pd_receive(void *data, uint8_t *buf, int len)
 
 int test_setup_devices(struct test *t, osdp_t **cp, osdp_t **pd)
 {
-	osdp_logger_init(t->loglevel, NULL);
+	osdp_logger_init("osdp", t->loglevel, NULL);
 
 	uint8_t scbk[16] = {
 		0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,

diff --git a/utils b/utils