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

Check if cmd arg file exist for g2o_viewer #785

Merged

Conversation

FelixSchladt
Copy link
Contributor

Hi All,

thanks for this awesome library and the continued support.

We have been working with g2o and g2o_viewer in the last months and normally open the graph files with the viewer providing them as command line argument.

Unfortunately I am not perfect and sometimes provided an incorrect filename, which led to some wrong debugging as I rather questioned my code instead of the filename.

Unfortunately g2o_viewer does not give an error message if the provided file does not exists.
This would be something I would appreciate if this would be added.

Greetings Felix and Jan

g2o_viewer does not check if a specified file exists and opens an empty graph without any error
@@ -55,6 +55,12 @@ int RunG2OViewer::run(int argc, char** argv, CommandArgs& arg) {
"graph file which will be processed", true);
arg.parseArgs(argc, argv);

// Check if given file exists
if (inputFilename.size() > 0 && !std::ifstream(inputFilename)) {
Copy link
Owner

Choose a reason for hiding this comment

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

Call fileExists instead? see https://github.com/RainerKuemmerle/g2o/blob/master/g2o/stuff/filesys_tools.h#L80C20-L80C30
Downside would be it also passes for a directory. We probably could use some filessystem calls instead directly.

Copy link
Contributor Author

@FelixSchladt FelixSchladt Apr 20, 2024

Choose a reason for hiding this comment

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

The current approach with ifstream, does also not check if the file is a directory. Therefore we actually could use the function and would have the same result.

One approach would be to use stat to check the filetype:

bool is_regular_file(const std::string& path) {
    struct stat path_stat;
    stat(path.c_str(), &path_stat);
    return S_ISREG(path_stat.st_mode); // Return true if it's a regular file
}

This works just fine but will only work on posix compliant systems.
So far I did not find a good way to achieve this to work on windows as well as on posix without using preprocessor magic. Additionally I do not have any means to test on windows.

The right way in my opinion would be to use C++17's filesystem library, but for this the c++ version would require a bump.

So this might be a lean efficient compromise

@RainerKuemmerle RainerKuemmerle merged commit 5293e2d into RainerKuemmerle:master Apr 20, 2024
5 checks passed
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