diff --git a/include/rs232.hpp b/include/rs232.hpp index 469f6d9..05fb566 100644 --- a/include/rs232.hpp +++ b/include/rs232.hpp @@ -46,11 +46,11 @@ namespace sakurajin { std::atomic stopThread = false; std::string readBuffer; - std::shared_mutex readBufferMutex; + std::timed_mutex readBufferMutex; std::atomic readBufferHasData = false; std::string writeBuffer; - std::shared_mutex writeBufferMutex; + std::timed_mutex writeBufferMutex; std::atomic writeBufferHasData = false; /** @@ -153,7 +153,7 @@ namespace sakurajin { * @return std::string the content of the read buffer */ [[nodiscard]] [[maybe_unused]] - std::string&& retrieveReadBuffer(); + std::string retrieveReadBuffer(); /** * @brief load the read buffer and return the first match with a regex @@ -166,7 +166,7 @@ namespace sakurajin { * @return std::string the first match of the read buffer */ [[nodiscard]] [[maybe_unused]] - std::string&& retrieveFirstMatch(const std::regex& pattern); + std::string retrieveFirstMatch(const std::regex& pattern); /** * @brief print a string to the currently connected device @@ -178,7 +178,7 @@ namespace sakurajin { * @param text the text to send */ [[maybe_unused]] - void Print(const std::string& text); + void Print(std::string text); /** diff --git a/samples/interfaceTest.cpp b/samples/interfaceTest.cpp index b7ace7c..0fa7fd8 100644 --- a/samples/interfaceTest.cpp +++ b/samples/interfaceTest.cpp @@ -3,36 +3,32 @@ using namespace sakurajin; -void printNextBuffer(RS232& h){ - auto buffer = h.ReadNextMessage(); - - if(std::get<1>(buffer) < 0){ - std::cerr << "Error while reading the buffer" << std::endl; - }else{ - std::cout << std::get<0>(buffer) ; - } +void printNextBuffer(RS232& h) { + auto buffer = h.retrieveReadBuffer(); + + std::cout << buffer; } -int main (int argc, char **argv) { +int main(int argc, char** argv) { // get the port form the parameter or set the default value - std::string serialPort = ""; + std::string serialPort; - if(argc == 2){ + if (argc == 2) { serialPort = argv[1]; - }else{ - #if defined(__unix__) || defined(__unix) || (defined(__APPLE__) && defined(__MACH__)) - serialPort = "/dev/ttyUSB0"; - #else - serialPort = "\\\\.\\COM3"; - #endif + } else { +#if defined(__unix__) || defined(__unix) || (defined(__APPLE__) && defined(__MACH__)) + serialPort = "/dev/ttyUSB0"; +#else + serialPort = "\\\\.\\COM3"; +#endif } - + // create the rs232 handle and check if the connection worked - RS232 h{serialPort,baud19200}; + RS232 h{serialPort, baud19200}; - if(!h.IsAvailable()){ - std::cerr << "Serial port " << h.GetDeviceName() << " is not available!" << std::endl; + if (!h.IsAvailable()) { + std::cerr << "Serial port " << h.getCurrentDevice()->getDeviceName() << " is not available!" << std::endl; return -1; } @@ -41,11 +37,11 @@ int main (int argc, char **argv) { // read the first message printNextBuffer(h); - - //send a message to the arduino + + // send a message to the arduino h.Print("Hello from the other side!"); - //get the response from the arduino + // get the response from the arduino printNextBuffer(h); // Closing port @@ -54,4 +50,3 @@ int main (int argc, char **argv) { return 0; } - diff --git a/samples/readInfinite.cpp b/samples/readInfinite.cpp index 4650376..eaf6814 100644 --- a/samples/readInfinite.cpp +++ b/samples/readInfinite.cpp @@ -36,7 +36,7 @@ int main (int argc, char **argv) { while (true) { //do a small delay to prevent too much locking - std::this_thread::sleep_for(std::chrono::milliseconds(100)); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); //get the first match of the pattern in the read buffer auto result = rs232_interface.retrieveFirstMatch(pattern); @@ -52,7 +52,7 @@ int main (int argc, char **argv) { }else{ while(true){ //do a small delay to prevent too much locking - std::this_thread::sleep_for(std::chrono::milliseconds(1)); + std::this_thread::sleep_for(std::chrono::milliseconds(10)); //read the string and save the result in a variable auto readString = rs232_interface.retrieveReadBuffer(); diff --git a/src/rs232.cpp b/src/rs232.cpp index bf524f6..fee2ba6 100644 --- a/src/rs232.cpp +++ b/src/rs232.cpp @@ -113,51 +113,66 @@ void sakurajin::RS232::work() { // if there is something to write to the device, write it if (writeBufferHasData) { // lock the mutex to prevent the buffer from being changed while it is being moved + // try lock is not used here because the buffer is only locked for a short time + // both the print function and the work function only do a copy/move operation writeBufferMutex.lock(); auto localWriteBuffer = std::move(writeBuffer); writeBufferHasData = false; writeBufferMutex.unlock(); // write the data - for (auto c : localWriteBuffer) { - if (transferDevice->writeRawData(&c, 1) < 1) { - std::cerr << "error writing [" << c << "] to serial port"; - } + auto err = native::Print(transferDevice, localWriteBuffer); + if (err < 0) { + std::cerr << "Error while writing to the device: " << err << std::endl; + } + } + + // the read is a bit more complicated because the retrieve functions might block the code for a long time + // because of this the read is performed every call to work but first stored into a local static buffer. + static std::string queuedBuffer{}; + char IOBuf = '\0'; + if (transferDevice->readRawData(&IOBuf, 1) > 0) { + if (queuedBuffer.empty()) { + queuedBuffer = std::string(1, IOBuf); + } else { + queuedBuffer.append(1, IOBuf); } } - // read the data - char IOBuf = '\0'; - int64_t dataSize = transferDevice->readRawData(&IOBuf, 1); - if (dataSize > 0) { - // if there is data, add it to the buffer and set the hasData flag - readBufferMutex.lock(); + // if there is data in the local buffer try locking the readBuffer mutex and add the data to the buffer + // if it takes too long to lock the mutex, try again during the next call to work + // this prevents long blocking of actual write operations while making sure no read data is lost. + if (!queuedBuffer.empty() && readBufferMutex.try_lock_for(1ms)) { + // if there is data, add it to the buffer if (readBufferHasData) { - readBuffer.append(1, IOBuf); + readBuffer.append(queuedBuffer); + queuedBuffer.clear(); } else { - readBuffer = std::string(1, IOBuf); + // if there is no data, create a new buffer + readBuffer = std::move(queuedBuffer); readBuffer.reserve(10); readBufferHasData = true; } + readBufferMutex.unlock(); } } // io functions -void sakurajin::RS232::Print(const std::string& text) { +void sakurajin::RS232::Print(std::string text) { std::scoped_lock lock(writeBufferMutex); if (writeBufferHasData) { writeBuffer.append(text); } else { - writeBuffer = text; + writeBuffer = std::move(text); writeBufferHasData = true; } } -std::string&& sakurajin::RS232::retrieveReadBuffer() { +std::string sakurajin::RS232::retrieveReadBuffer() { if (!readBufferHasData) { - return std::string(); + return std::string{}; } std::scoped_lock lock(readBufferMutex); @@ -166,21 +181,20 @@ std::string&& sakurajin::RS232::retrieveReadBuffer() { return std::move(readBuffer); } -std::string&& sakurajin::RS232::retrieveFirstMatch(const std::regex& pattern) { +std::string sakurajin::RS232::retrieveFirstMatch(const std::regex& pattern) { if (!readBufferHasData) { - return std::string(); + return std::string{}; } std::scoped_lock lock(readBufferMutex); std::smatch s_match_result; std::regex_search(readBuffer, s_match_result, pattern); if (s_match_result.empty()) { - return std::string(); + return std::string{}; } - auto result = s_match_result.str(); - readBuffer = s_match_result.suffix(); - return std::move(result); + readBuffer = s_match_result.suffix(); + return s_match_result.str(); } // device access functions