partial lint fix

HypersonicED · Feb 13, 2023 · 30816f8 · 30816f8
1 parent 038eac6
commit 30816f8
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 32 deletions.
diff --git a/include/aniray/IOInterfaceModbus.hpp b/include/aniray/IOInterfaceModbus.hpp
@@ -24,23 +24,23 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-#ifndef ANIRAY_INPUTINTERFACEMODBUS_HPP
-#define ANIRAY_INPUTINTERFACEMODBUS_HPP
+#ifndef ANIRAY_IOINTERFACEMODBUS_HPP
+#define ANIRAY_IOINTERFACEMODBUS_HPP
 
 #include <chrono>
+#include <cstddef>
 #include <cstdint>
 #include <shared_mutex>
 #include <string>
+#include <unordered_map>
 #include <vector>
 
 #include <modbus/modbus.h>
 
 #include <aniray/IOInterface.hpp>
 #include <aniray/PeriodicThread.hpp>
 
-namespace aniray {
-namespace IOInterface {
-namespace Modbus {
+namespace aniray::IOInterface::Modbus {
 
 enum class ConfigFunctionsAddressLayout {
     ADDRESS, // each address is an input
@@ -72,6 +72,9 @@ const std::uint8_t FUNCTION_CODE_FORCE_MULTIPLE_COILS = 15;
 const std::uint8_t FUNCTION_CODE_PRESET_SINGLE_REGISTERS = 6;
 const std::uint8_t FUNCTION_CODE_PRESET_MULTIPLE_REGISTERS = 16;
 
+const std::size_t MODBUS_BITS_PER_BYTE = 8;
+const std::size_t MODBUS_BITS_PER_REGISTER = 16;
+
 const std::uint8_t CLEAR_BIT_VALUE_LOW = 0;
 const std::uint16_t CLEAR_REGISTER_VALUE_LOW = 0;
 const std::uint8_t CLEAR_BIT_VALUE_HIGH = 1;
@@ -82,13 +85,13 @@ class IOInterfaceModbus : public aniray::IOInterface::IOInterfaceGeneric {
         IOInterfaceModbus(std::string tcpAddress, std::uint16_t tcpPort);
         ~IOInterfaceModbus();
         void refreshInputs() override;
-        void setupInputDiscrete(std::string name,
+        void setupInputDiscrete(const std::string &name,
                                 std::uint8_t slaveID,
                                 std::uint8_t functionCode,
                                 ConfigFunctionsAddressLayout addressLayout,
                                 std::uint16_t startAddress,
                                 std::uint16_t numAddressedItems);
-        void setupInputDiscrete(std::string name,
+        void setupInputDiscrete(const std::string &name,
                                 std::uint8_t slaveID,
                                 std::uint8_t functionCode,
                                 ConfigFunctionsAddressLayout addressLayout,
@@ -104,8 +107,8 @@ class IOInterfaceModbus : public aniray::IOInterface::IOInterfaceGeneric {
         modbus_t *mCTX;
 
         void setupConnectionTCP(std::string tcpAddress, std::uint16_t tcpPort);
-        void updateInputDiscrete(ConfigInputDiscrete configInputDiscrete);
-        void setupInputDiscreteNoLock(std::string name,
+        void updateInputDiscrete(const ConfigInputDiscrete &configInputDiscrete);
+        void setupInputDiscreteNoLock(const std::string &name,
                                       std::uint8_t slaveID,
                                       std::uint8_t functionCode,
                                       ConfigFunctionsAddressLayout addressLayout,
@@ -122,8 +125,6 @@ class IOInterfaceModbusThread : public IOInterfaceModbus, public aniray::Periodi
         void periodicAction() override;
 };
 
-} // namespace IOInterfaceModbus
-} // namespace IOInterface
-} // namespace aniray
+} // namespace aniray::IOInterface::Modbus
 
-#endif // ANIRAY_INPUTINTERFACEMODBUS_HPP
+#endif // ANIRAY_IOINTERFACEMODBUS_HPP
diff --git a/src/IOInterface.cpp b/src/IOInterface.cpp
@@ -25,6 +25,8 @@
  */
 
 #include <memory>
+#include <mutex>
+#include <shared_mutex>
 #include <stdexcept>
 #include <string>
 #include <unordered_map>

diff --git a/src/IOInterfaceModbus.cpp b/src/IOInterfaceModbus.cpp
@@ -24,25 +24,32 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#include <cerrno>
+#include <chrono>
+#include <cstddef>
 #include <cstdint>
 #include <memory>
+#include <mutex>
+#include <shared_mutex>
 #include <stdexcept>
 #include <string>
+#include <unordered_map>
+#include <utility>
 #include <vector>
 
 #include <boost/log/core/record.hpp>
 #include <boost/log/sources/record_ostream.hpp>
 #include <boost/log/trivial.hpp>
 #include <modbus/modbus.h>
 
+#include <aniray/IOInterface.hpp>
 #include <aniray/IOInterfaceModbus.hpp>
+#include <aniray/PeriodicThread.hpp>
 
-namespace aniray {
-namespace IOInterface {
-namespace Modbus {
+namespace aniray::IOInterface::Modbus {
 
 IOInterfaceModbus::IOInterfaceModbus(std::string tcpAddress, std::uint16_t tcpPort) {
-        setupConnectionTCP(tcpAddress, tcpPort);
+        setupConnectionTCP(std::move(tcpAddress), tcpPort);
 
         // Refresh inputs to initially populate values and confirm all addresses are reachable
         // refreshInputs();
@@ -55,9 +62,10 @@ IOInterfaceModbus::~IOInterfaceModbus() {
 
 void IOInterfaceModbus::setupConnectionTCP(std::string tcpAddress, std::uint16_t tcpPort) {
     mCTX = modbus_new_tcp_pi(tcpAddress.c_str(), std::to_string(tcpPort).c_str());
-    if (mCTX == NULL) {
+    if (mCTX == nullptr) {
         throw std::runtime_error("IOInterfaceModbus: Unable to allocate libmodbus context");
-    } else if (modbus_connect(mCTX) == -1) {
+    }
+    if (modbus_connect(mCTX) == -1) {
         modbus_free(mCTX);
         throw std::runtime_error("IOInterfaceModbus: Connection failed: " + std::string(modbus_strerror(errno)));
     }
@@ -66,7 +74,7 @@ void IOInterfaceModbus::setupConnectionTCP(std::string tcpAddress, std::uint16_t
 }
 
 // WARNING: Not thread safe! Use setupInputDiscrete publicly!
-void IOInterfaceModbus::setupInputDiscreteNoLock(std::string name,
+void IOInterfaceModbus::setupInputDiscreteNoLock(const std::string &name,
                                                  std::uint8_t slaveID,
                                                  std::uint8_t functionCode,
                                                  ConfigFunctionsAddressLayout addressLayout,
@@ -84,7 +92,7 @@ void IOInterfaceModbus::setupInputDiscreteNoLock(std::string name,
         .enableClear = false
     };
 }
-void IOInterfaceModbus::setupInputDiscrete(std::string name,
+void IOInterfaceModbus::setupInputDiscrete(const std::string &name,
                                            std::uint8_t slaveID,
                                            std::uint8_t functionCode,
                                            ConfigFunctionsAddressLayout addressLayout,
@@ -94,7 +102,7 @@ void IOInterfaceModbus::setupInputDiscrete(std::string name,
     // Use below instead of initializer so that mutex lock covers
     setupInputDiscreteNoLock(name, slaveID, functionCode, addressLayout, startAddress, numAddressedItems);
 }
-void IOInterfaceModbus::setupInputDiscrete(std::string name,
+void IOInterfaceModbus::setupInputDiscrete(const std::string &name,
                                            std::uint8_t slaveID,
                                            std::uint8_t functionCode,
                                            ConfigFunctionsAddressLayout addressLayout,
@@ -122,7 +130,7 @@ void IOInterfaceModbus::refreshInputs() {
     // inputsDiscreteLock.unlock(); // will need this when other types of input are added
 }
 
-void IOInterfaceModbus::updateInputDiscrete(ConfigInputDiscrete configInputDiscrete) {
+void IOInterfaceModbus::updateInputDiscrete(const ConfigInputDiscrete &configInputDiscrete) {
     if (modbus_set_slave(mCTX, configInputDiscrete.slaveID) == -1) {
         throw std::runtime_error("IOInterfaceModbus: Invalid slave ID: " + std::to_string(configInputDiscrete.slaveID));
     }
@@ -189,7 +197,7 @@ void IOInterfaceModbus::updateInputDiscrete(ConfigInputDiscrete configInputDiscr
                 throw std::runtime_error("IOInterfaceModbus: Incorrect discrete input clear function code!");
                 break;
         }
-        if (res_read == -1) {
+        if (res_clear == -1) {
             throw std::runtime_error("IOInterfaceModbus: Error clearing discrete values: " + std::string(modbus_strerror(errno)));
         }
     }
@@ -220,17 +228,17 @@ void IOInterfaceModbus::updateInputDiscrete(ConfigInputDiscrete configInputDiscr
                 case FUNCTION_CODE_READ_BITS:
                 case FUNCTION_CODE_READ_INPUT_BITS:
                     for (std::size_t i = 0; i < configInputDiscrete.numAddressedItems; i++) {
-                        for (std::size_t bit = 0; bit < 8; bit++) {
-                            auto val = (dest8[i] >> bit) & 1;
+                        for (std::size_t bit = 0; bit < MODBUS_BITS_PER_BYTE; bit++) {
+                            auto val = static_cast<std::size_t>(dest8[i] >> bit) & 1U;
                             out.push_back(static_cast<bool>(val));
                         }
                     }
                     break;
                 case FUNCTION_CODE_READ_REGISTERS:
                 case FUNCTION_CODE_READ_INPUT_REGISTERS:
                     for (std::size_t i = 0; i < configInputDiscrete.numAddressedItems; i++) {
-                        for (std::size_t bit = 0; bit < 16; bit++) {
-                            auto val = (dest16[i] >> bit) & 1;
+                        for (std::size_t bit = 0; bit < MODBUS_BITS_PER_REGISTER; bit++) {
+                            auto val = static_cast<std::size_t>(dest16[i] >> bit) & 1U;
                             out.push_back(static_cast<bool>(val));
                         }
                     }
@@ -270,13 +278,11 @@ void IOInterfaceModbus::updateInputDiscrete(ConfigInputDiscrete configInputDiscr
 }
 
 IOInterfaceModbusThread::IOInterfaceModbusThread(std::string tcpAddress, std::uint16_t tcpPort, std::chrono::milliseconds updateRateMs)
-    : IOInterfaceModbus(tcpAddress, tcpPort)
+    : IOInterfaceModbus(std::move(tcpAddress), tcpPort)
     , PeriodicThread(updateRateMs) {}
 
 void IOInterfaceModbusThread::periodicAction() {
     refreshInputs();
 }
 
-} // namespace Modbus
-} // namespace IOInterface
-} // namespace aniray
+} // namespace aniray::IOInterface::Modbus