diff --git a/common/device-model.cpp b/common/device-model.cpp index 1584b937bb..c0e97c1575 100644 --- a/common/device-model.cpp +++ b/common/device-model.cpp @@ -1865,7 +1865,12 @@ namespace rs2 auto itr = sub->options_metadata.find(RS2_OPTION_VISUAL_PRESET); if (itr != sub->options_metadata.end()) { - itr->second.endpoint->set_option(RS2_OPTION_VISUAL_PRESET, RS2_RS400_VISUAL_PRESET_CUSTOM); + // Make sure we actually have a "Custom" preset + if( std::string( "Custom", 6 ) + == itr->second.endpoint->get_option_value_description( + RS2_OPTION_VISUAL_PRESET, + RS2_RS400_VISUAL_PRESET_CUSTOM ) ) + itr->second.endpoint->set_option(RS2_OPTION_VISUAL_PRESET, RS2_RS400_VISUAL_PRESET_CUSTOM); } } } diff --git a/src/dds/rs-dds-device-proxy.cpp b/src/dds/rs-dds-device-proxy.cpp index 4a741c75b5..276b28860c 100644 --- a/src/dds/rs-dds-device-proxy.cpp +++ b/src/dds/rs-dds-device-proxy.cpp @@ -696,4 +696,24 @@ void dds_device_proxy::update( const void * /*image*/, int /*image_size*/, rs2_u } +std::vector< sensor_interface * > dds_device_proxy::get_serializable_sensors() +{ + std::vector< sensor_interface * > sensors; + auto const n_sensors = get_sensors_count(); + for( auto i = 0; i < n_sensors; ++i ) + sensors.push_back( &get_sensor( i ) ); + return sensors; +} + + +std::vector< sensor_interface const * > dds_device_proxy::get_serializable_sensors() const +{ + std::vector< sensor_interface const * > sensors; + auto const n_sensors = get_sensors_count(); + for( auto i = 0; i < n_sensors; ++i ) + sensors.push_back( &get_sensor( i ) ); + return sensors; +} + + } // namespace librealsense diff --git a/src/dds/rs-dds-device-proxy.h b/src/dds/rs-dds-device-proxy.h index e5bc9e064c..bc6a2b0b68 100644 --- a/src/dds/rs-dds-device-proxy.h +++ b/src/dds/rs-dds-device-proxy.h @@ -6,6 +6,7 @@ #include #include #include "sid_index.h" +#include "rsdds-serializable.h" #include #include @@ -40,6 +41,7 @@ class dds_device_proxy , public debug_interface , public updatable // unsigned, non-recovery-mode , public update_device_interface // signed, recovery-mode + , public dds_serializable { std::shared_ptr< realdds::dds_device > _dds_dev; std::map< std::string, std::vector< std::shared_ptr< stream_profile_interface > > > _stream_name_to_profiles; @@ -90,6 +92,11 @@ class dds_device_proxy private: void update( const void * image, int image_size, rs2_update_progress_callback_sptr = nullptr ) const override; + // dds_serializable +private: + device_interface const & get_serializable_device() const override { return *this; } + std::vector< sensor_interface * > get_serializable_sensors() override; + std::vector< sensor_interface const * > get_serializable_sensors() const override; }; diff --git a/src/dds/rs-dds-option.cpp b/src/dds/rs-dds-option.cpp index a624b0687a..67d07db989 100644 --- a/src/dds/rs-dds-option.cpp +++ b/src/dds/rs-dds-option.cpp @@ -126,6 +126,9 @@ void rs_dds_option::set( float value ) << "use rs2_set_option_value to set " << get_string( _rs_type ) << " value" ); } + if( is_read_only() ) + throw invalid_value_exception( "option is read-only: " + _dds_opt->get_name() ); + _set_opt_cb( j_value ); } @@ -158,6 +161,9 @@ void rs_dds_option::set_value( json value ) if( ! _set_opt_cb ) throw std::runtime_error( "Set option callback is not set for option " + _dds_opt->get_name() ); + if( is_read_only() ) + throw invalid_value_exception( "option is read-only: " + _dds_opt->get_name() ); + _set_opt_cb( std::move( value ) ); } diff --git a/src/dds/rsdds-serializable.cpp b/src/dds/rsdds-serializable.cpp new file mode 100644 index 0000000000..806a627793 --- /dev/null +++ b/src/dds/rsdds-serializable.cpp @@ -0,0 +1,170 @@ +// License: Apache 2.0. See LICENSE file in root directory. +// Copyright(c) 2024 Intel Corporation. All Rights Reserved. + +#include "rsdds-serializable.h" + +#include +#include +#include + +#include +#include + +using json = nlohmann::json; + + +static std::set< rs2_option > get_options_to_ignore() +{ + return { + RS2_OPTION_FRAMES_QUEUE_SIZE, // Internally added and is not an option we need to record/load + RS2_OPTION_REGION_OF_INTEREST // The RoI is temporary, uses another mechanism for get/set, and we don't load it + }; +} + + +namespace librealsense { + + +std::vector< uint8_t > dds_serializable::serialize_json() const +{ + serialized_utilities::json_preset_writer writer; + writer.set_device_info( get_serializable_device() ); + + // Set of options that should not be written + auto const options_to_ignore = get_options_to_ignore(); + + for( auto p_sensor : get_serializable_sensors() ) + { + std::string const sensor_name = p_sensor->get_info( RS2_CAMERA_INFO_NAME ); + for( auto o : p_sensor->get_supported_options() ) + { + auto & opt = p_sensor->get_option( o ); + if( opt.is_read_only() ) + continue; + if( options_to_ignore.find( o ) != options_to_ignore.end() ) + continue; + + auto val = opt.get_value(); + writer.write_param( sensor_name + '/' + get_string( o ), val ); + } + } + + auto str = writer.to_string(); + return std::vector< uint8_t >( str.begin(), str.end() ); +} + + +void dds_serializable::load_json( const std::string & json_content ) +{ + serialized_utilities::json_preset_reader reader( json_content ); + + // Verify if device information in preset file is compatible with the connected device. + reader.check_device_info( get_serializable_device() ); + auto const sensors = get_serializable_sensors(); + + // Do the visual preset first, as it sets the basis on top of which we apply the regular controls + { + auto const visual_preset_subkey = '/' + get_string( RS2_OPTION_VISUAL_PRESET ); + for( auto const p_sensor : sensors ) + { + std::string const sensor_name = p_sensor->get_info( RS2_CAMERA_INFO_NAME ); + auto const key = sensor_name + visual_preset_subkey; + auto it = reader.find( key ); + if( it == reader.end() ) + continue; + + try + { + auto & opt = p_sensor->get_option( RS2_OPTION_VISUAL_PRESET ); + opt.set_value( it.value() ); + LOG_DEBUG( "loaded '" << key << "' value " << it.value() ); + } + catch( std::exception const & e ) + { + LOG_ERROR( "Failed to load '" << key << "' (value " << it.value() << "): " << e.what() ); + throw; + } + } + } + + // Create a set of options we have to change + std::map< std::string /*key*/, std::string /*error*/ > keys_to_change; + { + // Set of options that should not be set in the loop + auto options_to_ignore = get_options_to_ignore(); + options_to_ignore.insert( RS2_OPTION_VISUAL_PRESET ); // We don't want to do this again + + for( auto const p_sensor : sensors ) + { + std::string const sensor_name = p_sensor->get_info( RS2_CAMERA_INFO_NAME ); + for( auto option_id : p_sensor->get_supported_options() ) + { + if( options_to_ignore.find( option_id ) != options_to_ignore.end() ) + continue; + auto key = sensor_name + '/' + get_string( option_id ); + if( reader.find( key ) == reader.end() ) + continue; + + keys_to_change.emplace( std::move( key ), std::string() ); + } + } + } + + // Some options may depend on others being set first (e.g., they are read-only until another option is set); these + // will generate errors. Since we don't know which order to set options in, we track failures and keep trying as + // long as at least one option value is changed: + while( true ) + { + std::map< std::string /*key*/, std::string /*error*/ > keys_left; + for( auto const p_sensor : sensors ) + { + std::string const sensor_name = p_sensor->get_info( RS2_CAMERA_INFO_NAME ); + for( auto option_id : p_sensor->get_supported_options() ) + { + auto key = sensor_name + '/' + get_string( option_id ); + if( keys_to_change.find( key ) == keys_to_change.end() ) + continue; + + auto it = reader.find( key ); + if( it != reader.end() ) + { + try + { + auto & opt = p_sensor->get_option( option_id ); + opt.set_value( it.value() ); + LOG_DEBUG( "loaded '" << key << "' value " << it.value() ); + continue; + } + catch( unrecoverable_exception const & ) + { + throw; + } + catch( std::exception const & e ) + { + std::string error = e.what(); + LOG_DEBUG( "Failed to load '" << key << "' (value " << it.value() << "): " << error ); + keys_left.emplace( std::move( key ), std::move( error ) ); + } + } + } + } + if( keys_left.empty() ) + // Nothing more to do; we can stop + break; + if( keys_left.size() == keys_to_change.size() ) + { + // We still have failures, but nothing's changed; we must stop + for( auto const & key_error : keys_left ) + { + auto it = reader.find( key_error.first ); + if( it != reader.end() ) + LOG_ERROR( "Failed to load '" << key_error.first << "' (value " << it.value() << "): " << key_error.second ); + } + break; + } + keys_to_change = std::move( keys_left ); + } +} + + +} // namespace librealsense diff --git a/src/dds/rsdds-serializable.h b/src/dds/rsdds-serializable.h new file mode 100644 index 0000000000..0db91c1f00 --- /dev/null +++ b/src/dds/rsdds-serializable.h @@ -0,0 +1,28 @@ +// License: Apache 2.0. See LICENSE file in root directory. +// Copyright(c) 2024 Intel Corporation. All Rights Reserved. +#pragma once + +#include + + +namespace librealsense { + + +class device_interface; +class sensor_interface; + + +class dds_serializable : public serializable_interface +{ +public: + std::vector< uint8_t > serialize_json() const override; + void load_json( const std::string & json_content ) override; + +protected: + virtual device_interface const & get_serializable_device() const = 0; + virtual std::vector< sensor_interface * > get_serializable_sensors() = 0; + virtual std::vector< sensor_interface const * > get_serializable_sensors() const = 0; +}; + + +} // namespace librealsense diff --git a/src/serializable-interface.h b/src/serializable-interface.h index 1d805de4d3..93fe485895 100644 --- a/src/serializable-interface.h +++ b/src/serializable-interface.h @@ -1,17 +1,25 @@ // License: Apache 2.0. See LICENSE file in root directory. -// Copyright(c) 2020 Intel Corporation. All Rights Reserved. - +// Copyright(c) 2020-4 Intel Corporation. All Rights Reserved. #pragma once -#include "types.h" + #include "core/extension.h" +#include +#include + + +namespace librealsense { + -namespace librealsense +class serializable_interface { - class serializable_interface - { - public: - virtual std::vector serialize_json() const = 0; - virtual void load_json(const std::string& json_content) = 0; - }; - MAP_EXTENSION(RS2_EXTENSION_SERIALIZABLE, serializable_interface); -} +public: + virtual ~serializable_interface() {} + + virtual std::vector< uint8_t > serialize_json() const = 0; + virtual void load_json( const std::string & json_content ) = 0; +}; + +MAP_EXTENSION( RS2_EXTENSION_SERIALIZABLE, serializable_interface ); + + +} // namespace librealsense diff --git a/third-party/realdds/py/pyrealdds.cpp b/third-party/realdds/py/pyrealdds.cpp index a4c8b8a4ac..d072bba1ab 100644 --- a/third-party/realdds/py/pyrealdds.cpp +++ b/third-party/realdds/py/pyrealdds.cpp @@ -913,6 +913,10 @@ PYBIND11_MODULE(NAME, m) { .def( "publish_metadata", &dds_device_server::publish_metadata, py::call_guard< py::gil_scoped_release >() ) .def( "broadcast", &dds_device_server::broadcast ) .def( "broadcast_disconnect", &dds_device_server::broadcast_disconnect, py::arg( "ack-timeout" ) = dds_time() ) + .def( FN_FWD( dds_device_server, on_set_option, + (dds_device_server &, std::shared_ptr< realdds::dds_option > const &, json_ref &&), + ( std::shared_ptr< realdds::dds_option > const & option, json & value ), + callback( self, option, json_ref{ value } ); ) ) .def( FN_FWD_R( dds_device_server, on_control, false, (dds_device_server &, std::string const &, py::object &&, json_ref &&), diff --git a/unit-tests/dds/test-librs-options.py b/unit-tests/dds/test-librs-options.py index 0ebe6e6f79..105071b04a 100644 --- a/unit-tests/dds/test-librs-options.py +++ b/unit-tests/dds/test-librs-options.py @@ -26,11 +26,18 @@ dds.option.from_json( ['Backlight Compensation', 0, 0, 1, 1, 0, 'Backlight custom description'] ), dds.option.from_json( ['Boolean Option', False, False, 'Something'] ), dds.option.from_json( ['Integer Option', 1, None, 'Something', ['optional']] ), - dds.option.from_json( ['Enum Option', 'First', ['First','Last','Everything'], 'Last', 'My'] ) + dds.option.from_json( ['Enum Option', 'First', ['First','Last','Everything'], 'Last', 'My'] ), + dds.option.from_json( ['R/O Option', 'Value', 'Read-only string option'] ), + dds.option.from_json( ['Visual Preset', 'Default', ['Default','Preset-1','Preset-2'], 'Default', 'Should enable serialization'] ) ] ) server = dds.device_server( participant, device_info.topic_root ) server.init( [s1], [], {} ) + with test.closure( 'Set up a handler to keep track of the change order' ): + def _on_set_option( server, option, value ): + print( option.get_name() ) + server.on_set_option( _on_set_option ) + with test.closure( 'Broadcast the device', on_fail=test.ABORT ): server.broadcast( device_info ) @@ -52,7 +59,7 @@ for s in dev.query_sensors(): break options = test.info( "supported options", s.get_supported_options() ) - test.check_equal( len(options), 5 ) # 'Frames Queue Size' gets added to all sensors!!?!?! + test.check_equal( len(options), 7 ) # 'Frames Queue Size' gets added to all sensors!!?!?! with test.closure( 'Play with integer option' ): io = next( o for o in options if str(o) == 'Integer Option' ) @@ -93,8 +100,57 @@ s.set_option_value( eo, 'Last' ) test.check_equal( s.get_option( eo ), 1. ) + with test.closure( 'Play with read-only option' ): + ro = next( o for o in options if str(o) == 'R/O Option' ) + test.check_throws( lambda: + s.get_option( ro ), + RuntimeError, 'use rs2_get_option_value to get string values' ) + rv = s.get_option_value( ro ) + test.check_equal( rv.type, rs.option_type.string ) + test.check_equal( rv.value, 'Value' ) + test.check( s.is_option_read_only( rv.id ) ) + rr = s.get_option_range( rv.id ) + test.check_equal( rr.min, 0. ) + test.check_equal( rr.max, 0. ) + test.check_equal( rr.default, 0. ) + test.check_equal( rr.step, 0. ) + test.check_throws( lambda: + s.set_option( ro, 2. ), + RuntimeError, 'use rs2_set_option_value to set string values' ) + test.check_throws( lambda: + s.set_option_value( ro, 'Blah' ), + RuntimeError, 'option is read-only: R/O Option' ) + + with test.closure( 'Check serialization through serialized_device' ): + sdev = rs.serializable_device( dev ) + test.check( sdev, on_fail=test.RAISE ) + import json + j = json.loads( sdev.serialize_json() ) + test.info( "serialize_json()", j ) + params = j.get( 'parameters', [] ) + test.check_equal( len( params ), 5 ) # Without FRAMES_QUEUE_SIZE + test.check_equal( params.get( 'sensor/Boolean Option' ), True ) + + # Confirm previous value was set + bv = s.get_option_value( bo ) + test.check_equal( bv.type, rs.option_type.boolean ) + test.check_equal( bv.value, True ) + test.check_equal( s.get_option( bo ), 1. ) + + # Change using load_json() and verify + params.update({ 'sensor/Boolean Option': False }) + log.d( f'updated json: {j}' ) + remote.capture_stdout() + sdev.load_json( json.dumps( j ) ) + rv = remote.get_stdout() + test.check_equal( s.get_option( bo ), 0. ) + test.info( "option change order", rv ) + test.check_equal( len(rv), 5 ) # all options should have been changed + test.check_equal( rv[0], 'Visual Preset' ) # but Visual Preset should always update first + del sdev + with test.closure( 'All done' ): - dev = None - context = None + del dev + del context test.print_results() diff --git a/unit-tests/py/rspy/test.py b/unit-tests/py/rspy/test.py index 8d527a5c17..df9f9221aa 100644 --- a/unit-tests/py/rspy/test.py +++ b/unit-tests/py/rspy/test.py @@ -710,6 +710,7 @@ def __init__( self, script, interactive=True, name="remote", nested_indent="svr" self._status = None # last return-code self._on_finish = None # callback self._exception = None + self._stdout = None def __enter__( self ): """ @@ -780,7 +781,10 @@ def _output_reader( self ): missing_ready = None if not line: continue - print( nested_prefix + line, end='', flush=True ) + if self._stdout is not None and not line.startswith( '-D-' ): + self._stdout.append( line[:-1] ) + else: + print( nested_prefix + line, end='', flush=True ) else: if x > 0 and line[:x] == '___ready\n'[:x]: missing_ready = '___ready\n'[x:] @@ -855,6 +859,31 @@ def run( self, command, timeout=5, on_fail=RAISE ): self.wait_until_ready( timeout=timeout, on_fail=on_fail ) + def capture_stdout( self ): + """ + Start collecting stdout from the remote, if not already collecting it. + Stdout collected will not be printed to our stdout, and will instead be returned in get_stdout() + as a list of lines. Debug lines (-D-...) are filtered out. + """ + if self._stdout is None: + self._stdout = [] + + + def get_stdout( self, flush=True, stop=True, timeout=5 ): + """ + Returns the stdout collected since capture_stdout(), as a list of lines. + :param stop: if True, will stop collecting stdout + :param flush: remote output may have not made it to us yet; if True, wait until we've seen everything + """ + if flush: + # Run empty command and wait until ready; will throw + self.run( '', timeout=timeout ) + stdout = self._stdout + if stop: + self._stdout = None + return stdout + + def wait( self, timeout=10 ): """ Waits until all stdout has been consumed and the remote exited