PR IntelRealSense#12650 from Eran: JSON-based DDS options

common/device-model.cpp

@@ -2493,7 +2493,7 @@ namespace rs2
         const float left_space = 3.f;
         const float upper_space = 3.f;
 
-        bool update_read_only_options = _update_readonly_options_timer;
+        bool update_read_only_options = false; // _update_readonly_options_timer;
 
         const ImVec2 initial_screen_pos = ImGui::GetCursorScreenPos();
         //Upper Space

common/option-model.cpp

@@ -32,12 +32,12 @@ namespace rs2
         option.value = 0;
 
         option.supported = options->supports(opt);
+        option.range = options->get_option_range( opt );
+        option.read_only = options->is_option_read_only( opt );
         if (option.supported)
         {
             try
             {
-                option.range = options->get_option_range(opt);
-                option.read_only = options->is_option_read_only(opt);
                 option.value = options->get_option(opt);
             }
             catch (const error& e)

include/librealsense2/h/rs_option.h

@@ -144,7 +144,7 @@ extern "C" {
     */
     typedef enum rs2_option_type
     {
-        RS2_OPTION_TYPE_NUMBER, /**< 64-bit integer value, either as_number_signed or as_number_unsigned */
+        RS2_OPTION_TYPE_INTEGER, /**< 64-bit signed integer value */
         RS2_OPTION_TYPE_FLOAT,
         RS2_OPTION_TYPE_STRING,
 
@@ -167,8 +167,7 @@ extern "C" {
         union {
             char const * as_string;       /**< valid only while rs2_option_value is alive! */
             float as_float;
-            int64_t as_number_signed;
-            uint64_t as_number_unsigned;
+            int64_t as_integer;
         };
     } rs2_option_value;
 

src/core/options-watcher.cpp

@@ -139,7 +139,13 @@ options_watcher::options_and_values options_watcher::update_options()
         }
         catch( ... )
         {
-            // Some options cannot be queried all the time (i.e. streaming only)
+            // Some options cannot be queried all the time (i.e. streaming only) - so if we HAD a value, it needs to be
+            // removed!
+            if( opt.second.p_last_known_value && ! opt.second.p_last_known_value->is_null() )
+            {
+                opt.second.p_last_known_value = std::make_shared< const json >();
+                updated_options[opt.first] = opt.second;
+            }
         }
 
         // Checking stop conditions after each query to ensure stop when requested.

src/dds/rs-dds-option.cpp

@@ -1,5 +1,5 @@
 // License: Apache 2.0. See LICENSE file in root directory.
-// Copyright(c) 2023 Intel Corporation. All Rights Reserved.
+// Copyright(c) 2024 Intel Corporation. All Rights Reserved.
 
 #include "rs-dds-option.h"
 
@@ -9,13 +9,24 @@
 namespace librealsense {
 
 
+static option_range range_from_realdds( std::shared_ptr< realdds::dds_option > const & dds_opt )
+{
+    if( dds_opt->get_minimum_value().is_number() && dds_opt->get_maximum_value().is_number()
+        && dds_opt->get_stepping().is_number() )
+    {
+        return { dds_opt->get_minimum_value(),
+                 dds_opt->get_maximum_value(),
+                 dds_opt->get_stepping(),
+                 dds_opt->get_default_value() };
+    }
+    return { 0, 0, 0, 0 };
+}
+
+
 rs_dds_option::rs_dds_option( const std::shared_ptr< realdds::dds_option > & dds_opt,
                               set_option_callback set_opt_cb,
                               query_option_callback query_opt_cb )
-    : option_base( { dds_opt->get_range().min,
-                     dds_opt->get_range().max,
-                     dds_opt->get_range().step,
-                     dds_opt->get_range().default_value } )
+    : option_base( range_from_realdds( dds_opt ) )
     , _dds_opt( dds_opt )
     , _set_opt_cb( set_opt_cb )
     , _query_opt_cb( query_opt_cb )
@@ -47,6 +58,17 @@ float rs_dds_option::get_last_known_value() const
 }
 
 
+bool rs_dds_option::is_read_only() const
+{
+    return _dds_opt->is_read_only();
+}
+
+
+bool rs_dds_option::is_enabled() const
+{
+    return ! _dds_opt->get_value().is_null();
+}
+
 
 const char * rs_dds_option::get_description() const
 {

src/dds/rs-dds-option.h

@@ -1,5 +1,5 @@
 // License: Apache 2.0. See LICENSE file in root directory.
-// Copyright(c) 2023 Intel Corporation. All Rights Reserved.
+// Copyright(c) 2024 Intel Corporation. All Rights Reserved.
 
 #pragma once
 
@@ -41,7 +41,8 @@ class rs_dds_option : public option_base
 
     float query() const override;
 
-    bool is_enabled() const override { return true; }
+    bool is_read_only() const override;
+    bool is_enabled() const override;
     const char * get_description() const override;
 };
 

src/dds/rs-dds-sensor-proxy.cpp

@@ -543,10 +543,29 @@ void dds_sensor_proxy::add_option( std::shared_ptr< realdds::dds_option > option
     if( get_option_handler( option_id ) )
         throw std::runtime_error( "option '" + option->get_name() + "' already exists in sensor" );
 
+    LOG_DEBUG( "... option -> " << option->get_name() );
     auto opt = std::make_shared< rs_dds_option >(
         option,
         [=]( const std::string & name, float value ) { _dev->set_option_value( option, value ); },
-        [=]( const std::string & name ) -> float { return _dev->query_option_value( option ); } );
+        [=]( const std::string & name ) -> float
+        {
+            // We don't have to constantly query the option: we expect to get new values automatically, so can return
+            // the "last-known" value:
+            json value = option->get_value();
+            // If the value is null, we shouldn't get here (is_enabled() should return false) from the user but it's
+            // still possible from internal mechanisms (like the options-watcher).
+            // If we actually query for the current value:
+            //    value = _dev->query_option_value( option );
+            // Then we may have get a null even when is_enabled() returned true!
+            try
+            {
+                return value;  // try to convert to float
+            }
+            catch( std::exception const & )
+            {
+                throw invalid_value_exception( rsutils::string::from() << "option '" << name << "' value (" << value << ") is not a float" );
+            }
+        } );
     register_option( option_id, opt );
     _options_watcher.register_option( option_id, opt );
 }

src/rs.cpp

@@ -1,5 +1,5 @@
 // License: Apache 2.0 See LICENSE file in root directory.
-// Copyright(c) 2015 Intel Corporation. All Rights Reserved.
+// Copyright(c) 2024 Intel Corporation. All Rights Reserved.
 
 #include <functional>   // For function
 
@@ -104,7 +104,12 @@ struct rs2_option_value_wrapper : rs2_option_value
             if( p_json->is_number_float() )
             {
                 type = RS2_OPTION_TYPE_FLOAT;
-                as_float = p_json->get< float >();
+                p_json->get_to( as_float );
+            }
+            if( p_json->is_number_integer() )
+            {
+                type = RS2_OPTION_TYPE_INTEGER;
+                p_json->get_to( as_integer );
             }
             else if( p_json->is_string() )
             {
@@ -205,7 +210,7 @@ struct rs2_error
 
 rs2_error *rs2_create_error(const char* what, const char* name, const char* args, rs2_exception_type type) BEGIN_API_CALL
 {
-    LOG_ERROR( "[" << name << "][" << rs2_exception_type_to_string( type ) << "] " << what << ": " << args );
+    LOG_ERROR( "[" << name << "( " << args << " ) " << rs2_exception_type_to_string( type ) << "] " << what );
     return new rs2_error{ what, name, args, type };
 }
 NOEXCEPT_RETURN(nullptr, what, name, args, type)

src/to-string.cpp

@@ -1,5 +1,5 @@
 // License: Apache 2.0. See LICENSE file in root directory.
-// Copyright(c) 2021 Intel Corporation. All Rights Reserved.
+// Copyright(c) 2024 Intel Corporation. All Rights Reserved.
 
 #include "core/options-registry.h"
 #include "core/enum-helpers.h"
@@ -732,7 +732,7 @@ std::string const & get_string( rs2_option_type value )
 #define CASE( X ) STRARR( arr, OPTION_TYPE, X );
         CASE( FLOAT )
         CASE( STRING )
-        CASE( NUMBER )
+        CASE( INTEGER )
 #undef CASE
             return arr;
     }();

third-party/realdds/doc/initialization.md

@@ -84,22 +84,28 @@ This is optional: not all devices have options. See [device](device.md).
 ```
 
 * `"options"` is an array of options
-    * Each option is an array of `[name, value, range..., default-value, description, options...]`:
+    * Each option is an array of `[name, value, range..., default-value, description, [properties...]]`:
         * The `name` is what will be displayed to the user
         * The current `value`
         * An optional `range` of valid values
-            * Numeric options (`float`, `int`), defined by a `minimum`, `maximum`, and `stepping`, e.g. "Domain" in the example above
+            * Numeric options (`float`, `int`), defined by a `minimum`, `maximum`, and `stepping`
                 * I.e., is-valid = one-of( `minimum`, `minimum+1*stepping`, `minimum+2*stepping`, ..., `maximum` )
             * Booleans can remove the range, e.g. `["Enabled", true, true, "Description"]`
+                * Booleans can be expressed as a range with `minimum=0`, `maximum=1`, `stepping=1`
             * Free string options would likewise have no range, e.g. `["Name", "Bob", "", "The customer's name"]`
+                * `"IPv4"` is a string option that conforms to `W.X.Y.Z` (IP address) format
             * Choice options are strings with an array of choices, e.g. `["Preset", "Maximum Quality", ["Maximum Range", "Maximum Quality", "Maximum Speed"], "Maximum Speed", "Standard preset combination of options"]`
-            * **NOTE**: Only numeric options are implemented at this time
-                * Booleans can be expressed as a range with `minimum=0`, `maximum=1`, `stepping=1`
         * A `default-value` which also adheres to the range
+            * If this and the range are missing, the option is read-only
         * A user-friendly description that describes the option, to be shown in any tooltip
-        * Additional `options` describing behavior
-            * E.g., `"read-only"`, `"volatile"`, etc.
-            * **NOTE**: none are implemented at this time
+        * Additional `properties` describing behavior or nature, as an array of (case-sensitive) strings
+            * `"optional"` to note that it's possible for it to not have a value; lack of a value is denoted as `null` in the JSON
+                * If optional, a type must be deducible or present in the properties
+                * E.g., `["name", null, "description", ["optional", "string"]]` is an optional read-only string value that's currently unset
+            * `"string"`, `"int"`, `"boolean"`, `"float"`, `"IPv4"` can (and sometime must) indicate the value type
+                * If missing, the type will be deduced, if possible, from the values
+            * `"read-only"` options are not settable
+                * `set-option` will fail for these, though their value may change on the server side
     * The device server has final say whether an option value is valid or not, and return an error if `set-option` specifies an unsupported or invalid value based on context
 
 Device options will not be shown in the Viewer.

third-party/realdds/include/realdds/dds-device-server.h

@@ -94,7 +94,7 @@ class dds_device_server
     bool has_metadata_readers() const;
 
     typedef std::function< void( const std::shared_ptr< realdds::dds_option > & option, float value ) > set_option_callback;
-    typedef std::function< float( const std::shared_ptr< realdds::dds_option > & option ) > query_option_callback;
+    typedef std::function< rsutils::json( const std::shared_ptr< realdds::dds_option > & option ) > query_option_callback;
     void on_set_option( set_option_callback callback ) { _set_option_callback = std::move( callback ); }
     void on_query_option( query_option_callback callback ) { _query_option_callback = std::move( callback ); }
 
@@ -114,7 +114,7 @@ class dds_device_server
     void on_query_option( control_sample const &, rsutils::json & reply );
     void on_query_options( control_sample const &, rsutils::json & reply );
 
-    float query_option( std::shared_ptr< dds_option > const & ) const;
+    rsutils::json query_option( std::shared_ptr< dds_option > const & ) const;
 
     std::shared_ptr< dds_publisher > _publisher;
     std::shared_ptr< dds_subscriber > _subscriber;

third-party/realdds/include/realdds/dds-device.h

@@ -1,5 +1,5 @@
 // License: Apache 2.0. See LICENSE file in root directory.
-// Copyright(c) 2022 Intel Corporation. All Rights Reserved.
+// Copyright(c) 2024 Intel Corporation. All Rights Reserved.
 #pragma once
 
 #include "dds-defines.h"
@@ -78,8 +78,8 @@ class dds_device : public dds_discovery_sink
 
     void open( const dds_stream_profiles & profiles );
 
-    void set_option_value( const std::shared_ptr< dds_option > & option, float new_value );
-    float query_option_value( const std::shared_ptr< dds_option > & option );
+    void set_option_value( const std::shared_ptr< dds_option > & option, rsutils::json new_value );
+    rsutils::json query_option_value( const std::shared_ptr< dds_option > & option );
 
     void send_control( topics::flexible_msg &&, rsutils::json * reply = nullptr );