Skip to content

Options registry for non-built-in DDS options - #12163

Merged
maloel merged 13 commits into
realsenseai:developmentfrom
maloel:byname
Sep 6, 2023
Merged

maloel merged 13 commits into
realsenseai:developmentfrom
maloel:byname

Conversation

@maloel

@maloel maloel commented Sep 4, 2023

Copy link
Copy Markdown
Contributor

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
Comment thread src/core/CMakeLists.txt Resolved

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Comment thread src/core/options-registry.cpp Resolved
Comment thread third-party/realdds/py/pyrealdds.cpp Resolved

@OhadMeir OhadMeir left a comment

Copy link
Copy Markdown
Contributor

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 realsenseai: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