From 9bb9b384a2fd77d104eae4df762ef6235ee7d2c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Kub=C3=A1nek?= Date: Thu, 9 Apr 2026 15:22:44 +0000 Subject: [PATCH 1/7] FPGASerialPort file manipulation. --- doc/version-history.rst | 5 ++ include/Transports/FPGASerialPort.h | 66 ++++++++++++++++++++++ src/Transports/FPGASerialPort.cpp | 88 +++++++++++++++++++++++++++++ 3 files changed, 159 insertions(+) create mode 100644 include/Transports/FPGASerialPort.h create mode 100644 src/Transports/FPGASerialPort.cpp diff --git a/doc/version-history.rst b/doc/version-history.rst index 43985b5..614a272 100644 --- a/doc/version-history.rst +++ b/doc/version-history.rst @@ -2,6 +2,11 @@ Version History ############### +v1.17.0 +------- + +* FPGASerialPort class - opens virtual terminal to communicate with FPGA device. + v1.16.1 ------- diff --git a/include/Transports/FPGASerialPort.h b/include/Transports/FPGASerialPort.h new file mode 100644 index 0000000..a81bd69 --- /dev/null +++ b/include/Transports/FPGASerialPort.h @@ -0,0 +1,66 @@ +/* + * Serial communication through FPGA FIFOs. + * + * Developed for the Vera C. Rubin Observatory Telescope & Site Software Systems. + * This product includes software developed by the Vera C.Rubin Observatory Project + * (https://www.lsst.org). See the COPYRIGHT file at the top-level directory of + * this distribution for details of code ownership. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +#ifndef __Transports_FPGASerialPort__ +#define __Transports_FPGASerialPort__ + +#include + +#include "cRIO/Thread.h" +#include "FPGASerialDevice.h" + +namespace Transports { + +/** + * Thread running serial port communication, using FPGA resources. A virtual + * (pseudo serial) port is opened and offered to external programs to + * communicate with the device. + */ +class FPGASerialPort : public FPGASerialDevice, public LSST::cRIO::Thread { +public: + FPGASerialPort(uint32_t fpga_session, int write_fifo, int read_fifo, const char* device_name, + std::chrono::microseconds quiet_time); + + virtual ~FPGASerialPort(); + + /** + * Opens and initialize pseudo-serial connection. + */ + void init_pt(); + + std::string tty_name; + +protected: + void run(std::unique_lock& lock) override; + +private: + int _port_fd; + + std::string _device_name; + + int _buffer_len = 100; + std::chrono::microseconds _read_timeout; +}; + +} // namespace Transports + +#endif // !__Transports_FPGASerialPort__ diff --git a/src/Transports/FPGASerialPort.cpp b/src/Transports/FPGASerialPort.cpp new file mode 100644 index 0000000..a191dac --- /dev/null +++ b/src/Transports/FPGASerialPort.cpp @@ -0,0 +1,88 @@ +/* + * Serial communication through FPGA FIFOs. + * + * Developed for the Vera C. Rubin Observatory Telescope & Site Software Systems. + * This product includes software developed by the Vera C.Rubin Observatory Project + * (https://www.lsst.org). See the COPYRIGHT file at the top-level directory of + * this distribution for details of code ownership. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation, either version 3 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +#define _XOPEN_SOURCE 700 + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +#include + +#include "Transports/FPGASerialPort.h" + +using namespace Transports; + +FPGASerialPort::FPGASerialPort(uint32_t fpga_session, int write_fifo, int read_fifo, const char* device_name, + std::chrono::microseconds quiet_time) + : FPGASerialDevice(fpga_session, write_fifo, read_fifo, quiet_time), + _device_name(device_name), + _read_timeout(1) { + _port_fd = -1; +} + +FPGASerialPort::~FPGASerialPort() { + if (_port_fd > 0) { + ::close(_port_fd); + } +} + +void FPGASerialPort::init_pt() { + if (_port_fd > 0) { + throw std::runtime_error(fmt::format("Port for device {} already assigned.", _device_name)); + } + + int _port_fd = posix_openpt(O_RDWR); + grantpt(_port_fd); + unlockpt(_port_fd); + + tty_name = ptsname(_port_fd); + + SPDLOG_DEBUG("Port {} created for device {}.", tty_name, _device_name); +} + +void FPGASerialPort::run(std::unique_lock& lock) { + while (keepRunning) { + auto data = read(_buffer_len, _read_timeout); + + if (data.size() > 0) { + ::write(_port_fd, data.data(), data.size()); + } + + unsigned char buffer[_buffer_len]; + + auto bytes = ::read(_port_fd, buffer, _buffer_len); + if (bytes > 0) { + write(buffer, bytes); + } + + runCondition.wait(lock); + } +} From 506a1c76f82cad85ff16968a542ad85b57308fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Kub=C3=A1nek?= Date: Tue, 14 Apr 2026 09:47:03 +0000 Subject: [PATCH 2/7] Reformatted, fixed getcwd call. --- include/ILC/ILCBusList.h | 6 +++--- include/Modbus/BusList.h | 6 +++--- include/Modbus/Parser.h | 10 +++++----- include/PID/PID.h | 2 +- include/PID/PIDParameters.h | 6 +++--- include/cRIO/Application.h | 7 ++++++- include/cRIO/Command.h | 2 +- include/cRIO/IntelHex.h | 10 +++++----- include/cRIO/MPU.h | 4 ++-- include/cRIO/NiError.h | 10 +++++----- include/cRIO/PrintILC.h | 4 ++-- include/cRIO/SimpleFPGACliApp.h | 6 ++++-- include/cRIO/Task.h | 2 +- include/cRIO/version.h | 2 +- src/ILC/ILCBusList.cpp | 12 ++++++------ src/LSST/PID/PID.cpp | 2 +- src/LSST/cRIO/FPGA.cpp | 8 ++++---- src/LSST/cRIO/IntelHex.cpp | 10 +++++----- src/LSST/cRIO/MPU.cpp | 6 +++--- src/LSST/cRIO/MPUTelemetry.cpp | 8 ++++---- src/LSST/cRIO/NiError.cpp | 6 +++--- src/LSST/cRIO/NiStatus.cpp | 6 +++--- src/LSST/cRIO/PrintILC.cpp | 16 ++++++++-------- src/LSST/cRIO/version.cpp | 2 +- src/Modbus/BusList.cpp | 2 +- src/Modbus/Parser.cpp | 4 ++-- tests/test_LimitTrigger.cpp | 12 ++++++------ tests/test_NiError.cpp | 4 ++-- tests/test_PID.cpp | 4 ++-- 29 files changed, 93 insertions(+), 86 deletions(-) diff --git a/include/ILC/ILCBusList.h b/include/ILC/ILCBusList.h index ba7f410..4b9cc7f 100644 --- a/include/ILC/ILCBusList.h +++ b/include/ILC/ILCBusList.h @@ -195,7 +195,7 @@ class ILCBusList : public Modbus::BusList { * * @return status description (enabled, standby,..). */ - const char *getModeStr(uint8_t mode); + const char* getModeStr(uint8_t mode); /** * Return string with short text describing all code 18 status response. @@ -204,7 +204,7 @@ class ILCBusList : public Modbus::BusList { * * @return vector of strings with status description */ - virtual std::vector getStatusString(uint16_t status); + virtual std::vector getStatusString(uint16_t status); /** * Returns last know mode (state) of the @glos{ILC} at the address. @@ -224,7 +224,7 @@ class ILCBusList : public Modbus::BusList { * * @return vector of strings with fault description */ - virtual std::vector getFaultString(uint16_t fault); + virtual std::vector getFaultString(uint16_t fault); /** * Callback for reponse to ServerID request. See LTS-646 Code 17 (0x11) for diff --git a/include/Modbus/BusList.h b/include/Modbus/BusList.h index a7b3e71..f61007f 100644 --- a/include/Modbus/BusList.h +++ b/include/Modbus/BusList.h @@ -245,7 +245,7 @@ class BusList : public std::vector { * @endcode */ template - void callFunction(uint8_t address, uint8_t func, uint32_t timing, const dt &...params) { + void callFunction(uint8_t address, uint8_t func, uint32_t timing, const dt&... params) { emplace(end(), CommandRecord(Buffer(address, func, params...), timing)); } @@ -258,7 +258,7 @@ class BusList : public std::vector { * @return Total size of the response expected. If the passed response doesn't contain * all data needed to calculate its length, returns -1. */ - virtual int responseLength(const std::vector &response); + virtual int responseLength(const std::vector& response); /** * Process @glos{ILC} response. Address and function of the response are checked if @@ -278,7 +278,7 @@ class BusList : public std::vector { * * @see ::parse(Parser) */ - void parse(uint8_t *data, size_t len) { parse(std::vector(data, data + len)); } + void parse(uint8_t* data, size_t len) { parse(std::vector(data, data + len)); } /** * Add response callbacks. Both function code and error response code shall diff --git a/include/Modbus/Parser.h b/include/Modbus/Parser.h index 79bed64..9c186a4 100644 --- a/include/Modbus/Parser.h +++ b/include/Modbus/Parser.h @@ -44,7 +44,7 @@ namespace Modbus { * @param len length of the buffer */ template -static const std::string hexDump(const dt *buf, size_t len) { +static const std::string hexDump(const dt* buf, size_t len) { std::ostringstream os; os << std::setfill('0') << std::hex; for (size_t i = 0; i < len; i++) { @@ -58,7 +58,7 @@ static const std::string hexDump(const dt *buf, size_t len) { } template -static const std::string hexDump(const std::vector
&data) { +static const std::string hexDump(const std::vector
& data) { return hexDump
(data.data(), data.size()); } @@ -88,7 +88,7 @@ class LongResponse : public std::runtime_error { * @param buf Buffer with data received * @param len Buffer length */ - LongResponse(uint8_t *buf, size_t len) + LongResponse(uint8_t* buf, size_t len) : std::runtime_error(std::string("Too long response - received ") + hexDump(buf, len)) {} }; @@ -129,7 +129,7 @@ class Parser : public std::vector { * @param buf target buffer to read the data * @param len how many bytes shall be read */ - void readBuffer(void *buf, size_t len) { + void readBuffer(void* buf, size_t len) { if (_data + len > size()) { throw std::out_of_range(fmt::format( "Attempt to access data beyond buffer end (buffer index {}, but buffer length is {}).", @@ -249,7 +249,7 @@ inline uint64_t Parser::read() { template <> inline float Parser::read() { uint32_t d = read(); - float *db = reinterpret_cast(&d); + float* db = reinterpret_cast(&d); return *db; } diff --git a/include/PID/PID.h b/include/PID/PID.h index 08508bd..3bde242 100644 --- a/include/PID/PID.h +++ b/include/PID/PID.h @@ -77,7 +77,7 @@ class PID { */ void thaw() { _frozen = false; } - double get_offset(bool *changed); + double get_offset(bool* changed); protected: PIDParameters _current_parameters; diff --git a/include/PID/PIDParameters.h b/include/PID/PIDParameters.h index 4ccfe10..23ee45e 100644 --- a/include/PID/PIDParameters.h +++ b/include/PID/PIDParameters.h @@ -37,7 +37,7 @@ namespace PID { struct PIDParameters { PIDParameters() { timestep = P = I = D = N = NAN; } - PIDParameters(const PIDParameters &pid) { + PIDParameters(const PIDParameters& pid) { timestep = pid.timestep; P = pid.P; I = pid.I; @@ -60,7 +60,7 @@ struct PIDParameters { * * @raise YAML::Exception on error */ - virtual void load(const YAML::Node &node) { + virtual void load(const YAML::Node& node) { timestep = node["Timestep"].as(1); P = node["P"].as(); I = node["I"].as(); @@ -76,7 +76,7 @@ struct PIDParameters { * * @raise YAML::Exception on error */ - virtual void load(const YAML::Node &node, const PIDParameters &default_params) { + virtual void load(const YAML::Node& node, const PIDParameters& default_params) { timestep = node["Timestep"].as(default_params.timestep); P = node["P"].as(default_params.P); I = node["I"].as(default_params.I); diff --git a/include/cRIO/Application.h b/include/cRIO/Application.h index 610aeea..d782f0f 100644 --- a/include/cRIO/Application.h +++ b/include/cRIO/Application.h @@ -60,7 +60,7 @@ struct Argument { AClass : public cRIO::Application { public: - AClass(const char* description) : Application(description), counter(0) {} + AClass(const char* description); float counter; protected: @@ -68,6 +68,11 @@ AClass : public cRIO::Application { void processArg(int opt, char* optarg) override; }; +AClass::AClass(const char* description) : Application(description), counter(0) { + addArgument('a', "Append - increase counter."); + addArgument('h', "Print application help."); +} + void AClass::printUsage() { std::cout << "A simple app. Accept -h for help. Pass -i to start interactive mode." << std::endl; Application::printUsage(); diff --git a/include/cRIO/Command.h b/include/cRIO/Command.h index fdda90c..7fcd0e0 100644 --- a/include/cRIO/Command.h +++ b/include/cRIO/Command.h @@ -51,7 +51,7 @@ class Command : public Task { */ virtual void execute() = 0; - void reportException(const std::exception &ex) override { ackFailed(ex.what()); } + void reportException(const std::exception& ex) override { ackFailed(ex.what()); } /** * Acknowledges the command is in progress. diff --git a/include/cRIO/IntelHex.h b/include/cRIO/IntelHex.h index 8c0f9e1..19275fd 100644 --- a/include/cRIO/IntelHex.h +++ b/include/cRIO/IntelHex.h @@ -48,7 +48,7 @@ struct IntelHexLine { class LoadError : public std::runtime_error { public: - LoadError(size_t line, uint16_t address, const std::string &arg) : std::runtime_error(arg) { + LoadError(size_t line, uint16_t address, const std::string& arg) : std::runtime_error(arg) { _line = line; _address = address; } @@ -73,8 +73,8 @@ class IntelHex { * * @throws LoadError on error */ - void load(const std::string &fileName); - void load(std::istream &inputStream); + void load(const std::string& fileName); + void load(std::istream& inputStream); /** * Returns data to be written into ILC. Only three bytes out of every four @@ -84,10 +84,10 @@ class IntelHex { * @param startAddress returns memory start address * @return vector with data */ - std::vector getData(uint16_t &startAddress); + std::vector getData(uint16_t& startAddress); private: - void _processLine(const char *line, IntelHexLine *hexLine, IntelRecordType::Types &recordType); + void _processLine(const char* line, IntelHexLine* hexLine, IntelRecordType::Types& recordType); void _sortByAddress(); std::vector _hexData; diff --git a/include/cRIO/MPU.h b/include/cRIO/MPU.h index acf44ed..4038b14 100644 --- a/include/cRIO/MPU.h +++ b/include/cRIO/MPU.h @@ -72,7 +72,7 @@ class MPU : public Modbus::BusList { */ MPU(uint8_t node_address); - int responseLength(const std::vector &response) override; + int responseLength(const std::vector& response) override; void missing_response() override; @@ -106,7 +106,7 @@ class MPU : public Modbus::BusList { * @param values register values * @param timing timeout (in microseconds) */ - void presetHoldingRegisters(uint16_t start_register_address, const std::vector &values, + void presetHoldingRegisters(uint16_t start_register_address, const std::vector& values, uint32_t timing = 100000); /** diff --git a/include/cRIO/NiError.h b/include/cRIO/NiError.h index 941f420..14036f7 100644 --- a/include/cRIO/NiError.h +++ b/include/cRIO/NiError.h @@ -45,7 +45,7 @@ class NiError : public std::runtime_error { * @param msg Message associated with the error. * @param status Ni error status. */ - NiError(const std::string &msg, NiFpga_Status status); + NiError(const std::string& msg, NiFpga_Status status); }; /** @@ -60,7 +60,7 @@ class NiWarning : public std::runtime_error { * @param msg message associated with warning * @param status NI error status */ - NiWarning(const std::string &msg, NiFpga_Status status); + NiWarning(const std::string& msg, NiFpga_Status status); }; /** @@ -74,7 +74,7 @@ class NiWarning : public std::runtime_error { * * @see NiError */ -inline void NiThrowError(const std::string &msg, NiFpga_Status status) { +inline void NiThrowError(const std::string& msg, NiFpga_Status status) { if (status == 0) { return; } @@ -86,7 +86,7 @@ inline void NiThrowError(const std::string &msg, NiFpga_Status status) { } template -void NiThrowError(NiFpga_Status status, const char *msg, const dt &...params) { +void NiThrowError(NiFpga_Status status, const char* msg, const dt&... params) { if (status == 0) { return; } @@ -110,7 +110,7 @@ void NiThrowError(NiFpga_Status status, const char *msg, const dt &...params) { * * @see NiError */ -void NiThrowError(const char *func, const char *ni_func, NiFpga_Status status); +void NiThrowError(const char* func, const char* ni_func, NiFpga_Status status); } // namespace cRIO } // namespace LSST diff --git a/include/cRIO/PrintILC.h b/include/cRIO/PrintILC.h index 7ba3de9..bbad3de 100644 --- a/include/cRIO/PrintILC.h +++ b/include/cRIO/PrintILC.h @@ -111,7 +111,7 @@ class PrintILC : public virtual ILC::ILCBusList { * @param address @glos{ILC} address * @param hex Intel Hex file to load into ILC */ - void programILC(FPGA *fpga, uint8_t address, IntelHex &hex); + void programILC(FPGA* fpga, uint8_t address, IntelHex& hex); /** * Please consult LTS-646 for details about the ILC commands. @@ -192,7 +192,7 @@ class PrintILC : public virtual ILC::ILCBusList { uint16_t _startAddress; uint16_t _dataLength; - void _writeHex(FPGA *fpga, uint8_t address, IntelHex &hex); + void _writeHex(FPGA* fpga, uint8_t address, IntelHex& hex); }; } // namespace cRIO diff --git a/include/cRIO/SimpleFPGACliApp.h b/include/cRIO/SimpleFPGACliApp.h index 54263e9..dd63c5e 100644 --- a/include/cRIO/SimpleFPGACliApp.h +++ b/include/cRIO/SimpleFPGACliApp.h @@ -134,16 +134,18 @@ class TemplateFPGACliApp : public CliApp { std::cerr << "FPGA already opened!" << std::endl; return 1; } - char dir[255]; + char* dir; if (cmds.size() == 0) { - getcwd(dir, 255); + dir = get_current_dir_name(); } else { + dir = new char[cmds[0].length() + 1]; memcpy(dir, cmds[0].c_str(), cmds[0].length() + 1); } _fpga_singleton = false; _fpga = newFPGA(dir, _fpga_singleton); _fpga->initialize(); _fpga->open(); + free(dir); return 0; } diff --git a/include/cRIO/Task.h b/include/cRIO/Task.h index 86790eb..d668aff 100644 --- a/include/cRIO/Task.h +++ b/include/cRIO/Task.h @@ -54,7 +54,7 @@ class Task { * * @param ex exception raised during processing */ - virtual void reportException(const std::exception &ex) {}; + virtual void reportException(const std::exception& ex) {}; static constexpr task_return_t DONT_RESCHEDULE = 0xFFFFFFFF; }; diff --git a/include/cRIO/version.h b/include/cRIO/version.h index 37b3ed3..6cfa9a8 100644 --- a/include/cRIO/version.h +++ b/include/cRIO/version.h @@ -29,7 +29,7 @@ namespace cRIO { * * @return version of cRIO library */ -const char *version(); +const char* version(); } // namespace cRIO } // namespace LSST diff --git a/src/ILC/ILCBusList.cpp b/src/ILC/ILCBusList.cpp index 40d48d2..695daf6 100644 --- a/src/ILC/ILCBusList.cpp +++ b/src/ILC/ILCBusList.cpp @@ -92,7 +92,7 @@ uint8_t ILCBusList::nextBroadcastCounter() { return _broadcastCounter; } -const char *ILCBusList::getModeStr(uint8_t mode) { +const char* ILCBusList::getModeStr(uint8_t mode) { switch (mode) { case Mode::Standby: return "Standby"; @@ -109,8 +109,8 @@ const char *ILCBusList::getModeStr(uint8_t mode) { } } -std::vector ILCBusList::getStatusString(uint16_t status) { - std::vector ret; +std::vector ILCBusList::getStatusString(uint16_t status) { + std::vector ret; if (status & Status::MajorFault) { ret.push_back("Major Fault"); @@ -127,8 +127,8 @@ std::vector ILCBusList::getStatusString(uint16_t status) { return ret; } -std::vector ILCBusList::getFaultString(uint16_t fault) { - std::vector ret; +std::vector ILCBusList::getFaultString(uint16_t fault) { + std::vector ret; if (fault & Fault::UniqueIdCRC) { ret.push_back("Unique ID CRC error"); @@ -179,7 +179,7 @@ void ILCBusList::changeILCMode(uint8_t address, uint16_t mode) { (getLastMode(address) == Mode::Bootloader && mode == Mode::Standby)) { timeout = 100000; } - } catch (std::out_of_range &err) { + } catch (std::out_of_range& err) { } callFunction(address, ILC_CMD::CHANGE_MODE, timeout, mode); } diff --git a/src/LSST/PID/PID.cpp b/src/LSST/PID/PID.cpp index 2d161a9..cfe357c 100644 --- a/src/LSST/PID/PID.cpp +++ b/src/LSST/PID/PID.cpp @@ -67,7 +67,7 @@ void PID::freeze() { _frozen = true; } -double PID::get_offset(bool *changed) { +double PID::get_offset(bool* changed) { if (abs(_offset) > 0 && _frozen == false) { if (abs(_offset) < (THAW_STEP + 1)) { _offset = 0; diff --git a/src/LSST/cRIO/FPGA.cpp b/src/LSST/cRIO/FPGA.cpp index 84db827..09883e5 100644 --- a/src/LSST/cRIO/FPGA.cpp +++ b/src/LSST/cRIO/FPGA.cpp @@ -47,7 +47,7 @@ FPGA::FPGA(fpgaType type) : SimpleFPGA(type) { } } -void FPGA::ilcCommands(ILC::ILCBusList &ilc, int32_t timeout) { +void FPGA::ilcCommands(ILC::ILCBusList& ilc, int32_t timeout) { // no messages to send if (ilc.size() == 0) { return; @@ -108,7 +108,7 @@ void FPGA::ilcCommands(ILC::ILCBusList &ilc, int32_t timeout) { // data received from ILCs (& 0x9000) // end of frame (0xA000) // 8 bytes of end timestamp (& 0xB000) - uint64_t *beginD = reinterpret_cast(buffer); + uint64_t* beginD = reinterpret_cast(buffer); uint64_t beginTs = le64toh(*beginD); uint64_t endTs = 0; int endTsShift = 0; @@ -119,7 +119,7 @@ void FPGA::ilcCommands(ILC::ILCBusList &ilc, int32_t timeout) { int wrong_response_counter = 0; - for (uint16_t *p = buffer + 4; p < buffer + responseLen; p++) { + for (uint16_t* p = buffer + 4; p < buffer + responseLen; p++) { switch (*p & 0xF000) { // data.. case FIFO::RX_MASK & 0xF000: @@ -140,7 +140,7 @@ void FPGA::ilcCommands(ILC::ILCBusList &ilc, int32_t timeout) { ilc.parse(decoded); wrong_response_counter = 0; break; - } catch (Modbus::WrongResponse &wr) { + } catch (Modbus::WrongResponse& wr) { if (wrong_response_counter == 0) { SPDLOG_WARN( "While processing ILC response, {}. Most likely an ILC is not " diff --git a/src/LSST/cRIO/IntelHex.cpp b/src/LSST/cRIO/IntelHex.cpp index 7d94e55..1c38429 100644 --- a/src/LSST/cRIO/IntelHex.cpp +++ b/src/LSST/cRIO/IntelHex.cpp @@ -31,13 +31,13 @@ using namespace LSST::cRIO; IntelHex::IntelHex() {} -void IntelHex::load(const std::string &fileName) { +void IntelHex::load(const std::string& fileName) { std::ifstream inputStream(fileName); load(inputStream); inputStream.close(); } -void IntelHex::load(std::istream &inputStream) { +void IntelHex::load(std::istream& inputStream) { _hexData.clear(); _lineNo = 0; std::string lineText; @@ -61,7 +61,7 @@ void IntelHex::load(std::istream &inputStream) { _lineNo, 0xFFFF, fmt::format("Invalid extension size - expected 2, got {}", hexLine.data.size())); } - extensionData = *(reinterpret_cast(hexLine.data.data())) > 0; + extensionData = *(reinterpret_cast(hexLine.data.data())) > 0; break; case IntelRecordType::EndOfFile: return; @@ -71,7 +71,7 @@ void IntelHex::load(std::istream &inputStream) { } } -std::vector IntelHex::getData(uint16_t &startAddress) { +std::vector IntelHex::getData(uint16_t& startAddress) { _sortByAddress(); startAddress = _hexData.front().address; @@ -95,7 +95,7 @@ std::vector IntelHex::getData(uint16_t &startAddress) { return ret; } -void IntelHex::_processLine(const char *line, IntelHexLine *hexLine, IntelRecordType::Types &recordType) { +void IntelHex::_processLine(const char* line, IntelHexLine* hexLine, IntelRecordType::Types& recordType) { int offset = 1; if (line[0] != ':') { throw LoadError(_lineNo, 0xFFFF, diff --git a/src/LSST/cRIO/MPU.cpp b/src/LSST/cRIO/MPU.cpp index b8735eb..dc47a17 100644 --- a/src/LSST/cRIO/MPU.cpp +++ b/src/LSST/cRIO/MPU.cpp @@ -119,7 +119,7 @@ MPU::MPU(uint8_t node_address) : _bus(0), _node_address(node_address) { }); } -int MPU::responseLength(const std::vector &response) { +int MPU::responseLength(const std::vector& response) { if (response.size() < 2) { return -1; } @@ -165,7 +165,7 @@ void MPU::presetHoldingRegister(uint16_t register_address, uint16_t value, uint3 _commanded_info.emplace_back(register_address, 0); } -void MPU::presetHoldingRegisters(uint16_t start_register_address, const std::vector &values, +void MPU::presetHoldingRegisters(uint16_t start_register_address, const std::vector& values, uint32_t timing) { callFunction(_node_address, PRESET_HOLDING_REGISTERS, timing, start_register_address, static_cast(values.size()), static_cast(values.size() * 2), values); @@ -178,7 +178,7 @@ uint16_t MPU::getRegister(uint16_t address) { std::lock_guard lg(_registerMutex); try { return _registers.at(address); - } catch (std::out_of_range &e) { + } catch (std::out_of_range& e) { throw std::runtime_error( fmt::format("Cannot retrieve holding register {} (0x{:04x})", address, address)); } diff --git a/src/LSST/cRIO/MPUTelemetry.cpp b/src/LSST/cRIO/MPUTelemetry.cpp index 8f21343..a69beab 100644 --- a/src/LSST/cRIO/MPUTelemetry.cpp +++ b/src/LSST/cRIO/MPUTelemetry.cpp @@ -31,12 +31,12 @@ namespace LSST { namespace cRIO { -MPUTelemetry::MPUTelemetry(const uint8_t *data) { - writeBytes = be64toh(*(reinterpret_cast(data + 0))); - readBytes = be64toh(*(reinterpret_cast(data + 8))); +MPUTelemetry::MPUTelemetry(const uint8_t* data) { + writeBytes = be64toh(*(reinterpret_cast(data + 0))); + readBytes = be64toh(*(reinterpret_cast(data + 8))); } -std::ostream &operator<<(std::ostream &os, const MPUTelemetry &tel) { +std::ostream& operator<<(std::ostream& os, const MPUTelemetry& tel) { os << std::setw(20) << "Write bytes: " << tel.writeBytes << std::endl << std::setw(20) << "Read bytes: " << tel.readBytes << std::endl; return os; diff --git a/src/LSST/cRIO/NiError.cpp b/src/LSST/cRIO/NiError.cpp index 4dec6c0..46c29f8 100644 --- a/src/LSST/cRIO/NiError.cpp +++ b/src/LSST/cRIO/NiError.cpp @@ -28,21 +28,21 @@ namespace LSST { namespace cRIO { -NiError::NiError(const std::string &msg, NiFpga_Status status) +NiError::NiError(const std::string& msg, NiFpga_Status status) : std::runtime_error(msg + ": " + NiStatus(status)) { if (status != 0) { SPDLOG_ERROR("FPGA error {0} in {1}: {2}", status, msg, NiStatus(status)); } } -NiWarning::NiWarning(const std::string &msg, NiFpga_Status status) +NiWarning::NiWarning(const std::string& msg, NiFpga_Status status) : std::runtime_error(msg + ": " + NiStatus(status)) { if (status != 0) { SPDLOG_WARN("FPGA warning {0} in {1}: {2}", status, msg, NiStatus(status)); } } -void NiThrowError(const char *func, const char *ni_func, NiFpga_Status status) { +void NiThrowError(const char* func, const char* ni_func, NiFpga_Status status) { NiThrowError(std::string(func) + " " + ni_func, status); } diff --git a/src/LSST/cRIO/NiStatus.cpp b/src/LSST/cRIO/NiStatus.cpp index a577401..23e12bd 100644 --- a/src/LSST/cRIO/NiStatus.cpp +++ b/src/LSST/cRIO/NiStatus.cpp @@ -29,7 +29,7 @@ namespace cRIO { struct NiErrors { int32_t status; - const char *desc; + const char* desc; }; NiErrors errors[] = { @@ -194,9 +194,9 @@ NiErrors errors[] = { "some: open sessions"}, {0, nullptr}}; -const char *NiStatus(int32_t status) { +const char* NiStatus(int32_t status) { // non-0 negative status are errors, positive are warnings - for (NiErrors *err = errors; err->desc != nullptr; err++) { + for (NiErrors* err = errors; err->desc != nullptr; err++) { if (err->status == abs(status)) return err->desc; } return "Unknow error/warning status"; diff --git a/src/LSST/cRIO/PrintILC.cpp b/src/LSST/cRIO/PrintILC.cpp index 8ef54a6..a5aa3f8 100644 --- a/src/LSST/cRIO/PrintILC.cpp +++ b/src/LSST/cRIO/PrintILC.cpp @@ -52,22 +52,22 @@ void PrintILC::writeApplicationStats(uint8_t address, uint16_t dataCRC, uint16_t SimulatedILC buf; uint16_t v = htole16(dataCRC); - buf.writeBuffer(reinterpret_cast(&v), 2); + buf.writeBuffer(reinterpret_cast(&v), 2); buf.write(0); v = htole16(startAddress); - buf.writeBuffer(reinterpret_cast(&v), 2); + buf.writeBuffer(reinterpret_cast(&v), 2); buf.write(0); v = htole16(dataLength); - buf.writeBuffer(reinterpret_cast(&v), 2); + buf.writeBuffer(reinterpret_cast(&v), 2); buf.write(0); callFunction(address, ILC_CLI_CMD::WRITE_APPLICATION_STATS, 500000, dataCRC, startAddress, dataLength, buf.getCalcCrc()); } -void PrintILC::programILC(FPGA *fpga, uint8_t address, IntelHex &hex) { +void PrintILC::programILC(FPGA* fpga, uint8_t address, IntelHex& hex) { clear(); static constexpr int32_t ILC_TIMEOUT = 1000; @@ -98,7 +98,7 @@ void PrintILC::programILC(FPGA *fpga, uint8_t address, IntelHex &hex) { changeILCMode(address, ILC::Mode::Bootloader); fpga->ilcCommands(*this, ILC_TIMEOUT); clear(); - } catch (std::runtime_error &er) { + } catch (std::runtime_error& er) { // particularly bootloader version 5.0 (used at M2 ILCs) shows // problems reporting status. The following code rectivies that. clear(); @@ -266,7 +266,7 @@ void PrintILC::printSepline() { _printout++; } -void PrintILC::_writeHex(FPGA *fpga, uint8_t address, IntelHex &hex) { +void PrintILC::_writeHex(FPGA* fpga, uint8_t address, IntelHex& hex) { // align data to 256 bytes pages std::vector data = hex.getData(_startAddress); @@ -288,8 +288,8 @@ void PrintILC::_writeHex(FPGA *fpga, uint8_t address, IntelHex &hex) { data.push_back(((i % 4) == 3) ? 0x00 : 0xFF); } - uint8_t *startData = data.data(); - uint8_t *endData = data.data() + data.size(); + uint8_t* startData = data.data(); + uint8_t* endData = data.data() + data.size(); uint16_t dataAddress = _startAddress; while (startData < endData) { std::vector page(APPLICATION_PAGE_LENGTH); diff --git a/src/LSST/cRIO/version.cpp b/src/LSST/cRIO/version.cpp index f82a5b0..bca41b2 100644 --- a/src/LSST/cRIO/version.cpp +++ b/src/LSST/cRIO/version.cpp @@ -23,7 +23,7 @@ namespace LSST { namespace cRIO { -const char *version() { return VERSION; } +const char* version() { return VERSION; } } // namespace cRIO } // namespace LSST diff --git a/src/Modbus/BusList.cpp b/src/Modbus/BusList.cpp index cef9397..201af1c 100644 --- a/src/Modbus/BusList.cpp +++ b/src/Modbus/BusList.cpp @@ -43,7 +43,7 @@ ErrorResponse::ErrorResponse(uint8_t address, uint8_t func) BusList::BusList() {} -int BusList::responseLength(const std::vector &response) { return -1; } +int BusList::responseLength(const std::vector& response) { return -1; } void BusList::parse(Parser parser) { auto exp_address = at(_parsed_index).buffer.address(); diff --git a/src/Modbus/Parser.cpp b/src/Modbus/Parser.cpp index e033dcb..c256d03 100644 --- a/src/Modbus/Parser.cpp +++ b/src/Modbus/Parser.cpp @@ -57,12 +57,12 @@ void Parser::checkCRC() { uint64_t Parser::readU48() { uint64_t ret = 0; - readBuffer(reinterpret_cast(&ret) + 2, 6); + readBuffer(reinterpret_cast(&ret) + 2, 6); return be64toh(ret); } std::string Parser::readString(size_t length) { uint8_t buf[length]; readBuffer(buf, length); - return std::string((const char *)buf, length); + return std::string((const char*)buf, length); } diff --git a/tests/test_LimitTrigger.cpp b/tests/test_LimitTrigger.cpp index 6e278fb..1d6b30a 100644 --- a/tests/test_LimitTrigger.cpp +++ b/tests/test_LimitTrigger.cpp @@ -26,7 +26,7 @@ using namespace LSST::cRIO; -class MyLimitTrigger : public LimitTrigger { +class MyLimitTrigger : public LimitTrigger { public: MyLimitTrigger(int minOccurence) : executedCount(0) { reset(); @@ -39,11 +39,11 @@ class MyLimitTrigger : public LimitTrigger { } int executedCount; - const char *message; + const char* message; protected: bool trigger() override { return ++_counter >= _minOccurence; } - void execute(const char *msg) override { + void execute(const char* msg) override { executedCount++; message = msg; } @@ -60,12 +60,12 @@ TEST_CASE("Simple LimitTrigger", "[LimitTrigger]") { REQUIRE(limitTrigger.executedCount == 0); REQUIRE(limitTrigger.message == NULL); - const char *m1 = "should trigger"; + const char* m1 = "should trigger"; limitTrigger.check(m1); REQUIRE(limitTrigger.executedCount == 1); REQUIRE(limitTrigger.message == m1); - const char *m2 = "next trigger"; + const char* m2 = "next trigger"; limitTrigger.check(m2); REQUIRE(limitTrigger.executedCount == 2); REQUIRE(limitTrigger.message == m2); @@ -76,7 +76,7 @@ TEST_CASE("Simple LimitTrigger", "[LimitTrigger]") { REQUIRE(limitTrigger.executedCount == 2); - const char *m3 = "triggers again!"; + const char* m3 = "triggers again!"; limitTrigger.check(m3); REQUIRE(limitTrigger.executedCount == 3); REQUIRE(limitTrigger.message == m3); diff --git a/tests/test_NiError.cpp b/tests/test_NiError.cpp index 96c5e25..44b37fd 100644 --- a/tests/test_NiError.cpp +++ b/tests/test_NiError.cpp @@ -32,7 +32,7 @@ using namespace LSST::cRIO; void callError() { try { NiThrowError(-61060, "In a Galaxy Far, {}", "Far Away"); - } catch (NiError &ni) { + } catch (NiError& ni) { REQUIRE(ni.what() == std::string("In a Galaxy Far, Far Away: NiFpga_Status_IrqTimeout: The timeout " "expired before any of the IRQs were asserted")); } @@ -41,7 +41,7 @@ void callError() { void callWarning() { try { NiThrowError(63195, "What do you get if you multiply {} by {}? {:+.02f}", "six", "nine", 42.); - } catch (NiWarning &ni) { + } catch (NiWarning& ni) { REQUIRE(ni.what() == std::string("What do you get if you multiply six by nine? +42.00: " "NiFpga_Status_InvalidSession: The session is invalid or has been closed")); diff --git a/tests/test_PID.cpp b/tests/test_PID.cpp index 8ecb282..953dd43 100644 --- a/tests/test_PID.cpp +++ b/tests/test_PID.cpp @@ -32,7 +32,7 @@ using namespace LSST::PID; TEST_CASE("Constant_PID", "[PID]") { - PID *pids[6]; + PID* pids[6]; PIDParameters pparams; pparams.timestep = 1; pparams.P = 1; @@ -60,7 +60,7 @@ TEST_CASE("Constant_PID", "[PID]") { } TEST_CASE("PID_convergence", "[PID]") { - PID *pids[6]; + PID* pids[6]; PIDParameters pparams; pparams.timestep = 0.1; pparams.P = 0.5; From 5c382c662b05309bfcf24a6ab2b584ce4b6533d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Kub=C3=A1nek?= Date: Wed, 15 Jul 2026 13:15:34 +0000 Subject: [PATCH 3/7] FPGASerialPort renamed to PseudoSerialPort. --- etc/ospl.service | 18 -------- .../{FPGASerialPort.h => PseudoSerialPort.h} | 34 ++++++++------- include/Transports/Transport.h | 2 + include/cRIO/SimpleFPGACliApp.h | 2 +- ...PGASerialPort.cpp => PseudoSerialPort.cpp} | 42 ++++++++++++++----- tests/test_FPGACliApp.cpp | 3 +- 6 files changed, 56 insertions(+), 45 deletions(-) delete mode 100644 etc/ospl.service rename include/Transports/{FPGASerialPort.h => PseudoSerialPort.h} (58%) rename src/Transports/{FPGASerialPort.cpp => PseudoSerialPort.cpp} (60%) diff --git a/etc/ospl.service b/etc/ospl.service deleted file mode 100644 index a1b7c31..0000000 --- a/etc/ospl.service +++ /dev/null @@ -1,18 +0,0 @@ -[Unit] -Description=OpenSplice daemon - -[Service] -Type=forking -UMask=0002 -User=ospl -# Paths and user shall be modified as needed -Environment=OSPL_HOME=/opt/OpenSpliceDDS/V6.10.4/HDE/x86_64.linux -Environment=PATH=/opt/OpenSpliceDDS/V6.10.4/HDE/x86_64.linux/bin -Environment=OSPL_URI=file:///usr/lib/python3.10/site-packages/lsst/ts/ddsconfig/data/config/ospl-shmem-debug-no-network.xml -Environment=LD_LIBRARY_PATH=/opt/OpenSpliceDDS/V6.10.4/HDE/x86_64.linux/lib -WorkingDirectory=/tmp -ExecStart=/opt/OpenSpliceDDS/V6.10.4/HDE/x86_64.linux/bin/ospl start -ExecStop=/opt/OpenSpliceDDS/V6.10.4/HDE/x86_64.linux/bin/ospl stop - -[Install] -WantedBy=multi-user.target diff --git a/include/Transports/FPGASerialPort.h b/include/Transports/PseudoSerialPort.h similarity index 58% rename from include/Transports/FPGASerialPort.h rename to include/Transports/PseudoSerialPort.h index a81bd69..09d4105 100644 --- a/include/Transports/FPGASerialPort.h +++ b/include/Transports/PseudoSerialPort.h @@ -20,33 +20,37 @@ * this program. If not, see . */ -#ifndef __Transports_FPGASerialPort__ -#define __Transports_FPGASerialPort__ +#ifndef __Transports_PseudoSerialPort__ +#define __Transports_PseudoSerialPort__ #include #include "cRIO/Thread.h" -#include "FPGASerialDevice.h" +#include "Transport.h" namespace Transports { /** - * Thread running serial port communication, using FPGA resources. A virtual - * (pseudo serial) port is opened and offered to external programs to - * communicate with the device. + * Encapsulates Transport to work as pseudo-serial port. */ -class FPGASerialPort : public FPGASerialDevice, public LSST::cRIO::Thread { +class PseudoSerialPort : public Transport, public LSST::cRIO::Thread { public: - FPGASerialPort(uint32_t fpga_session, int write_fifo, int read_fifo, const char* device_name, - std::chrono::microseconds quiet_time); + PseudoSerialPort(std::shared_ptr real_port, const char* device_name); - virtual ~FPGASerialPort(); + virtual ~PseudoSerialPort(); - /** - * Opens and initialize pseudo-serial connection. - */ void init_pt(); + void open() override; + void close() override; + void write(const unsigned char* buf, size_t len) override; + std::vector read(size_t len, std::chrono::microseconds timeout, + LSST::cRIO::Thread* calling_thread = NULL) override; + void commands(Modbus::BusList& bus_list, std::chrono::microseconds timeout, + LSST::cRIO::Thread* calling_thread = NULL) override; + void flush() override; + void telemetry(uint64_t& write_bytes, uint64_t& read_bytes) override; + std::string tty_name; protected: @@ -59,8 +63,10 @@ class FPGASerialPort : public FPGASerialDevice, public LSST::cRIO::Thread { int _buffer_len = 100; std::chrono::microseconds _read_timeout; + + std::shared_ptr _real_port; }; } // namespace Transports -#endif // !__Transports_FPGASerialPort__ +#endif // !__Transports_PseudoSerialPort__ diff --git a/include/Transports/Transport.h b/include/Transports/Transport.h index 1d3b281..45eea62 100644 --- a/include/Transports/Transport.h +++ b/include/Transports/Transport.h @@ -66,6 +66,8 @@ class CommunicationError : public std::runtime_error { */ class Transport { public: + virtual ~Transport() {} + /** * Opens transport connection. * diff --git a/include/cRIO/SimpleFPGACliApp.h b/include/cRIO/SimpleFPGACliApp.h index dd63c5e..4793df9 100644 --- a/include/cRIO/SimpleFPGACliApp.h +++ b/include/cRIO/SimpleFPGACliApp.h @@ -138,7 +138,7 @@ class TemplateFPGACliApp : public CliApp { if (cmds.size() == 0) { dir = get_current_dir_name(); } else { - dir = new char[cmds[0].length() + 1]; + dir = (char*)malloc(cmds[0].length() + 1); memcpy(dir, cmds[0].c_str(), cmds[0].length() + 1); } _fpga_singleton = false; diff --git a/src/Transports/FPGASerialPort.cpp b/src/Transports/PseudoSerialPort.cpp similarity index 60% rename from src/Transports/FPGASerialPort.cpp rename to src/Transports/PseudoSerialPort.cpp index a191dac..dffd9ec 100644 --- a/src/Transports/FPGASerialPort.cpp +++ b/src/Transports/PseudoSerialPort.cpp @@ -36,30 +36,28 @@ #include -#include "Transports/FPGASerialPort.h" +#include "Transports/PseudoSerialPort.h" using namespace Transports; -FPGASerialPort::FPGASerialPort(uint32_t fpga_session, int write_fifo, int read_fifo, const char* device_name, - std::chrono::microseconds quiet_time) - : FPGASerialDevice(fpga_session, write_fifo, read_fifo, quiet_time), - _device_name(device_name), - _read_timeout(1) { +PseudoSerialPort::PseudoSerialPort(std::shared_ptr real_port, const char* device_name) + : _device_name(device_name), _read_timeout(1) { + _real_port = real_port; _port_fd = -1; } -FPGASerialPort::~FPGASerialPort() { +PseudoSerialPort::~PseudoSerialPort() { if (_port_fd > 0) { ::close(_port_fd); } } -void FPGASerialPort::init_pt() { +void PseudoSerialPort::init_pt() { if (_port_fd > 0) { throw std::runtime_error(fmt::format("Port for device {} already assigned.", _device_name)); } - int _port_fd = posix_openpt(O_RDWR); + _port_fd = posix_openpt(O_RDWR); grantpt(_port_fd); unlockpt(_port_fd); @@ -68,7 +66,29 @@ void FPGASerialPort::init_pt() { SPDLOG_DEBUG("Port {} created for device {}.", tty_name, _device_name); } -void FPGASerialPort::run(std::unique_lock& lock) { +void PseudoSerialPort::open() { _real_port->open(); } + +void PseudoSerialPort::close() { _real_port->close(); } + +void PseudoSerialPort::write(const unsigned char* buf, size_t len) { _real_port->write(buf, len); } + +std::vector PseudoSerialPort::read(size_t len, std::chrono::microseconds timeout, + LSST::cRIO::Thread* calling_thread) { + return _real_port->read(len, timeout, calling_thread); +} + +void PseudoSerialPort::commands(Modbus::BusList& bus_list, std::chrono::microseconds timeout, + LSST::cRIO::Thread* calling_thread) { + _real_port->commands(bus_list, timeout, calling_thread); +} + +void PseudoSerialPort::flush() { _real_port->flush(); } + +void PseudoSerialPort::telemetry(uint64_t& write_bytes, uint64_t& read_bytes) { + _real_port->telemetry(write_bytes, read_bytes); +} + +void PseudoSerialPort::run(std::unique_lock& lock) { while (keepRunning) { auto data = read(_buffer_len, _read_timeout); @@ -83,6 +103,6 @@ void FPGASerialPort::run(std::unique_lock& lock) { write(buffer, bytes); } - runCondition.wait(lock); + runCondition.wait_for(lock, std::chrono::microseconds(1)); } } diff --git a/tests/test_FPGACliApp.cpp b/tests/test_FPGACliApp.cpp index 89392fe..74c765b 100644 --- a/tests/test_FPGACliApp.cpp +++ b/tests/test_FPGACliApp.cpp @@ -95,7 +95,8 @@ TEST_CASE("Test disable/enable ILC", "[FPGACliApp]") { REQUIRE_NOTHROW(cli.processCmdVector({"open"})); - CHECK(testRuns == 0); + testRuns = 0; + REQUIRE_NOTHROW(cli.processCmdVector({"test", "0/2"})); CHECK(testRuns == 1); From f100147833ee6e54e85e00bdbf72a6675bdcd486 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Kub=C3=A1nek?= Date: Mon, 20 Jul 2026 15:56:46 +0000 Subject: [PATCH 4/7] stopInterruptWatcherTasks - fixed core dumps in tests. --- include/cRIO/ControllerThread.h | 2 + include/cRIO/InterruptWatcherTask.h | 4 +- include/cRIO/Thread.h | 8 ++-- src/LSST/cRIO/ControllerThread.cpp | 5 +++ src/Transports/PseudoSerialPort.cpp | 3 ++ tests/test_ControllerThread.cpp | 66 ++++++++++++++++++----------- 6 files changed, 57 insertions(+), 31 deletions(-) diff --git a/include/cRIO/ControllerThread.h b/include/cRIO/ControllerThread.h index 766045f..5cc300c 100644 --- a/include/cRIO/ControllerThread.h +++ b/include/cRIO/ControllerThread.h @@ -52,6 +52,8 @@ class ControllerThread final : public Thread, public Singleton void startInterruptWatcherTask(FPGA* fpga); + void stopInterruptWatcherTask(); + /* Add task into queue. * * @param task Task to enqueue. diff --git a/include/cRIO/InterruptWatcherTask.h b/include/cRIO/InterruptWatcherTask.h index 0179c6b..d321e0e 100644 --- a/include/cRIO/InterruptWatcherTask.h +++ b/include/cRIO/InterruptWatcherTask.h @@ -34,7 +34,7 @@ namespace cRIO { class InterruptWatcherThread : public Thread { public: - InterruptWatcherThread(FPGA* fpga) : _fpga(fpga) {} + InterruptWatcherThread(FPGA* fpga) : _fpga(fpga), _triggeredInterrupts(0) {} ~InterruptWatcherThread() { stop(std::chrono::seconds(2)); } protected: @@ -42,7 +42,7 @@ class InterruptWatcherThread : public Thread { private: FPGA* _fpga; - uint32_t _triggeredInterrupts = 0; + uint32_t _triggeredInterrupts; }; class InterruptWatcherTask : public Task { diff --git a/include/cRIO/Thread.h b/include/cRIO/Thread.h index 12a9dba..256e4be 100644 --- a/include/cRIO/Thread.h +++ b/include/cRIO/Thread.h @@ -44,7 +44,7 @@ class Thread { /** * Starts the thread. Starts new thread running the loop. * - * @param timeout start timeout. Defaults to 1ms. + * @param timeout start timeout. Defaults to 25ms. * * @note If the call blocks indefinitely, most likely cause is the * overridden run method not releasing locks (either directly, or @@ -52,7 +52,7 @@ class Thread { * * @throw runtime_error when thread was already started */ - void start(std::chrono::microseconds timeout = 5ms); + void start(std::chrono::microseconds timeout = 25ms); /** * Stops and join thread. @@ -61,7 +61,7 @@ class Thread { * * @throw runtime_error when timeout is crossed */ - void stop(std::chrono::microseconds timeout = 2ms); + void stop(std::chrono::microseconds timeout = 200ms); /** * Returns true if thread is joinable (~is running). @@ -92,7 +92,7 @@ class Thread { * Mutex protecting keepRunning access, can be used in condition variable. */ std::mutex runMutex; - std::atomic keepRunning = false; + bool keepRunning = false; /** * Condition variable for outside notifications. Notified when keepRunning diff --git a/src/LSST/cRIO/ControllerThread.cpp b/src/LSST/cRIO/ControllerThread.cpp index 3be1cad..f6dd2f3 100644 --- a/src/LSST/cRIO/ControllerThread.cpp +++ b/src/LSST/cRIO/ControllerThread.cpp @@ -71,6 +71,11 @@ void ControllerThread::startInterruptWatcherTask(FPGA* fpga) { _interrupt_watcher_thread->start(); } +void ControllerThread::stopInterruptWatcherTask() { + _interrupt_watcher_thread->stop(); + _interrupt_watcher_thread = NULL; +} + void ControllerThread::setInterruptHandler(std::shared_ptr handler, uint8_t irq) { if ((irq == 0) || (irq > CRIO_INTERRUPTS)) { throw std::runtime_error(fmt::format("Interrupt number should fall between 1 and {} - {} specified", diff --git a/src/Transports/PseudoSerialPort.cpp b/src/Transports/PseudoSerialPort.cpp index dffd9ec..2712860 100644 --- a/src/Transports/PseudoSerialPort.cpp +++ b/src/Transports/PseudoSerialPort.cpp @@ -23,6 +23,7 @@ #define _XOPEN_SOURCE 700 #include +#include #include #include #include @@ -89,6 +90,7 @@ void PseudoSerialPort::telemetry(uint64_t& write_bytes, uint64_t& read_bytes) { } void PseudoSerialPort::run(std::unique_lock& lock) { + std::cout << "Keep running: " << keepRunning << " " << (keepRunning == true) << std::endl; while (keepRunning) { auto data = read(_buffer_len, _read_timeout); @@ -105,4 +107,5 @@ void PseudoSerialPort::run(std::unique_lock& lock) { runCondition.wait_for(lock, std::chrono::microseconds(1)); } + std::cout << "Stopped: " << keepRunning << " " << (keepRunning == true) << std::endl; } diff --git a/tests/test_ControllerThread.cpp b/tests/test_ControllerThread.cpp index 131a574..964e3ce 100644 --- a/tests/test_ControllerThread.cpp +++ b/tests/test_ControllerThread.cpp @@ -95,68 +95,76 @@ TEST_CASE("Queue at different times", "[ControllerThread]") { tv = 0; REQUIRE(tv == 0); - auto when = std::chrono::steady_clock::now() + 500ms; + auto start = std::chrono::steady_clock::now(); + auto when = start + 500ms; ControllerThread::instance().clear(); + TestFPGA fpga; + REQUIRE_NOTHROW(ControllerThread::instance().startInterruptWatcherTask(&fpga)); + for (int i = 0; i < 3; i++) { ControllerThread::instance().enqueue_at(std::make_shared(), when); when -= 1ms; } - when = std::chrono::steady_clock::now() + 200ms; + when = start + 300ms; for (int i = 0; i < 4; i++) { ControllerThread::instance().enqueue_at(std::make_shared(), when); when -= 1ms; } - when = std::chrono::steady_clock::now(); + when = start; for (int i = 0; i < 2; i++) { ControllerThread::instance().enqueue_at(std::make_shared(), when); when += 1ms; } - ControllerThread::instance().start(); + ControllerThread::instance().start(25ms); - std::this_thread::sleep_for(100ms); + std::this_thread::sleep_until(start + 50ms); CHECK(tv == 2); - std::this_thread::sleep_for(200ms); + std::this_thread::sleep_until(start + 350ms); CHECK(tv == 6); - std::this_thread::sleep_for(300ms); + std::this_thread::sleep_until(start + 550ms); CHECK(tv == 9); ControllerThread::instance().stop(); + REQUIRE_NOTHROW(ControllerThread::instance().stopInterruptWatcherTask()); + REQUIRE(tv == 9); } TEST_CASE("Removing tasks from the queue", "[ControllerThread]") { tv = 0; - auto when = std::chrono::steady_clock::now() + 100ms; auto test_task = std::make_shared(); ControllerThread::instance().clear(); + auto start = std::chrono::steady_clock::now(); + auto when = start + 400ms; + for (int i = 0; i < 100; i++) { ControllerThread::instance().enqueue_at(test_task, when); - when += 5ms; + when += 40ms; } REQUIRE(ControllerThread::instance().size() == 100); - ControllerThread::instance().start(); + ControllerThread::instance().start(25ms); - REQUIRE(ControllerThread::instance().size() == 100); + CHECK(ControllerThread::instance().size() == 100); - std::this_thread::sleep_for(101ms); + std::this_thread::sleep_until(start + 401ms); CHECK(tv == 1); CHECK(ControllerThread::instance().size() == 99); @@ -165,7 +173,7 @@ TEST_CASE("Removing tasks from the queue", "[ControllerThread]") { CHECK(ControllerThread::instance().size() == 0); - std::this_thread::sleep_for(200ms); + std::this_thread::sleep_until(start + 601ms); CHECK(tv == 1); @@ -201,23 +209,25 @@ TEST_CASE("Task rescheduling", "[ControllerThread]") { ControllerThread::instance().enqueue(std::make_shared(30)); } + auto start = std::chrono::steady_clock::now(); + ControllerThread::instance().start(); - std::this_thread::sleep_for(22ms); + std::this_thread::sleep_until(start + 22ms); - CHECK(rv == 8); + CHECK(rv >= 8); - std::this_thread::sleep_for(10ms); + std::this_thread::sleep_until(start + 32ms); - CHECK(rv == 10); + CHECK(rv >= 10); - std::this_thread::sleep_for(70ms); + std::this_thread::sleep_until(start + 102ms); - CHECK(rv == 26); + CHECK(rv >= 26); ControllerThread::instance().stop(); - REQUIRE(rv == 26); + REQUIRE(rv >= 26); } class TestHandler : public InterruptHandler { @@ -243,7 +253,7 @@ TEST_CASE("Handling interrupts", "[ControllerThread]") { REQUIRE(iv == 0); for (int i = 0; i < 3; i++) { - ControllerThread::instance().enqueue(std::make_shared(20)); + ControllerThread::instance().enqueue(std::make_shared(100)); } for (int i = 0; i < 3; i++) { @@ -257,13 +267,19 @@ TEST_CASE("Handling interrupts", "[ControllerThread]") { REQUIRE_NOTHROW(ControllerThread::instance().startInterruptWatcherTask(&fpga)); - ControllerThread::instance().start(); + auto start = std::chrono::steady_clock::now(); + + ControllerThread::instance().start(25ms); - std::this_thread::sleep_for(22ms); + std::this_thread::sleep_until(start + 102ms); - CHECK(iv == 6); + int t1 = iv; + CHECK(t1 != 0); ControllerThread::instance().stop(); - REQUIRE(iv == 6); + REQUIRE_NOTHROW(ControllerThread::instance().stopInterruptWatcherTask()); + + int t2 = iv; + REQUIRE(t2 >= t1); } From 408433c8f4c7aa11a7f305dba92d45536a16644b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Kub=C3=A1nek?= Date: Wed, 22 Jul 2026 09:06:16 +0000 Subject: [PATCH 5/7] _startLog for CLI logger setup. --- include/Transports/PseudoSerialPort.h | 10 +++++++--- include/cRIO/CliApp.h | 5 +++++ include/cRIO/SimpleFPGACliApp.h | 2 ++ src/LSST/cRIO/CSC.cpp | 2 +- src/LSST/cRIO/CliApp.cpp | 10 ++++++++++ src/Transports/PseudoSerialPort.cpp | 17 +++++++++++------ 6 files changed, 36 insertions(+), 10 deletions(-) diff --git a/include/Transports/PseudoSerialPort.h b/include/Transports/PseudoSerialPort.h index 09d4105..1e4e584 100644 --- a/include/Transports/PseudoSerialPort.h +++ b/include/Transports/PseudoSerialPort.h @@ -24,8 +24,8 @@ #define __Transports_PseudoSerialPort__ #include +#include -#include "cRIO/Thread.h" #include "Transport.h" namespace Transports { @@ -33,7 +33,7 @@ namespace Transports { /** * Encapsulates Transport to work as pseudo-serial port. */ -class PseudoSerialPort : public Transport, public LSST::cRIO::Thread { +class PseudoSerialPort : public Transport { public: PseudoSerialPort(std::shared_ptr real_port, const char* device_name); @@ -41,6 +41,8 @@ class PseudoSerialPort : public Transport, public LSST::cRIO::Thread { void init_pt(); + void start(); + void open() override; void close() override; void write(const unsigned char* buf, size_t len) override; @@ -54,7 +56,7 @@ class PseudoSerialPort : public Transport, public LSST::cRIO::Thread { std::string tty_name; protected: - void run(std::unique_lock& lock) override; + void run(); private: int _port_fd; @@ -65,6 +67,8 @@ class PseudoSerialPort : public Transport, public LSST::cRIO::Thread { std::chrono::microseconds _read_timeout; std::shared_ptr _real_port; + + std::thread* _thread; }; } // namespace Transports diff --git a/include/cRIO/CliApp.h b/include/cRIO/CliApp.h index a891777..b7690e3 100644 --- a/include/cRIO/CliApp.h +++ b/include/cRIO/CliApp.h @@ -253,6 +253,11 @@ get_the_answer command. */ void printCommands(); + /** + * Starts terminal logging. + */ + void _startLog(); + /** * Exits the application. * diff --git a/include/cRIO/SimpleFPGACliApp.h b/include/cRIO/SimpleFPGACliApp.h index 4793df9..5982ebe 100644 --- a/include/cRIO/SimpleFPGACliApp.h +++ b/include/cRIO/SimpleFPGACliApp.h @@ -79,6 +79,8 @@ class TemplateFPGACliApp : public CliApp { virtual int run(int argc, char* const argv[]) { command_vec cmds = processArgs(argc, argv); + _startLog(); + if (_autoOpen) { command_vec cmds; openFPGA(cmds); diff --git a/src/LSST/cRIO/CSC.cpp b/src/LSST/cRIO/CSC.cpp index 6f457fd..5853798 100644 --- a/src/LSST/cRIO/CSC.cpp +++ b/src/LSST/cRIO/CSC.cpp @@ -62,7 +62,7 @@ CSC::CSC(const char* name, const char* description) : Application(name, descript addArgument('u', ": run under user & group", ':'); } -CSC::~CSC() {} +CSC::~CSC() { spdlog::shutdown(); } void sigHandler(int sig) { SPDLOG_INFO("Exiting on signal {}", sig); diff --git a/src/LSST/cRIO/CliApp.cpp b/src/LSST/cRIO/CliApp.cpp index f2750ae..2730482 100644 --- a/src/LSST/cRIO/CliApp.cpp +++ b/src/LSST/cRIO/CliApp.cpp @@ -32,6 +32,9 @@ #include #include +#include +#include + #include #include @@ -524,5 +527,12 @@ void CliApp::printCommandHelp(const Command* cmd) { std::cout << cmd->help << std::endl << std::endl; } +void CliApp::_startLog() { + spdlog::init_thread_pool(8192, 1); + addSink(std::make_shared()); + + setSinks(); +} + } // namespace cRIO } // namespace LSST diff --git a/src/Transports/PseudoSerialPort.cpp b/src/Transports/PseudoSerialPort.cpp index 2712860..b2a0698 100644 --- a/src/Transports/PseudoSerialPort.cpp +++ b/src/Transports/PseudoSerialPort.cpp @@ -67,6 +67,15 @@ void PseudoSerialPort::init_pt() { SPDLOG_DEBUG("Port {} created for device {}.", tty_name, _device_name); } +void PseudoSerialPort::start() { + if (_thread) { + delete _thread; + } + + _thread = new std::thread(&PseudoSerialPort::run, this); + _thread->detach(); +} + void PseudoSerialPort::open() { _real_port->open(); } void PseudoSerialPort::close() { _real_port->close(); } @@ -89,9 +98,8 @@ void PseudoSerialPort::telemetry(uint64_t& write_bytes, uint64_t& read_bytes) { _real_port->telemetry(write_bytes, read_bytes); } -void PseudoSerialPort::run(std::unique_lock& lock) { - std::cout << "Keep running: " << keepRunning << " " << (keepRunning == true) << std::endl; - while (keepRunning) { +void PseudoSerialPort::run() { + while (true) { auto data = read(_buffer_len, _read_timeout); if (data.size() > 0) { @@ -104,8 +112,5 @@ void PseudoSerialPort::run(std::unique_lock& lock) { if (bytes > 0) { write(buffer, bytes); } - - runCondition.wait_for(lock, std::chrono::microseconds(1)); } - std::cout << "Stopped: " << keepRunning << " " << (keepRunning == true) << std::endl; } From edf8808c26699180e0015e7b41862ac34ed42845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Kub=C3=A1nek?= Date: Wed, 22 Jul 2026 10:27:33 +0000 Subject: [PATCH 6/7] O_NONBLOCK fd. --- src/Transports/PseudoSerialPort.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Transports/PseudoSerialPort.cpp b/src/Transports/PseudoSerialPort.cpp index b2a0698..64662fd 100644 --- a/src/Transports/PseudoSerialPort.cpp +++ b/src/Transports/PseudoSerialPort.cpp @@ -64,6 +64,12 @@ void PseudoSerialPort::init_pt() { tty_name = ptsname(_port_fd); + int flags = fcntl(_port_fd, F_GETFL, 0); + if (fcntl(_port_fd, F_SETFL, flags | O_NONBLOCK) == -1) { + throw std::runtime_error(fmt::format("Cannot set flags for {} device {}: {}.", tty_name, _device_name, + strerror(errno))); + } + SPDLOG_DEBUG("Port {} created for device {}.", tty_name, _device_name); } @@ -111,6 +117,11 @@ void PseudoSerialPort::run() { auto bytes = ::read(_port_fd, buffer, _buffer_len); if (bytes > 0) { write(buffer, bytes); + } else if (bytes == -1) { + if (errno != EAGAIN) { + SPDLOG_ERROR("Cannot read from {} device {}: {}.", tty_name, _device_name, strerror(errno)); + break; + } } } } From 6c4e13621d6d9e2c63d9709b3f50d1fdfceed926 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Kub=C3=A1nek?= Date: Sun, 26 Jul 2026 18:50:27 +0000 Subject: [PATCH 7/7] Address review's comments. --- src/LSST/cRIO/ControllerThread.cpp | 1 + src/Transports/PseudoSerialPort.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/src/LSST/cRIO/ControllerThread.cpp b/src/LSST/cRIO/ControllerThread.cpp index f6dd2f3..dd8a586 100644 --- a/src/LSST/cRIO/ControllerThread.cpp +++ b/src/LSST/cRIO/ControllerThread.cpp @@ -73,6 +73,7 @@ void ControllerThread::startInterruptWatcherTask(FPGA* fpga) { void ControllerThread::stopInterruptWatcherTask() { _interrupt_watcher_thread->stop(); + delete _interrupt_watcher_thread; _interrupt_watcher_thread = NULL; } diff --git a/src/Transports/PseudoSerialPort.cpp b/src/Transports/PseudoSerialPort.cpp index 64662fd..784b666 100644 --- a/src/Transports/PseudoSerialPort.cpp +++ b/src/Transports/PseudoSerialPort.cpp @@ -45,6 +45,7 @@ PseudoSerialPort::PseudoSerialPort(std::shared_ptr real_port, const c : _device_name(device_name), _read_timeout(1) { _real_port = real_port; _port_fd = -1; + _thread = NULL; } PseudoSerialPort::~PseudoSerialPort() {