Significant streamlining. Removed the two Port sub-classes, we now ju…

…st have the Port class, which takes an is_server bool argument in it's constructor. All ports create their own in-fifo, that way a server-client pair between them create both required fifos. The is_server flag determines which order each Port attempts to open it's in and out fifos. Previously when opening it's out fifo, we polled until timeout trying to open the fifo for writing, ignoring ENXIO in case the corresponding reader hadn't yet opened the file for reading. Now we explicitly catch (and ignore) ENOENT, in case the corresponding reader hasn't yet created the fifo file. This means we don't need to first create the server ports (so they can create all the required fifo files), then launch the child process, then open all the ports. Now we can launch the child process *first* and then create the ports in both parent and child process. As long as the ports in both server and client are all created within timeout_seconds of one-another, they will all link up. More comments to follow on significantly revised shutdown & destructors.

demos/server_demo.cpp

@@ -7,7 +7,7 @@
 
 int main (void) {
 
-	SatTerm_Server sts("test_server", "./client_demo", true, {"com_1", "com_2"});
+	SatTerm_Server sts("test_server", "./client_demo", true, {"com_1", "com_2", "com_3", "com_4"});
 
 	if (sts.IsConnected()) {
 		size_t message_count = 10;

src/satellite_terminal.h

@@ -20,7 +20,7 @@
 class SatTerm_Agent {
 	public:
 		SatTerm_Agent() {}
-		virtual ~SatTerm_Agent() {}
+		virtual ~SatTerm_Agent();
 
 		std::string GetMessage(bool capture_end_char = false, unsigned long timeout_seconds = 0);
 		std::string GetMessage(std::string const& port_identifier, bool capture_end_char = false, unsigned long timeout_seconds = 0);
@@ -36,8 +36,8 @@ class SatTerm_Agent {
 		void SetConnectedFlag(bool is_connected);
 
 	protected:
-		bool OpenPorts(std::map<std::string, std::unique_ptr<Port>>& ports, unsigned long timeout_seconds = 5);
-
+		bool CreatePorts(bool is_server, std::string const& working_path, std::vector<std::string> port_identifiers,
+		                 bool display_messages, char end_char, std::map<std::string, std::unique_ptr<Port>>& ports);
 		error_descriptor m_error_code = {0, ""};
 		bool m_display_messages = false;
 		std::map<std::string, std::unique_ptr<Port>> m_ports = {};
@@ -60,8 +60,6 @@ class SatTerm_Server : public SatTerm_Agent {
 
 	private:
 		std::string GetWorkingPath(void);
-		bool CreateServerPorts(std::string const& working_path, std::vector<std::string> port_identifiers,
-		                       bool display_messages, char end_char, std::map<std::string, std::unique_ptr<Port>>& ports);
 		std::vector<std::string> LoadTerminalEmulatorPaths(std::string const& file_path);
 		pid_t StartClient(std::string const& path_to_terminal_emulator_paths, std::string const& path_to_client_binary, std::string const& working_path,
                           char end_char, std::string const& stop_message, std::vector<std::string> port_identifiers);
@@ -75,6 +73,5 @@ class SatTerm_Client : public SatTerm_Agent {
 	private:
 		size_t GetArgStartIndex(std::string const& arg_start_delimiter, int argc, char* argv[]);
 		std::vector<std::string> ParseFifoPaths(size_t argv_start_index, size_t argv_count, char* argv[]);
-		void CreateClientPorts(std::string const& working_path, std::vector<std::string> port_identifiers,
-		                       bool display_messages, char end_char, std::map<std::string, std::unique_ptr<Port>>& ports);
+
 };

src/satterm_agent.cpp

@@ -19,12 +19,24 @@
 
 #include "satellite_terminal.h"
 
-bool SatTerm_Agent::OpenPorts(std::map<std::string, std::unique_ptr<Port>>& ports, unsigned long timeout_seconds) {
+SatTerm_Agent::~SatTerm_Agent() {
+	// NOTE - STL containers apart from std::array make no guarantees about order of element destruction.
+	//
+	// If we want to destroy all the Ports 'in order' at each end, we need to force the issue rather than relying on the automatic behaviour.
+	std::map<std::string, std::unique_ptr<Port>>::iterator itr = m_ports.begin();
+	while (itr != m_ports.end()) {
+		itr = m_ports.erase(itr);
+	}
+}
+
+bool SatTerm_Agent::CreatePorts(bool is_server, std::string const& working_path, std::vector<std::string> port_identifiers,
+                                bool display_messages, char end_char, std::map<std::string, std::unique_ptr<Port>>& ports) {
 	bool success = true;
-	for (auto const& current_port : ports) {
-		if (!(current_port.second->Open(timeout_seconds))) {
+	for (auto const& port_identifier : port_identifiers) {
+		ports.emplace(port_identifier, std::make_unique<Port>(is_server, working_path, port_identifier, display_messages, end_char));
+		if (!(ports.at(port_identifier).get()->IsOpened())) {
 			success = false;
-			m_error_code = current_port.second->GetErrorCode();
+			m_error_code = ports.at(port_identifier)->GetErrorCode();
 			break;
 		}
 	}

src/satterm_client.cpp

@@ -28,7 +28,7 @@ SatTerm_Client::SatTerm_Client(std::string const& identifier, int argc, char* ar
 
 	bool success = false;
 	if (argv_start_index != 0) {
-		success = true;
+
 		m_working_path = std::string(argv[argv_start_index]);
 		m_end_char = (char)(std::stoi(std::string(argv[argv_start_index + 1])));
 		m_stop_message = std::string(argv[argv_start_index + 2]);
@@ -42,13 +42,7 @@ SatTerm_Client::SatTerm_Client(std::string const& identifier, int argc, char* ar
 			std::cerr << message << std::endl;
 		}
 
-		CreateClientPorts(m_working_path, port_identifiers, m_display_messages, m_end_char, m_ports);
-
-		unsigned long timeout_seconds = 5;
-
-		if (success) {
-			success = OpenPorts(m_ports, timeout_seconds);
-		}
+		success = CreatePorts(false, m_working_path, port_identifiers, m_display_messages, m_end_char, m_ports);
 
 		if (success) {
 			if (m_display_messages) {
@@ -72,18 +66,9 @@ SatTerm_Client::SatTerm_Client(std::string const& identifier, int argc, char* ar
 }
 
 SatTerm_Client::~SatTerm_Client() {
-	// If shutdown is occurring, the server will poll m_default_port.in until it gets EOF, then everything will be closed and unlinked at the
-	// server end. To make sure that unlink() deletes the files, we close any that are open here, saving m_default_port.out at this end for last.
 	if (IsConnected()) {
-		for (const auto& port : m_ports) {
-			if (port.first != m_default_port_identifier) {
-				port.second->Close();
-			}
-		}
-		m_ports.at(m_default_port_identifier)->Close();
+		SendMessage(m_stop_message, m_stop_port_identifier);
 	}
-	// There should not be any more to do. Pointers in m_in_ports and m_out_ports are stored as unique_ptr<Port>, so when the maps
-	// are destroyed now the destructors for the Ports should be called automatically. In these the fifos will be unlink()ed if m_is_server is set.
 }
 
 size_t SatTerm_Client::GetArgStartIndex(std::string const& arg_start_delimiter, int argc, char* argv[]) {
@@ -115,10 +100,3 @@ std::vector<std::string> SatTerm_Client::ParseFifoPaths(size_t argv_start_index,
 	}
 	return paths_container;
 }
-
-void SatTerm_Client::CreateClientPorts(std::string const& working_path, std::vector<std::string> port_identifiers,
-                                       bool display_messages, char end_char, std::map<std::string, std::unique_ptr<Port>>& ports) {
-	for (auto const& port_identifier : port_identifiers) {
-		ports.emplace(port_identifier, std::make_unique<Port_Client>(working_path, port_identifier, display_messages, end_char));
-	}
-}

src/satterm_port.cpp

@@ -17,12 +17,103 @@
 #include <ctime>                      // time().
 
 #include <stdio.h>                    // perror().
+#include <sys/stat.h>                 // open() and O_RDONLY, O_WRONLY, etc.
 #include <fcntl.h>                    // open() and O_RDONLY, O_WRONLY, etc.
 #include <unistd.h>                   // write(), read(), close(), unlink().
 #include <errno.h>                    // errno.
+#include <signal.h>                   // SIGPIPE, SIG_IGN.
+
 
 #include "satterm_port.h"
 
+Port::Port(bool is_server, std::string const& working_path, std::string const& identifier, bool display_messages, char end_char) {
+	m_working_path = working_path;
+	m_identifier = identifier;
+
+	m_display_messages = display_messages;
+	m_end_char = end_char;
+	if (is_server) {
+		m_fifos.in.identifier = identifier + "_sin";
+		m_fifos.out.identifier = identifier + "_sout";
+	} else {
+		m_fifos.in.identifier = identifier + "_sout";
+		m_fifos.out.identifier = identifier + "_sin";
+	}
+
+	signal(SIGPIPE, SIG_IGN);
+
+	m_fifos.in.created = CreateFifo(m_working_path + m_fifos.in.identifier);
+
+	OpenFifos(is_server, 5);
+}
+
+Port::~Port() {
+	CloseFifos();
+	UnlinkInFifo();
+}
+
+bool Port::OpenFifos(bool is_server, unsigned long timeout_seconds) {
+	if (is_server) {
+		m_fifos.in.opened = OpenRxFifo(m_working_path + m_fifos.in.identifier, timeout_seconds);
+		if (m_fifos.in.opened) {
+			m_fifos.out.opened = OpenTxFifo(m_working_path + m_fifos.out.identifier, timeout_seconds);
+		}
+	} else {
+		m_fifos.out.opened = OpenTxFifo(m_working_path + m_fifos.out.identifier, timeout_seconds);
+		if (m_fifos.out.opened) {
+			m_fifos.in.opened = OpenRxFifo(m_working_path + m_fifos.in.identifier, timeout_seconds);
+		}
+	}
+	return (m_fifos.in.opened && m_fifos.out.opened);
+}
+
+void Port::CloseFifos(void) {
+	if (m_fifos.out.opened) {
+		m_fifos.out.opened = false;
+		close(m_fifos.out.descriptor);
+	}
+	if (m_fifos.in.opened) {
+		while (m_fifos.in.opened) {
+			usleep(10);
+			GetMessage(false, 0);
+		}
+		close(m_fifos.in.descriptor);
+	}
+}
+
+void Port::UnlinkInFifo(void) {
+	if (m_fifos.in.created) {
+		std::string fifo_path = m_working_path + m_fifos.in.identifier;
+		int status = unlink(fifo_path.c_str());
+		if ((status < 0) && m_display_messages) {
+			std::string error_message = "Unable to unlink() fifo at " + fifo_path;
+			perror(error_message.c_str());
+		}
+	}
+}
+
+bool Port::CreateFifo(std::string const& fifo_path) {
+	m_error_code = {0, ""};
+	bool success = false;
+
+	remove(fifo_path.c_str());	// If temporary file already exists, delete it.
+
+	int status = mkfifo(fifo_path.c_str(), S_IFIFO|0666);
+
+	if (status < 0) {
+		// Info on possible errors for mkfifo() - https://pubs.opengroup.org/onlinepubs/009696799/functions/mkfifo.html
+		m_error_code = {errno, "mkfifo()"};
+		if (m_display_messages) {
+			std::string error_message = "mkfifo() error trying to create fifo " + fifo_path;
+			perror(error_message.c_str());
+		}
+		success = false;
+	} else {
+		success = true;
+	}
+	return success;
+}
+
 bool Port::OpenRxFifo(std::string const& fifo_path, unsigned long timeout_seconds) {
 	m_error_code = {0, ""};
 
@@ -112,6 +203,12 @@ int Port::PollToOpenTxFifo(std::string const& fifo_path, unsigned long timeout_s
 						m_error_code = {-1, "PollToOpenTxFifo()_rx_conn_timeout"};
 					}
 					break;
+				case ENOENT:
+					finished = ((time(0) - start_time) > timeout_seconds);
+					if (finished) {
+						m_error_code = {-1, "PollToOpenTxFifo()_no_tx_fifo_timeout"};
+					}
+					break;
 				default:
 					m_error_code = {errno, "open()_tx"};
 					if (m_display_messages) {

src/satterm_port.h

@@ -17,22 +17,23 @@
 
 class Port {
 	public:
-		Port() {}
-		virtual ~Port() {}
-
-		virtual bool Open(unsigned long timeout_seconds) = 0;
-		virtual void Close(void) = 0;
+		Port(bool is_server, std::string const& working_path, std::string const& identifier, bool display_messages, char end_char);
+		~Port();
 
+		bool IsOpened(void);
 		std::string GetMessage(bool capture_end_char, unsigned long timeout_seconds);
 		std::string SendMessage(std::string const& message, unsigned long timeout_seconds);
 		size_t SendBytes(const char* bytes, size_t byte_count, unsigned long timeout_seconds);
 		error_descriptor GetErrorCode(void);
-		bool IsOpened(void);
 
 	protected:
+		bool CreateFifo(std::string const& fifo_path);
+		bool OpenFifos(bool is_server, unsigned long timeout_seconds);
 		bool OpenRxFifo(std::string const& fifo_path, unsigned long timeout_seconds);
 		bool OpenTxFifo(std::string const& fifo_path, unsigned long timeout_seconds);
 		int PollToOpenTxFifo(std::string const& fifo_path, unsigned long timeout_seconds);
+		void CloseFifos(void);
+		void UnlinkInFifo(void);
 
 		error_descriptor m_error_code = {0, ""};
 		bool m_display_messages = false;
@@ -43,28 +44,3 @@ class Port {
 		std::string m_current_message = "";
 
 };
-
-class Port_Server : public Port {
-	public:
-		Port_Server(std::string const& working_path, std::string const& identifier, bool display_messages, char end_char);
-		~Port_Server();
-
-		bool Open(unsigned long timeout_seconds) override;
-		void Close(void) override;
-
-		bool IsCreated(void);
-
-	private:
-		bool CreateFifo(std::string const& fifo_path);
-
-};
-
-class Port_Client : public Port {
-	public:
-		Port_Client(std::string const& working_path, std::string const& identifier, bool display_messages, char end_char);
-		~Port_Client();
-
-		bool Open(unsigned long timeout_seconds) override;
-		void Close(void) override;
-
-};