Skip to content

Conversation

maloel
Copy link
Contributor

@maloel maloel commented Sep 4, 2023

New options can now be registered by their name/string:

  • Options must have unique names/strings
  • New options (custom options) have negative values
  • DDS sensor options register automatically, so DDS devices can now have freedom from librealsense built-in options
  • Added rs2_option_from_string, the reverse of rs2_option_to_string
  • Support in Python

Various other minor changes along the way. I recommend to just review by commit...

Tracked by [LRS-864]

@maloel maloel requested a review from OhadMeir September 4, 2023 11:37

inline rs2_option index_to_option( size_t index )
{
return rs2_option( -( int( index ) + 1 ) );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider checking if index >= INT_MAX.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must say this function is not readable.
I asked Remi to look and he also didn't understand.
Why all the values are negative?

@maloel please add some comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added:

    // Index = index into the _name_by_index array -> 0-based
    // Registered rs2_option value will be negative, and starting at -1:

Will be in next PR

Copy link
Contributor

@OhadMeir OhadMeir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maloel maloel merged commit a438d85 into IntelRealSense:development Sep 6, 2023
@maloel maloel deleted the byname branch September 6, 2023 07:30
maloel added a commit to maloel/librealsense that referenced this pull request Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants