More cleanup and fixes from testing failures

platforms/nuttx/src/px4/common/SerialImpl.cpp

@@ -60,7 +60,6 @@ SerialImpl::SerialImpl(const char *port, uint32_t baudrate, ByteSize bytesize, P
 	} else {
 		_port[0] = 0;
 	}
-
 }
 
 SerialImpl::~SerialImpl()
@@ -72,18 +71,14 @@ SerialImpl::~SerialImpl()
 
 bool SerialImpl::validateBaudrate(uint32_t baudrate)
 {
-	if ((baudrate == 9600) ||
+	return ((baudrate == 9600) ||
-	    (baudrate == 19200) ||
+		(baudrate == 19200) ||
-	    (baudrate == 38400) ||
+		(baudrate == 38400) ||
-	    (baudrate == 57600) ||
+		(baudrate == 57600) ||
-	    (baudrate == 115200) ||
+		(baudrate == 115200) ||
-	    (baudrate == 230400) ||
+		(baudrate == 230400) ||
-	    (baudrate == 460800) ||
+		(baudrate == 460800) ||
-	    (baudrate == 921600)) {
+		(baudrate == 921600));
-		return true;
-	}
-
-	return false;
 }
 
 bool SerialImpl::configure()
@@ -131,7 +126,10 @@ bool SerialImpl::configure()
 	int termios_state;
 
 	/* fill the struct for the new configuration */
-	tcgetattr(_serial_fd, &uart_config);
+	if ((termios_state = tcgetattr(_serial_fd, &uart_config)) < 0) {
+		PX4_ERR("ERR: %d (tcgetattr)", termios_state);
+		return false;
+	}
 
 	/* properly configure the terminal (see also https://en.wikibooks.org/wiki/Serial_Programming/termios ) */
 
@@ -202,15 +200,14 @@ bool SerialImpl::open()
 		return false;
 	}
 
-	// Configure the serial port if a baudrate has been configured
+	_serial_fd = serial_fd;
-	if (_baudrate) {
+
-		if (! configure()) {
+	// Configure the serial port
-			PX4_ERR("failed to configure %s err: %d", _port, errno);
+	if (! configure()) {
-			return false;
+		PX4_ERR("failed to configure %s err: %d", _port, errno);
-		}
+		return false;
 	}
 
-	_serial_fd = serial_fd;
 	_open = true;
 
 	return _open;
@@ -245,10 +242,6 @@ ssize_t SerialImpl::read(uint8_t *buffer, size_t buffer_size)
 
 	if (ret < 0) {
 		PX4_DEBUG("%s read error %d", _port, ret);
-
-	} else {
-		// Track total bytes read
-		_bytes_read += ret;
 	}
 
 	return ret;
@@ -311,10 +304,6 @@ ssize_t SerialImpl::write(const void *buffer, size_t buffer_size)
 
 	if (written < 0) {
 		PX4_ERR("%s write error %d", _port, written);
-
-	} else {
-		// Track total bytes written
-		_bytes_written += written;
 	}
 
 	return written;

platforms/nuttx/src/px4/common/include/SerialImpl.hpp

@@ -85,9 +85,6 @@ private:
 
 	int _serial_fd{-1};
 
-	size_t _bytes_read{0};
-	size_t _bytes_written{0};
-
 	bool _open{false};
 
 	char _port[32] {};

platforms/posix/include/SerialImpl.hpp

@@ -1,4 +1,35 @@
-
+/****************************************************************************
+ *
+ *   Copyright (C) 2023 PX4 Development Team. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name PX4 nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
 
 #pragma once
 
@@ -34,7 +65,6 @@ public:
 	ssize_t write(const void *buffer, size_t buffer_size);
 
 	const char *getPort() const;
-	bool setPort(const char *port);
 
 	uint32_t getBaudrate() const;
 	bool setBaudrate(uint32_t baudrate);
@@ -55,9 +85,6 @@ private:
 
 	int _serial_fd{-1};
 
-	size_t _bytes_read{0};
-	size_t _bytes_written{0};
-
 	bool _open{false};
 
 	char _port[32] {};
@@ -71,12 +98,6 @@ private:
 
 	bool validateBaudrate(uint32_t baudrate);
 	bool configure();
-
-	// Mutex used to lock the read functions
-	//pthread_mutex_t read_mutex;
-
-	// Mutex used to lock the write functions
-	//pthread_mutex_t write_mutex;
 };
 
 } // namespace device

platforms/posix/src/px4/common/SerialImpl.cpp

@@ -1,7 +1,39 @@
+/****************************************************************************
+ *
+ *   Copyright (C) 2023 PX4 Development Team. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name PX4 nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
 
 #include <SerialImpl.hpp>
 #include <string.h> // strncpy
-#include <termios.h> // strncpy
+#include <termios.h>
 #include <px4_log.h>
 #include <fcntl.h>
 #include <errno.h>
@@ -26,7 +58,6 @@ SerialImpl::SerialImpl(const char *port, uint32_t baudrate, ByteSize bytesize, P
 	} else {
 		_port[0] = 0;
 	}
-
 }
 
 SerialImpl::~SerialImpl()
@@ -38,18 +69,14 @@ SerialImpl::~SerialImpl()
 
 bool SerialImpl::validateBaudrate(uint32_t baudrate)
 {
-	if ((baudrate == 9600) ||
+	return ((baudrate == 9600) ||
-	    (baudrate == 19200) ||
+		(baudrate == 19200) ||
-	    (baudrate == 38400) ||
+		(baudrate == 38400) ||
-	    (baudrate == 57600) ||
+		(baudrate == 57600) ||
-	    (baudrate == 115200) ||
+		(baudrate == 115200) ||
-	    (baudrate == 230400) ||
+		(baudrate == 230400) ||
-	    (baudrate == 460800) ||
+		(baudrate == 460800) ||
-	    (baudrate == 921600)) {
+		(baudrate == 921600));
-		return true;
-	}
-
-	return false;
 }
 
 bool SerialImpl::configure()
@@ -97,7 +124,10 @@ bool SerialImpl::configure()
 	int termios_state;
 
 	/* fill the struct for the new configuration */
-	tcgetattr(_serial_fd, &uart_config);
+	if ((termios_state = tcgetattr(_serial_fd, &uart_config)) < 0) {
+		PX4_ERR("ERR: %d (tcgetattr)", termios_state);
+		return false;
+	}
 
 	/* properly configure the terminal (see also https://en.wikibooks.org/wiki/Serial_Programming/termios ) */
 
@@ -168,15 +198,14 @@ bool SerialImpl::open()
 		return false;
 	}
 
-	// Configure the serial port if a baudrate has been configured
+	_serial_fd = serial_fd;
-	if (_baudrate) {
+
-		if (! configure()) {
+	// Configure the serial port
-			PX4_ERR("failed to configure %s err: %d", _port, errno);
+	if (! configure()) {
-			return false;
+		PX4_ERR("failed to configure %s err: %d", _port, errno);
-		}
+		return false;
 	}
 
-	_serial_fd = serial_fd;
 	_open = true;
 
 	return _open;
@@ -212,9 +241,6 @@ ssize_t SerialImpl::read(uint8_t *buffer, size_t buffer_size)
 	if (ret < 0) {
 		PX4_DEBUG("%s read error %d", _port, ret);
 
-	} else {
-		// Track total bytes read
-		_bytes_read += ret;
 	}
 
 	return ret;
@@ -278,9 +304,6 @@ ssize_t SerialImpl::write(const void *buffer, size_t buffer_size)
 	if (written < 0) {
 		PX4_ERR("%s write error %d", _port, written);
 
-	} else {
-		// Track total bytes written
-		_bytes_written += written;
 	}
 
 	return written;
@@ -291,24 +314,6 @@ const char *SerialImpl::getPort() const
 	return _port;
 }
 
-bool SerialImpl::setPort(const char *port)
-{
-	if (strcmp(port, _port) == 0) {
-		return true;
-	}
-
-	strncpy(_port, port, sizeof(_port) - 1);
-	_port[sizeof(_port) - 1] = '\0';
-
-	// If old port is already opened then close it and reopen it on new port
-	if (_open) {
-		close();
-		return open();
-	}
-
-	return true;
-}
-
 uint32_t SerialImpl::getBaudrate() const
 {
 	return _baudrate;

platforms/qurt/include/SerialImpl.hpp

@@ -1,4 +1,35 @@
-
+/****************************************************************************
+ *
+ *   Copyright (C) 2023 PX4 Development Team. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name PX4 nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
 
 #pragma once
 
@@ -54,9 +85,6 @@ private:
 
 	int _serial_fd{-1};
 
-	size_t _bytes_read{0};
-	size_t _bytes_written{0};
-
 	bool _open{false};
 
 	char _port[32] {};

platforms/qurt/src/px4/SerialImpl.cpp

@@ -1,3 +1,35 @@
+/****************************************************************************
+ *
+ *   Copyright (C) 2023 PX4 Development Team. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ * 3. Neither the name PX4 nor the names of its contributors may be
+ *    used to endorse or promote products derived from this software
+ *    without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ * ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ ****************************************************************************/
 
 #include <SerialImpl.hpp>
 #include <string.h> // strncpy
@@ -132,8 +164,6 @@ ssize_t SerialImpl::read(uint8_t *buffer, size_t buffer_size)
 	if (ret_read < 0) {
 		PX4_DEBUG("%s read error %d", _port, ret_read);
 
-	} else {
-		_bytes_read += ret_read;
 	}
 
 	return ret_read;
@@ -216,8 +246,6 @@ ssize_t SerialImpl::write(const void *buffer, size_t buffer_size)
 	if (ret_write < 0) {
 		PX4_ERR("%s write error %d", _port, ret_write);
 
-	} else {
-		_bytes_written += ret_write;
 	}
 
 	return ret_write;
@@ -228,15 +256,6 @@ const char *SerialImpl::getPort() const
 	return _port;
 }
 
-bool SerialImpl::setPort(const char *port)
-{
-	if (strcmp(port, _port) == 0) {
-		return true;
-	}
-
-	return false;
-}
-
 uint32_t SerialImpl::getBaudrate() const
 {
 	return _baudrate;

src/drivers/gps/gps.cpp

@@ -795,7 +795,9 @@ GPS::run()
 				px4_sleep(1);
 				continue;
 			}
+		}
 
+		if ((_interface == GPSHelper::Interface::UART) && (! _uart->isOpen())) {
 			// Configure the desired baudrate if one was specified by the user.
 			// Otherwise the default baudrate will be used.
 			if (_configured_baudrate) {