Skip to content
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

viewer presets for dds devices #13074

Merged
merged 7 commits into from
Jun 27, 2024
Merged

Conversation

maloel
Copy link
Collaborator

@maloel maloel commented Jun 24, 2024

Enabled Viewer presets in DDS devices.
The Viewer detects that a device has presets based on implementation of serializable_interface.
Added a unit-test making sure that DDS devices are serializable, and have their Visual Preset set first when loading JSON.

The values for all serialized sensors are kept in <sensor-name>/<option-name> keys when serializing.
On loading, the Visual Preset option is special: it is the baseline for all other options, so it is changed first. Then all the rest of the options are loaded.

Enhanced test.remote so capture of remote stdout is possible.

Read-only options were not tested before. Added a test for those as well, but noticed that they are actually settable (i.e., it was up to the server to check read-only). I don't know what's right at this point, but I think it's safe for LibRS to fail and throw if a read-only option is modified. The code for the regular non-DDS options also seems to not check.

Tracked on [RSDEV-986]

@maloel maloel requested a review from OhadMeir June 24, 2024 08:28
@maloel maloel changed the title Dds preset viewer presets for dds devices Jun 24, 2024
@OhadMeir
Copy link
Contributor

Read-only options were not tested before. Added a test for those as well, but noticed that they are actually settable (i.e., it was up to the server to check read-only). I don't know what's right at this point, but I think it's safe for LibRS to fail and throw if a read-only option is modified. The code for the regular non-DDS options also seems to not check.

I think that back than we discussed it and decided not to check read only. Leave it for the server to fail if trying to update read only.

@OhadMeir
Copy link
Contributor

OhadMeir commented Jun 25, 2024

the Visual Preset option is special: it is the baseline for all other options, so it is changed first. Then all the rest of the options are loaded.

Not sure I understand what is special in the preset value. Changing preset triggers a change in a group of "advanced mode" options, but it does not affect regular options like exposure and such.
I think that "Advanced mode" options are not exposed via DDS

@maloel
Copy link
Collaborator Author

maloel commented Jun 25, 2024

Not sure I understand what is special in the preset value. Changing preset triggers a change in a group of "advanced mode" options, but it does not affect regular options like exposure and such. I think that "Advanced mode" options are not exposed via DDS

The Visual Preset sets parameters that may not even have equivalents available to the user: internal settings. The idea here is that the preset determines the "baseline" on top of which the rest of the settings are then applied.
If still unclear, let's talk.

@maloel maloel merged commit a203640 into IntelRealSense:development Jun 27, 2024
21 of 22 checks passed
@maloel maloel deleted the dds-preset branch June 27, 2024 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants