Skip to content

Commit

Permalink
Updated findfile() to search localpath first (#1292)
Browse files Browse the repository at this point in the history
As the issue above mentions, this tutorial for Garden, is not able to parse the SDF file if it was named model.sdf. It works if the file name is changed. This was happening due to the search order used in the sdf::findFile() function. The used search order was causing the findFile() to find the model.sdf from the install path instead of the one present locally. Since the found model.sdf is actually the SDF spec file for the <model> and it is not a valid SDFormat file, it caused the script to throw a not valid error.
This PR changes the search order used in the findFile() function by searching for the local path (current directory) first and solves the issue.
---------

Signed-off-by: Jasmeet Singh <jasmeet0915@gmail.com>
Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org>
  • Loading branch information
jasmeet0915 and azeey authored Jun 26, 2023
1 parent b363e0e commit da1f325
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 17 deletions.
1 change: 1 addition & 0 deletions Migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ but with improved human-readability..

1. ParserConfig defaults to WARN instead of LOG when parsing unrecognized
elements.
2. Updated search order for `sdf::findFile()` making local path (current directory) the first to be searched.

### Deprecations

Expand Down
12 changes: 6 additions & 6 deletions include/sdf/SDFImpl.hh
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ namespace sdf
/// The search order in the function is as follows:
/// 1. Using the global URI path map, search in paths associated with the URI
/// scheme of the input.
/// 2. Seach in the path defined by the macro `SDF_SHARE_PATH`.
/// 3. Search in the the libsdformat install path. The path is formed by
/// has the pattern `SDF_SHARE_PATH/sdformat<major version>/<version>/`
/// 4. Directly check if the input path exists in the filesystem.
/// 5. Seach in the path defined by the environment variable `SDF_PATH`.
/// 6. If enabled via _searchLocalPath, prepend the input with the current
/// 2. If enabled via _searchLocalPath, prepend the input with the current
/// working directory and check if the result path exists.
/// 3. Seach in the path defined by the macro `SDF_SHARE_PATH`.
/// 4. Search in the the libsdformat install path. The path is formed by
/// has the pattern `SDF_SHARE_PATH/sdformat<major version>/<version>/`
/// 5. Directly check if the input path exists in the filesystem.
/// 6. Seach in the path defined by the environment variable `SDF_PATH`.
/// 7. If enabled via _useCallback and the global callback function is set,
/// invoke the function and return its result.
///
Expand Down
23 changes: 12 additions & 11 deletions src/SDF.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,17 @@ std::string findFile(const std::string &_filename, bool _searchLocalPath,
filename = filename.substr(idx + sep.length());
}

// First check the local path, if the flag is set.
if (_searchLocalPath)
{
std::string path = sdf::filesystem::append(sdf::filesystem::current_path(),
filename);
if (sdf::filesystem::exists(path))
{
return path;
}
}

// Next check the install path.
std::string path = sdf::filesystem::append(SDF_SHARE_PATH, filename);
if (sdf::filesystem::exists(path))
Expand All @@ -113,7 +124,7 @@ std::string findFile(const std::string &_filename, bool _searchLocalPath,
return path;
}

// Next check to see if the given file exists.
// Finally check to see if the given file exists.
path = filename;
if (sdf::filesystem::exists(path))
{
Expand All @@ -135,16 +146,6 @@ std::string findFile(const std::string &_filename, bool _searchLocalPath,
}
}

// Finally check the local path, if the flag is set.
if (_searchLocalPath)
{
path = sdf::filesystem::append(sdf::filesystem::current_path(), filename);
if (sdf::filesystem::exists(path))
{
return path;
}
}

// If we still haven't found the file, use the registered callback if the
// flag has been set
if (_useCallback)
Expand Down
28 changes: 28 additions & 0 deletions src/SDF_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -788,4 +788,32 @@ TEST(SDF, WriteURIPath)
ASSERT_EQ(std::remove(tempFile.c_str()), 0);
ASSERT_EQ(rmdir(tempDir.c_str()), 0);
}

/////////////////////////////////////////////////
TEST(SDF, FindFileModelSDFCurrDir)
{
std::string currDir;

// Get current directory path from $PWD env variable
currDir = sdf::filesystem::current_path();

// A file named model.sdf in current directory
auto tempFile = currDir + "/model.sdf";

sdf::SDF tempSDF;
tempSDF.Write(tempFile);

// Check file was created
auto fp = fopen(tempFile.c_str(), "r");
ASSERT_NE(nullptr, fp);

// Get path of the file returned from findFile()
std::string foundFile = sdf::findFile("model.sdf", true, false);

// Check that the returned file is model.sdf from curr directory
ASSERT_EQ(tempFile, foundFile);

// Cleanup
ASSERT_EQ(std::remove(tempFile.c_str()), 0);
}
#endif // _WIN32

0 comments on commit da1f325

Please sign in to comment.