From c8e38ce19a16f326bb529b7991788b9fa7c9a8b2 Mon Sep 17 00:00:00 2001
From: donker <donker@astron.nl>
Date: Tue, 31 Aug 2021 06:46:14 +0200
Subject: [PATCH] L2SDP-454, process review comment.

---
 src/node.cpp        |  5 ++---
 src/node.h          |  6 +-----
 src/periph/fpga.cpp | 40 ++++++++++++++--------------------------
 src/periph/fpga.h   | 11 ++++++++---
 src/sdptr.conf      | 14 +++++++-------
 src/sdptr.cpp       | 39 +++++++++++++++------------------------
 src/sdptr.h         |  2 +-
 7 files changed, 48 insertions(+), 69 deletions(-)

diff --git a/src/node.cpp b/src/node.cpp
index 24785218..97779b14 100644
--- a/src/node.cpp
+++ b/src/node.cpp
@@ -161,8 +161,7 @@ void Node::worker()
     //      << " leaving!!" << endl;
 }
 
-Node::Node(const string ipaddr, const uint unb, const uint localnr, const string type,
-           const string firmware, const uint firmware_version)
+Node::Node(const string ipaddr, const uint unb, const uint localnr, const string type)
 {
     // int ret;
     UniboardNr = unb;
@@ -170,7 +169,7 @@ Node::Node(const string ipaddr, const uint unb, const uint localnr, const string
     GlobalNr = localnr + 4 * unb;
     
     myIPaddr = ipaddr;
-    periph_fpga = new Periph_fpga(GlobalNr, ipaddr, firmware, firmware_version);
+    periph_fpga = new Periph_fpga(GlobalNr, ipaddr);
 
 
     if (type != "pn") {
diff --git a/src/node.h b/src/node.h
index e9c7253c..d873d883 100644
--- a/src/node.h
+++ b/src/node.h
@@ -47,9 +47,7 @@ class NODE_config {
 public:
     int gn;
     std::string ipaddr;
-    std::string firmware;
     int n_beamsets;
-    uint version;
 };
 
 #define GLOBALNODE_to_UNB(n)   (n>>2)
@@ -79,9 +77,7 @@ class Node {
   }
 
  public:
-  Node(const std::string ipaddr,
-       const uint unb, const uint localnr, const std::string type,
-       const std::string firmware, const uint firmware_version);
+  Node(const std::string ipaddr, const uint unb, const uint localnr, const std::string type);
   ~Node();
 
   uint ipaddr_to_id(const std::string ipaddr);
diff --git a/src/periph/fpga.cpp b/src/periph/fpga.cpp
index ff238852..4fb2601c 100644
--- a/src/periph/fpga.cpp
+++ b/src/periph/fpga.cpp
@@ -58,12 +58,10 @@ extern int debug;
 #define R_MEM 0  // read mem value
 #define R_UCP 1  // read value from hardware
 
-Periph_fpga::Periph_fpga(uint global_nr, string ipaddr, string expected_design_name, uint expected_firmware_version):
+Periph_fpga::Periph_fpga(uint global_nr, string ipaddr):
     GlobalNr(global_nr),
     Masked(false),
     Online(false),
-    my_expected_design_name(expected_design_name),
-    my_expected_firmware_version(expected_firmware_version),
     my_current_design_name("-"),
     my_current_hw_version(0),
     my_current_fw_version("-.-"),
@@ -82,6 +80,7 @@ Periph_fpga::Periph_fpga(uint global_nr, string ipaddr, string expected_design_n
     ucp = new UCP(ipaddr);
     mmap = new CMMap();
 
+    // flash settings
     Flash_fact_sector_start = 0;
     Flash_fact_sector_end   = 159;
     Flash_user_sector_start = 160;
@@ -611,6 +610,14 @@ uint32_t Periph_fpga::shift_mask(const string addr_str, uint32_t data)
 */
 bool Periph_fpga::read_system_info(TermOutput& termout)
 {
+    // mask and shift values for getting version info from SYSTEM_INFO
+    #define FW_VERSION_MASK     0x00F00000
+    #define FW_VERSION_SHIFT    20
+    #define FW_SUBVERSION_MASK  0x000F0000
+    #define FW_SUBVERSION_SHIFT 16
+    #define HW_VERSION_MASK     0x0000300
+    #define HW_VERSION_SHIFT    8
+
     bool retval = false;
     uint32_t nvalues = REG_ADDR_SYSTEM_INFO_SPAN;
     uint32_t addr = REG_ADDR_SYSTEM_INFO;
@@ -654,33 +661,14 @@ bool Periph_fpga::read_system_info(TermOutput& termout)
         mmap->clear();
     }
     my_current_design_name = design_name;
-
     // cout << "node " << GlobalNr << " design_name= " << my_current_design_name << endl;
 
-    // FIXME: get rid of magic constants in masks, should be in CCFG:
-    uint firmware_version    = (data[0] & 0x00F00000) >> 20;
-    uint firmware_subversion = (data[0] & 0x000F0000) >> 16;
+    uint32_t firmware_version = (data[0] & FW_VERSION_MASK) >> FW_VERSION_SHIFT;
+    uint firmware_subversion = (data[0] & FW_SUBVERSION_MASK) >> FW_SUBVERSION_SHIFT;
     my_current_fw_version = to_string(firmware_version) + "." + to_string(firmware_subversion);
-
-    my_current_hw_version = (data[0] & 0x0000300) >> 8;
-
+    my_current_hw_version = (data[0] & HW_VERSION_MASK) >> HW_VERSION_SHIFT;
     retval = true;
-    // if (my_current_design_name == my_expected_design_name && firmware_version >= my_expected_firmware_version) {
-    //     Online = true;
-    //     retval = true;
-    // }
-    // else {
-    //     retval = false;
-    //     cerr << "Warning: Node configuration mismatch!! (read_design_name/version=" << my_current_design_name
-    //               << "/" << firmware_version << "), expected=" << my_expected_design_name
-    //               << "/" << my_expected_firmware_version << ")" << endl;
-    //     //syslog(LOG_WARNING,"Node configuration mismatch!! (read_design_name/version=%s/%d, expected=%s/%d)\n",
-    //     //       my_current_design_name.c_str(), firmware_version,
-    //     //       my_expected_design_name.c_str(), my_expected_firmware_version);
-
-    //     Online = false;
-    //     // registerMap->setAllPermission_NA();
-    // }
+    
     return retval;
 }
 
diff --git a/src/periph/fpga.h b/src/periph/fpga.h
index 7a4f0386..b282586e 100644
--- a/src/periph/fpga.h
+++ b/src/periph/fpga.h
@@ -50,8 +50,6 @@ private:
   bool Masked;
   bool Online;
 
-  std::string my_expected_design_name;
-  uint my_expected_firmware_version;
   std::string my_current_design_name;
   uint my_current_hw_version;
   std::string my_current_fw_version;
@@ -69,6 +67,13 @@ private:
   bool    my_xst_processing_enable;
   uint32_t my_pps_offset_cnt;
 
+  uint32_t fw_version_mask;
+  uint32_t fw_version_shift;
+  uint32_t fw_subversion_mask;
+  uint32_t fw_subversion_shift;
+  uint32_t hw_version_mask;
+  uint32_t hw_version_shift;
+
   std::ofstream rbf_wf;
   uint32_t Flash_page_start, Flash_page_size_bytes, Flash_user_sector_start,
            Flash_user_sector_end, Flash_pages_per_sector,
@@ -185,7 +190,7 @@ private:
   CMMap read_reg_map();
 
 public:
-  Periph_fpga(uint global_nr, std::string ipaddr, std::string expected_design_name, uint expected_firmware_version);
+  Periph_fpga(uint global_nr, std::string ipaddr);
   ~Periph_fpga();
 
 
diff --git a/src/sdptr.conf b/src/sdptr.conf
index 557b8e1a..f2d51796 100644
--- a/src/sdptr.conf
+++ b/src/sdptr.conf
@@ -4,44 +4,44 @@
 # this config file holds settings for all [type].
 #
 # # settings per type
-# [LB_Core]						  # [band_stationtype]
+# [LB_CORE]						  # [ant_band_station_type]
 # n_fpgas = 16                    # 8 or 16
 # first_pfga_nr = 0               # 0 for LB or 16 for HB
 # ip_prefix = 10.99.              # first part of ip (last part is hardware dependent)
 # n_beamsets = 1                  # 1 for 'LB', 'HB Remote' and 'HB International' and 2 for 'HB Core'
 
 
-[LB_Core]
+[LB_CORE]
 n_fpgas = 16
 first_fpga_nr = 0
 ip_prefix = 10.99.
 n_beamsets = 1
 
-[LB_Remote]
+[LB_REMOTE]
 n_fpgas = 16
 first_fpga_nr = 0
 ip_prefix = 10.99.
 n_beamsets = 1
 
-[LB_International]
+[LB_INTERNATIONAL]
 n_fpgas = 16
 first_fpga_nr = 0
 ip_prefix = 10.99.
 n_beamsets = 1
 
-[HB_Core]
+[HB_CORE]
 n_fpgas = 8
 first_fpga_nr = 16
 ip_prefix = 10.99.
 n_beamsets = 2
 
-[HB_Remote]
+[HB_REMOTE]
 n_fpgas = 8
 first_fpga_nr = 16
 ip_prefix = 10.99.
 n_beamsets = 1
 
-[HB_International]
+[HB_INTERNATIONAL]
 n_fpgas = 16
 first_fpga_nr = 16
 ip_prefix = 10.99.
diff --git a/src/sdptr.cpp b/src/sdptr.cpp
index bcf0e224..87a1b016 100644
--- a/src/sdptr.cpp
+++ b/src/sdptr.cpp
@@ -45,8 +45,6 @@ namespace po = boost::program_options;
 #include "sdptr.h"
 #include "config_reader.h"
 #include "tools/parse.h"
-// #include "unb_config.h"
-// #include "io/tcpsocket.h"
 #include "opcua/ua_server.h"
 
 using namespace std;
@@ -104,7 +102,6 @@ void server_init(bool warm_start)
 {
     cout << "read sdptr config file '" << SD.configfile << "'" << endl;
     ConfigReader* p = ConfigReader::getInstance();  // create object of the class ConfigReader
-    // p->parseFile("/etc/sdptr.conf");  // parse the configuration file
     p->parseFile(SD.configfile);  // parse the configuration file
     p->dumpFileValues();  // print map on the console after parsing it
     
@@ -112,18 +109,14 @@ void server_init(bool warm_start)
     int32_t first_fpga_nr = 0;
     int32_t n_beamsets = 0;
     string ip_prefix = "";
-    string firmware_name = "";
-    int32_t firmware_version = 0;
-
-    p->getValue(SD.ant_station_type, "n_fpgas", n_fpgas);
-    p->getValue(SD.ant_station_type, "first_fpga_nr", first_fpga_nr);
-    p->getValue(SD.ant_station_type, "n_beamsets", n_beamsets);
-    p->getValue(SD.ant_station_type, "ip_prefix", ip_prefix);
-    p->getValue(SD.ant_station_type, "fw_name", firmware_name);
-    p->getValue(SD.ant_station_type, "fw_version", firmware_version);
+
+    p->getValue(SD.ant_band_station_type, "n_fpgas", n_fpgas);
+    p->getValue(SD.ant_band_station_type, "first_fpga_nr", first_fpga_nr);
+    p->getValue(SD.ant_band_station_type, "n_beamsets", n_beamsets);
+    p->getValue(SD.ant_band_station_type, "ip_prefix", ip_prefix);
     
     if (n_fpgas == 0) {
-        cerr << "ERROR, n_fpgas = 0" << endl;
+        cerr << "ERROR, no settings found for '" << SD.ant_band_station_type << "'" <<endl;
         exit(EXIT_FAILURE);
     }
 
@@ -134,14 +127,12 @@ void server_init(bool warm_start)
         int32_t board_nr = (int32_t)(nc.gn / FPGAS_PER_BOARD);
         int32_t fpga_nr = (int32_t)(nc.gn % FPGAS_PER_BOARD);
         nc.ipaddr = ip_prefix + to_string(board_nr) + "." + to_string(fpga_nr+1);
-        nc.firmware = firmware_name;
         nc.n_beamsets = n_beamsets;
-        nc.version = (uint)firmware_version;
         NC.push_back(nc);
     }
 
     for (auto nc : NC) {
-        cout << nc.gn << ", " << nc.ipaddr << ", " << nc.firmware << ", " << nc.version << endl;
+        cout << nc.gn << ", " << nc.ipaddr << ", " << nc.n_beamsets << endl;
     }
 
     cerr << "done, it is mine. Now (re)init all" << endl;
@@ -158,7 +149,7 @@ void server_init(bool warm_start)
         uint n = GLOBALNODE_to_NODE(gn);
         string type = GLOBALNODE_to_TYPE(gn);
 
-        Node *node = new Node(nc.ipaddr, uniboardnr, n, type, nc.firmware, nc.version);
+        Node *node = new Node(nc.ipaddr, uniboardnr, n, type);
         nodelist.push_back(node);
     }
     SD.unb = new UniboardMap(nodelist);
@@ -191,8 +182,7 @@ int main (int argc, char* argv[])
     debug=0;
 
     SD.configfile = "/etc/sdptr.conf";
-    // SD.configfile = "sdptr.conf";
-    SD.ant_station_type = "-";
+    SD.ant_band_station_type = "-";
 
     atomic<size_t> ncthreads(0);
     thread *monitor_thread;
@@ -202,12 +192,12 @@ int main (int argc, char* argv[])
         ("help,h",      "shows this help text")
         ("version,v",   "prints sdptr version info")
         ("nodaemon",    po::value<bool>(&nodaemon)->zero_tokens(),
-                        "With this flag, the server runs NOT as daemon")
+                        "With this flag, sdptr runs NOT as daemon")
         ("debug",       po::value(&debug), "Prints out debug info (default=0)")
         ("configfile",  po::value<string>(&SD.configfile)->default_value(SD.configfile),
                         "Specify uniboard configuration file location")
-        ("type",        po::value<string>(&SD.ant_station_type)->default_value(SD.ant_station_type),
-                        "Specify ant-station type")
+        ("type",        po::value<string>(&SD.ant_band_station_type)->default_value(SD.ant_band_station_type),
+                        "Specify antenna band and station type")
     ;
 
     try {
@@ -227,8 +217,9 @@ int main (int argc, char* argv[])
         if (debug) {
             cout << "Debug printing enabled. Level=" << debug << endl;
         }
-        if (SD.ant_station_type == "-") {
-            cout << "need type [LB_Core, LB_Remote, LB_International, HB_Core, HB_Remote, HB_International or LTS]" << endl;
+        if (SD.ant_band_station_type == "-") {
+            cout << "need --type see sdptr.conf for accepted [keys]" << endl;
+            exit(EXIT_FAILURE);
         }
     } catch (po::error& e) {
         cerr << "ERROR: " << e.what() << endl << endl;
diff --git a/src/sdptr.h b/src/sdptr.h
index a2fb4990..6ba538c1 100644
--- a/src/sdptr.h
+++ b/src/sdptr.h
@@ -49,7 +49,7 @@ public:
     TranslatorMap *tr;
 
     std::string configfile;
-    std::string ant_station_type;
+    std::string ant_band_station_type;
 
     struct timespec t0;
     volatile time_t timetick;
-- 
GitLab