Skip to content

Commit

Permalink
Add in support for a version tag. (#133)
Browse files Browse the repository at this point in the history
* Add in support for a version tag.

This is already specified by the xsd, so just make sure we
implement it and enforce it in the code.

Signed-off-by: Chris Lalancette <[email protected]>

* Switch to a class for version parsing.

The previous attempt used a float to store the version number.
That has two problems:

1.  It runs into locale issues, and
2.  The XSD specifies it as a string

This alternative introduces a class to do parsing of the version
string.  Besides conforming to the XSD, and getting us away
from the locale issues, it also allows us to provide additional
constraints on the version (like not allowing negative numbers).
The UDFVersion class ends up living in the header file so we
can write tests against it.

Signed-off-by: Chris Lalancette <[email protected]>

* Remove bogus comment.

Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
clalancette authored and scpeters committed Jan 13, 2020
1 parent 3ffc4ee commit 1aef805
Show file tree
Hide file tree
Showing 4 changed files with 260 additions and 4 deletions.
85 changes: 84 additions & 1 deletion urdf_parser/include/urdf_parser/urdf_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,15 @@
#ifndef URDF_PARSER_URDF_PARSER_H
#define URDF_PARSER_URDF_PARSER_H

#include <stdexcept>
#include <string>
#include <map>
#include <vector>

#include <tinyxml.h>
#include <urdf_model/model.h>
#include <urdf_model/color.h>
#include <urdf_world/types.h>
#include <urdf_model/utils.h>

#include "exportdecl.h"

Expand All @@ -54,6 +57,86 @@ URDFDOM_DLLAPI std::string values2str(urdf::Rotation rot);
URDFDOM_DLLAPI std::string values2str(urdf::Color c);
URDFDOM_DLLAPI std::string values2str(double d);

// This lives here (rather than in model.cpp) so we can run tests on it.
class URDFVersion final
{
public:
explicit URDFVersion(const char *attr)
{
// If the passed in attribute is NULL, it means it wasn't specified in the
// XML, so we just assume version 1.0.
if (attr == nullptr)
{
major_ = 1;
minor_ = 0;
return;
}

// We accept any of the following strings:
// 1
// 1.0
std::vector<std::string> split;
urdf::split_string(split, std::string(attr), ".");
if (split.size() == 2)
{
major_ = strToUnsigned(split[0].c_str());
minor_ = strToUnsigned(split[1].c_str());
}
else
{
throw std::runtime_error("The version attribute should be in the form 'x.y'");
}
}

bool equal(uint32_t maj, uint32_t min)
{
return this->major_ == maj && this->minor_ == min;
}

uint32_t getMajor() const
{
return major_;
}

uint32_t getMinor() const
{
return minor_;
}

private:
uint32_t strToUnsigned(const char *str)
{
if (str[0] == '\0')
{
// This would get caught below, but we can make a nicer error message
throw std::runtime_error("One of the fields of the version attribute is blank");
}
char *end = const_cast<char *>(str);
long value = strtol(str, &end, 10);
if (end == str)
{
// If the pointer didn't move at all, then we couldn't convert any of
// the string to an integer.
throw std::runtime_error("Version attribute is not an integer");
}
if (*end != '\0')
{
// Here, we didn't go all the way to the end of the string, which
// means there was junk at the end
throw std::runtime_error("Extra characters after the version number");
}
if (value < 0)
{
throw std::runtime_error("Version number must be positive");
}

return value;
}

uint32_t major_;
uint32_t minor_;
};

}

namespace urdf{
Expand Down
21 changes: 19 additions & 2 deletions urdf_parser/src/model.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@

/* Author: Wim Meeussen */

#include <vector>
#include <fstream>
#include <map>
#include <stdexcept>
#include <string>
#include "urdf_parser/urdf_parser.h"
#include <console_bridge/console.h>
#include <fstream>

namespace urdf{

Expand Down Expand Up @@ -119,6 +121,21 @@ ModelInterfaceSharedPtr parseURDF(const std::string &xml_string)
}
model->name_ = std::string(name);

try
{
urdf_export_helpers::URDFVersion version(robot_xml->Attribute("version"));
if (!version.equal(1, 0))
{
throw std::runtime_error("Invalid 'version' specified; only version 1.0 is currently supported");
}
}
catch (const std::runtime_error & err)
{
CONSOLE_BRIDGE_logError(err.what());
model.reset();
return model;
}

// Get all Material elements
for (TiXmlElement* material_xml = robot_xml->FirstChildElement("material"); material_xml; material_xml = material_xml->NextSiblingElement("material"))
{
Expand Down
3 changes: 2 additions & 1 deletion urdf_parser/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ include_directories(${GTEST_INCLUDE_DIRS})
set(tests
urdf_double_convert.cpp
urdf_unit_test.cpp
urdf_version_test.cpp
)

#################################################
Expand All @@ -39,7 +40,7 @@ foreach(GTEST_SOURCE_file ${tests})
--gtest_output=xml:${CMAKE_BINARY_DIR}/test_results/${BINARY_NAME}.xml)

set_tests_properties(${BINARY_NAME} PROPERTIES TIMEOUT 240 ENVIRONMENT LC_ALL=C)

add_test(NAME ${BINARY_NAME}_locale
COMMAND ${BINARY_NAME}
--gtest_output=xml:${CMAKE_BINARY_DIR}/test_results/${BINARY_NAME}_locale.xml)
Expand Down
155 changes: 155 additions & 0 deletions urdf_parser/test/urdf_version_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
#include <iostream>
#include <string>

#include <gtest/gtest.h>

#include <urdf_parser/urdf_parser.h>

#define EXPECT_THROW_OF_TYPE(type, statement, msg) \
do { \
try { \
statement; \
} catch (const type & err) { \
if (std::string(err.what()).find(msg) == std::string::npos) { \
FAIL() << "Expected error msg containing:" << std::endl \
<< msg << std::endl \
<< "Saw error msg:" << std::endl \
<< err.what() << std::endl; \
} \
} catch (const std::exception & err) { \
FAIL() << "Expected " #type << std::endl \
<< "Saw exception type: " << typeid(err).name() << std::endl; \
} \
} while (0)

#define EXPECT_RUNTIME_THROW(st, msg) \
EXPECT_THROW_OF_TYPE(std::runtime_error, st, msg)


TEST(URDF_VERSION, test_version_wrong_type)
{
std::string test_str =
"<robot name=\"test\" version=\"foo\">"
"</robot>";

urdf::ModelInterfaceSharedPtr urdf = urdf::parseURDF(test_str);

EXPECT_EQ(urdf, nullptr);
}

TEST(URDF_VERSION, test_version_unsupported_version)
{
std::string test_str =
"<robot name=\"test\" version=\"2\">"
"</robot>";

urdf::ModelInterfaceSharedPtr urdf = urdf::parseURDF(test_str);

EXPECT_EQ(urdf, nullptr);
}

TEST(URDF_VERSION, test_version_not_specified)
{
std::string test_str =
"<robot name=\"test\">"
" <link name=\"l1\"/>"
"</robot>";

urdf::ModelInterfaceSharedPtr urdf = urdf::parseURDF(test_str);

EXPECT_NE(urdf, nullptr);
}

TEST(URDF_VERSION, test_version_one_int)
{
std::string test_str =
"<robot name=\"test\" version=\"1\">"
" <link name=\"l1\"/>"
"</robot>";

urdf::ModelInterfaceSharedPtr urdf = urdf::parseURDF(test_str);

EXPECT_EQ(urdf, nullptr);
}

TEST(URDF_VERSION, test_version_one_float)
{
std::string test_str =
"<robot name=\"test\" version=\"1.0\">"
" <link name=\"l1\"/>"
"</robot>";

urdf::ModelInterfaceSharedPtr urdf = urdf::parseURDF(test_str);

EXPECT_NE(urdf, nullptr);
}

TEST(URDF_VERSION_CLASS, test_null_ptr)
{
urdf_export_helpers::URDFVersion vers1(nullptr);

EXPECT_EQ(vers1.getMajor(), 1);
EXPECT_EQ(vers1.getMinor(), 0);
}

TEST(URDF_VERSION_CLASS, test_correct_string)
{
urdf_export_helpers::URDFVersion vers2("1.0");

EXPECT_EQ(vers2.getMajor(), 1);
EXPECT_EQ(vers2.getMinor(), 0);
}

TEST(URDF_VERSION_CLASS, test_too_many_dots)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("1.0.0"),
"The version attribute should be in the form 'x.y'");
}

TEST(URDF_VERSION_CLASS, test_not_enough_numbers)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("1."),
"The version attribute should be in the form 'x.y'");
}

TEST(URDF_VERSION_CLASS, test_no_major_number)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2(".1"),
"One of the fields of the version attribute is blank");
}

TEST(URDF_VERSION_CLASS, test_negative_major_number)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("-1.0"),
"Version number must be positive");
}

TEST(URDF_VERSION_CLASS, test_negative_minor_number)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("1.-1"),
"Version number must be positive");
}

TEST(URDF_VERSION_CLASS, test_no_numbers)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("abc"),
"The version attribute should be in the form 'x.y'");
}

TEST(URDF_VERSION_CLASS, test_dots_no_numbers)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("a.c"),
"Version attribute is not an integer");
}

TEST(URDF_VERSION_CLASS, test_dots_one_number)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("1.c"),
"Version attribute is not an integer");
}

TEST(URDF_VERSION_CLASS, test_trailing_junk)
{
EXPECT_RUNTIME_THROW(urdf_export_helpers::URDFVersion vers2("1.0~pre6"),
"Extra characters after the version number");
}

0 comments on commit 1aef805

Please sign in to comment.