Change in ...osmo-bts[master]: bts-trx: trx_if.c: Introduce logging macro LOGPL1H

2019-06-04 Thread Harald Welte
Harald Welte has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/14373 )

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


Patch Set 1:

(1 comment)

https://gerrit.osmocom.org/#/c/14373/1/src/osmo-bts-trx/trx_if.c
File src/osmo-bts-trx/trx_if.c:

https://gerrit.osmocom.org/#/c/14373/1/src/osmo-bts-trx/trx_if.c@61
PS1, Line 61: LOGPL1H
a) this is not "L1 handle" (which is bts model specific) but "phy instance" so 
something like LOGPPHI or the like would be more logical.

b) as phy_instance is common, the macro should be shared wih other backends so 
they can use it, too.



--
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 
Gerrit-CC: Harald Welte 
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Tue, 04 Jun 2019 15:21:04 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment


Change in ...osmo-bts[master]: bts-trx: trx_if.c: Introduce logging macro LOGPL1H

2019-06-04 Thread pespin
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(>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(>list, >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: " " */
if (sscanf(rsp->params, "%u %u", , _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) {
-