-
Notifications
You must be signed in to change notification settings - Fork 26
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
Update Python package with new ScenarI/O APIs #176
Changes from all commits
7b0b014
aaf3395
ceccd7f
a113c01
d111c9a
7658ea3
5f802f0
146e397
511a4c0
e49366d
afe72d5
70404c1
3981215
acba20e
2663f19
6fbb653
9e08947
03e8a54
7260696
4d6db91
97241fc
889d455
57338d1
e684c60
db84c8e
5ed481e
6062f12
80f0db7
cf1743a
e632660
5ec561a
2325327
981285e
4ad6ed9
9e8f58e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,18 +38,130 @@ | |
namespace scenario { | ||
namespace gazebo { | ||
namespace utils { | ||
/** | ||
* Set the verbosity process-wise. | ||
* | ||
* Accepted levels are the following: | ||
* | ||
* - ``<= 0``: No messages. | ||
* - ``1``: Error messages. | ||
* - ``2``: Error and warning messages. | ||
* - ``3``: Error, warning, and info messages. | ||
* - ``>= 4``: Error, warning, info, and debug messages. | ||
* | ||
* If called without specifying the level, it will use | ||
* level 2 or level 4 depending if the project was compiled | ||
* respectively with Release or Debug flags. | ||
* | ||
* @param level The verbosity level. | ||
*/ | ||
void setVerbosity(const int level = DEFAULT_VERBOSITY); | ||
|
||
/** | ||
* Find a SDF file in the filesystem. | ||
* | ||
* The search path is defined with the ``IGN_GAZEBO_RESOURCE_PATH`` | ||
* environment variable. | ||
* | ||
* @param fileName The SDF file name. | ||
* @return The absolute path to the file if found, an empty string | ||
* otherwise. | ||
*/ | ||
std::string findSdfFile(const std::string& fileName); | ||
|
||
/** | ||
* Check if a SDF string is valid. | ||
* | ||
* An SDF string could contain for instance an SDF model or | ||
* an SDF world, and it is valid if it can be parsed successfully | ||
* by the SDFormat library. | ||
* | ||
* @param sdfString The SDF string to check. | ||
* @return True if the SDF string is valid, false otherwise. | ||
*/ | ||
bool sdfStringValid(const std::string& sdfString); | ||
|
||
/** | ||
* Get an SDF string from a SDF file. | ||
* | ||
* @param fileName An SDF file. It could be either an absolute path | ||
* to the file or the file name if the parent folder is part | ||
* of the ``IGN_GAZEBO_RESOURCE_PATH`` environment variable. | ||
* @return The SDF string if the file was found and is valid, an | ||
* empty string otherwise. | ||
*/ | ||
std::string getSdfString(const std::string& fileName); | ||
|
||
/** | ||
* Get the name of a model from a SDF file. | ||
* | ||
* @param fileName An SDF file. It could be either an absolute path | ||
* to the file or the file name if the parent folder is part | ||
* of the ``IGN_GAZEBO_RESOURCE_PATH`` environment variable. | ||
* @param modelIndex The index of the model in the SDF file. By | ||
* default it finds the first model. | ||
* @return The name of the model. | ||
*/ | ||
std::string getModelNameFromSdf(const std::string& fileName, | ||
const size_t modelIndex = 0); | ||
|
||
/** | ||
* Get the name of a world from a SDF file. | ||
* | ||
* @param fileName An SDF file. It could be either an absolute path | ||
* to the file or the file name if the parent folder is part | ||
* of the ``IGN_GAZEBO_RESOURCE_PATH`` environment variable. | ||
* @param worldIndex The index of the world in the SDF file. By | ||
* default it finds the first world. | ||
* @return The name of the world. | ||
*/ | ||
std::string getWorldNameFromSdf(const std::string& fileName, | ||
const size_t worldIndex = 0); | ||
|
||
/** | ||
* Return a SDF string with an empty world. | ||
* | ||
* An empty world only has a sun and the default DART | ||
* physics profile enabled. | ||
* | ||
* @note The empty world does not have any ground plane. | ||
* | ||
* @return A SDF string with the empty world. | ||
*/ | ||
std::string getEmptyWorld(); | ||
|
||
/** | ||
* Get a SDF model file from a Fuel URI. | ||
* | ||
* A valid URI has the following form: | ||
* | ||
* ``https://fuel.ignitionrobotics.org/openrobotics/models/model_name`` | ||
* | ||
* @param URI A valid Fuel URI. | ||
* @param useCache Load the model from the local cache. | ||
* @return The absolute path to the SDF model. | ||
*/ | ||
std::string getModelFileFromFuel(const std::string& URI, | ||
const bool useCache = false); | ||
|
||
/** | ||
* Generate a random alpha numeric string. | ||
* | ||
* @param length The length of the string. | ||
* @return The random string. | ||
*/ | ||
std::string getRandomString(const size_t length); | ||
|
||
/** | ||
* Get the install prefix used by the CMake project. | ||
* | ||
* @note It is defined only if the project is installed in | ||
* Developer mode. | ||
* | ||
* @return A string with the install prefix if the project is | ||
* installed in Developer mode, an empty string otherwise. | ||
*/ | ||
std::string getInstallPrefix(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suggest to document that this is only valid if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I take advantage and I will document all the utils functions together in a new commit There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 9e8f58e |
||
} // namespace utils | ||
} // namespace gazebo | ||
} // namespace scenario | ||
|
This file was deleted.
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will make life complicated for anyone packaging the project in any form, being it as part of a superbuild, as Debian packages or a conda recipe. Instead of conflating build logic and packaging logic, could we have an explicit CMake variable to select if the
gym-ignition
project get installed in User mode or Developer mode, and then just specify the default based on theCMAKE_BUILD_TYPE
, something like:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the suggestion of propagating the build type as normal option. I will have a look at it. I don't remember precisely why we had to define a new build type for PyPI in the first place, for sure I tried to use a normal CMake variable as first attempt and then I faced problems that led me to this build type workaround, that is much more convoluted. What I'm sure is that I didn't do it if there were more simple solutions :)
I am planning to give a refreshing also to the CMake project, I will keep you comment in mind.
This is something I don't understand. The PyPI type is not meant to be used by any downstream user or packager. It is meant to be used to create packages for PyPI, as the name (and we are the maintainers of the packages). Any other usage is not supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. With this modification, now the only way to properly install
gym-ignition
in a relocatable way is to set theCMAKE_BUILD_TYPE
toPyPI
, and I guess that this is not supported if you are not installing forPyPI
. I guess we want to support "proper" installation also if the project is not installed withPyPI
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ow ok now I understood your initial point: supporting a relocatable installation which could be implemented in the same way we package the project for PyPI.
Note the using PyPI for this purpose is wrong. In fact, due to how the shared libraries must be placed in the
site_package
folder, you need to have the following installation tree:As you can see, shared libraries are installed in the root of the
CMAKE_INSTALL_PREFIX
, and plugins insidegym_ignition/plugins
. This is not what you want to obtain in a regular installation! In light of this, we should start thinking how to make coexist a (static) relocatable Developer installation with the PyPI build type without messing too much with the CMake project.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can also have a separate issue for that.