diff --git a/cmake/gtest.cmake b/cmake/gtest.cmake index 3b0b8dfd..107c1559 100644 --- a/cmake/gtest.cmake +++ b/cmake/gtest.cmake @@ -12,6 +12,7 @@ FetchContent_MakeAvailable(googletest) # Add test files set(TEST_SOURCES test/cpp/CanDevice_test.cpp + test/cpp/CanDeviceConfiguration_test.cpp test/cpp/CanFrame_test.cpp test/cpp/LogIt_test.cpp test/cpp/CanVersion_test.cpp diff --git a/src/include/CanDeviceConfiguration.h b/src/include/CanDeviceConfiguration.h index 93731a99..be194608 100644 --- a/src/include/CanDeviceConfiguration.h +++ b/src/include/CanDeviceConfiguration.h @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -15,6 +16,18 @@ * depending on the type of CAN device being used. */ struct CanDeviceConfiguration { + /** + * @brief Builds a configuration from a map of string parameters, where + * each key must be the name of one of the parameters below. + * + * @param parameters The configuration parameters, indexed by name. + * @return The parsed configuration. + * @throws std::invalid_argument if a key is not a configuration parameter or + * if a value cannot be converted to the type of its parameter. + */ + static CanDeviceConfiguration from_map( + const std::map& parameters); + /** * @brief The name of the CAN bus. * diff --git a/src/main/CanDeviceConfiguration.cpp b/src/main/CanDeviceConfiguration.cpp index 207c75f6..5244e1bd 100644 --- a/src/main/CanDeviceConfiguration.cpp +++ b/src/main/CanDeviceConfiguration.cpp @@ -1,10 +1,108 @@ #include "CanDeviceConfiguration.h" +#include +#include #include #include +#include +#include #include +#include #include +namespace { + +/** + * @brief Converts a string to an unsigned 32 bits integer. + * + * @param key The name of the parameter being converted, used in the error. + * @param value The value to convert, a decimal number without sign. + * @return The converted value. + * @throws std::invalid_argument if the value is not an unsigned 32 bits + * integer. + */ +uint32_t to_uint32(const std::string& key, const std::string& value) { + const bool is_decimal = + !value.empty() && + std::all_of(value.begin(), value.end(), [](unsigned char character) { + return std::isdigit(character) != 0; + }); + + if (is_decimal) { + try { + const uint64_t parsed = std::stoull(value); + + if (parsed <= std::numeric_limits::max()) { + return static_cast(parsed); + } + } catch (const std::out_of_range&) { + // Reported below, together with the values that are not decimal. + } + } + + throw std::invalid_argument("Invalid value '" + value + + "' for CAN device " + "configuration parameter '" + + key + "': expected an unsigned 32 bits integer"); +} + +/** + * @brief Converts a string to a boolean. + * + * @param key The name of the parameter being converted, used in the error. + * @param value The value to convert, either "true" or "false". + * @return The converted value. + * @throws std::invalid_argument if the value is not a boolean. + */ +bool to_bool(const std::string& key, const std::string& value) { + if (value == "true") { + return true; + } + + if (value == "false") { + return false; + } + + throw std::invalid_argument("Invalid value '" + value + + "' for CAN device " + "configuration parameter '" + + key + "': expected 'true' or 'false'"); +} + +} // namespace + +CanDeviceConfiguration CanDeviceConfiguration::from_map( + const std::map& parameters) { + CanDeviceConfiguration config; + + for (const auto& [key, value] : parameters) { + if (key == "bus_name") { + config.bus_name = value; + } else if (key == "bus_number") { + config.bus_number = to_uint32(key, value); + } else if (key == "host") { + config.host = value; + } else if (key == "bitrate") { + config.bitrate = to_uint32(key, value); + } else if (key == "enable_termination") { + config.enable_termination = to_bool(key, value); + } else if (key == "high_speed") { + config.high_speed = to_bool(key, value); + } else if (key == "timeout") { + config.timeout = to_uint32(key, value); + } else if (key == "vcan") { + config.vcan = to_bool(key, value); + } else if (key == "sent_acknowledgement") { + config.sent_acknowledgement = to_uint32(key, value); + } else { + throw std::invalid_argument( + "Unknown CAN device configuration parameter '" + key + "'"); + } + } + + return config; +} + /** * @brief Converts the CanDeviceConfiguration object to a string representation. * diff --git a/src/main/CanVendorAnagate.cpp b/src/main/CanVendorAnagate.cpp index 744f853e..40af348d 100644 --- a/src/main/CanVendorAnagate.cpp +++ b/src/main/CanVendorAnagate.cpp @@ -62,8 +62,12 @@ void anagate_receive(AnaInt32 nIdentifier, const char* pcBuffer, */ CanVendorAnagate::CanVendorAnagate(const CanDeviceArguments& args) : CanDevice("anagate", args) { - if (!args.config.bus_number.has_value() || !args.config.host.has_value()) { - throw std::invalid_argument("Missing required configuration parameters"); + if (!args.config.host.has_value()) { + throw std::invalid_argument("Missing required host"); + } + + if (!args.config.bus_number.has_value()) { + throw std::invalid_argument("Missing required bus number"); } } diff --git a/src/main/CanVendorSocketCan.cpp b/src/main/CanVendorSocketCan.cpp index 9e52b166..1078c4ee 100644 --- a/src/main/CanVendorSocketCan.cpp +++ b/src/main/CanVendorSocketCan.cpp @@ -32,7 +32,7 @@ constexpr auto LIBSOCKETCAN_SUCCESS = 0; CanVendorSocketCan::CanVendorSocketCan(const CanDeviceArguments& args) : CanDevice("socketcan", args) { if (!args.config.bus_name.has_value()) { - throw std::invalid_argument("Missing required configuration parameters"); + throw std::invalid_argument("Missing required bus name"); } } /** diff --git a/src/python/CanModule.cpp b/src/python/CanModule.cpp index bdbe963e..a951b581 100644 --- a/src/python/CanModule.cpp +++ b/src/python/CanModule.cpp @@ -2,6 +2,8 @@ #include #include +#include +#include #include #include "CanDevice.h" @@ -77,6 +79,8 @@ PYBIND11_MODULE(canmodule, m) { py::class_(m, "CanDeviceConfiguration") .def(py::init<>()) + .def_static("from_map", &CanDeviceConfiguration::from_map, + py::arg("parameters")) .def_readwrite("bus_name", &CanDeviceConfiguration::bus_name) .def_readwrite("bus_number", &CanDeviceConfiguration::bus_number) .def_readwrite("host", &CanDeviceConfiguration::host) diff --git a/test/cpp/CanDeviceConfiguration_test.cpp b/test/cpp/CanDeviceConfiguration_test.cpp new file mode 100644 index 00000000..f2fc19fc --- /dev/null +++ b/test/cpp/CanDeviceConfiguration_test.cpp @@ -0,0 +1,127 @@ +#include "CanDeviceConfiguration.h" + +#include + +#include +#include +#include +#include +#include + +class CanDeviceConfigurationTest : public ::testing::Test {}; + +TEST_F(CanDeviceConfigurationTest, DefaultConstructorLeavesEverythingUnset) { + const CanDeviceConfiguration config; + + ASSERT_FALSE(config.bus_name.has_value()); + ASSERT_FALSE(config.bus_number.has_value()); + ASSERT_FALSE(config.host.has_value()); + ASSERT_FALSE(config.bitrate.has_value()); + ASSERT_FALSE(config.enable_termination.has_value()); + ASSERT_FALSE(config.high_speed.has_value()); + ASSERT_FALSE(config.timeout.has_value()); + ASSERT_FALSE(config.vcan.has_value()); + ASSERT_FALSE(config.sent_acknowledgement.has_value()); +} + +TEST_F(CanDeviceConfigurationTest, AssignsEveryParameterOfTheMap) { + const CanDeviceConfiguration config = CanDeviceConfiguration::from_map({ + {"bus_name", "can0"}, + {"bus_number", "2"}, + {"host", "127.0.0.1"}, + {"bitrate", "125000"}, + {"enable_termination", "true"}, + {"high_speed", "false"}, + {"timeout", "6000"}, + {"vcan", "true"}, + {"sent_acknowledgement", "1"}, + }); + + ASSERT_EQ(config.bus_name.value(), "can0"); + ASSERT_EQ(config.bus_number.value(), 2); + ASSERT_EQ(config.host.value(), "127.0.0.1"); + ASSERT_EQ(config.bitrate.value(), 125000); + ASSERT_TRUE(config.enable_termination.value()); + ASSERT_FALSE(config.high_speed.value()); + ASSERT_EQ(config.timeout.value(), 6000); + ASSERT_TRUE(config.vcan.value()); + ASSERT_EQ(config.sent_acknowledgement.value(), 1); +} + +TEST_F(CanDeviceConfigurationTest, LeavesTheAbsentParametersUnset) { + const CanDeviceConfiguration config = + CanDeviceConfiguration::from_map({{"bus_name", "can0"}}); + + ASSERT_EQ(config.bus_name.value(), "can0"); + ASSERT_FALSE(config.bus_number.has_value()); + ASSERT_FALSE(config.host.has_value()); + ASSERT_FALSE(config.bitrate.has_value()); + ASSERT_FALSE(config.enable_termination.has_value()); + ASSERT_FALSE(config.high_speed.has_value()); + ASSERT_FALSE(config.timeout.has_value()); + ASSERT_FALSE(config.vcan.has_value()); + ASSERT_FALSE(config.sent_acknowledgement.has_value()); +} + +TEST_F(CanDeviceConfigurationTest, AssignsEveryParameterPositionally) { + const CanDeviceConfiguration config{"can0", 2, "127.0.0.1", 125000, true, + false, 6000, true, 1}; + + ASSERT_EQ(config.bus_name.value(), "can0"); + ASSERT_EQ(config.bus_number.value(), 2); + ASSERT_EQ(config.host.value(), "127.0.0.1"); + ASSERT_EQ(config.bitrate.value(), 125000); + ASSERT_TRUE(config.enable_termination.value()); + ASSERT_FALSE(config.high_speed.value()); + ASSERT_EQ(config.timeout.value(), 6000); + ASSERT_TRUE(config.vcan.value()); + ASSERT_EQ(config.sent_acknowledgement.value(), 1); +} + +TEST_F(CanDeviceConfigurationTest, LeavesTrailingPositionalParametersUnset) { + const CanDeviceConfiguration config{"can0", 2}; + + ASSERT_EQ(config.bus_name.value(), "can0"); + ASSERT_EQ(config.bus_number.value(), 2); + ASSERT_FALSE(config.host.has_value()); + ASSERT_FALSE(config.bitrate.has_value()); + ASSERT_FALSE(config.enable_termination.has_value()); + ASSERT_FALSE(config.high_speed.has_value()); + ASSERT_FALSE(config.timeout.has_value()); + ASSERT_FALSE(config.vcan.has_value()); + ASSERT_FALSE(config.sent_acknowledgement.has_value()); +} + +TEST_F(CanDeviceConfigurationTest, RejectsUnknownParameters) { + ASSERT_THROW(CanDeviceConfiguration::from_map({{"buss_name", "can0"}}), + std::invalid_argument); + ASSERT_THROW(CanDeviceConfiguration::from_map({{"vendor", "socketcan"}}), + std::invalid_argument); + ASSERT_THROW(CanDeviceConfiguration::from_map({{"", ""}}), + std::invalid_argument); +} + +TEST_F(CanDeviceConfigurationTest, RejectsValuesOfTheWrongType) { + const std::vector> invalid_values = { + {"bus_number", ""}, + {"bus_number", "-1"}, + {"bus_number", "+1"}, + {"bus_number", "1.5"}, + {"bus_number", "12a"}, + {"bus_number", "0x2"}, + {"bus_number", " 2 "}, + {"bitrate", "4294967296"}, + {"bitrate", "99999999999999999999"}, + {"enable_termination", ""}, + {"enable_termination", "yes"}, + {"high_speed", "TRUE"}, + {"vcan", "1"}, + {"sent_acknowledgement", "abc"}, + }; + + for (const auto& [key, value] : invalid_values) { + ASSERT_THROW(CanDeviceConfiguration::from_map({{key, value}}), + std::invalid_argument) + << "Expected '" << value << "' to be invalid for '" << key << "'"; + } +} diff --git a/test/python/test_common.py b/test/python/test_common.py index ba153828..81472fbb 100644 --- a/test/python/test_common.py +++ b/test/python/test_common.py @@ -65,6 +65,24 @@ def test_loopback_multiple_messages(): assert received_frames[1].message() == ["W", "o", "r", "l", "d"] +def test_configuration_from_parameters(): + config = CanDeviceConfiguration.from_map( + {"bus_name": "can0", "bitrate": "500000", "vcan": "true"} + ) + assert config.bus_name == "can0" + assert config.bitrate == 500000 + assert config.vcan is True + assert config.host is None + + +def test_configuration_from_parameters_unknown_key(): + with pytest.raises(ValueError) as e: + CanDeviceConfiguration.from_map({"not_a_parameter": "value"}) + assert ( + str(e.value) == "Unknown CAN device configuration parameter 'not_a_parameter'" + ) + + def test_loopback_construction_empty_callback(): myDevice = CanDevice.create( "loopback", CanDeviceArguments(CanDeviceConfiguration())