Skip to content

revise dds metadata handling with overrides & depth-units #12394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Nov 13, 2023

Conversation

maloel
Copy link
Contributor

@maloel maloel commented Nov 12, 2023

  • add nested functionality in rsutils::json
  • improve reliability debug output for writers/readers
  • fix software-device matcher creation:
    • the syncer showed: (TS: C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 C0 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR4 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 IR6 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 D2 M8 M8)
    • it now takes only unique (stream-uid) values, so now (TS: C0 D2 IR4 IR6)
    • device-proxy now uses the DLR_C matcher, which is also overridable in the settings
  • increase default reply timeout to 2 seconds
    • we were almost consistently running into the timeout right after stopping the sensor, even on the same computer
  • fix depth-unit
    • now set even when no MD
    • localized in depth-sensor-proxy
    • has a default value of 0.001, without which depth shows up as black

Tracked on [LRS-956]

@maloel maloel requested a review from OhadMeir November 12, 2023 07:51
{
if( auto value = rsutils::json::nested( md, keystr ) )
{
if( value->is_number_integer() )
Copy link
Contributor

Choose a reason for hiding this comment

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

If we will get something else, don't we want to infrom the user?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. This is metadata; we ignore anything we cannot parse.
Same behavior as before -- just doesn't throw exceptions as much (which in Debug mode are constantly triggered).

Copy link
Contributor

Choose a reason for hiding this comment

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

Are we getting bad values in debug? DDS layer is supposed to check the message...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the exceptions were when we just didn't find the values for the metadata

for (auto&& p : s->get_stream_profiles())
profiles.push_back(p.get());
for( auto const & s : _software_sensors )
for( auto const & p : s->_sw_profiles )
Copy link
Contributor

Choose a reason for hiding this comment

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

Calling get_raw_stream_profiles() is clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't exist any more :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is get_stream_profiles() or something like it but it returns a vector, not a reference. Since we're a software device and these are software sensors, I elected to access directly.

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

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.

2 participants