[RSDK-13421] Improve C++ code quality: constexpr, consistent logging, namespace cleanup (#39)

* Clean up namespace polution

* Remove all couts

* Use constexpr instead of defines

* Use string for device type env

Author: seanavery

constraints.h

@@ -1,40 +1,42 @@
 #pragma once
 
+#include <string>
+
 // Module
-#define DEFAULT_SOCKET_PATH "/tmp/viam.csi.sock"
-#define RESOURCE_TYPE "CSICamera"
-#define API_NAMESPACE "viam"
-#define API_TYPE "camera"
-#define DEFAULT_API_SUBTYPE "csi"
+constexpr const char* DEFAULT_SOCKET_PATH = "/tmp/viam.csi.sock";
+constexpr const char* RESOURCE_TYPE = "CSICamera";
+constexpr const char* API_NAMESPACE = "viam";
+constexpr const char* API_TYPE = "camera";
+constexpr const char* DEFAULT_API_SUBTYPE = "csi";
 
 // GST
-#define GST_GET_STATE_TIMEOUT 1
-#define GST_CHANGE_STATE_TIMEOUT 5
+constexpr int GST_GET_STATE_TIMEOUT = 1;
+constexpr int GST_CHANGE_STATE_TIMEOUT = 5;
 
 // Pipeline
-#define DEFAULT_INPUT_SOURCE "libcamerasrc"
-#define DEFAULT_INPUT_SENSOR "0"
-#define DEFAULT_INPUT_FORMAT "video/x-raw"
-#define DEFAULT_INPUT_WIDTH 1920
-#define DEFAULT_INPUT_HEIGHT 1080
-#define DEFAULT_INPUT_FRAMERATE 30
-#define DEFAULT_VIDEO_CONVERTER "videoconvert"
-#define DEFAULT_OUTPUT_ENCODER "nvjpegenc"
-#define DEFAULT_OUTPUT_MIMETYPE "image/jpeg"
+constexpr const char* DEFAULT_INPUT_SOURCE = "libcamerasrc";
+constexpr const char* DEFAULT_INPUT_SENSOR = "0";
+constexpr const char* DEFAULT_INPUT_FORMAT = "video/x-raw";
+constexpr int DEFAULT_INPUT_WIDTH = 1920;
+constexpr int DEFAULT_INPUT_HEIGHT = 1080;
+constexpr int DEFAULT_INPUT_FRAMERATE = 30;
+constexpr const char* DEFAULT_VIDEO_CONVERTER = "videoconvert";
+constexpr const char* DEFAULT_OUTPUT_ENCODER = "nvjpegenc";
+constexpr const char* DEFAULT_OUTPUT_MIMETYPE = "image/jpeg";
 
 // Jetson
-#define JETSON_API_SUBTYPE "csi"
-#define JETSON_INPUT_SOURCE "nvarguscamerasrc"
-#define JETSON_INPUT_FORMAT "video/x-raw(memory:NVMM)"
-#define JETSON_VIDEO_CONVERTER "nvvidconv"
-#define JETSON_OUTPUT_ENCODER "nvjpegenc"
+constexpr const char* JETSON_API_SUBTYPE = "csi";
+constexpr const char* JETSON_INPUT_SOURCE = "nvarguscamerasrc";
+constexpr const char* JETSON_INPUT_FORMAT = "video/x-raw(memory:NVMM)";
+constexpr const char* JETSON_VIDEO_CONVERTER = "nvvidconv";
+constexpr const char* JETSON_OUTPUT_ENCODER = "nvjpegenc";
 
 // Pi
-#define PI_API_SUBTYPE "csi-pi"
-#define PI_INPUT_SOURCE "libcamerasrc"
-#define PI_INPUT_FORMAT "video/x-raw,format=NV12"
-#define PI_VIDEO_CONVERTER "videoconvert"
-#define PI_OUTPUT_ENCODER "jpegenc"
+constexpr const char* PI_API_SUBTYPE = "csi-pi";
+constexpr const char* PI_INPUT_SOURCE = "libcamerasrc";
+constexpr const char* PI_INPUT_FORMAT = "video/x-raw,format=NV12";
+constexpr const char* PI_VIDEO_CONVERTER = "videoconvert";
+constexpr const char* PI_OUTPUT_ENCODER = "jpegenc";
 
 // Integration Tests
 inline const std::string TEST_GST_PIPELINE =

csi_camera.cpp

@@ -1,7 +1,6 @@
 #include <chrono>
 #include <cstring>
 #include <fstream>
-#include <iostream>
 #include <sstream>
 #include <thread>
 
@@ -116,8 +115,7 @@ void CSICamera::init_csi(const std::string pipeline_args) {
     GError* error = nullptr;
     pipeline = gst_parse_launch(pipeline_args.c_str(), &error);
     if (!pipeline) {
-        std::cerr << "Failed to create the pipeline" << std::endl;
-        g_print("Error: %s\n", error->message);
+        VIAM_SDK_LOG(error) << "Failed to create the pipeline: " << error->message;
         g_error_free(error);
         throw Exception("Failed to create the pipeline");
     }
@@ -171,14 +169,14 @@ void CSICamera::wait_pipeline() {
     }
 
     if (ret == GST_STATE_CHANGE_SUCCESS) {
-        std::cout << "GST pipeline state change success" << std::endl;
+        VIAM_SDK_LOG(debug) << "GST pipeline state change success";
     } else if (ret == GST_STATE_CHANGE_FAILURE) {
-        std::cerr << "GST pipeline failed to change state" << std::endl;
+        VIAM_SDK_LOG(error) << "GST pipeline failed to change state";
         throw Exception("GST pipeline failed to change state");
     } else if (ret == GST_STATE_CHANGE_NO_PREROLL) {
-        std::cout << "GST pipeline changed but not enough data for preroll" << std::endl;
+        VIAM_SDK_LOG(warn) << "GST pipeline changed but not enough data for preroll";
     } else {
-        std::cerr << "GST pipeline failed to change state" << std::endl;
+        VIAM_SDK_LOG(error) << "GST pipeline failed to change state";
         throw Exception("GST pipeline failed to change state");
     }
 }

csi_camera.h

@@ -17,9 +17,7 @@
 
 #include "utils.h"
 
-using namespace viam::sdk;
-
-class CSICamera : public Camera, public Reconfigurable {
+class CSICamera : public viam::sdk::Camera, public viam::sdk::Reconfigurable {
    private:
     // Device
     device_type device;
@@ -37,22 +35,22 @@ class CSICamera : public Camera, public Reconfigurable {
 
    public:
     // Module
-    explicit CSICamera(const std::string name, const ProtoStruct& attrs);
+    explicit CSICamera(const std::string name, const viam::sdk::ProtoStruct& attrs);
     ~CSICamera();
-    void init(const ProtoStruct& attrs);
+    void init(const viam::sdk::ProtoStruct& attrs);
     void init_csi(const std::string pipeline_args);
-    void validate_attrs(const ProtoStruct& attrs);
+    void validate_attrs(const viam::sdk::ProtoStruct& attrs);
     template <typename T>
-    void set_attr(const ProtoStruct& attrs, const std::string& name, T CSICamera::* member, T de);
+    void set_attr(const viam::sdk::ProtoStruct& attrs, const std::string& name, T CSICamera::* member, T de);
 
     // Camera
     // overrides camera component interface
-    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override;
-    raw_image get_image(const std::string mime_type, const ProtoStruct& extra) override;
-    image_collection get_images(std::vector<std::string> /* filter_source_names */, const ProtoStruct& /* extra */) override;
-    ProtoStruct do_command(const ProtoStruct& command) override;
-    point_cloud get_point_cloud(const std::string mime_type, const ProtoStruct& extra) override;
-    std::vector<GeometryConfig> get_geometries(const ProtoStruct& extra) override;
+    void reconfigure(const viam::sdk::Dependencies& deps, const viam::sdk::ResourceConfig& cfg) override;
+    raw_image get_image(const std::string mime_type, const viam::sdk::ProtoStruct& extra) override;
+    image_collection get_images(std::vector<std::string> /* filter_source_names */, const viam::sdk::ProtoStruct& /* extra */) override;
+    viam::sdk::ProtoStruct do_command(const viam::sdk::ProtoStruct& command) override;
+    point_cloud get_point_cloud(const std::string mime_type, const viam::sdk::ProtoStruct& extra) override;
+    std::vector<viam::sdk::GeometryConfig> get_geometries(const viam::sdk::ProtoStruct& extra) override;
     properties get_properties() override;
 
     // GST

utils.cpp

@@ -5,16 +5,16 @@
 
 device_type get_device_type() {
     // Check for environment variable override
-    const char* env_device = std::getenv("VIAM_CSI_DEVICE");
-    if (env_device != nullptr) {
-        std::string device_str(env_device);
-        std::transform(device_str.begin(), device_str.end(), device_str.begin(), ::tolower);
+    const char* env_raw = std::getenv("VIAM_CSI_DEVICE");
+    std::string env_device = env_raw ? env_raw : "";
+    if (!env_device.empty()) {
+        std::transform(env_device.begin(), env_device.end(), env_device.begin(), ::tolower);
 
-        if (device_str == "jetson") {
+        if (env_device == "jetson") {
             return device_type(device_type::jetson, "Jetson");
-        } else if (device_str == "pi") {
+        } else if (env_device == "pi") {
             return device_type(device_type::pi, "Raspberry Pi");
-        } else if (device_str == "test") {
+        } else if (env_device == "test") {
             return device_type(device_type::test, "Test");
         }
     }
@@ -37,7 +37,7 @@ device_type get_device_type() {
         device_name.close();
     }
 
-    return device_type(device_type::unknown, "unkwnown");
+    return device_type(device_type::unknown, "unknown");
 }
 
 device_params get_device_params(device_type device) {

utils.h

@@ -4,7 +4,7 @@
 
 #include "constraints.h"
 
-#define DEVICE_PATH "/proc/device-tree/model"
+const std::string DEVICE_PATH = "/proc/device-tree/model";
 
 struct device_type {
     enum type { unknown, jetson, pi, test };