PR Review

This commit is contained in:
2026-05-29 15:34:36 +02:00
parent f07baa197c
commit 1422b41b0f
11 changed files with 113 additions and 86 deletions
@@ -41,6 +41,8 @@ class BaseMatterhornServer
ReturnCode initial_checks(ServerInterface &socket);
ReturnCode get_num_udp_interfaces(ServerInterface &socket) const;
/**
* @brief call function corresponding to the function ID received from the
* client and send back the result
@@ -52,6 +54,9 @@ class BaseMatterhornServer
private:
static std::string getMatterhornServerVersion();
static constexpr uint8_t numUDPInterfaces =
1; // only one udp per module for now
};
template <typename DerivedServer>
@@ -67,6 +72,13 @@ BaseMatterhornServer<DerivedServer>::processFunction(const detFuncs function_id,
}
}
template <typename DerivedServer>
ReturnCode BaseMatterhornServer<DerivedServer>::get_num_udp_interfaces(
ServerInterface &socket) const {
return static_cast<ReturnCode>(
socket.sendResult(static_cast<int>(numUDPInterfaces)));
}
template <typename DerivedServer>
ReturnCode
BaseMatterhornServer<DerivedServer>::get_version(ServerInterface &socket) {
@@ -8,15 +8,23 @@
namespace sls {
struct DetectorServerOptions {
// TODO: careful changed for other detectors
uint16_t port{DEFAULT_TCP_CNTRL_PORTNO};
bool isControlServer{true};
bool debugflag{false};
bool updateFlag{false};
bool checkModuleFlag{true};
/// @brief ignore firmware version compatibility
bool ignoreFirmwareCompatibility{false};
/// @brief safe startup - skip initial detector setup and checks
bool safeStartup{false};
bool versionRequested{false};
bool helpRequested{false};
};
template <typename Server>
struct SpecificDetectorServerOptions : DetectorServerOptions {};
// template specialization
// template <>
// struct SpecificDetectorServerOptions<BaseMatterhornServer> {};
// TODO should be a general server specific class or even shared with
// CommandLIneOptions in Receiver
class CommandLineOptions {
@@ -28,22 +36,32 @@ class CommandLineOptions {
DetectorServerOptions parse(int argc, char *argv[]);
std::string getHelpMessage(const std::string &executable) const;
std::string printOptions() const;
std::string getVersion() const;
private:
std::string getHelpMessage(const std::string &executable) const;
uint16_t parsePort(const char *optarg) const;
private:
static constexpr std::array<option, 6> options{
void parse_deprecated(const int &opt, char *argv[]);
DetectorServerOptions detectorserveroptions{};
static constexpr std::array<option, 8> options{
{{"help", no_argument, nullptr, 'h'},
{"version", no_argument, nullptr, 'v'},
{"port", required_argument, nullptr, 'p'},
{"devel", no_argument, nullptr, 'd'},
{"update", no_argument, nullptr, 'u'},
{"ignore_fw_compatibility", no_argument, nullptr,
'f'}, // ignore firmware compatibility check
{"safe_startup", no_argument, nullptr, 's'}, // safe startup
// deprecated options for backward compatibility
{"devel", no_argument, nullptr, 'd'}, // safe_startup mode
{"update", no_argument, nullptr, 'u'}, // firmware compatibility check
{nullptr, 0, nullptr, 0}}};
inline static const std::string optstring{"hvp:du"};
inline static const char optstring[] = "hvp:fs"
"du"; // second part is deprecated
};
} // namespace sls
@@ -1,7 +1,6 @@
#include "CommandLineOptions.h"
#include "sls/ToString.h"
#include "sls/sls_detector_exceptions.h"
#include "sls/versionAPI.h"
#include <cstdint>
#include <fmt/format.h>
@@ -10,24 +9,14 @@
namespace sls {
std::string CommandLineOptions::getVersion() const {
return fmt::format(
"MatterhornServer Version: {}",
APIMATTERHORN); // TODO check that it is updated correctly !!!
}
uint16_t CommandLineOptions::parsePort(const char *optarg) const {
uint16_t val = 0; // TODO: in c code its unsigned int
try {
val = sls::StringTo<uint16_t>(optarg);
} catch (...) {
throw("Could not parse port number " + std::string(optarg));
}
if (val == std::numeric_limits<uint16_t>::max()) {
throw sls::RuntimeError("Cannot parse stop server port number. "
"Value must be in range 0 - 65535.");
} catch (const std::exception &e) {
throw sls::RuntimeError(fmt::format(
"Could not parse port number {}. {}", optarg, e.what()));
}
if (val < 1024) {
@@ -45,51 +34,83 @@ CommandLineOptions::getHelpMessage(const std::string &executable) const {
"Usage: {}"
" [arguments]\n"
"Possible arguments are:\n"
"\t-v, --version : Software version\n"
"\t-p, --port <port> : TCP communication port with client. "
"\t-v, --version : Software version\n"
"\t-p, --port <port> : TCP communication port with client. "
"\n"
"\t-d, --devel : Developer mode. Skips firmware "
"checks. \n"
"\t-u, --update : Update mode. Skips firmware checks "
"and "
"initial detector setup. \n",
"\t-s, --safe_startup : Safe startup mode. Skips initial "
"detector setup and checks. \n"
"\t-f, --ignore_fw_compatibility : Ignore firmware compatibility "
"check. \n",
executable);
return helpmessage;
}
void CommandLineOptions::parse_deprecated(const int &opt, char *argv[]) {
switch (opt) {
case 'd':
std::cout << "Warning: -d/--devel option is deprecated. Use "
"-l/--safe_startup instead."
<< std::endl;
detectorserveroptions.safeStartup = true;
break;
case 'u':
std::cout << "Warning: -u/--update option is deprecated. Use "
"-f/--ignore_fw_compatibility instead."
<< std::endl;
detectorserveroptions.ignoreFirmwareCompatibility = true;
break;
default:
throw std::runtime_error(fmt::format("Wrong command line arguments. {}",
getHelpMessage(argv[0])));
}
}
std::string CommandLineOptions::printOptions() const {
std::string msg = "setting up detector server";
if (detectorserveroptions.ignoreFirmwareCompatibility) {
msg += " skipping firmware compatibility checks";
msg += detectorserveroptions.safeStartup ? " and" : "";
}
if (detectorserveroptions.safeStartup) {
msg += " in safe startup mode e.g. skipping any initial detector setup "
"and checks";
}
return msg;
}
DetectorServerOptions CommandLineOptions::parse(int argc, char *argv[]) {
int opt, option_index = 0;
DetectorServerOptions serverOptionsValues{};
while ((opt = getopt_long(argc, argv, optstring.c_str(), options.data(),
while ((opt = getopt_long(argc, argv, optstring, options.data(),
&option_index)) != -1) {
switch (opt) {
case 'h':
std::cout << getHelpMessage(argv[0]) << std::endl;
serverOptionsValues.helpRequested = true; // to exit in main
detectorserveroptions.helpRequested = true; // to exit in main
break;
case 'v':
serverOptionsValues.versionRequested = true; // to exit in main
std::cout << getVersion() << std::endl;
detectorserveroptions.versionRequested = true; // to exit in main
break;
case 'p':
serverOptionsValues.port = parsePort(optarg);
detectorserveroptions.port = parsePort(optarg);
break;
case 'd':
serverOptionsValues.debugflag = true;
case 'f':
detectorserveroptions.ignoreFirmwareCompatibility = true;
break;
case 'u':
serverOptionsValues.updateFlag = true;
case 's':
detectorserveroptions.safeStartup = true;
break;
default:
std::cout << getHelpMessage(argv[0]) << std::endl;
throw std::runtime_error("Wrong command line arguments.");
parse_deprecated(opt, argv); // to handle deprecated options and
// throw error for wrong options
}
}
return serverOptionsValues;
return detectorserveroptions;
}
} // namespace sls
@@ -2,6 +2,7 @@
#include "VirtualMatterhornServer.h"
#include "sls/logger.h"
#include "sls/sls_detector_exceptions.h"
#include "sls/versionAPI.h"
#include <semaphore.h>
#include <csignal>
@@ -47,10 +48,19 @@ int main(int argc, char *argv[]) {
} catch (sls::RuntimeError &e) {
return EXIT_FAILURE;
}
if (opts.versionRequested || opts.helpRequested) {
if (opts.versionRequested) {
std::cout << fmt::format("MatterhornServer Version: {}", APIMATTERHORN)
<< std::endl; // might go back to costum CommandLIneOptions
// getVersion
return EXIT_SUCCESS;
}
if (opts.helpRequested) {
return EXIT_SUCCESS;
}
LOG(TLogLevel::logINFOMAGENTA) << cli.printOptions();
// Register Ctrl+C handler
std::signal(SIGINT, sigInterruptHandler);
@@ -15,7 +15,6 @@ target_link_libraries(slsServerObject
PUBLIC
slsProjectOptions
slsSupportStatic
slsDetectorObject
PRIVATE
slsProjectWarnings
)
@@ -18,17 +18,11 @@ namespace sls {
/// @brief struct saving udp details (one UDP port per module)
struct UDPInfo {
uint16_t srcport{};
uint16_t srcport2{};
uint16_t dstport{};
uint16_t dstport2{};
uint64_t srcmac{};
uint64_t srcmac2{};
uint64_t dstmac{};
uint64_t dstmac2{};
uint32_t srcip{};
uint32_t srcip2{};
uint32_t dstip{};
uint32_t dstip2{};
};
template <typename DerivedDetectorServer> class DetectorServer {
@@ -57,10 +51,6 @@ template <typename DerivedDetectorServer> class DetectorServer {
ReturnCode processFunction(const detFuncs function_id,
ServerInterface &socket);
size_t num_udp_interfaces() const;
ReturnCode get_num_udp_interfaces(ServerInterface &socket) const;
// TODO dont know what this does?
ReturnCode get_update_mode(ServerInterface &socket) const;
@@ -151,18 +141,6 @@ ReturnCode DetectorServer<DerivedDetectorServer>::processFunction(
return ReturnCode::FAIL;
}
template <typename DerivedDetectorServer>
size_t DetectorServer<DerivedDetectorServer>::num_udp_interfaces() const {
return udpDetails.size();
}
template <typename DerivedDetectorServer>
ReturnCode DetectorServer<DerivedDetectorServer>::get_num_udp_interfaces(
ServerInterface &socket) const {
int numUDPInterfaces = static_cast<int>(num_udp_interfaces());
return static_cast<ReturnCode>(socket.sendResult(numUDPInterfaces));
}
template <typename DerivedDetectorServer>
ReturnCode DetectorServer<DerivedDetectorServer>::get_update_mode(
ServerInterface &socket) const {
@@ -185,6 +163,7 @@ ReturnCode DetectorServer<DerivedDetectorServer>::set_source_udp_mac(
}
udpDetails[0].srcmac = newsrcudpMac;
// TODO: configuremac, check unicast address
return ReturnCode::OK;
}
@@ -217,13 +196,6 @@ ReturnCode DetectorServer<DerivedDetectorServer>::get_source_udp_ip(
return static_cast<ReturnCode>(socket.sendResult(udpDetails[0].srcip));
}
template <typename DerivedDetectorServer>
ReturnCode DetectorServer<DerivedDetectorServer>::get_source_udp_port(
ServerInterface &socket) const {
return static_cast<ReturnCode>(
socket.sendResult(static_cast<int>(udpDetails[0].srcport)));
}
template <typename DerivedDetectorServer>
ReturnCode DetectorServer<DerivedDetectorServer>::set_destination_udp_mac(
ServerInterface &socket) {
@@ -238,6 +210,7 @@ ReturnCode DetectorServer<DerivedDetectorServer>::set_destination_udp_mac(
}
udpDetails[0].dstmac = newDstMac;
// TODO: configuremac, check unicast address
return ReturnCode::OK;
}
@@ -44,7 +44,7 @@ void TCPInterface::startTCPServerClientConnection() {
socket.Receive(function_id);
if (function_id < 0 || function_id >= NUM_DET_FUNCTIONS) {
throw RuntimeError(fmt::format(
UNRECOGNIZED_FNUM_ENUM,
"{}:{}", UNRECOGNIZED_FNUM_ENUM,
getFunctionNameFromEnum((enum detFuncs)function_id)));
}
auto returncode = processReceivedData(
-1
View File
@@ -184,7 +184,6 @@ void Detector::setHostname(const std::vector<std::string> &hostname) {
setDetectorSize(numChannels);
setInitialChecks(initialChecks);
}
LOG(logDEBUG3) << "not in shared memory yet, setting hostname";
pimpl->setHostname(hostname);
}
-2
View File
@@ -95,7 +95,6 @@ std::string Module::getControlServerLongVersion() const {
}
void Module::throwDeprecatedServerVersion() const {
LOG(logDEBUG3) << "throw deprecated version error";
uint64_t res = sendToDetectorStop<int64_t>(F_GET_SERVER_VERSION);
std::cout << std::endl;
std::ostringstream os;
@@ -3513,7 +3512,6 @@ void Module::initialDetectorServerChecks() {
}
void Module::checkDetectorVersionCompatibility() {
LOG(logDEBUG) << "Checking detector version compatibility with client...";
std::string detServers[2] = {getControlServerLongVersion(),
getStopServerLongVersion()};
LOG(logDEBUG1)
+2 -3
View File
@@ -97,9 +97,8 @@ class Logger {
gettimeofday(&tv, nullptr);
constexpr size_t result_len = 100;
char result[result_len];
// snprintf(result, result_len, "%s.%03ld", buffer,
//(long)tv.tv_usec / 1000);
snprintf(result, result_len, "%s.%03ld", buffer, (long)tv.tv_usec);
snprintf(result, result_len, "%s.%03ld", buffer,
(long)tv.tv_usec / 1000);
result[result_len - 1] = '\0';
return result;
}
-2
View File
@@ -90,8 +90,6 @@ void ClientSocket::readReply(int &ret, void *retval, size_t retval_size) {
}
// get retval
Receive(retval, retval_size);
LOG(logDEBUG1) << "Received size " << retval_size << " bytes from "
<< socketType << " with return code: " << ret;
}
// debugging
catch (SocketError &e) {