Skip to content

Add cmdline args to realsense-viewer - #12175

Merged
maloel merged 10 commits into
realsenseai:developmentfrom
maloel:tclap
Sep 11, 2023
Merged

Add cmdline args to realsense-viewer#12175
maloel merged 10 commits into
realsenseai:developmentfrom
maloel:tclap

Conversation

@maloel

@maloel maloel commented Sep 10, 2023

Copy link
Copy Markdown
Contributor
  • add --debug to force debug output to console
  • add --sw-only to enable easier DDS-only debugging (same flag as in rs-enumerate-devices)

@maloel
maloel requested a review from OhadMeir September 10, 2023 11:15
Comment thread src/context.cpp
Comment thread common/viewer.h
press_button_model grid_object_button{ u8"\uf1cb", u8"\uf1cb", "Configure Grid", "Configure Grid", false };

viewer_model(context &ctx_);
viewer_model(context &ctx_, bool disable_log_to_console = false );

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.

For now this is OK, but if there will be more options controlled then we need to pass in a more flexible way, like JSON.

@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

{
TCLAP::CmdLine cmd( "realsense-viewer", ' ', RS2_API_FULL_VERSION_STR );
#ifdef BUILD_EASYLOGGINGPP
TCLAP::SwitchArg debug_arg( "", "debug", "Turn on LibRS debug logs" );

@Nir-Az Nir-Az Sep 10, 2023

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.

Add to docs please.
Both tools needs CLI parameters documented

@Nir-Az Nir-Az left a comment

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.

Add to docs please.
Both tools needs CLI parameters documented

@maloel

maloel commented Sep 10, 2023

Copy link
Copy Markdown
Contributor Author

Add to docs please. Both tools needs CLI parameters documented

Need is quite a strong word. They document it by supporting --help. Both tools are development tools. Adding the same lines to some web page (which I've never seen, BTW) is not helpful IMO...

@Nir-Az

Nir-Az commented Sep 10, 2023

Copy link
Copy Markdown
Collaborator

Add to docs please. Both tools needs CLI parameters documented

Need is quite a strong word. They document it by supporting --help. Both tools are development tools. Adding the same lines to the some web page (which I've never seen, BTW) is not helpful IMO...

Need means that we post a table of parameters and if someone add a new flag or change the table needs to be changed.
image

If you think the table is not needed we can speak about it.
But if it's there it should reflect the code.

@maloel

maloel commented Sep 10, 2023

Copy link
Copy Markdown
Contributor Author

If you think the table is not needed we can speak about it.

I think it's not needed.

@Nir-Az

Nir-Az commented Sep 10, 2023

Copy link
Copy Markdown
Collaborator

If you think the table is not needed we can speak about it.

I think it's not needed.

I think it gives the user the ability to see the tool capabilities before trying to build and run.
I use it alot when users ask how to check or do something.
I sent a link to the readme.
Not every time the user want to do the whole process just to see what can be done with this tool.
Not all are SW developers..

Comment thread src/context.cpp
}

if( mask & RS2_PRODUCT_LINE_NON_INTEL )
if( mask & RS2_PRODUCT_LINE_NON_INTEL && ! ( mask & RS2_PRODUCT_LINE_SW_ONLY ) )

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.

Note that this kind of changes needs to be added on other products as well and will not shown as conflicts.

We need to be carefull with those refactors

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.

This is compatible with all products. The flag is the --sw-only flag is is mostly on for our development and in unit-tests. We are careful.

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.

I'll add the flag when I do the PR with the 555e

@Nir-Az Nir-Az changed the title Add cmdline args to realsense-viewer Add cmdline args to realsense-viewer & rs-enumerate Sep 11, 2023
@maloel
maloel merged commit cb49aca into realsenseai:development Sep 11, 2023
@maloel
maloel deleted the tclap branch September 11, 2023 07:12
@Nir-Az Nir-Az changed the title Add cmdline args to realsense-viewer & rs-enumerate Add cmdline args to realsense-viewer Sep 11, 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