Skip to content

fix non-DDS settings initialization in rs-terminal and -enumerate-devices - #12567

Merged
maloel merged 1 commit into
realsenseai:developmentfrom
maloel:settings
Jan 11, 2024
Merged

fix non-DDS settings initialization in rs-terminal and -enumerate-devices#12567
maloel merged 1 commit into
realsenseai:developmentfrom
maloel:settings

Conversation

@maloel

@maloel maloel commented Jan 11, 2024

Copy link
Copy Markdown
Contributor

It's really hard to see, but when we pass in settings to the context they must be either empty or an object. But json initializes to null and librealsense correctly throws.

This fixes two use-cases which were missed before, in rs-terminal and rs-enumerate-devices -- especially outside of DDS mode which usually force an object.

Tracked on [HSD-15015096962]

@maloel
maloel requested a review from OhadMeir January 11, 2024 12:25

// Obtain a list of devices currently present on the system
json settings;
json settings = json::object();

@OhadMeir OhadMeir Jan 11, 2024

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.

Should we update all occurrences of json j; to json j = json::object() or is it relevant only to the settings usage?

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.

Relevant only to the settings

@OhadMeir

Copy link
Copy Markdown
Contributor

"It's really hard to see ..."

Can we test it here? Prevent further cases of null json

@maloel

maloel commented Jan 11, 2024

Copy link
Copy Markdown
Contributor Author

Can we test it here? Prevent further cases of null json

No, I think it's a valid complaint that the settings should make sense -- and sending null is not valid...

@maloel
maloel merged commit 61601c6 into realsenseai:development Jan 11, 2024
@maloel
maloel deleted the settings branch January 11, 2024 12:48
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