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

[apps/src, openni files] Inconsistencies in the command line args #5491

Closed
filbert14 opened this issue Oct 25, 2022 · 2 comments
Closed

[apps/src, openni files] Inconsistencies in the command line args #5491

filbert14 opened this issue Oct 25, 2022 · 2 comments
Labels

Comments

@filbert14
Copy link
Contributor

filbert14 commented Oct 25, 2022

Now that #5475 is closed, I hope to able able to make following changes:

As several maintainers pointed out, currently,

  1. Several files require you to input -h to be able to get the output of usage(), some allow -h and --help
    (compare openni_3d_convex_hull.cpp and openni_voxel_grid.cpp)
  2. The variable name arg is used for both device_id and -h or --help
    (see openni_3d_convex_hull.cpp)
  3. For some files, --help or -help can only be used to call usage() if they are the first arguments in the cmd line, some use pcl::console::find_argument(argc, argv, "-h"), where it could be anywhere (same example as 1.)
  4. options is shown for the output of usage() for some files where no options can be given
    (compare openni_3d_convex_hull.cpp and openni_ii_normal_estimation.cpp, where options is not displayed if options can't be used in main().

Idea:
So that everything says uniform, use std::string device_id instead of std::string arg exclusively for the device id, make it have to be at argv[1] (while checking if it begins with '-' as to not confuse it with options parameter -> see openni_planar_convex_hull.cpp), initialize it to the default value "" so that device_id can safely be passed everytime (grabber looks for first device), allowing it to be optional when its not a cmd line arg and separate it completely from -h or --help (can be at any position, using pcl::console::find_argument(argc, argv, "-h") bzw. pcl::console::find_argument(argc, argv, "--help"))

+Make small changes by deleting options in usage() where none can/needs to be passed

I think it would be a good idea to start with the same files worked on the last time:

apps/3d_rec_framework/src/tools/openni_frame_source.cpp
apps/src/openni_tracking.cpp
apps/src/openni_boundary_estimation.cpp
apps/src/openni_klt.cpp
apps/src/openni_organized_edge_detection.cpp
apps/src/openni_mls_smoothing.cpp
apps/src/openni_uniform_sampling.cpp
apps/src/openni_ii_normal_estimation.cpp
apps/src/openni_grab_images.cpp
apps/src/openni_voxel_grid.cpp
apps/src/openni_feature_persistence.cpp
apps/src/openni_3d_concave_hull.cpp
apps/src/openni_fast_mesh.cpp
apps/src/openni_planar_convex_hull.cpp
apps/src/face_detection/openni_frame_source.cpp
apps/src/openni_planar_segmentation.cpp
apps/src/openni_color_filter.cpp
apps/src/openni_3d_convex_hull.cpp
apps/src/openni_mobile_server.cpp
@filbert14 filbert14 added the status: triage Labels incomplete label Oct 25, 2022
@mvieth
Copy link
Member

mvieth commented Oct 26, 2022

Sounds good, feel free to open a pull request 👍

mvieth pushed a commit that referenced this issue Jan 31, 2023
* #5491

* Fixed typo at line 212 in parse.h

* Fixed formatting.

* Deleted whitespaces.

* Dependency update before attempting fix.

* Fixed formatting style.

* Revert . . formatting?

* Weird formatting error test?

* Fixed dependency, this should hopefully build.

* Removed unused variable in openni_ii_normal_estimation.cpp

* Build test after improvements and refactors.

* Build Test #2.

* deleted whitespace.

* Fixed formatting errors.

* Fixed formatting openni_boundary_estimation.cpp.

* Fixed formatting openni_boundary_estimation.cpp.

* Fixed formatting openni_boundary_estimation.cpp.

* Formatting check #1.

* Formatting check 2.

* Deleted whitespace.

* Changed general parsing structure, improved usage().

* Fixed formatting errors.

* Fixed!

* resolve merge conflict

* resolve merge conflict

* format test

* new format test

* test formatting

* test formatting

* test formatting

* command line behavior mimics previous version(s), consistent help flags

* rest of the files
@mvieth
Copy link
Member

mvieth commented Jan 31, 2023

I assume your pull request fixes this issue, please correct me if I am wrong.

@mvieth mvieth closed this as completed Jan 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants