treewide: simplify line lookup
authorBartosz Golaszewski <bgolaszewski@baylibre.com>
Mon, 18 Jan 2021 15:51:47 +0000 (16:51 +0100)
committerBartosz Golaszewski <bgolaszewski@baylibre.com>
Thu, 18 Mar 2021 08:34:15 +0000 (09:34 +0100)
We're preparing to entirely remove the line objects from the API and
split their functionality between two new objects: line_info and
line_request. The lookup functions must be limited in the process.

This reworks all the find_line methods to: a) always assume that names
looked for are unique within a single chip (because while it's
technically possible for GPIO line names to be non-unique, it doesn't
make sense to look for two lines named the same) and b) return the
hardware offset within the chip instead of the line object.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
bindings/cxx/chip.cpp
bindings/cxx/examples/gpiofindcxx.cpp
bindings/cxx/gpiod.hpp
bindings/cxx/tests/tests-chip.cpp
bindings/python/examples/gpiofind.py
bindings/python/gpiodmodule.c
bindings/python/tests/gpiod_py_test.py
include/gpiod.h
lib/helpers.c
tests/tests-chip.c
tools/gpiofind.c

index 5b8125b9edceb935197d71d8906a7603dc725a7c..527fb34c389152ac410bdedd5ba578a31bcef2d3 100644 (file)
@@ -92,22 +92,18 @@ line chip::get_line(unsigned int offset) const
        return line(line_handle, *this);
 }
 
-::std::vector<line> chip::find_line(const ::std::string& name, bool unique) const
+int chip::find_line(const ::std::string& name) const
 {
        this->throw_if_noref();
 
-       ::std::vector<line> lines;
+       for (unsigned int offset = 0; offset < this->num_lines(); offset++) {
+               auto line = this->get_line(offset);
 
-       for (auto& line: ::gpiod::line_iter(*this)) {
                if (line.name() == name)
-                       lines.push_back(line);
+                       return offset;
        }
 
-       if (unique && lines.size() > 1)
-               throw ::std::system_error(ERANGE, ::std::system_category(),
-                                         "multiple lines with the same name found");
-
-       return lines;
+       return -1;
 }
 
 line_bulk chip::get_lines(const ::std::vector<unsigned int>& offsets) const
index 0bccd947c3b61cbcd463eea56a41ab695e3ca2b5..ec4d79b6a8da38f7e3ae58cb51e6ec2092793e45 100644 (file)
@@ -20,10 +20,9 @@ int main(int argc, char **argv)
                if (::gpiod::is_gpiochip_device(entry.path())) {
                        ::gpiod::chip chip(entry.path());
 
-                       auto lines = chip.find_line(argv[1], true);
-                       if (!lines.empty()) {
-                               ::std::cout << lines.front().name() << " " <<
-                                              lines.front().offset() << ::std::endl;
+                       auto offset = chip.find_line(argv[1]);
+                       if (offset >= 0) {
+                               ::std::cout << chip.name() << " " << offset << ::std::endl;
                                return EXIT_SUCCESS;
                        }
                }
index d987b3ad611e3f9cd1496e7ee26a113e70bd6dd1..3a043a15d178bec28f9c0861e11f4a454f1293d3 100644 (file)
@@ -129,15 +129,12 @@ public:
        GPIOD_API line get_line(unsigned int offset) const;
 
        /**
-        * @brief Find all GPIO lines by name among lines exposed by this GPIO
-        *        chip.
-        * @param name Line name.
-        * @param unique If set to true: throw an error if multiple lines match
-        *               the name.
-        * @return Vector of all matching lines.
-        */
-       GPIOD_API ::std::vector<line> find_line(const ::std::string& name,
-                                               bool unique = false) const;
+        * @brief Map a GPIO line's name to its offset within the chip.
+        * @param name Name of the GPIO line to map.
+        * @return Offset of the line within the chip or -1 if a line with
+        *         given name is not exposed by the chip.
+        */
+       GPIOD_API int find_line(const ::std::string& name) const;
 
        /**
         * @brief Get a set of lines exposed by this chip at given offsets.
index a84b150782f2cd8762ca5cf77bab5a5805339103..aea00fa62985dc04f299388e5dae303d0096e0b2 100644 (file)
@@ -113,8 +113,8 @@ TEST_CASE("Lines can be retrieved from chip objects", "[chip]")
 
        SECTION("find single line by name")
        {
-               auto line = chip.find_line("gpio-mockup-B-3", true).front();
-               REQUIRE(line.offset() == 3);
+               auto offset = chip.find_line("gpio-mockup-B-3");
+               REQUIRE(offset == 3);
        }
 
        SECTION("get multiple lines by offsets")
@@ -168,6 +168,6 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
 
        SECTION("line not found by name")
        {
-               REQUIRE(chip.find_line("nonexistent-line").empty());
+               REQUIRE(chip.find_line("nonexistent-line") == -1);
        }
 }
index 117d583e5b5ed44682ab550f10cca3ac255f4140..a9ec7342287c27e5042e24dd669440d440ea72dd 100755 (executable)
@@ -12,10 +12,9 @@ if __name__ == '__main__':
     for entry in os.scandir('/dev/'):
         if gpiod.is_gpiochip_device(entry.path):
             with gpiod.Chip(entry.path) as chip:
-                lines = chip.find_line(sys.argv[1], unique=True)
-                if lines is not None:
-                     line = lines.to_list()[0]
-                     print('{} {}'.format(line.owner().name(), line.offset()))
+                offset = chip.find_line(sys.argv[1], unique=True)
+                if offset is not None:
+                     print('{} {}'.format(line.owner().name(), offset))
                      sys.exit(0)
 
     sys.exit(1)
index dca6a981f31b6687efc2600e3b63a60852aff350..ac0fd83072c87058d884878535e631a875c54fd1 100644 (file)
@@ -2151,59 +2151,40 @@ gpiod_LineBulkObjectFromBulk(gpiod_ChipObject *chip, struct gpiod_line_bulk *bul
 }
 
 PyDoc_STRVAR(gpiod_Chip_find_line_doc,
-"find_line(name) -> gpiod.LineBulk object or None\n"
+"find_line(name) -> integer or None\n"
 "\n"
-"Find all GPIO lines by name among lines exposed by this GPIO chip..\n"
+"Find the offset of the line with given name among lines exposed by this\n"
+"GPIO chip.\n"
 "\n"
 "  name\n"
 "    Line name (string)\n"
-"  unique\n"
-"    Indicates whether an exception should be raised if more than one lines\n"
-"    matches the name\n"
 "\n"
-"Returns a gpiod.LineBulk object containing all matching lines or None if\n"
-"line with given name is not associated with this chip.");
+"Returns the offset of the line with given name or None if it is not\n"
+"associated with this chip.");
 
-static gpiod_LineBulkObject *
-gpiod_Chip_find_line(gpiod_ChipObject *self, PyObject *args, PyObject *kwds)
+static PyObject *gpiod_Chip_find_line(gpiod_ChipObject *self, PyObject *args)
 {
-       static char *kwlist[] = { "", "unique", NULL };
-
-       gpiod_LineBulkObject *bulk_obj;
-       struct gpiod_line_bulk *bulk;
-       int rv, unique = 0;
        const char *name;
+       int rv, offset;
 
        if (gpiod_ChipIsClosed(self))
                return NULL;
 
-       rv = PyArg_ParseTupleAndKeywords(args, kwds, "s|p",
-                                        kwlist, &name, &unique);
+       rv = PyArg_ParseTuple(args, "s", &name);
        if (!rv)
                return NULL;
 
        Py_BEGIN_ALLOW_THREADS;
-       bulk = gpiod_chip_find_line(self->chip, name);
+       offset = gpiod_chip_find_line(self->chip, name);
        Py_END_ALLOW_THREADS;
-       if (!bulk) {
-               if (errno == ENOENT) {
-                       Py_INCREF(Py_None);
-                       return (gpiod_LineBulkObject *)Py_None;
-               }
-
-               return (gpiod_LineBulkObject *)PyErr_SetFromErrno(
-                                                       PyExc_OSError);
-       }
+       if (offset < 0) {
+               if (errno == ENOENT)
+                       Py_RETURN_NONE;
 
-       if (unique && gpiod_line_bulk_num_lines(bulk) > 1) {
-               gpiod_line_bulk_free(bulk);
-               PyErr_SetString(PyExc_RuntimeError, "line not unique");
-               return NULL;
+               return PyErr_SetFromErrno(PyExc_OSError);
        }
 
-       bulk_obj = gpiod_LineBulkObjectFromBulk(self, bulk);
-       gpiod_line_bulk_free(bulk);
-       return bulk_obj;
+       return Py_BuildValue("i", offset);
 }
 
 PyDoc_STRVAR(gpiod_Chip_get_lines_doc,
@@ -2353,8 +2334,8 @@ static PyMethodDef gpiod_Chip_methods[] = {
        },
        {
                .ml_name = "find_line",
-               .ml_meth = (PyCFunction)(void (*)(void))gpiod_Chip_find_line,
-               .ml_flags = METH_VARARGS | METH_KEYWORDS,
+               .ml_meth = (PyCFunction)gpiod_Chip_find_line,
+               .ml_flags = METH_VARARGS,
                .ml_doc = gpiod_Chip_find_line_doc,
        },
        {
index f264db334c47fbda11c85f1df1417581a0655c66..c8ee85b23bf53190b899ceec1b6e6b569107c8d0 100755 (executable)
@@ -136,8 +136,9 @@ class ChipGetLines(MockupTestCase):
 
     def test_find_single_line_by_name(self):
         with gpiod.Chip(mockup.chip_path(1)) as chip:
-            lines = chip.find_line('gpio-mockup-B-4', unique=True).to_list()
-            self.assertEqual(lines[0].offset(), 4)
+            offset = chip.find_line('gpio-mockup-B-4')
+            self.assertIsNotNone(offset)
+            self.assertEqual(offset, 4)
 
     def test_get_single_line_invalid_offset(self):
         with gpiod.Chip(mockup.chip_path(1)) as chip:
@@ -148,8 +149,8 @@ class ChipGetLines(MockupTestCase):
 
     def test_find_single_line_nonexistent(self):
         with gpiod.Chip(mockup.chip_path(1)) as chip:
-            lines = chip.find_line('nonexistent-line', unique=True)
-            self.assertEqual(lines, None)
+            offset = chip.find_line('nonexistent-line')
+            self.assertIsNone(offset)
 
     def test_get_multiple_lines_by_offsets_in_tuple(self):
         with gpiod.Chip(mockup.chip_path(1)) as chip:
index 76aa1dd43a3fef9ae34191807dcab84c2bf244d9..6ed9c6c4b49e4c54d8418784aeccb0eea18dd60b 100644 (file)
@@ -136,31 +136,13 @@ struct gpiod_line_bulk *
 gpiod_chip_get_all_lines(struct gpiod_chip *chip) GPIOD_API;
 
 /**
- * @brief Find all GPIO lines by name among lines exposed by this GPIO chip.
+ * @brief Map a GPIO line's name to its offset within the chip.
  * @param chip The GPIO chip object.
- * @param name GPIO line name to look for.
- * @return New line bulk object containing all matching lines or NULL on error.
- *
- * If no line with given name is associated with this chip, the function sets
- * errno to ENOENT.
- */
-struct gpiod_line_bulk *
-gpiod_chip_find_line(struct gpiod_chip *chip, const char *name) GPIOD_API;
-
-/**
- * @brief Find a unique line by name among lines exposed by this GPIO chip.
- * @param chip The GPIO chip object.
- * @param name Name of the GPIO line.
- * @return Pointer to the GPIO line handle or NULL if the line could not be
- *         found or an error occurred.
- *
- * If no line with given name is associated with this chip, the function sets
- * errno to ENOENT. If more than one line with given name is associated with
- * this chip, the function sets errno to ERANGE.
+ * @param name Name of the GPIO line to map.
+ * @return Offset of the line within the chip or -1 if a line with given name
+ *         is not exposed by the chip.
  */
-struct gpiod_line *
-gpiod_chip_find_line_unique(struct gpiod_chip *chip,
-                           const char *name) GPIOD_API;
+int gpiod_chip_find_line(struct gpiod_chip *chip, const char *name) GPIOD_API;
 
 /**
  * @}
index 0071a2d9df093f98c6d58aa560c339dd4c7de836..76d8fc26c3adc097935332d4afd11df1914ec5d3 100644 (file)
@@ -59,10 +59,8 @@ struct gpiod_line_bulk *gpiod_chip_get_all_lines(struct gpiod_chip *chip)
        return bulk;
 }
 
-struct gpiod_line_bulk *
-gpiod_chip_find_line(struct gpiod_chip *chip, const char *name)
+int gpiod_chip_find_line(struct gpiod_chip *chip, const char *name)
 {
-       struct gpiod_line_bulk *bulk = NULL;
        unsigned int offset, num_lines;
        struct gpiod_line *line;
        const char *tmp;
@@ -70,62 +68,17 @@ gpiod_chip_find_line(struct gpiod_chip *chip, const char *name)
        num_lines = gpiod_chip_num_lines(chip);
 
        for (offset = 0; offset < num_lines; offset++) {
-               line = gpiod_chip_get_line(chip, offset);
-               if (!line) {
-                       if (bulk)
-                               gpiod_line_bulk_free(bulk);
-
-                       return NULL;
-               }
-
-               tmp = gpiod_line_name(line);
-               if (tmp && strcmp(tmp, name) == 0) {
-                       if (!bulk) {
-                               bulk = gpiod_line_bulk_new(num_lines);
-                               if (!bulk)
-                                       return NULL;
-                       }
-
-                       gpiod_line_bulk_add_line(bulk, line);
-               }
-       }
-
-       if (!bulk)
-               errno = ENOENT;
-
-       return bulk;
-}
-
-struct gpiod_line *
-gpiod_chip_find_line_unique(struct gpiod_chip *chip, const char *name)
-{
-       struct gpiod_line *line, *matching = NULL;
-       unsigned int offset, num_found = 0;
-       const char *tmp;
-
-       for (offset = 0; offset < gpiod_chip_num_lines(chip); offset++) {
                line = gpiod_chip_get_line(chip, offset);
                if (!line)
-                       return NULL;
+                       return -1;
 
                tmp = gpiod_line_name(line);
-               if (tmp && strcmp(tmp, name) == 0) {
-                       matching = line;
-                       num_found++;
-               }
-       }
-
-       if (matching) {
-               if (num_found > 1) {
-                       errno = ERANGE;
-                       return NULL;
-               }
-
-               return matching;
+               if (tmp && strcmp(tmp, name) == 0)
+                       return gpiod_line_offset(line);
        }
 
        errno = ENOENT;
-       return NULL;
+       return -1;
 }
 
 int gpiod_line_request_input(struct gpiod_line *line, const char *consumer)
index 928dc49df9097b04e3bd067f68b57df7d5d1cf18..a87dc9a4b7acb9a310bd618e91d2339223b9eeb3 100644 (file)
@@ -190,19 +190,24 @@ GPIOD_TEST_CASE(get_all_lines, 0, { 4 })
        g_assert_cmpuint(gpiod_line_offset(line3), ==, 3);
 }
 
-GPIOD_TEST_CASE(find_line_good_unique, GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
+GPIOD_TEST_CASE(find_line_good, GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
 {
        g_autoptr(gpiod_chip_struct) chip = NULL;
        struct gpiod_line *line;
+       int offset;
 
        chip = gpiod_chip_open(gpiod_test_chip_path(1));
        g_assert_nonnull(chip);
        gpiod_test_return_if_failed();
 
-       line = gpiod_chip_find_line_unique(chip, "gpio-mockup-B-4");
+       offset = gpiod_chip_find_line(chip, "gpio-mockup-B-4");
+       g_assert_cmpint(offset, ==, 4);
+       gpiod_test_return_if_failed();
+
+       line = gpiod_chip_get_line(chip, 4);
        g_assert_nonnull(line);
        gpiod_test_return_if_failed();
-       g_assert_cmpuint(gpiod_line_offset(line), ==, 4);
+
        g_assert_cmpstr(gpiod_line_name(line), ==, "gpio-mockup-B-4");
 }
 
@@ -210,13 +215,13 @@ GPIOD_TEST_CASE(find_line_unique_not_found,
                GPIOD_TEST_FLAG_NAMED_LINES, { 8, 8, 8 })
 {
        g_autoptr(gpiod_chip_struct) chip = NULL;
-       struct gpiod_line *line;
+       int offset;
 
        chip = gpiod_chip_open(gpiod_test_chip_path(1));
        g_assert_nonnull(chip);
        gpiod_test_return_if_failed();
 
-       line = gpiod_chip_find_line_unique(chip, "nonexistent");
-       g_assert_null(line);
+       offset = gpiod_chip_find_line(chip, "nonexistent");
+       g_assert_cmpint(offset, ==, -1);
        g_assert_cmpint(errno, ==, ENOENT);
 }
index 4936c4f49e41d7e7ca9596cad9b01f976051cc06..0fc07d98de551a245d45762925e8c0a6952240ca 100644 (file)
@@ -31,9 +31,8 @@ static void print_help(void)
 
 int main(int argc, char **argv)
 {
-       int i, num_chips, optc, opti;
+       int i, num_chips, optc, opti, offset;
        struct gpiod_chip *chip;
-       struct gpiod_line *line;
        struct dirent **entries;
 
        for (;;) {
@@ -74,10 +73,10 @@ int main(int argc, char **argv)
                        die_perror("unable to open %s", entries[i]->d_name);
                }
 
-               line = gpiod_chip_find_line_unique(chip, argv[0]);
-               if (line) {
+               offset = gpiod_chip_find_line(chip, argv[0]);
+               if (offset >= 0) {
                        printf("%s %u\n",
-                              gpiod_chip_name(chip), gpiod_line_offset(line));
+                              gpiod_chip_name(chip), offset);
                        gpiod_chip_close(chip);
                        return EXIT_SUCCESS;
                }