chore(hw): apply suggestions of review/sonar to VSCANPlugin

Open-Agriculture · Jan 11, 2025 · 5d8e219 · 5d8e219
1 parent 48fd9a8
commit 5d8e219
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 14 deletions.
diff --git a/hardware_integration/CMakeLists.txt b/hardware_integration/CMakeLists.txt
@@ -358,12 +358,12 @@ if("VSCAN" IN_LIST CAN_DRIVER)
     set(CMAKE_SYSTEM_PROCESSOR ${MSVC_CXX_ARCHITECTURE_ID})
   endif()
   if(DEFINED ENV{VSCAN_API_DIR})
-    message(
-      AUTHOR_WARNING
-        "The VSCAN driver requires vs_can_api.dll to be in the same directory as the executable. You can find this file in the VSCAN API directory."
-    )
     target_include_directories(HardwareIntegration PUBLIC $ENV{VSCAN_API_DIR})
     if(WIN32)
+      message(
+        AUTHOR_WARNING
+          "The VSCAN driver requires vs_can_api.dll to be in the same directory as the executable. You can find this file in the VSCAN API directory."
+      )
       if(CMAKE_SYSTEM_PROCESSOR STREQUAL "AMD64" OR CMAKE_SYSTEM_PROCESSOR
                                                     STREQUAL "x64")
         message(STATUS "Detected Windows x64, linking to VSCAN API x64 library")
@@ -390,8 +390,8 @@ if("VSCAN" IN_LIST CAN_DRIVER)
                                                          STREQUAL "X86")
         message(STATUS "Detected Linux x86, linking to VSCAN API x86 library")
         target_link_libraries(
-          HardwareIntegration PRIVATE $ENV{VSCAN_API_DIR}/Linux
-                                      x86-64/libvs_can_api.a)
+          HardwareIntegration
+          PRIVATE "$ENV{VSCAN_API_DIR}/Linux x86-64/libvs_can_api.a")
       else()
         message(
           FATAL_ERROR

diff --git a/hardware_integration/include/isobus/hardware_integration/vscan_plugin.hpp b/hardware_integration/include/isobus/hardware_integration/vscan_plugin.hpp
@@ -27,7 +27,7 @@ namespace isobus
 		/// @brief Constructor for the VSCOM VSCAN CAN driver
 		/// @param[in] channel The COM port or IP address of the VSCAN device to use.
 		/// @param[in] baudrate The baudrate to use for the CAN connection.
-		VSCANPlugin(std::string channel, void *baudrate = VSCAN_SPEED_250K);
+		VSCANPlugin(const std::string &channel, void *baudrate = VSCAN_SPEED_250K);
 
 		/// @brief The destructor for VSCANPlugin
 		virtual ~VSCANPlugin() = default;
@@ -56,13 +56,13 @@ namespace isobus
 		/// @param[in] channel The COM port or IP address of the VSCAN device to use.
 		/// @param[in] baudrate The baudrate to use for the CAN connection.
 		/// @returns True if the configuration was changed, otherwise false (if the device is open false will be returned)
-		bool reconfigure(std::string channel, void *baudrate = VSCAN_SPEED_250K);
+		bool reconfigure(const std::string &channel, void *baudrate = VSCAN_SPEED_250K);
 
 	private:
 		/// @brief Parses the error from the status code
 		/// @param[in] status The status code to parse
 		/// @returns The error message
-		std::string parse_error_from_status(VSCAN_STATUS status);
+		static std::string parse_error_from_status(VSCAN_STATUS status);
 
 		std::string channel; ///< The COM port or IP address of the VSCAN device to use.
 		void *baudrate; ///< The baudrate to use for the CAN connection.

diff --git a/hardware_integration/src/vscan_plugin.cpp b/hardware_integration/src/vscan_plugin.cpp
@@ -13,11 +13,12 @@
 #include "isobus/isobus/can_stack_logger.hpp"
 
 #include <chrono>
+#include <cstring>
 #include <thread>
 
 namespace isobus
 {
-	VSCANPlugin::VSCANPlugin(std::string channel, void *baudrate) :
+	VSCANPlugin::VSCANPlugin(const std::string &channel, void *baudrate) :
 	  channel(channel),
 	  baudrate(baudrate)
 	{
@@ -138,7 +139,7 @@ namespace isobus
 		return retVal;
 	}
 
-	bool VSCANPlugin::reconfigure(std::string channel, void *baudrate)
+	bool VSCANPlugin::reconfigure(const std::string &channel, void *baudrate)
 	{
 		bool retVal = false;
 
@@ -153,9 +154,16 @@ namespace isobus
 
 	std::string VSCANPlugin::parse_error_from_status(VSCAN_STATUS status)
 	{
-		char errorBuffer[256] = { 0 };
-		VSCAN_GetErrorString(status, errorBuffer, sizeof(errorBuffer));
-		return std::string(errorBuffer);
+		// Arbitrary buffer size, should be enough for most error messages
+		size_t bufferSize = 256;
+		std::vector<char> errorBuffer(bufferSize, 0);
+
+		VSCAN_GetErrorString(status, errorBuffer.data(), bufferSize);
+
+		// Ensure the string is null-terminated, just in case
+		errorBuffer[bufferSize - 1] = '\0';
+
+		return std::string(errorBuffer.data());
 	}
 }
 // namespace isobus