Skip to content

Commit

Permalink
remove unsigned option value
Browse files Browse the repository at this point in the history
  • Loading branch information
maloel committed Feb 11, 2024
1 parent 7d75b55 commit 75e2443
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 136 deletions.
5 changes: 2 additions & 3 deletions include/librealsense2/h/rs_option.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand All @@ -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;

Expand Down
9 changes: 7 additions & 2 deletions src/rs.cpp
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -103,7 +103,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() )
{
Expand Down
4 changes: 2 additions & 2 deletions src/to-string.cpp
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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;
}();
Expand Down
17 changes: 0 additions & 17 deletions third-party/realdds/include/realdds/dds-option.h
Original file line number Diff line number Diff line change
Expand Up @@ -137,23 +137,6 @@ class dds_integer_option : public dds_option
};


class dds_unsigned_option : public dds_option
{
using super = dds_option;

public:
using type = rsutils::json::number_unsigned_t;

type get_unsigned() const { return get_value().get< type >(); }
type get_default_unsigned() const { return super::get_default_value().get< type >(); }

char const * value_type() const override { return "unsigned"; }

void check_type( rsutils::json const & ) const override;
static type check_unsigned( rsutils::json const & );
};


class dds_string_option : public dds_option
{
using super = dds_option;
Expand Down
99 changes: 6 additions & 93 deletions third-party/realdds/src/dds-option.cpp
Original file line number Diff line number Diff line change
@@ -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.

#include <realdds/dds-option.h>
#include <realdds/dds-exceptions.h>
Expand Down Expand Up @@ -183,49 +183,6 @@ static dds_option::option_properties parse_option_properties( json const & j )
}


bool type_from_value( json const & j, std::string & type )
{
switch( j.type() )
{
case json::value_t::number_float:
if( type.length() == 5 && type == "float" )
return true;
if( type.empty() || type == "int" || type == "unsigned" )
return type.assign( "float", 5 ), true; // float trumps other number typs
break;
case json::value_t::string:
if( type.length() == 6 && type == "string" )
return true;
if( type.empty() )
return type.assign( "string", 6 ), true;
break;
case json::value_t::number_unsigned:
if( type.length() == 8 && type == "unsigned" )
return true;
if( type.empty() || type == "int" )
return type.assign( "unsigned", 8 ), true; // unsigned trumps integer
if( type == "float" )
return true;
break;
case json::value_t::number_integer:
if( type.length() == 3 && type == "int" )
return true;
if( type.empty() )
return type.assign( "int", 3 ), true;
else if( type == "float" || type == "unsigned" )
return true;
break;
case json::value_t::boolean:
if( type.length() == 7 && type == "boolean" )
return true;
if( type.empty() )
return type.assign( "boolean", 7 ), true;
break;
}
return false;
}


template< typename... Rest >
bool type_from_value( std::string & type, json const & j )
{
Expand All @@ -234,8 +191,8 @@ bool type_from_value( std::string & type, json const & j )
case json::value_t::number_float:
if( type.length() == 5 && type == "float" )
return true;
if( type.empty() || type == "int" || type == "unsigned" )
return type.assign( "float", 5 ), true; // float trumps other number typs
if( type.empty() || type == "int" )
return type.assign( "float", 5 ), true; // float > int
break;

case json::value_t::string:
Expand All @@ -246,22 +203,11 @@ bool type_from_value( std::string & type, json const & j )
break;

case json::value_t::number_unsigned:
// Numbers are UNSIGNED by default: only if a sign is used will it go to 'integer'.
// I.e., 0 is unsigned, -0 is signed.
// Float > Integer > Unsigned
if( type.length() == 8 && type == "unsigned" )
return true;
if( type.empty() )
return type.assign( "unsigned", 8 ), true; // unsigned trumps integer
if( type == "float" || type == "int" )
return true; // float & integer trump unsigned
break;

case json::value_t::number_integer:
// Float > Integer > Unsigned
// Float > Integer/Unsigned
if( type.length() == 3 && type == "int" )
return true;
if( type.empty() || type == "unsigned" )
if( type.empty() )
return type.assign( "int", 3 ), true;
if( type == "float" )
return true; // float trumps integer
Expand Down Expand Up @@ -313,10 +259,6 @@ static std::string parse_type( json const & j, size_t size, dds_option::option_p
if( p == "boolean" )
return props.erase( p ), p;
break;
case 8:
if( p == "unsigned" )
return props.erase( p ), p;
break;
case 4:
if( p == "IPv4" )
return props.erase( p ), p;
Expand All @@ -343,7 +285,7 @@ static std::string parse_type( json const & j, size_t size, dds_option::option_p
break;

default:
DDS_THROW( runtime_error, "unexected size " << size << " of unsigned option json" );
DDS_THROW( runtime_error, "unexected size " << size << " of option json" );
}
DDS_THROW( runtime_error, "cannot deduce value type: " << j );
}
Expand All @@ -357,8 +299,6 @@ static std::string parse_type( json const & j, size_t size, dds_option::option_p
return std::make_shared< dds_string_option >();
if( type == "int" )
return std::make_shared< dds_integer_option >();
if( type == "unsigned" )
return std::make_shared< dds_unsigned_option >();
//if( type == "boolean" )
// return std::make_shared< dds_boolean_option >();
if( type == "IPv4" )
Expand Down Expand Up @@ -512,33 +452,6 @@ void dds_integer_option::check_type( json const & value ) const
}


/*static*/ dds_unsigned_option::type dds_unsigned_option::check_unsigned( json const & value )
{
switch( value.type() )
{
case json::value_t::number_unsigned:
return value.get< type >();

case json::value_t::number_integer:
if( value.get< json::number_integer_t >() < 0 )
break;
return value.get< type >();

case json::value_t::number_float:
if( value.get< type >() != value.get< json::number_float_t >() )
break;
return value.get< type >();
}
DDS_THROW( runtime_error, "not convertible to an unsigned integer: " << value );
}


void dds_unsigned_option::check_type( json const & value ) const
{
check_unsigned( value );
}


void dds_string_option::check_type( json const & value ) const
{
if( ! value.is_string() )
Expand Down
4 changes: 2 additions & 2 deletions tools/dds/dds-adapter/lrs-device-controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -675,8 +675,8 @@ lrs_device_controller::lrs_device_controller( rs2::device dev, std::shared_ptr<
case RS2_OPTION_TYPE_STRING:
value = changed_option->as_string;
break;
case RS2_OPTION_TYPE_NUMBER:
value = changed_option->as_number_signed;
case RS2_OPTION_TYPE_INTEGER:
value = changed_option->as_integer;
break;
case RS2_OPTION_TYPE_COUNT:
// No value available
Expand Down
26 changes: 13 additions & 13 deletions unit-tests/dds/test-option-value.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@

with test.closure( 'r/o options are still settable' ):
# NOTE: the DDS options do not enforce logic post initialization; they serve only to COMMUNICATE any state and limits
test.check_equal( dds.option.from_json( ['1', 0, 'desc'] ).value_type(), "unsigned" )
dds.option.from_json( ['1', 0, 'desc'] ).set_value( 20. ) # OK because 20.0 can be expressed as unsigned
test.check_equal( dds.option.from_json( ['1', 0, 'desc'] ).value_type(), 'int' )
dds.option.from_json( ['1', 0, 'desc'] ).set_value( 20. ) # OK because 20.0 can be expressed as int
test.check_throws( lambda:
dds.option.from_json( ['1', 0, 'desc'] ).set_value( 20.5 ),
RuntimeError, 'not convertible to an unsigned integer: 20.5' )
RuntimeError, 'not convertible to a signed integer: 20.5' )

with test.closure( 'optional (default) value' ):
test.check_false( dds.option.from_json( ['1', 0, 'desc'] ).is_optional() )
Expand All @@ -32,21 +32,21 @@
dds.option.from_json( ['5', None, 'default-string-value', 'desc', ['optional']] )

with test.closure( 'mixed types' ):
test.check_equal( dds.option.from_json( ['i', 0, 'desc'] ).value_type(), 'unsigned' )
test.check_equal( dds.option.from_json( ['i', 0, 'desc', ['float']] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['i', 0, 'desc'] ).value_type(), 'int' )
test.check_equal( dds.option.from_json( ['f', 0, 'desc', ['float']] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['1', 0., 0, 1, 1, 0, 'desc'] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['2', 0, 0., 1, 1, 0, 'desc'] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['3', 0, -1, 4, 1, 0, 'desc'] ).value_type(), 'int' )
test.check_throws( lambda:
dds.option.from_json( ['3u', 0, -1, 4, 1, 0, 'desc', ['unsigned']] ),
RuntimeError, 'not convertible to an unsigned integer: -1' )
test.check_equal( dds.option.from_json( ['3f', 3, 1, 5, 1, 2., ''] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['3f', 3, 1, 5, 1, 2., '', ['int']] ).value_type(), 'int' )
test.check_throws( lambda:
test.check_equal( dds.option.from_json( ['3', 0, 0, 1., 1, 0, 'desc'] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['4', 0, 0, 1, 1., 0, 'desc'] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['5', 0, 0, 1, 1, 0., 'desc'] ).value_type(), 'float' )
test.check_equal( dds.option.from_json( ['-1', 0, -1, 4, 1, 0, 'desc'] ).value_type(), 'int' )
test.check_equal( # float, but forced to an int
dds.option.from_json( ['3f', 3, 1, 5, 1, 2., '', ['int']] ).value_type(), 'int' )
test.check_throws( lambda: # same, but with an actual float
dds.option.from_json( ['3f', 3, 1, 5, 1, 2.2, '', ['int']] ),
RuntimeError, 'not convertible to a signed integer: 2.2' )

test.check_equal( dds.option.from_json( ["Brightness",0,-64,64,1,0,"UVC image brightness"] ).value_type(), 'int' )
test.check_equal( dds.option.from_json( ['Brightness',0,-64,64,1,0,'UVC image brightness'] ).value_type(), 'int' )

with test.closure( 'range' ):
test.check_throws( lambda:
Expand Down
8 changes: 4 additions & 4 deletions wrappers/python/pyrs_options.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* License: Apache 2.0. See LICENSE file in root directory.
Copyright(c) 2017 Intel Corporation. All Rights Reserved. */
// License: Apache 2.0. See LICENSE file in root directory.
// Copyright(c) 2024 Intel Corporation. All Rights Reserved.

#include "pyrealsense2.h"
#include <librealsense2/hpp/rs_options.hpp>
Expand All @@ -20,8 +20,8 @@ void init_options(py::module &m) {
value = py::float_( value_->as_float );
else if( RS2_OPTION_TYPE_STRING == value_->type )
value = py::str( value_->as_string );
else if( RS2_OPTION_TYPE_NUMBER == value_->type )
value = py::int_( value_->as_number_signed );
else if( RS2_OPTION_TYPE_INTEGER == value_->type )
value = py::int_( value_->as_integer );
else
value = py::cast< py::none >( Py_None );
}
Expand Down

0 comments on commit 75e2443

Please sign in to comment.