Code review fixes.

IntelRealSense · Jan 30, 2020 · bc277fe · bc277fe
1 parent cc59ae7
commit bc277fe
Show file tree

Hide file tree

Showing 27 changed files with 493 additions and 447 deletions.
diff --git a/common/model-views.cpp b/common/model-views.cpp
@@ -32,8 +32,16 @@
 using namespace rs400;
 using namespace nlohmann;
 
-const int XGA = 0;
-const int VGA = 1;
+
+rs2_sensor_mode resolution_from_width_height(int width, int height)
+{
+    if ((width == 640 && height == 480) || (height == 640 && width == 480))
+        return RS2_SENSOR_MODE_VGA;
+    else if ((width == 1024 && height == 768) || (height == 768 && width == 1024))
+        return RS2_SENSOR_MODE_XGA;
+    else
+        return RS2_SENSOR_MODE_COUNT;
+}
 
 ImVec4 flip(const ImVec4& c)
 {
@@ -739,7 +747,7 @@ namespace rs2
                 ImGui::PopStyleColor();
 
                 if (ImGui::IsItemHovered())
-                    ImGui::SetTooltip("Selectcustom region of interest for the auto-exposure algorithm\nClick the button, then draw a rect on the frame");
+                    ImGui::SetTooltip("Select custom region of interest for the auto-exposure algorithm\nClick the button, then draw a rect on the frame");
             }
         }
 
@@ -825,8 +833,7 @@ namespace rs2
         bool* options_invalidated,
         std::string& error_message)
     {
-
-        for (auto&& i:options->get_supported_options())
+        for (auto&& i: options->get_supported_options())
         {
             auto opt = static_cast<rs2_option>(i);
 
@@ -903,6 +910,7 @@ namespace rs2
         depth_decoder(std::make_shared<rs2::depth_huffman_decoder>()),
         viewer(viewer)
     {
+        supported_options = s->get_supported_options();
         restore_processing_block("colorizer", depth_colorizer);
         restore_processing_block("yuy2rgb", yuy2rgb);
 
@@ -1121,8 +1129,8 @@ namespace rs2
             get_default_selection_index(res_values, resolution_constrain, &selection_index);
             ui.selected_res_id = selection_index;
 
-            if (s->supports(RS2_OPTION_CAMERA_MODE))
-                s->set_option(RS2_OPTION_CAMERA_MODE, res_values[ui.selected_res_id].first == 640 || res_values[ui.selected_res_id].second == 640 ? VGA : XGA);
+            if (s->supports(RS2_OPTION_SENSOR_MODE))
+                s->set_option(RS2_OPTION_SENSOR_MODE, resolution_from_width_height(res_values[ui.selected_res_id].first, res_values[ui.selected_res_id].second));
 
             while (ui.selected_res_id >= 0 && !is_selected_combination_supported()) ui.selected_res_id--;
             last_valid_ui = ui;
@@ -1221,17 +1229,13 @@ namespace rs2
                     res = true;
                     _options_invalidated = true;
 
-                    if (s->supports(RS2_OPTION_CAMERA_MODE))
+                    if (s->supports(RS2_OPTION_SENSOR_MODE))
                     {
-                        const int XGA = 0;
-                        const int VGA = 1;
-
                         auto width = res_values[ui.selected_res_id].first;
                         auto height = res_values[ui.selected_res_id].second;
-                        if (width == 640 || height == 640)
-                            s->set_option(RS2_OPTION_CAMERA_MODE, VGA);
-                        else
-                            s->set_option(RS2_OPTION_CAMERA_MODE, XGA);
+                        auto res = resolution_from_width_height(width, height);
+                        if (res >= RS2_SENSOR_MODE_XGA && res <= RS2_SENSOR_MODE_VGA)
+                            s->set_option(RS2_OPTION_SENSOR_MODE, res);
                     }
                 }
                 ImGui::PopStyleColor();
@@ -1869,12 +1873,13 @@ namespace rs2
 
             for (auto&& pbm : post_processing) pbm->save_to_config_file();
         }
-        auto option = 0;
-        if (option++ < s->get_supported_options().size())
+
+        if (next_option < supported_options.size())
         {
-            if (options_metadata.find(static_cast<rs2_option>(next_option)) != options_metadata.end())
+            auto next = supported_options[next_option];
+            if (options_metadata.find(static_cast<rs2_option>(next)) != options_metadata.end())
             {
-                auto& opt_md = options_metadata[static_cast<rs2_option>(next_option)];
+                auto& opt_md = options_metadata[static_cast<rs2_option>(next)];
                 opt_md.update_all_fields(error_message, notifications);
 
                 if (next_option == RS2_OPTION_ENABLE_AUTO_EXPOSURE)
@@ -4960,8 +4965,9 @@ namespace rs2
         ImGui::PushStyleColor(ImGuiCol_TextSelectedBg, light_grey);
         ImGui::PushStyleVar(ImGuiStyleVar_WindowPadding, ImVec2(5, 5));
         ImGui::PushFont(window.get_font());
+        auto serializable = dev.as<serializable_device>();
 
-        const auto load_json = [&](const std::string f) {
+        const auto load_json = [&, serializable](const std::string f) {
             std::ifstream file(f);
             if (!file.good())
             {
@@ -4971,9 +4977,10 @@ namespace rs2
                 throw std::runtime_error(to_string() << "Failed to read configuration file:\n\"" << f << "\"\nRemoving it from presets.");
             }
             std::string str((std::istreambuf_iterator<char>(file)), std::istreambuf_iterator<char>());
-            if (auto advanced = dev.as<serializable_device>())
+
+            if (serializable)
             {
-                advanced.load_json(str);
+                serializable.load_json(str);
                 for (auto&& sub : subdevices)
                 {
                     //If json was loaded correctly, we want the presets combo box to show the name of the configuration file
@@ -4996,15 +5003,14 @@ namespace rs2
             viewer.not_model.add_log(to_string() << "Loaded settings from \"" << f << "\"...");
         };
 
-        const auto save_to_json = [&](std::string full_filename)
+        const auto save_to_json = [&, serializable](std::string full_filename)
         {
-            auto advanced = dev.as<serializable_device>();
             if (!ends_with(to_lower(full_filename), ".json")) full_filename += ".json";
             std::ofstream outfile(full_filename);
             json saved_configuraion;
-            if (auto advanced = dev.as<serializable_device>())
+            if (serializable)
             {
-                saved_configuraion = json::parse(advanced.serialize_json());
+                saved_configuraion = json::parse(serializable.serialize_json());
             }
             save_viewer_configurations(outfile, saved_configuraion);
             outfile << saved_configuraion.dump(4);
@@ -5104,7 +5110,10 @@ namespace rs2
                                 else
                                 {
                                     //File was chosen
-                                    auto f = full_files_names[selected - static_cast<int>(labels.size() - files_labels.size())];
+                                    auto file = selected - static_cast<int>(labels.size() - files_labels.size());
+                                    if(file < 0 || file > full_files_names.size())
+                                        throw std::runtime_error("not a valid format");
+                                    auto f = full_files_names[file];
                                     error_message = safe_call([&]() { load_json(f); });
                                     selected_file_preset = f;
                                 }
@@ -5135,17 +5144,17 @@ namespace rs2
         const ImVec2 icons_size{ 20, 20 };
         //TODO: Change this once we have support for loading jsons with more data than only advanced controls
         bool is_streaming = std::any_of(subdevices.begin(), subdevices.end(), [](const std::shared_ptr<subdevice_model>& sm) { return sm->streaming; });
-        const int buttons_flags = dev.is<serializable_device>() ? 0 : ImGuiButtonFlags_Disabled;
+        const int buttons_flags = serializable ? 0 : ImGuiButtonFlags_Disabled;
         static bool require_advanced_mode_enable_prompt = false;
         auto advanced_dev = dev.as<advanced_mode>();
-        bool is_advanced_mode_enabled = true;
+        auto is_advanced_device = false;
+        auto is_advanced_mode_enabled = false;
         if (advanced_dev)
         {
+            is_advanced_device = true;
             is_advanced_mode_enabled = advanced_dev.is_enabled();
         }
 
-        auto serializable_dev = dev.is<serializable_device>();
-
         ImGui::SetCursorPosX(ImGui::GetCursorPosX() + 3);
 
         ////////////////////////////////////////
@@ -5157,7 +5166,7 @@ namespace rs2
 
         if (ImGui::ButtonEx(upload_button_name.c_str(), icons_size, (is_streaming && !load_json_if_streaming) ? ImGuiButtonFlags_Disabled : buttons_flags))
         {
-            if (serializable_dev && is_advanced_mode_enabled)
+            if (serializable && (!is_advanced_device || is_advanced_mode_enabled))
             {
                 json_loading([&]()
                 {
@@ -5189,7 +5198,7 @@ namespace rs2
         ImGui::SetCursorPosY(ImGui::GetCursorPosY() + 1); //Align the two icons to buttom
         if (ImGui::ButtonEx(save_button_name.c_str(), icons_size, buttons_flags))
         {
-            if (serializable_dev && is_advanced_mode_enabled)
+            if (serializable && (!is_advanced_device || is_advanced_mode_enabled))
             {
                 auto ret = file_dialog_open(save_file, "JavaScript Object Notation (JSON)\0*.json\0", NULL, NULL);
                 if (ret)
@@ -5412,7 +5421,8 @@ namespace rs2
         ////////////////////////////////////////
         // draw advanced mode panel
         ////////////////////////////////////////
-        if (dev.is<serializable_device>())
+        auto serializable = dev.is<serializable_device>();
+        if (serializable)
         {
             pos = ImGui::GetCursorPos();
             const float vertical_space_before_advanced_mode_control = 10.0f;
@@ -5674,7 +5684,7 @@ namespace rs2
                 if (show_stream_selection)
                     sub->draw_stream_selection();
 
-                static const std::vector<rs2_option> drawing_order = dev.is<serializable_device>() ?
+                static const std::vector<rs2_option> drawing_order = serializable ?
                     std::vector<rs2_option>{                           RS2_OPTION_EMITTER_ENABLED, RS2_OPTION_ENABLE_AUTO_EXPOSURE }
                   : std::vector<rs2_option>{ RS2_OPTION_VISUAL_PRESET, RS2_OPTION_EMITTER_ENABLED, RS2_OPTION_ENABLE_AUTO_EXPOSURE };
 
@@ -5698,7 +5708,7 @@ namespace rs2
                             if (skip_option(opt)) continue;
                             if (std::find(drawing_order.begin(), drawing_order.end(), opt) == drawing_order.end())
                             {
-                                if (dev.is<serializable_device>() && opt == RS2_OPTION_VISUAL_PRESET)
+                                if (serializable && opt == RS2_OPTION_VISUAL_PRESET)
                                     continue;
                                 if (sub->draw_option(opt, dev.is<playback>() || update_read_only_options, error_message, viewer.not_model))
                                 {

diff --git a/common/model-views.h b/common/model-views.h
@@ -270,7 +270,7 @@ namespace rs2
             opt == RS2_OPTION_STREAM_FORMAT_FILTER ||
             opt == RS2_OPTION_STREAM_INDEX_FILTER ||
             opt == RS2_OPTION_FRAMES_QUEUE_SIZE ||
-            opt == RS2_OPTION_CAMERA_MODE)
+            opt == RS2_OPTION_SENSOR_MODE)
             return true;
         return false;
     }
@@ -689,7 +689,8 @@ namespace rs2
         frame_queues queues;
         std::mutex _queue_lock;
         bool _options_invalidated = false;
-        int next_option = RS2_OPTION_COUNT;
+        int next_option = 0;
+        std::vector<rs2_option> supported_options;
         bool streaming = false;
 
         rect normalized_zoom{0, 0, 1, 1};

diff --git a/include/CMakeLists.txt b/include/CMakeLists.txt
@@ -20,6 +20,7 @@ target_sources(${LRS_TARGET}
         "${CMAKE_CURRENT_LIST_DIR}/librealsense2/hpp/rs_types.hpp"
         "${CMAKE_CURRENT_LIST_DIR}/librealsense2/hpp/rs_context.hpp"
         "${CMAKE_CURRENT_LIST_DIR}/librealsense2/hpp/rs_device.hpp"
+        "${CMAKE_CURRENT_LIST_DIR}/librealsense2/hpp/rs_serializable_device.hpp"
         "${CMAKE_CURRENT_LIST_DIR}/librealsense2/hpp/rs_export.hpp"
         "${CMAKE_CURRENT_LIST_DIR}/librealsense2/hpp/rs_frame.hpp"
         "${CMAKE_CURRENT_LIST_DIR}/librealsense2/hpp/rs_processing.hpp"

diff --git a/include/librealsense2/h/rs_device.h b/include/librealsense2/h/rs_device.h
@@ -347,6 +347,12 @@ const rs2_raw_data_buffer* rs2_get_calibration_table(const rs2_device* dev, rs2_
 */
 void rs2_set_calibration_table(const rs2_device* device, const void* calibration, int calibration_size, rs2_error** error);
 
+/* Serialize JSON content, returns ASCII-serialized JSON string on success. otherwise nullptr */
+rs2_raw_data_buffer* rs2_serialize_json(rs2_device* dev, rs2_error** error);
+
+/* Load JSON and apply advanced-mode controls */
+void rs2_load_json(rs2_device* dev, const void* json_content, unsigned content_size, rs2_error** error);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/include/librealsense2/h/rs_option.h b/include/librealsense2/h/rs_option.h
@@ -84,13 +84,13 @@ extern "C" {
         RS2_OPTION_LED_POWER, /**< Power of the LED (light emitting diode), with 0 meaning LED off*/
         RS2_OPTION_ZERO_ORDER_ENABLED, /**< Toggle Zero-Order mode */
         RS2_OPTION_ENABLE_MAP_PRESERVATION, /**< Preserve previous map when starting */
-        RS2_OPTION_AVALANCHE_PHOTO_DIODE, /**< Changes the Avalanche Photo Diode in the reciever */
+        RS2_OPTION_AVALANCHE_PHOTO_DIODE, /**< Changes the exposure time of Avalanche Photo Diode in the reciever */
         RS2_OPTION_POST_PROCESSING_SHARPENING,  /**< Changes the amount of sharpening in the post-processed image */
         RS2_OPTION_PRE_PROCESSING_SHARPENING, /**< Changes the amount of sharpening in the pre-processed image */
         RS2_OPTION_NOISE_FILTERING, /**< Control edges and background noise */
         RS2_OPTION_INVALIDATION_BYPASS, /**< Enable\disable pixel invalidation */
-        RS2_OPTION_SENSETIVITY, /**< Canges the depth sensetivity to ambient: 1 for no ambient and 2 for low ambient*/
-        RS2_OPTION_CAMERA_MODE, /**< Device specific for L515: 0 for XGA and 1 for VGA */
+        RS2_OPTION_AMBIENT_LIGHT, /**< Change the depth ambient light to ambient: 1 for no ambient and 2 for low ambient */
+        RS2_OPTION_SENSOR_MODE, /**< Device specific for L515: 0 for XGA and 1 for VGA */
         RS2_OPTION_COUNT /**< Number of enumeration values. Not a valid input: intended to be used in for-loops. */
     } rs2_option;
 
@@ -129,6 +129,28 @@ extern "C" {
     } rs2_rs400_visual_preset;
     const char* rs2_rs400_visual_preset_to_string(rs2_rs400_visual_preset preset);
 
+    /** \brief For L500 devices: provides optimized settings (presets) for specific types of usage. */
+    typedef enum rs2_l500_visual_preset
+    {
+        RS2_L500_VISUAL_PRESET_CUSTOM,
+        RS2_L500_VISUAL_PRESET_DEFAULT,
+        RS2_L500_VISUAL_PRESET_NO_AMBIENT,
+        RS2_L500_VISUAL_PRESET_LOW_AMBIENT,
+        RS2_L500_VISUAL_PRESET_MAX_RANGE,
+        RS2_L500_VISUAL_PRESET_SHORT_RANGE,
+        RS2_L500_VISUAL_PRESET_COUNT
+    }rs2_l500_visual_preset;
+    const char* rs2_l500_visual_preset_to_string(rs2_l500_visual_preset preset);
+
+    /** \brief For setting the camera_mode option */
+    typedef enum rs2_sensor_mode
+    {
+        RS2_SENSOR_MODE_XGA,
+        RS2_SENSOR_MODE_VGA,
+        RS2_SENSOR_MODE_COUNT
+    } rs2_sensor_mode;
+    const char* rs2_sensor_mode_to_string(rs2_sensor_mode preset);
+
     /**
     * check if an option is read-only
     * \param[in] sensor   the RealSense sensor

diff --git a/include/librealsense2/h/rs_types.h b/include/librealsense2/h/rs_types.h
@@ -178,7 +178,7 @@ typedef enum rs2_extension
     RS2_EXTENSION_MOTION_SENSOR,
     RS2_EXTENSION_FISHEYE_SENSOR,
     RS2_EXTENSION_DEPTH_HUFFMAN_DECODER,
-	RS2_EXTENSION_SERIALIZABLE_DEVICE,
+    RS2_EXTENSION_SERIALIZABLE_DEVICE,
     RS2_EXTENSION_COUNT
 } rs2_extension;
 const char* rs2_extension_type_to_string(rs2_extension type);

diff --git a/include/librealsense2/hpp/rs_serializable_device.hpp b/include/librealsense2/hpp/rs_serializable_device.hpp
@@ -0,0 +1,57 @@
+// License: Apache 2.0. See LICENSE file in root directory.
+// Copyright(c) 2020 Intel Corporation. All Rights Reserved.
+
+#pragma once
+#include "rs_types.hpp"
+#include "rs_device.hpp"
+#include <array>
+#include  <string>
+
+namespace rs2
+{
+    class serializable_device : public rs2::device
+    {
+    public:
+        serializable_device(rs2::device d)
+            : rs2::device(d.get())
+        {
+            rs2_error* e = nullptr;
+            if (rs2_is_device_extendable_to(_dev.get(), RS2_EXTENSION_SERIALIZABLE_DEVICE, &e) == 0 && !e)
+            {
+                _dev = nullptr;
+            }
+            rs2::error::handle(e);
+        }
+
+        std::string serialize_json() const
+        {
+            std::string results;
+
+            rs2_error* e = nullptr;
+            std::shared_ptr<rs2_raw_data_buffer> json_data(
+                rs2_serialize_json(_dev.get(), &e),
+                rs2_delete_raw_data);
+            rs2::error::handle(e);
+
+            auto size = rs2_get_raw_data_size(json_data.get(), &e);
+            rs2::error::handle(e);
+
+            auto start = rs2_get_raw_data(json_data.get(), &e);
+            rs2::error::handle(e);
+
+            results.insert(results.begin(), start, start + size);
+
+            return results;
+        }
+
+        void load_json(const std::string& json_content) const
+        {
+            rs2_error* e = nullptr;
+            rs2_load_json(_dev.get(),
+                json_content.data(),
+                (unsigned int)json_content.size(),
+                &e);
+            rs2::error::handle(e);
+        }
+    };
+}
diff --git a/include/librealsense2/rs.hpp b/include/librealsense2/rs.hpp
@@ -50,5 +50,7 @@ inline std::ostream & operator << (std::ostream & o, rs2_notification_category n
 inline std::ostream & operator << (std::ostream & o, rs2_sr300_visual_preset preset) { return o << rs2_sr300_visual_preset_to_string(preset); }
 inline std::ostream & operator << (std::ostream & o, rs2_exception_type exception_type) { return o << rs2_exception_type_to_string(exception_type); }
 inline std::ostream & operator << (std::ostream & o, rs2_playback_status status) { return o << rs2_playback_status_to_string(status); }
+inline std::ostream & operator << (std::ostream & o, rs2_l500_visual_preset preset) {return o << rs2_l500_visual_preset_to_string(preset);}
+inline std::ostream & operator << (std::ostream & o, rs2_sensor_mode mode) { return o << rs2_sensor_mode_to_string(mode); }
 
 #endif // LIBREALSENSE_RS2_HPP
diff --git a/include/librealsense2/rs_advanced_mode.h b/include/librealsense2/rs_advanced_mode.h
@@ -93,12 +93,6 @@ void rs2_set_amp_factor(rs2_device* dev, const  STAFactor* group, rs2_error** er
 /* Gets new values for STAFactor, returns 0 if success */
 void rs2_get_amp_factor(rs2_device* dev, STAFactor* group, int mode, rs2_error** error);
 
-/* Load JSON and apply advanced-mode controls, returns 0 if success */
-void rs2_load_json(rs2_device* dev, const void* json_content, unsigned content_size, rs2_error** error);
-
-/* Serialize JSON content, returns 0 if success */
-rs2_raw_data_buffer* rs2_serialize_json(rs2_device* dev, rs2_error** error);
-
 #ifdef __cplusplus
 }
 #endif