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

Support for generic URIs #464

Merged
merged 58 commits into from
Aug 5, 2015
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
fbeb903
Added the ResourceRetriever concept.
Jul 12, 2015
f6b2708
Merge branch 'bugfix/DartLoaderSEGFAULT' into feature/ResourceRetriever
Jul 12, 2015
f2b619b
Cleanup.
Jul 12, 2015
f64234f
Added DAE rotation logic to MeshShape::loadMesh.
Jul 12, 2015
e896ccf
Added ResourceRetriever::exists.
Jul 13, 2015
755c00d
Switched to a stream-based URI.
Jul 14, 2015
0ae6658
Removed MemoryResource.
Jul 14, 2015
02028bc
Refactoring in preparation for static.
Jul 14, 2015
2b2121c
Added tests for LocalResourceRetriever.
Jul 18, 2015
82993bc
Added tests for PackageResourceRetriever.
Jul 18, 2015
6bf445b
Refactoring DartLoader to use ResourceRetriever.
Jul 18, 2015
224ff23
Tested RelativeResourceRetriever.
Jul 19, 2015
23eaddc
Added URI parsing utilities.
Jul 19, 2015
1853280
Better support for relative URIs.
Jul 20, 2015
8a7958c
Cleaned up URDF world loading.
Jul 20, 2015
5f05789
Removed more member variables from DartLoader.
Jul 20, 2015
0e3d718
Mesh loading cleanup.
Jul 20, 2015
533fa53
Implement removeDotSegments.
Jul 20, 2015
3dd3b11
Implemented URI joining.
Jul 20, 2015
ac28d8f
Made most of DartLoader static.
Jul 20, 2015
ccb1316
Loading Robonaut works again.
Jul 20, 2015
f347112
Optimized the regex.
Jul 20, 2015
483613e
Added unit tests for SchemaResourceRetriever.
Jul 21, 2015
f5e1cee
Non-backwards compatable SDF changes.
Jul 21, 2015
fce0a78
Added SDFParser backwards compatability.
Jul 21, 2015
9fe0c18
Use ResourceRetriever for SDF mesh loading.
Jul 21, 2015
f96e3e1
Modified openXMLFile to use ResourceRetriever.
Jul 21, 2015
956e0c5
Updated SkelParser w/o backwards compat.
Jul 21, 2015
4467332
Fixed backwards compatability.
Jul 21, 2015
a08abd5
Load meshes using relative URIs in SkelParser.
Jul 21, 2015
238b112
Removed unnecessary isUri flag.
Jul 21, 2015
6618f95
Renamed UriUtils to Uri.
Jul 21, 2015
c214bab
Moved ResourceRetriever and Uri into common.
Jul 21, 2015
d469307
Moved LocalResourceRetriever to common.
Jul 21, 2015
73fa9c3
Moved AssimpResourceAdaptor to dynamics.
Jul 21, 2015
7eb99be
Added documentation.
Jul 22, 2015
8d791ec
Renamed getFileSize to getSize.
Jul 22, 2015
c7e2245
Fixed some unittest bugs.
Jul 22, 2015
f44394e
Fixed a typo.
Jul 22, 2015
14414c5
Merge remote-tracking branch 'upstream/master' into feature/ResourceR…
Jul 22, 2015
d849427
Merge remote-tracking branch 'upstream/master' into feature/ResourceR…
Jul 27, 2015
af7cb60
Fixed line wrapping in comments.
Jul 27, 2015
450e06c
Added documentation to Uri.
Jul 27, 2015
7cc7cc3
Fixed indention.
Jul 27, 2015
9cb8e11
Removed dead code from MeshShape.
Jul 27, 2015
2729eb4
Removed default destructors to fix linking issues.
Aug 3, 2015
5a737b6
Added strerror to LocalResource warnings.
Aug 4, 2015
ac72ac7
Fixed log messages in LocalResourceRetriever.
Aug 4, 2015
dc4bc2c
Marked Uri and UriComponent as final.
Aug 4, 2015
20bbf8d
Switched dtwarn in AssimpInputResourceAdaptor.
Aug 4, 2015
b74ed66
Updated PackageResourceRetriever to use Uri.
Aug 4, 2015
a9c0042
Fixed a bunch of minor formatting issues.
Aug 4, 2015
54dceab
Fixed syntax errors.
Aug 4, 2015
913e945
Added delimiter comments to Uri.
Aug 4, 2015
9171d34
Added comments to AssimpInputResourceAdaptor.
Aug 4, 2015
7c423c3
Renamed Schema -> CompositeResourceRetriever.
Aug 4, 2015
0dd9c52
Removed unused function declaration.
Aug 4, 2015
0b4de8d
Fixed more formatting issues.
Aug 5, 2015
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ endif()
add_definitions(-DBOOST_TEST_DYN_LINK)
set(Boost_USE_MULTITHREADED ON)
set(Boost_USE_STATIC_RUNTIME OFF)
find_package(Boost ${DART_MIN_BOOST_VERSION} COMPONENTS system QUIET)
find_package(Boost ${DART_MIN_BOOST_VERSION} COMPONENTS regex system QUIET)
if(Boost_FOUND)
message(STATUS "Looking for Boost - ${Boost_MAJOR_VERSION}.${Boost_MINOR_VERSION}.${Boost_SUBMINOR_VERSION} found")
else()
Expand Down
139 changes: 139 additions & 0 deletions dart/common/LocalResource.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
#include <cstring>
Copy link
Member

Choose a reason for hiding this comment

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

License block is missing.

#include <iostream>
#include "dart/common/Console.h"
#include "LocalResource.h"

namespace dart {
namespace common {

Copy link
Member

Choose a reason for hiding this comment

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

We usually place horizontal separator before function definition as:

//==============================================================================
LocalResource::LocalResource(const std::string& _path)
{
  // ...
}

LocalResource::LocalResource(const std::string& _path)
: mFile(std::fopen(_path.c_str(), "rb"))
{
if(!mFile)
{
dtwarn << "[LocalResource::constructor] Failed opening file '" << _path
<< "' for reading: " << std::strerror(errno) << "\n";
Copy link
Member

Choose a reason for hiding this comment

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

vertical alignment (here and following comments)

}
}

LocalResource::~LocalResource()
{
if (!mFile)
return;

if (std::fclose(mFile) == EOF)
{
dtwarn << "[LocalResource::destructor] Failed closing file.\n";
Copy link
Member

Choose a reason for hiding this comment

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

We should probably print std::strerror(errno) here. According to this reference errno should get set when an error occurs in fclose.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Interesting. The documentation I found just says that the function returns EOF. It can't hurt to check errno and, if it's set, print the std::strerror(errno) anyway.

Copy link
Member

Choose a reason for hiding this comment

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

The reference I linked to is basically an internet copy of the GNU Linux Programmer's manual, so it should be legit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It might be true on GNU/Linux, but I don't know if it's guaranteed by the C++ standard.

}
}

bool LocalResource::isGood() const
{
return !!mFile;
}

size_t LocalResource::getSize()
{
if(!mFile)
return 0;

const long offset = std::ftell(mFile);
if(offset == -1L)
{
dtwarn << "[LocalResource::getSize] Unable to compute file size: Failed"
" getting current offset: " << std::strerror(errno) << "\n";
return 0;
}

// The SEEK_END option is not required by the C standard. However, it is
// required by POSIX.
// TODO: Does this work on Windows?
if(std::fseek(mFile, 0, SEEK_END) || std::ferror(mFile))
{
dtwarn << "[LocalResource::getSize] Unable to compute file size: Failed"
" seeking to the end of the file.\n";
Copy link
Member

Choose a reason for hiding this comment

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

I think fseek also sets errno.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The documentation I found says only that:

If a read or write error occurs, the error indicator for the stream (std::ferror) is set and the file position is unaffected.

I don't think there is any guarantee that the value returned by ferror has the same meaning as errno.

Again, the documentation that you found says that fseek sets errno. I'm not sure what the deal is.

return 0;
}

const long size = std::ftell(mFile);
if(size == -1L)
{
dtwarn << "[LocalResource::getSize] Unable to compute file size: Failed"
" getting end of file offset: " << std::strerror(errno) << "\n";
return 0;
}

if(std::fseek(mFile, offset, SEEK_SET) || std::ferror(mFile))
{
dtwarn << "[LocalResource::getSize] Unable to compute file size: Failed"
" seeking to the current position.\n";
return 0;
}

return size;
}

size_t LocalResource::tell()
{
if(!mFile)
return 0;

const long offset = std::ftell(mFile);
if (offset == -1L)
{
dtwarn << "[LocalResource::tell] Failed"
" seeking to the current position.\n";
Copy link
Member

Choose a reason for hiding this comment

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

Could use a std::strerror(errno) here as well, I think.

}

// We return -1 to match the beahvior of DefaultIoStream in Assimp.
return offset;
}

bool LocalResource::seek(ptrdiff_t _offset, SeekType _mode)
{
int origin;
switch(_mode)
{
case Resource::SEEKTYPE_CUR:
origin = SEEK_CUR;
break;

case Resource::SEEKTYPE_END:
origin = SEEK_END;
break;

case Resource::SEEKTYPE_SET:
origin = SEEK_SET;
break;

default:
dtwarn << "[LocalResource::Seek] Invalid origin. Expected"
" SEEKTYPE_CUR, SEEKTYPE_END, or SEEKTYPE_SET.\n";
return false;
}

if (!std::fseek(mFile, _offset, origin) && !std::ferror(mFile))
return true;
else
{
dtwarn << "[LocalResource::seek] Seeking failed.\n";
return false;
}
}

size_t LocalResource::read(void *_buffer, size_t _size, size_t _count)
{
if (!mFile)
return 0;

const size_t result = std::fread(_buffer, _size, _count, mFile);
if (std::ferror(mFile))
{
dtwarn << "[LocalResource::tell] Failed reading file.\n";
Copy link
Member

Choose a reason for hiding this comment

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

This should be [LocalResource::read].

}
return result;
}

} // namespace common
} // namespace dart

74 changes: 74 additions & 0 deletions dart/common/LocalResource.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
/*
* Copyright (c) 2015, Georgia Tech Research Corporation
* All rights reserved.
*
* Author(s): Michael Koval <mkoval@cs.cmu.edu>
*
* Georgia Tech Graphics Lab and Humanoid Robotics Lab
*
* Directed by Prof. C. Karen Liu and Prof. Mike Stilman
* <karenliu@cc.gatech.edu> <mstilman@cc.gatech.edu>
*
* This file is provided under the following "BSD-style" License:
* Redistribution and use in source and binary forms, with or
* without modification, are permitted provided that the following
* conditions are met:
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
* USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/
Copy link
Member

Choose a reason for hiding this comment

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

please add empty line after license block

#ifndef DART_COMMON_LOCALRESOURCE_H_
#define DART_COMMON_LOCALRESOURCE_H_
Copy link
Member

Choose a reason for hiding this comment

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

please add empty line after header guard

#include "dart/common/Resource.h"

namespace dart {
namespace common {

class LocalResource : public virtual Resource
{
public:
LocalResource(const std::string& _path);
Copy link
Member

Choose a reason for hiding this comment

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

explicit LocalResource(const std::string& _path);

to prevent unintended implicit type conversion

virtual ~LocalResource();

LocalResource(const LocalResource& _other) = delete;
LocalResource& operator=(const LocalResource& _other) = delete;

/// Return if the resource is open and in a valid state.
bool isGood() const;

// Documentation inherited.
size_t getSize() override;

// Documentation inherited.
size_t tell() override;

// Documentation inherited.
bool seek(ptrdiff_t _origin, SeekType _mode) override;

// Documentation inherited.
size_t read(void *_buffer, size_t _size, size_t _count) override;
Copy link
Member

Choose a reason for hiding this comment

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

please put * right after type as:

size_t read(void* _buffer, size_t _size, size_t _count) override;


private:
std::FILE* mFile;
};

} // namespace common
} // namespace dart

#endif // ifndef DART_COMMON_LOCALRESOURCERETRIEVER_H_
Copy link
Member

Choose a reason for hiding this comment

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

#endif // ifndef DART_COMMON_LOCALRESOURCE_H_

48 changes: 48 additions & 0 deletions dart/common/LocalResourceRetriever.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
#include <iostream>
Copy link
Member

Choose a reason for hiding this comment

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

license block missing

#include <fstream>
#include "dart/common/Console.h"
#include "dart/common/Uri.h"
#include "LocalResourceRetriever.h"
#include "LocalResource.h"

namespace dart {
namespace common {

bool LocalResourceRetriever::exists(const std::string& _uri)
{
common::Uri uri;
if(!uri.fromString(_uri))
{
dtwarn << "[exists] Failed parsing URI: " << _uri << "\n";
Copy link
Member

Choose a reason for hiding this comment

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

[LocalResourceRetriever::exists] would follow our usual pattern.

return false;
}

// Open and close the file to check if it exists. It would be more efficient
// to stat() it, but that is not portable.
if (uri.mScheme.get_value_or("file") != "file")
return false;

return std::ifstream(*uri.mPath, std::ios::binary).good();
}

common::ResourcePtr LocalResourceRetriever::retrieve(const std::string& _uri)
{
common::Uri uri;
if(!uri.fromString(_uri))
{
dtwarn << "[exists] Failed parsing URI: " << _uri << "\n";
Copy link
Member

Choose a reason for hiding this comment

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

This should be [LocalResourceRetriever::retrieve]

return nullptr;
}

if (uri.mScheme.get_value_or("file") != "file")
return nullptr;

const auto resource = std::make_shared<LocalResource>(*uri.mPath);
if(resource->isGood())
return resource;
else
return nullptr;
}

} // namespace common
} // namespace dart
62 changes: 62 additions & 0 deletions dart/common/LocalResourceRetriever.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright (c) 2015, Georgia Tech Research Corporation
* All rights reserved.
*
* Author(s): Michael Koval <mkoval@cs.cmu.edu>
*
* Georgia Tech Graphics Lab and Humanoid Robotics Lab
*
* Directed by Prof. C. Karen Liu and Prof. Mike Stilman
* <karenliu@cc.gatech.edu> <mstilman@cc.gatech.edu>
*
* This file is provided under the following "BSD-style" License:
* Redistribution and use in source and binary forms, with or
* without modification, are permitted provided that the following
* conditions are met:
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above
* copyright notice, this list of conditions and the following
* disclaimer in the documentation and/or other materials provided
* with the distribution.
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
* CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
* INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
* MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
* DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
* USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
* LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
* ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
* POSSIBILITY OF SUCH DAMAGE.
*/
Copy link
Member

Choose a reason for hiding this comment

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

please add empty line

#ifndef DART_COMMON_LOCALRESOURCERETRIEVER_H_
#define DART_COMMON_LOCALRESOURCERETRIEVER_H_
Copy link
Member

Choose a reason for hiding this comment

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

please add empty line

#include "dart/common/ResourceRetriever.h"

namespace dart {
namespace common {

/// LocalResourceRetriever provides access to local resources specified by
/// file:// URIs by wrapping the standard C and C++ file manipulation routines.
class LocalResourceRetriever : public virtual ResourceRetriever
{
public:
virtual ~LocalResourceRetriever() = default;

// Documentation inherited.
bool exists(const std::string& _uri) override;

// Documentation inherited.
ResourcePtr retrieve(const std::string& _uri) override;
};

typedef std::shared_ptr<LocalResourceRetriever> LocalResourceRetrieverPtr;
Copy link
Member

Choose a reason for hiding this comment

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

How about using using rather than typedef for type alias as:

using LocalResourceRetrieverPtr = std::shared_ptr<LocalResourceRetriever>;

They are same but it seems using is more preferred.


} // namespace common
} // namespace dart

#endif // ifndef DART_COMMON_LOCALRESOURCERETRIEVER_H_
Loading