Hi Alvaro,
A couple of comments below.
On Wed, Apr 02, 2014 at 01:36:57PM +0200, Alvaro Neira Ayuso wrote:
From: Álvaro Neira Ayuso <anayuso(a)sysmocom.de>
With this patch, the manager monitors the sensors and send
OML Failure message from the Manager to the BTS and the BTS
to BSC, for having a report system of the sensors.
Signed-off-by: Alvaro Neira Ayuso <anayuso(a)sysmocom.de>
---
v4: Fixed situation that we have a connection opened with the manager and we
have other connection with other manager.
src/osmo-bts-sysmo/main.c | 73 ++++++++++++++++
src/osmo-bts-sysmo/misc/sysmobts_mgr.c | 77 +++++++++++++++++
src/osmo-bts-sysmo/misc/sysmobts_misc.c | 140 ++++++++++++++++++++++++++++++-
src/osmo-bts-sysmo/misc/sysmobts_misc.h | 33 ++++++++
4 files changed, 322 insertions(+), 1 deletion(-)
diff --git a/src/osmo-bts-sysmo/main.c b/src/osmo-bts-sysmo/main.c
index 74ee47f..269f442 100644
--- a/src/osmo-bts-sysmo/main.c
+++ b/src/osmo-bts-sysmo/main.c
@@ -35,6 +35,7 @@
#include <osmocom/core/talloc.h>
#include <osmocom/core/application.h>
+#include <osmocom/core/socket.h>
#include <osmocom/vty/telnet_interface.h>
#include <osmocom/vty/logging.h>
@@ -45,8 +46,10 @@
#include <osmo-bts/vty.h>
#include <osmo-bts/bts_model.h>
#include <osmo-bts/pcu_if.h>
+#include <osmo-bts/oml.h>
#define SYSMOBTS_RF_LOCK_PATH "/var/lock/bts_rf_lock"
+#define SOCKET_PATH "/var/run/bts_oml"
#include "utils.h"
#include "eeprom.h"
@@ -258,6 +261,7 @@ static void signal_handler(int signal)
case SIGINT:
//osmo_signal_dispatch(SS_GLOBAL, S_GLOBAL_SHUTDOWN, NULL);
bts_shutdown(bts, "SIGINT");
+ unlink(SOCKET_PATH);
break;
case SIGABRT:
case SIGUSR1:
@@ -288,6 +292,62 @@ static int write_pid_file(char *procname)
return 0;
}
+static int read_sock(struct osmo_fd *fd, unsigned int what)
+{
+ struct msgb *msg;
+ struct gsm_abis_mo *mo;
+ int rc;
+
+ msg = oml_msgb_alloc();
+ if (msg == NULL)
+ return -1;
+
+ rc = recv(fd->fd, msg->tail, msg->data_len, 0);
+ if (rc <= 0) {
+ close(fd->fd);
+ osmo_fd_unregister(fd);
+ fd->fd = -1;
+ return -1;
+ }
+
+ msgb_put(msg, rc);
+
+ /*Remove the IPA header for removing the conflict with the new IPA
+ * header*/
+ msgb_pull(msg, sizeof(struct ipaccess_head *)-1);
+
+ mo = &bts->mo;
+ msg->trx = mo->bts->c0;
+ msg->l2h = msg->data;
+ msg->l3h = msg->data + sizeof(struct abis_om_fom_hdr);
+
+ return abis_oml_sendmsg(msg);
+}
+
+static int accept_unix_sock(struct osmo_fd *fd, unsigned int what)
+{
+ int sfd = fd->fd, cl;
+ struct osmo_fd *ofd = (struct osmo_fd *)fd->data;
+
+ if (ofd->fd > -1) {
+ close(ofd->fd);
+ osmo_fd_unregister(ofd);
+ ofd->fd = -1;
+ }
+
+ cl = accept(sfd, NULL, NULL);
+ if (cl < 0)
+ return -1;
+
+ ofd->fd = cl;
+ if (osmo_fd_register(ofd) != 0) {
+ close(cl);
+ return -1;
+ }
+
+ return 0;
+}
+
int main(int argc, char **argv)
{
struct stat st;
@@ -295,6 +355,7 @@ int main(int argc, char **argv)
struct gsm_bts_role_bts *btsb;
struct e1inp_line *line;
void *tall_msgb_ctx;
+ struct osmo_fd fd, rfd;
Perhaps you can rename fd and rfd to something a bit more descriptive,
eg. accept_ofd and ofd.
int rc;
tall_bts_ctx = talloc_named_const(NULL, 1, "OsmoBTS context");
@@ -370,6 +431,18 @@ int main(int argc, char **argv)
fprintf(stderr, "unable to connect to BSC\n");
exit(1);
}
+ fd.cb = accept_unix_sock;
+ rfd.cb = read_sock;
+ rfd.when = BSC_FD_READ;
+ rfd.fd = -1;
+ fd.data = &rfd;
+
+ rc = osmo_sock_unix_init_ofd(&fd, SOCK_SEQPACKET, 0, SOCKET_PATH,
+ OSMO_SOCK_F_BIND | OSMO_SOCK_F_NONBLOCK);
+ if (rc < 0) {
+ perror("Error creating socket domain creation");
+ exit(3);
+ }
if (daemonize) {
rc = osmo_daemonize();
diff --git a/src/osmo-bts-sysmo/misc/sysmobts_mgr.c
b/src/osmo-bts-sysmo/misc/sysmobts_mgr.c
index 6c64d0f..525e526 100644
--- a/src/osmo-bts-sysmo/misc/sysmobts_mgr.c
+++ b/src/osmo-bts-sysmo/misc/sysmobts_mgr.c
@@ -36,6 +36,7 @@
#include <osmocom/core/timer.h>
#include <osmocom/core/msgb.h>
#include <osmocom/core/serial.h>
+#include <osmocom/core/socket.h>
#include <osmocom/vty/telnet_interface.h>
#include <osmocom/vty/logging.h>
@@ -47,7 +48,11 @@
static int no_eeprom_write = 0;
static int daemonize = 0;
+static int trx_nr = -1;
+static int fd_unix = -1;
+static int state_connection = 0;
Please, use some enum for this small state machine, eg.
enum {
SYSMO_MGR_DISCONNECTED = 0,
SYSMO_MGR_CONNECTED,
};
It should help to make the code a bit more readable.
void *tall_mgr_ctx;
+static struct sbts2050_config_info confinfo;
/* every 6 hours means 365*4 = 1460 EEprom writes per year (max) */
#define TEMP_TIMER_SECS (6 * 3600)
@@ -55,7 +60,29 @@ void *tall_mgr_ctx;
/* every 1 hours means 365*24 = 8760 EEprom writes per year (max) */
#define HOURS_TIMER_SECS (1 * 3600)
+/* every 5 minutes try to reconnect if we have a problem in the communication*/
+#define CONNECT_TIMER_SECS (300)
^ ^
You can remove those parenthesis.
+
+/* unix domain socket file descriptor */
+#define SOCKET_PATH "/var/run/bts_oml"
+
#ifdef BUILD_SBTS2050
+static struct osmo_timer_list connect_timer;
+static void socket_connect_cb(void *data)
+{
+ if (state_connection == 0) {
+ fd_unix = osmo_sock_unix_init(SOCK_SEQPACKET, 0, SOCKET_PATH,
+ OSMO_SOCK_F_CONNECT);
+ if (fd_unix < 0) {
+ LOGP(DTEMP, LOGL_ERROR, "Error creating unix socket\n");
+ return;
+ }
+
+ state_connection = 1;
+ }
+ osmo_timer_schedule(&connect_timer, CONNECT_TIMER_SECS, 0);
This timer keeps running even if we're connected. I think you should
enable it only if we are in disconnected state.
+}
+
static struct osmo_timer_list temp_uc_timer;
static void check_uctemp_timer_cb(void *data)
{
@@ -64,6 +91,50 @@ static void check_uctemp_timer_cb(void *data)
sbts2050_uc_check_temp(ucontrol0, &temp_pa, &temp_board);
+ confinfo.temp_pa_cur = temp_pa;
+ confinfo.temp_board_cur = temp_board;
+
+ if (confinfo.temp_min_pa_warn_limit > temp_pa ||
+ confinfo.temp_max_pa_warn_limit < temp_pa) {
Should this be >= and <= ?
+ state_connection = sendto_osmobts(fd_unix,
ucontrol0,
+ SBTS2050_WARN_ALERT,
+ SBTS2050_TEMP_PA,
+ &confinfo, trx_nr);
+ } else if (confinfo.temp_min_pa_sever_limit > temp_pa ||
+ confinfo.temp_max_pa_sever_limit < temp_pa) {
Same here.
+ state_connection = sendto_osmobts(fd_unix,
ucontrol0,
+ SBTS2050_SEVER_ALERT,
+ SBTS2050_TEMP_PA,
+ &confinfo, trx_nr);
+ sbts2050_uc_power(ucontrol0,
+ sbts2050_uc_status(ucontrol0,
+ SBTS2050_STATUS_MASTER),
+ sbts2050_uc_status(ucontrol0,
+ SBTS2050_STATUS_SLAVE),
+ confinfo.pa_power_act);
+ }
+
+ if (confinfo.temp_min_board_warn_limit > temp_board ||
+ confinfo.temp_max_board_warn_limit < temp_board) {
+ state_connection = sendto_osmobts(fd_unix, ucontrol0,
+ SBTS2050_WARN_ALERT,
+ SBTS2050_TEMP_BOARD,
+ &confinfo, trx_nr);
This code looks very similar, perhaps you can encapsulate it in a
function, ie.
check_temperature(confinfo.temp_min_board_warn_limit,
confinfo.temp_max_board_warn_limit,
temp_board, SBTS2050_TEMP_BOARD);
+ } else if (confinfo.temp_min_board_sever_limit >
temp_board ||
+ confinfo.temp_max_board_sever_limit < temp_board) {
+ state_connection = sendto_osmobts(fd_unix, ucontrol0,
+ SBTS2050_SEVER_ALERT,
+ SBTS2050_TEMP_BOARD,
+ &confinfo, trx_nr);
+ sbts2050_uc_power(ucontrol0, confinfo.master_power_act,
+ confinfo.slave_power_act,
+ sbts2050_uc_status(ucontrol0,
+ SBTS2050_STATUS_PA));
+ }
+
+ if (state_connection == 0)
+ close(fd_unix);
+
osmo_timer_schedule(&temp_uc_timer, TEMP_TIMER_SECS, 0);
}
#endif
@@ -93,6 +164,7 @@ static void initialize_sbts2050(void)
if (val != 0)
return;
}
+ trx_nr = val;
ucontrol0.fd = osmo_serial_init(ucontrol0.path, 115200);
if (ucontrol0.fd < 0) {
@@ -169,6 +241,7 @@ static void signal_handler(int signal)
case SIGINT:
sysmobts_check_temp(no_eeprom_write);
sysmobts_update_hours(no_eeprom_write);
+ close(fd_unix);
exit(0);
break;
case SIGABRT:
@@ -363,6 +436,10 @@ int main(int argc, char **argv)
hours_timer.cb = hours_timer_cb;
hours_timer_cb(NULL);
+ /*start handle for reconnect the socket in case of error*/
^
^
Missing spaces.
/* Start handle ... */
+ connect_timer.cb = socket_connect_cb;
+ socket_connect_cb(NULL);
+
/* start uc temperature check timer */
initialize_sbts2050();
diff --git a/src/osmo-bts-sysmo/misc/sysmobts_misc.c
b/src/osmo-bts-sysmo/misc/sysmobts_misc.c
index 9ea26c2..0e89da6 100644
--- a/src/osmo-bts-sysmo/misc/sysmobts_misc.c
+++ b/src/osmo-bts-sysmo/misc/sysmobts_misc.c
@@ -29,13 +29,17 @@
#include <sys/signal.h>
#include <sys/types.h>
#include <sys/stat.h>
+#include <arpa/inet.h>
#include <osmocom/core/talloc.h>
#include <osmocom/core/utils.h>
#include <osmocom/core/msgb.h>
+#include <osmocom/core/socket.h>
#include <osmocom/core/application.h>
#include <osmocom/vty/telnet_interface.h>
#include <osmocom/vty/logging.h>
+#include <osmocom/gsm/abis_nm.h>
+#include <osmocom/gsm/protocol/ipaccess.h>
#include "btsconfig.h"
#include "sysmobts_misc.h"
@@ -49,10 +53,144 @@
#define SERIAL_ALLOC_SIZE 300
#define SIZE_HEADER_RSP 5
#define SIZE_HEADER_CMD 4
-
+#define OM_ALLOC_SIZE 1024
+#define OM_HEADROOM_SIZE 128
+#define IPA_OML_PROTO 0xFF
#ifdef BUILD_SBTS2050
/**********************************************************************
+ * Function send information to OsmoBts
+ *********************************************************************/
+static void add_sw_descr(struct msgb *msg)
+{
+ char file_version[255];
+ char file_id[255];
+
+ strcpy(file_id, "sysmomgr");
Better use strncpy here.
+ strncpy(file_version, PACKAGE_VERSION,
strlen(PACKAGE_VERSION));
And make sure you nul-terminate these strings.
file_version[strlen(PACKAGE_VERSION)-1] = '\0';
As strncpy doesn't append the \0.
I remember you couldn't use strlcpy, right?
+ msgb_v_put(msg, NM_ATT_SW_DESCR);
+ msgb_tl16v_put(msg, NM_ATT_FILE_ID, strlen(file_id),
+ (uint8_t *)file_id);
Coding style nitpick, perhaps:
msgb_tl16v_put(msg, NM_ATT_FILE_ID, strlen(file_id),
(uint8_t *)file_id);