uhdlib: Update constrained_device_args_t

- Allow enums to be non-consecutive
- Move to uhdlib/
- Add unit tests
- Updated N230 use of constrained_device_args_t

4 files changed, 170 insertions, 31 deletions

diff --git a/host/lib/usrp/common/constrained_device_args.hpp b/host/lib/include/uhdlib/usrp/constrained_device_args.hpp
index 8bf1d2a55..c9d573e5f 100644
--- a/host/lib/usrp/common/constrained_device_args.hpp
+++ b/host/lib/include/uhdlib/usrp/constrained_device_args.hpp
@@ -1,7 +1,7 @@
 //
 // Copyright 2014 Ettus Research LLC
 //
-// SPDX-License-Identifier: GPL-3.0
+// SPDX-License-Identifier: GPL-3.0+
 //
 
 #ifndef INCLUDED_LIBUHD_USRP_COMMON_CONSTRAINED_DEV_ARGS_HPP
@@ -14,6 +14,7 @@
 #include <boost/assign/list_of.hpp>
 #include <vector>
 #include <string>
+#include <unordered_map>
 
 namespace uhd {
 namespace usrp {
@@ -114,9 +115,6 @@ namespace usrp {
          * Enumeration argument type. The template type enum_t allows the
          * client to use their own enum and specify a string mapping for
          * the values of the enum
-         *
-         * NOTE: The constraint on enum_t is that the values must start with
-         * 0 and be sequential
          */
         template<typename enum_t>
         class enum_arg : public generic_arg {
@@ -124,44 +122,58 @@ namespace usrp {
             enum_arg(
                 const std::string& name,
                 const enum_t default_value,
-                const std::vector<std::string>& values) :
-                    generic_arg(name), _str_values(values)
-            { set(default_value); }
-
+                const std::unordered_map<std::string, enum_t>& values) :
+                    generic_arg(name), _str_values(_enum_map_to_lowercase<enum_t>(values))
+            {
+                set(default_value);
+            }
             inline void set(const enum_t value) {
                 _value = value;
             }
             inline const enum_t get() const {
                 return _value;
             }
-            inline void parse(const std::string& str_rep, bool assert_invalid = true) {
-                std::string valid_values_str;
-                for (size_t i = 0; i < _str_values.size(); i++) {
-                    if (boost::algorithm::to_lower_copy(str_rep) ==
-                        boost::algorithm::to_lower_copy(_str_values[i]))
-                    {
-                        valid_values_str += ((i==0)?"":", ") + _str_values[i];
-                        set(static_cast<enum_t>(static_cast<int>(i)));
+            inline void parse(
+                    const std::string& str_rep,
+                    const bool assert_invalid = true
+            ) {
+                const std::string str_rep_lowercase =
+                    boost::algorithm::to_lower_copy(str_rep);
+                if (_str_values.count(str_rep_lowercase) == 0) {
+                    if (assert_invalid) {
+                        std::string valid_values_str = "";
+                        for (const auto &value : _str_values) {
+                            valid_values_str +=
+                                (valid_values_str.empty()?"":", ")
+                                + value.first;
+                        }
+                        throw uhd::value_error(str(boost::format(
+                            "Invalid device arg value: %s=%s (Valid: {%s})") %
+                            key() % str_rep % valid_values_str
+                        ));
+                    } else {
                         return;
                     }
                 }
-                //If we reach here then, the string enum value was invalid
-                if (assert_invalid) {
-                    throw uhd::value_error(str(boost::format(
-                        "Invalid device arg value: %s=%s (Valid: {%s})") %
-                        key() % str_rep % valid_values_str
-                    ));
-                }
+
+                set(_str_values.at(str_rep_lowercase));
             }
             inline virtual std::string to_string() const {
-                size_t index = static_cast<size_t>(static_cast<int>(_value));
-                UHD_ASSERT_THROW(index < _str_values.size());
-                return key() + "=" + _str_values[index];
+                std::string repr;
+                for (const auto& value : _str_values) {
+                    if (value.second == _value) {
+                        repr = value.first;
+                        break;
+                    }
+                }
+
+                UHD_ASSERT_THROW(!repr.empty());
+                return key() + "=" + repr;
             }
 
         private:
-            enum_t                      _value;
-            std::vector<std::string>    _str_values;
+            enum_t                                         _value;
+            const std::unordered_map<std::string, enum_t>  _str_values;
         };
 
         /*!
@@ -265,6 +277,24 @@ namespace usrp {
                 }
             }
         }
+
+        //! Helper for enum_arg: Create a new map where keys are converted to
+        //  lowercase.
+        template<typename enum_t>
+        static std::unordered_map<std::string, enum_t> _enum_map_to_lowercase(
+            const std::unordered_map<std::string, enum_t>& in_map
+        ) {
+            std::unordered_map<std::string, enum_t> new_map;
+            for (const auto& str_to_enum : in_map) {
+                new_map.insert(
+                    std::pair<std::string, enum_t>(
+                        boost::algorithm::to_lower_copy(str_to_enum.first),
+                        str_to_enum.second
+                    )
+                );
+            }
+            return new_map;
+        }
     };
 }} //namespaces
 
diff --git a/host/lib/usrp/n230/n230_device_args.hpp b/host/lib/usrp/n230/n230_device_args.hpp
index de326a309..b0a4a23c9 100644
--- a/host/lib/usrp/n230/n230_device_args.hpp
+++ b/host/lib/usrp/n230/n230_device_args.hpp
@@ -9,9 +9,9 @@
 
 #include <uhd/types/wb_iface.hpp>
 #include <uhd/transport/udp_simple.hpp>
-#include <boost/thread/mutex.hpp>
-#include "../common/constrained_device_args.hpp"
+#include <uhdlib/usrp/constrained_device_args.hpp>
 #include "n230_defaults.h"
+#include <boost/thread/mutex.hpp>
 
 namespace uhd { namespace usrp { namespace n230 {
 
@@ -29,7 +29,9 @@ public:
         _send_buff_size("send_buff_size", n230::DEFAULT_SEND_BUFF_SIZE),
         _recv_buff_size("recv_buff_size", n230::DEFAULT_RECV_BUFF_SIZE),
         _safe_mode("safe_mode", false),
-        _loopback_mode("loopback_mode", LOOPBACK_OFF, boost::assign::list_of("off")("radio")("codec"))
+        _loopback_mode("loopback_mode",
+                       LOOPBACK_OFF,
+                       {{"off", LOOPBACK_OFF}, {"radio", LOOPBACK_RADIO}, {"codec", LOOPBACK_CODEC}})
     {}
 
     double get_master_clock_rate() const {
diff --git a/host/tests/CMakeLists.txt b/host/tests/CMakeLists.txt
index 6f78f8d3e..0741191f2 100644
--- a/host/tests/CMakeLists.txt
+++ b/host/tests/CMakeLists.txt
@@ -19,6 +19,7 @@ SET(test_sources
     cast_test.cpp
     cal_container_test.cpp
     chdr_test.cpp
+    constrained_device_args_test.cpp
     convert_test.cpp
     dict_test.cpp
     error_test.cpp
@@ -69,6 +70,8 @@ IF(ENABLE_C_API)
     )
 ENDIF(ENABLE_C_API)
 
+INCLUDE_DIRECTORIES("${CMAKE_SOURCE_DIR}/lib/include")
+
 #for each source: build an executable, register it as a test
 FOREACH(test_source ${test_sources})
     GET_FILENAME_COMPONENT(test_name ${test_source} NAME_WE)
diff --git a/host/tests/constrained_device_args_test.cpp b/host/tests/constrained_device_args_test.cpp
new file mode 100644
index 000000000..c5f256e9e
--- /dev/null
+++ b/host/tests/constrained_device_args_test.cpp
@@ -0,0 +1,104 @@
+//
+// Copyright 2018 Ettus Research, a National Instruments Company
+//
+// SPDX-License-Identifier: GPL-3.0+
+//
+
+#include <boost/test/unit_test.hpp>
+#include <uhdlib/usrp/constrained_device_args.hpp>
+#include <iostream>
+
+using uhd::usrp::constrained_device_args_t;
+
+namespace {
+    enum test_enum_t { VALUE1, VALUE2, VALUE3=4 };
+
+    static constexpr double      MAX_DOUBLE_ARG = 1e6;
+    static constexpr double      MIN_DOUBLE_ARG = 0.125;
+
+    static constexpr double      DEFAULT_DOUBLE_ARG = 2.25;
+    static constexpr size_t      DEFAULT_SIZE_T_ARG = 42;
+    static constexpr bool        DEFAULT_BOOL_ARG   = true;
+    static constexpr test_enum_t DEFAULT_ENUM_ARG   = VALUE1;
+
+
+    class test_device_args_t : public constrained_device_args_t
+    {
+    public:
+        test_device_args_t() {}
+        test_device_args_t(const std::string& dev_args) { parse(dev_args); }
+
+        double get_double_arg() const {
+            return _double_arg.get();
+        }
+        size_t get_size_t_arg() const {
+            return _size_t_arg.get();
+        }
+        bool get_bool_arg() const {
+            return _bool_arg.get();
+        }
+        test_enum_t get_enum_arg() const {
+            return _enum_arg.get();
+        }
+
+        inline virtual std::string to_string() const {
+            return  _double_arg.to_string() + ", " +
+                    _size_t_arg.to_string() + ", " +
+                    _bool_arg.to_string() + ", " +
+                    _enum_arg.to_string();
+        }
+    private:
+        virtual void _parse(const uhd::device_addr_t& dev_args) {
+            if (dev_args.has_key(_double_arg.key()))
+                _double_arg.parse(dev_args[_double_arg.key()]);
+            if (dev_args.has_key(_size_t_arg.key()))
+                _size_t_arg.parse(dev_args[_size_t_arg.key()]);
+            if (dev_args.has_key(_bool_arg.key()))
+                _bool_arg.parse(dev_args[_bool_arg.key()]);
+            if (dev_args.has_key(_enum_arg.key()))
+                _enum_arg.parse(dev_args[_enum_arg.key()]);
+            _enforce_range(_double_arg, MIN_DOUBLE_ARG, MAX_DOUBLE_ARG);
+        }
+
+        constrained_device_args_t::num_arg<double>       _double_arg
+            {"double_arg", DEFAULT_DOUBLE_ARG};
+        constrained_device_args_t::num_arg<size_t>       _size_t_arg
+            {"size_t_arg", DEFAULT_SIZE_T_ARG};
+        constrained_device_args_t::bool_arg              _bool_arg
+            {"bool_arg", DEFAULT_BOOL_ARG};
+        constrained_device_args_t::enum_arg<test_enum_t> _enum_arg
+            {"enum_arg", DEFAULT_ENUM_ARG,
+                  {{"value1", VALUE1}, {"value2", VALUE2}, {"VALUE3", VALUE3}}};
+    };
+
+}
+
+BOOST_AUTO_TEST_CASE(test_constrained_device_args) {
+    test_device_args_t test_dev_args("double_arg=3.5,bool_arg=0,foo=bar");
+    BOOST_CHECK_EQUAL(test_dev_args.get_double_arg(), 3.5);
+    BOOST_CHECK_EQUAL(
+        test_dev_args.get_size_t_arg(),
+        DEFAULT_SIZE_T_ARG
+    );
+    BOOST_CHECK_EQUAL(test_dev_args.get_bool_arg(), false);
+    BOOST_CHECK_EQUAL(
+        test_dev_args.get_enum_arg(),
+        DEFAULT_ENUM_ARG
+    );
+    BOOST_REQUIRE_THROW(
+        test_dev_args.parse("double_arg=2e6"),
+        uhd::value_error
+    ); // Note: test_dev_args is now in a bad state until we fix it!
+    test_dev_args.parse("double_arg=2.6"),
+    test_dev_args.parse("enum_arg=vaLue2");
+    BOOST_CHECK_EQUAL(
+        test_dev_args.get_enum_arg(),
+        VALUE2
+    );
+    test_dev_args.parse("enum_arg=VALUE3");
+    BOOST_CHECK_EQUAL(
+        test_dev_args.get_enum_arg(),
+        VALUE3
+    );
+}
+