From f6318c492627a669fb334022158550c8b2ef795e Mon Sep 17 00:00:00 2001 From: Eric Katzfey Date: Thu, 20 Jul 2023 15:43:07 -0700 Subject: [PATCH] More cleanup and fixes from testing failures --- platforms/nuttx/src/px4/common/SerialImpl.cpp | 47 ++++----- .../src/px4/common/include/SerialImpl.hpp | 3 - platforms/posix/include/SerialImpl.hpp | 43 +++++--- platforms/posix/src/px4/common/SerialImpl.cpp | 97 ++++++++++--------- platforms/qurt/include/SerialImpl.hpp | 36 ++++++- platforms/qurt/src/px4/SerialImpl.cpp | 45 ++++++--- src/drivers/gps/gps.cpp | 2 + 7 files changed, 167 insertions(+), 106 deletions(-) diff --git a/platforms/nuttx/src/px4/common/SerialImpl.cpp b/platforms/nuttx/src/px4/common/SerialImpl.cpp index cfb207b174..11b3d3ce4d 100644 --- a/platforms/nuttx/src/px4/common/SerialImpl.cpp +++ b/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) || - (baudrate == 19200) || - (baudrate == 38400) || - (baudrate == 57600) || - (baudrate == 115200) || - (baudrate == 230400) || - (baudrate == 460800) || - (baudrate == 921600)) { - return true; - } - - return false; + return ((baudrate == 9600) || + (baudrate == 19200) || + (baudrate == 38400) || + (baudrate == 57600) || + (baudrate == 115200) || + (baudrate == 230400) || + (baudrate == 460800) || + (baudrate == 921600)); } 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 - if (_baudrate) { - if (! configure()) { - PX4_ERR("failed to configure %s err: %d", _port, errno); - return false; - } + _serial_fd = serial_fd; + + // Configure the serial port + if (! configure()) { + 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; diff --git a/platforms/nuttx/src/px4/common/include/SerialImpl.hpp b/platforms/nuttx/src/px4/common/include/SerialImpl.hpp index ec5e774c0a..58d41bf759 100644 --- a/platforms/nuttx/src/px4/common/include/SerialImpl.hpp +++ b/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] {}; diff --git a/platforms/posix/include/SerialImpl.hpp b/platforms/posix/include/SerialImpl.hpp index 93765efac7..efc95d7d51 100644 --- a/platforms/posix/include/SerialImpl.hpp +++ b/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 diff --git a/platforms/posix/src/px4/common/SerialImpl.cpp b/platforms/posix/src/px4/common/SerialImpl.cpp index ef76c2ebf7..c216e32656 100644 --- a/platforms/posix/src/px4/common/SerialImpl.cpp +++ b/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 #include // strncpy -#include // strncpy +#include #include #include #include @@ -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) || - (baudrate == 19200) || - (baudrate == 38400) || - (baudrate == 57600) || - (baudrate == 115200) || - (baudrate == 230400) || - (baudrate == 460800) || - (baudrate == 921600)) { - return true; - } - - return false; + return ((baudrate == 9600) || + (baudrate == 19200) || + (baudrate == 38400) || + (baudrate == 57600) || + (baudrate == 115200) || + (baudrate == 230400) || + (baudrate == 460800) || + (baudrate == 921600)); } 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 - if (_baudrate) { - if (! configure()) { - PX4_ERR("failed to configure %s err: %d", _port, errno); - return false; - } + _serial_fd = serial_fd; + + // Configure the serial port + if (! configure()) { + 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; diff --git a/platforms/qurt/include/SerialImpl.hpp b/platforms/qurt/include/SerialImpl.hpp index dadda42e77..1b98d3bb40 100644 --- a/platforms/qurt/include/SerialImpl.hpp +++ b/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] {}; diff --git a/platforms/qurt/src/px4/SerialImpl.cpp b/platforms/qurt/src/px4/SerialImpl.cpp index f4606a8046..ec0fb73fce 100644 --- a/platforms/qurt/src/px4/SerialImpl.cpp +++ b/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 #include // 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; diff --git a/src/drivers/gps/gps.cpp b/src/drivers/gps/gps.cpp index 645b1d7737..ae92000bfc 100644 --- a/src/drivers/gps/gps.cpp +++ b/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) {