pespin has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/14373


Change subject: bts-trx: trx_if.c: Introduce logging macro LOGPL1H
......................................................................

bts-trx: trx_if.c: Introduce logging macro LOGPL1H

This way we unify format. We take the chance to add related information
to some log messages which were not printing that information (and was
confusing when using more than one phy instance).

Change-Id: I5b17a01638ade9a6c41da73e550d5947fa92f568
---
M src/osmo-bts-trx/trx_if.c
1 file changed, 33 insertions(+), 40 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/73/14373/1

diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c
index ec879c7..8d9fedb 100644
--- a/src/osmo-bts-trx/trx_if.c
+++ b/src/osmo-bts-trx/trx_if.c
@@ -58,6 +58,8 @@
  * socket helper functions
  */

+#define LOGPL1H(l1h, section, lvl, fmt, args...) LOGP(section, lvl, "%s: " 
fmt, phy_instance_name(l1h->phy_inst), ##args)
+
 /*! convenience wrapper to open socket + fill in osmo_fd */
 static int trx_udp_open(void *priv, struct osmo_fd *ofd, const char 
*host_local,
                        uint16_t port_local, const char *host_remote, uint16_t 
port_remote,
@@ -155,7 +157,7 @@
        len = snprintf(buf, sizeof(buf), "CMD %s%s%s", tcm->cmd, 
tcm->params_len ? " ":"", tcm->params);
        OSMO_ASSERT(len < sizeof(buf));

-       LOGP(DTRX, LOGL_DEBUG, "Sending control '%s' to %s\n", buf, 
phy_instance_name(l1h->phy_inst));
+       LOGPL1H(l1h, DTRX, LOGL_DEBUG, "Sending control '%s'\n", buf);
        /* send command */
        send(l1h->trx_ofd_ctrl.fd, buf, len+1, 0);

@@ -173,8 +175,7 @@
        OSMO_ASSERT(!llist_empty(&l1h->trx_ctrl_list));
        tcm = llist_entry(l1h->trx_ctrl_list.next, struct trx_ctrl_msg, list);

-       LOGP(DTRX, LOGL_NOTICE, "No satisfactory response from transceiver for 
%s (CMD %s%s%s)\n",
-               phy_instance_name(l1h->phy_inst),
+       LOGPL1H(l1h, DTRX, LOGL_NOTICE, "No satisfactory response from 
transceiver(CMD %s%s%s)\n",
                tcm->cmd, tcm->params_len ? " ":"", tcm->params);

        trx_ctrl_send(l1h);
@@ -208,8 +209,8 @@

        if (!transceiver_available &&
            !(!strcmp(cmd, "POWEROFF") || !strcmp(cmd, "POWERON"))) {
-               LOGP(DTRX, LOGL_ERROR, "CTRL %s ignored: No clock from "
-                    "transceiver, please fix!\n", cmd);
+               LOGPL1H(l1h, DTRX, LOGL_ERROR, "CTRL %s ignored: No clock from "
+                       "transceiver, please fix!\n", cmd);
                return -EIO;
        }

@@ -241,7 +242,7 @@

        if (!pending ||
            !(strcmp(tcm->cmd, prev->cmd) == 0 && strcmp(tcm->params, 
prev->params) == 0)) {
-               LOGP(DTRX, LOGL_INFO, "Enqueuing TRX control command 'CMD 
%s%s%s'\n",
+               LOGPL1H(l1h, DTRX, LOGL_INFO, "Enqueuing TRX control command 
'CMD %s%s%s'\n",
                        tcm->cmd, tcm->params_len ? " ":"", tcm->params);
                llist_add_tail(&tcm->list, &l1h->trx_ctrl_list);
        }
@@ -445,18 +446,16 @@
 static int trx_ctrl_rx_rsp_setslot(struct trx_l1h *l1h, struct trx_ctrl_rsp 
*rsp)
 {
        trx_if_cmd_setslot_cb *cb = (trx_if_cmd_setslot_cb*) rsp->cb;
-       struct phy_instance *pinst = l1h->phy_inst;
        unsigned int tn, ts_type;

        if (rsp->status)
-               LOGP(DTRX, LOGL_ERROR, "transceiver (%s) SETSLOT failed with 
status %d\n",
-                    phy_instance_name(pinst), rsp->status);
+               LOGPL1H(l1h, DTRX, LOGL_ERROR, "transceiver SETSLOT failed with 
status %d\n",
+                       rsp->status);

        /* Since message was already validated against CMD we sent, we know 
format
         * of params is: "<TN> <TS_TYPE>" */
        if (sscanf(rsp->params, "%u %u", &tn, &ts_type) < 2) {
-               LOGP(DTRX, LOGL_ERROR, "transceiver (%s) SETSLOT unable to 
parse params\n",
-                    phy_instance_name(pinst));
+               LOGPL1H(l1h, DTRX, LOGL_ERROR, "transceiver SETSLOT unable to 
parse params\n");
                return -EINVAL;
        }

@@ -481,9 +480,9 @@
                                phy_link_state_set(pinst->phy_link, 
PHY_LINK_CONNECTED);
                        return 0;
                } else {
-                       LOGP(DTRX, LOGL_NOTICE,
-                            "transceiver (%s) rejected POWERON command (%d), 
re-trying in a few seconds\n",
-                            phy_instance_name(pinst), rsp->status);
+                       LOGPL1H(l1h, DTRX, LOGL_NOTICE,
+                               "transceiver rejected POWERON command (%d), 
re-trying in a few seconds\n",
+                               rsp->status);
                        if (pinst->phy_link->state != PHY_LINK_SHUTDOWN)
                                phy_link_state_set(pinst->phy_link, 
PHY_LINK_SHUTDOWN);
                        return 5;
@@ -493,10 +492,10 @@
        }

        if (rsp->status) {
-               LOGP(DTRX, critical ? LOGL_FATAL : LOGL_NOTICE,
-                    "transceiver (%s) rejected TRX command with response: 
'%s%s%s %d'\n",
-                    phy_instance_name(pinst), rsp->cmd, rsp->params[0] != '\0' 
? " ":"",
-                    rsp->params, rsp->status);
+               LOGPL1H(l1h, DTRX, critical ? LOGL_FATAL : LOGL_NOTICE,
+                       "transceiver rejected TRX command with response: 
'%s%s%s %d'\n",
+                       rsp->cmd, rsp->params[0] != '\0' ? " ":"",
+                       rsp->params, rsp->status);
                if (critical)
                        return -EINVAL;
        }
@@ -521,7 +520,7 @@
        if (parse_rsp(buf, len, &rsp) < 0)
                return 0;

-       LOGP(DTRX, LOGL_INFO, "Response message: '%s'\n", buf);
+       LOGPL1H(l1h, DTRX, LOGL_INFO, "Response message: '%s'\n", buf);

        /* abort timer and send next message, if any */
        if (osmo_timer_pending(&l1h->trx_ctrl_timer))
@@ -531,12 +530,11 @@
        if (llist_empty(&l1h->trx_ctrl_list)) {
                /* RSP from a retransmission, skip it */
                if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, &rsp)) {
-                       LOGP(DTRX, LOGL_NOTICE, "Discarding duplicated RSP "
+                       LOGPL1H(l1h, DTRX, LOGL_NOTICE, "Discarding duplicated 
RSP "
                                "from old CMD '%s'\n", buf);
                        return 0;
                }
-               LOGP(DTRX, LOGL_NOTICE, "Response message without "
-                       "command\n");
+               LOGPL1H(l1h, DTRX, LOGL_NOTICE, "Response message without 
command\n");
                return -EINVAL;
        }
        tcm = llist_entry(l1h->trx_ctrl_list.next, struct trx_ctrl_msg,
@@ -546,11 +544,11 @@
        if (!cmd_matches_rsp(tcm, &rsp)) {
                /* RSP from a retransmission, skip it */
                if (l1h->last_acked && cmd_matches_rsp(l1h->last_acked, &rsp)) {
-                       LOGP(DTRX, LOGL_NOTICE, "Discarding duplicated RSP "
+                       LOGPL1H(l1h, DTRX, LOGL_NOTICE, "Discarding duplicated 
RSP "
                                "from old CMD '%s'\n", buf);
                        return 0;
                }
-               LOGP(DTRX, (tcm->critical) ? LOGL_FATAL : LOGL_NOTICE,
+               LOGPL1H(l1h, DTRX, (tcm->critical) ? LOGL_FATAL : LOGL_NOTICE,
                        "Response message '%s' does not match command "
                        "message 'CMD %s%s%s'\n",
                        buf, tcm->cmd, tcm->params_len ? " ":"", tcm->params);
@@ -609,7 +607,7 @@
                burst_len = EGPRS_BURST_LEN;
        /* Accept bursts ending with 2 bytes of padding (OpenBTS compatible 
trx) or without them: */
        } else if (len != GSM_BURST_LEN + 10 && len != GSM_BURST_LEN + 8) {
-               LOGP(DTRX, LOGL_NOTICE, "Got data message with invalid lenght "
+               LOGPL1H(l1h, DTRX, LOGL_NOTICE, "Got data message with invalid 
lenght "
                        "'%d'\n", len);
                return -EINVAL;
        }
@@ -627,15 +625,15 @@
        }

        if (tn >= 8) {
-               LOGP(DTRX, LOGL_ERROR, "Illegal TS %d\n", tn);
+               LOGPL1H(l1h, DTRX, LOGL_ERROR, "Illegal TS %d\n", tn);
                return -EINVAL;
        }
        if (fn >= GSM_HYPERFRAME) {
-               LOGP(DTRX, LOGL_ERROR, "Illegal FN %u\n", fn);
+               LOGPL1H(l1h, DTRX, LOGL_ERROR, "Illegal FN %u\n", fn);
                return -EINVAL;
        }

-       LOGP(DTRX, LOGL_DEBUG, "RX burst tn=%u fn=%u rssi=%d toa256=%d\n",
+       LOGPL1H(l1h, DTRX, LOGL_DEBUG, "RX burst tn=%u fn=%u rssi=%d 
toa256=%d\n",
                tn, fn, rssi, toa256);

 #ifdef TOA_RSSI_DEBUG
@@ -667,11 +665,11 @@
        uint8_t buf[TRX_MAX_BURST_LEN];

        if ((nbits != GSM_BURST_LEN) && (nbits != EGPRS_BURST_LEN)) {
-               LOGP(DTRX, LOGL_ERROR, "Tx burst length %u invalid\n", nbits);
+               LOGPL1H(l1h, DTRX, LOGL_ERROR, "Tx burst length %u invalid\n", 
nbits);
                return -1;
        }

-       LOGP(DTRX, LOGL_DEBUG, "TX burst tn=%u fn=%u pwr=%u\n", tn, fn, pwr);
+       LOGPL1H(l1h, DTRX, LOGL_DEBUG, "TX burst tn=%u fn=%u pwr=%u\n", tn, fn, 
pwr);

        buf[0] = tn;
        buf[1] = (fn >> 24) & 0xff;
@@ -688,7 +686,7 @@
        if (transceiver_available && llist_empty(&l1h->trx_ctrl_list)) {
                send(l1h->trx_ofd_data.fd, buf, nbits + 6, 0);
        } else
-               LOGP(DTRX, LOGL_ERROR, "Ignoring TX data, transceiver 
offline.\n");
+               LOGPL1H(l1h, DTRX, LOGL_ERROR, "Ignoring TX data, transceiver 
offline.\n");

        return 0;
 }
@@ -716,9 +714,7 @@
 /*! close the TRX for given handle (data + control socket) */
 void trx_if_close(struct trx_l1h *l1h)
 {
-       struct phy_instance *pinst = l1h->phy_inst;
-       LOGP(DTRX, LOGL_NOTICE, "Close transceiver for %s\n",
-               phy_instance_name(pinst));
+       LOGPL1H(l1h, DTRX, LOGL_NOTICE, "Close transceiver\n");

        trx_if_flush(l1h);

@@ -749,8 +745,7 @@
        struct phy_link *plink = pinst->phy_link;
        int rc;

-       LOGP(DTRX, LOGL_NOTICE, "Open transceiver for %s\n",
-               phy_instance_name(pinst));
+       LOGPL1H(l1h, DTRX, LOGL_NOTICE, "Open transceiver\n");

        /* initialize ctrl queue */
        INIT_LLIST_HEAD(&l1h->trx_ctrl_list);
@@ -806,15 +801,13 @@

        rc = trx_sched_init(&l1h->l1s, pinst->trx);
        if (rc < 0) {
-               LOGP(DL1C, LOGL_FATAL, "Cannot initialize scheduler for phy "
-                    "instance %d\n", pinst->num);
+               LOGPL1H(l1h, DL1C, LOGL_FATAL, "Cannot initialize scheduler\n");
                return -EIO;
        }

        rc = trx_if_open(l1h);
        if (rc < 0) {
-               LOGP(DL1C, LOGL_FATAL, "Cannot open TRX interface for phy "
-                    "instance %d\n", pinst->num);
+               LOGPL1H(l1h, DL1C, LOGL_FATAL, "Cannot open TRX interface\n");
                trx_phy_inst_close(pinst);
                return -EIO;
        }

--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/14373
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I5b17a01638ade9a6c41da73e550d5947fa92f568
Gerrit-Change-Number: 14373
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pes...@sysmocom.de>
Gerrit-MessageType: newchange

Reply via email to