Platform_extender: FT4222 library refactor

Updates to make the FT4222 platform extender more usable.

Previous implementation opened libft4222.so and loaded symbols as
needed.  This implementation removes dynamic loading of libft4222 in
favor of creating a new shared libary which links against libft4222.so.
The dynamic loading is now done in mraa.c.  One C method is exposed in
libmraa-platform-ft4222.so for finding/initializing an FT4222:

mraa_platform_t mraa_usb_platform_extender(mraa_board_t* board);

Mraa.c attempts to open this platform library and calls the
mraa_usb_platform_extender method.  If an ftdi4222 is connected, the
user gets added IO from the extender.  If no FT4222 device is connected,
continue as normal.

    * Create a new platform library for the FT4222
    * Expose only 1 C method from the library -
      mraa_usb_platform_extender
    * libmraa-platform-ft4222.so contains CXX code (as well as previous
      C code).  All *allocs have been removed in favor of global
      C++ stl containers.
    * Previously, the FT4222 would only initialize correctly if 2 ftdi
      devices existed.  Now, initialize FT4222 devices based on the
      device id.
    * Many fixes for various problems with the FT4222
    * Added unit test for platform extender (minimal functionality w/o hw)
    * Updated to FindFtd4222.cmake module to handle standard arguments
    * Removed CMAKE_C_FLAGS addition of -DFTDID2XX and -DFTDI4222 since
      these are NOT used anywhere in source.
    * Building the FTDI4222 shim requires libft4222.h which requires
      ftd2xx.h.  Updated CMakeLists.txt to require both when building
      the shim.

Signed-off-by: Noel Eck <noel.eck@intel.com>

cmake/modules/FindFtd2xx.cmake

@@ -27,11 +27,11 @@ else (LIBFTD2XX_LIBRARIES AND LIBFTD2XX_INCLUDE_DIRS)
     /usr/local/include
     /opt/local/include
     /sw/include
-	)
+    )
 
   SET(FTD2XX_LIBNAME ftd2xx)
   IF(WIN32)
-	SET(FTD2XX_LIBNAME ftd2xx.lib)
+    SET(FTD2XX_LIBNAME ftd2xx.lib)
   ENDIF(WIN32)
 
   find_library(LIBFTD2XX_LIBRARY
@@ -42,16 +42,16 @@ else (LIBFTD2XX_LIBRARIES AND LIBFTD2XX_INCLUDE_DIRS)
     /usr/local/lib
     /opt/local/lib
     /sw/lib
-	)
+    )
 
   if(LIBFTD2XX_INCLUDE_DIR)
-	set(LIBFTD2XX_INCLUDE_DIRS
+    set(LIBFTD2XX_INCLUDE_DIRS
       ${LIBFTD2XX_INCLUDE_DIR}
-	  )
+      )
   endif(LIBFTD2XX_INCLUDE_DIR)
   set(LIBFTD2XX_LIBRARIES
     ${LIBFTD2XX_LIBRARY}
-	)
+    )
 
   if (LIBFTD2XX_INCLUDE_DIRS AND LIBFTD2XX_LIBRARIES)
     set(LIBFTD2XX_FOUND TRUE)
@@ -59,14 +59,17 @@ else (LIBFTD2XX_LIBRARIES AND LIBFTD2XX_INCLUDE_DIRS)
 
   if (LIBFTD2XX_FOUND)
     if (NOT libftd2xx_FIND_QUIETLY)
-	  message(STATUS "Found libftd2xx: ${LIBFTD2XX_LIBRARIES}")
+      message(STATUS "Found libftd2xx: ${LIBFTD2XX_LIBRARIES}")
     endif (NOT libftd2xx_FIND_QUIETLY)
   else (LIBFTD2XX_FOUND)
     if (libftd2xx_FIND_REQUIRED)
-	  message(FATAL_ERROR "Could not find libftd2xx")
+      message(FATAL_ERROR "Could not find libftd2xx")
     endif (libftd2xx_FIND_REQUIRED)
   endif (LIBFTD2XX_FOUND)
 
+  find_package_handle_standard_args(Ftd2xx
+      REQUIRED_VARS LIBFTD2XX_INCLUDE_DIRS)
+
   # show the LIBFTD2XX_INCLUDE_DIRS and LIBFTD2XX_LIBRARIES variables only in the advanced view
   mark_as_advanced(LIBFTD2XX_INCLUDE_DIRS LIBFTD2XX_LIBRARIES)
 

cmake/modules/FindFtd4222.cmake

@@ -28,11 +28,11 @@ else (LIBFT4222_LIBRARIES AND LIBFT4222_INCLUDE_DIRS)
     /usr/local/include
     /opt/local/include
     /sw/include
-	)
+    )
 
   SET(FTD4222_LIBNAME ft4222)
   IF(WIN32)
-	SET(FTD4222_LIBNAME LibFT4222.lib)
+    SET(FTD4222_LIBNAME LibFT4222.lib)
   ENDIF(WIN32)
 
   find_library(LIBFT4222_LIBRARY
@@ -43,16 +43,16 @@ else (LIBFT4222_LIBRARIES AND LIBFT4222_INCLUDE_DIRS)
     /usr/local/lib
     /opt/local/lib
     /sw/lib
-	)
+    )
 
   if(LIBFT4222_INCLUDE_DIR)
-	set(LIBFT4222_INCLUDE_DIRS
+    set(LIBFT4222_INCLUDE_DIRS
       ${LIBFT4222_INCLUDE_DIR}
-	  )
+      )
   endif(LIBFT4222_INCLUDE_DIR)
   set(LIBFT4222_LIBRARIES
     ${LIBFT4222_LIBRARY}
-	)
+    )
 
   if (LIBFT4222_INCLUDE_DIRS AND LIBFT4222_LIBRARIES)
     set(LIBFT4222_FOUND TRUE)
@@ -60,14 +60,17 @@ else (LIBFT4222_LIBRARIES AND LIBFT4222_INCLUDE_DIRS)
 
   if (LIBFT4222_FOUND)
     if (NOT LIBFT4222_FIND_QUIETLY)
-	  message(STATUS "Found LIBFT4222: ${LIBFT4222_LIBRARIES}")
+      message(STATUS "Found LIBFT4222: ${LIBFT4222_LIBRARIES}")
     endif (NOT LIBFT4222_FIND_QUIETLY)
   else (LIBFT4222_FOUND)
     if (LIBFT4222_FIND_REQUIRED)
-	  message(FATAL_ERROR "Could not find LIBFT4222")
+      message(FATAL_ERROR "Could not find LIBFT4222")
     endif (LIBFT4222_FIND_REQUIRED)
   endif (LIBFT4222_FOUND)
 
+  find_package_handle_standard_args(Ftd4222
+      REQUIRED_VARS LIBFT4222_INCLUDE_DIRS LIBFT4222_LIBRARIES)
+
   # show the LIBFT4222_INCLUDE_DIRS and LIBFT4222_LIBRARIES variables only in the advanced view
   mark_as_advanced(LIBFT4222_INCLUDE_DIRS LIBFT4222_LIBRARIES)
 

docker-compose.yaml

@@ -108,6 +108,7 @@ services:
   ftdi4442:
     extends: all
     environment:
+      - USBPLAT=ON
       - FTDI4222=ON
     command: bash -c "./scripts/run-cmake.sh && make -Cbuild"
 

include/mraa_internal.h

@@ -79,13 +79,6 @@ mraa_platform_t mraa_mips_platform();
  */
 mraa_platform_t mraa_mock_platform();
 
-/**
- * runtime detect running usb platform extender
- *
- * @return mraa_platform_t of the detected platform extender
- */
-mraa_platform_t mraa_usb_platform_extender(mraa_board_t* board);
-
 /**
  * runtime detect iio subsystem
  *

include/mraa_internal_types.h

@@ -497,3 +497,15 @@ typedef struct {
     uint8_t iio_device_count; /**< IIO device count */
 } mraa_iio_info_t;
 #endif
+
+/**
+ * Function pointer typedef for use with platform extender libraries.
+ * Currently only the FT42222.
+ *
+ * @param board Pointer to valid board structure.  If a mraa_board_t
+ * is initialized, it will be added to board->sub_platform
+ *
+ * @return MRAA_SUCCESS if a valid subplaform has been initialized,
+ * otherwise return MRAA_ERROR_PLATFORM_NOT_INITIALISED
+ */
+typedef mraa_result_t (*fptr_add_platform_extender)(mraa_board_t* board);

src/CMakeLists.txt

@@ -176,22 +176,26 @@ endif()
 
 if (USBPLAT)
   message (STATUS "INFO - Adding USB platforms")
+  # USBPLAT is used in mraa.c to decide whether to attempt to load sub-platforms
   set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DUSBPLAT=1")
+  # Loading an FTDI platform extender library using dlopen requires
+  # mraa to link against libdl.  Mraa does NOT link directly to the
+  # FT4222 shim library.
+  set (mraa_LIBS ${mraa_LIBS} dl)
   if (FTDID2xx)
     find_package (Ftd2xx)
     if (${LIBFTD2XX_FOUND})
-      set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DFTDID2XX=1")
       set (mraa_LIBS ${mraa_LIBS} ${LIBFTD2XX_LIBRARIES})
     else ()
       message (SEND_ERROR "Enabled FTDID2xx support but library not found")
     endif ()
   endif ()
+  # The FTDI4222 shim includes libft4222.h (which includes ftd2xx.h), make sure
+  # both exist if building the FTDI4222 shim library
   if (FTDI4222)
+    find_package (Ftd2xx REQUIRED)
     find_package (Ftd4222)
-    if (${LIBFT4222_FOUND})
+    if (NOT ${LIBFT4222_FOUND})
-      set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -DFTDI4222=1")
-      set (mraa_LIBS ${mraa_LIBS} dl)
-    else ()
       message (SEND_ERROR "Enabled FTDI4222 support but library not found")
     endif ()
   endif ()

src/mraa.c

@@ -29,6 +29,7 @@
 #endif
 
 #include <stddef.h>
+#include <dlfcn.h>
 #include <stdlib.h>
 #include <sched.h>
 #include <string.h>
@@ -195,18 +196,21 @@ imraa_init()
     }
 
 #if defined(USBPLAT)
-    // Now detect sub platform, note this is not an else since we could be in
+    syslog(LOG_NOTICE, "Searching for USB plaform extender libraries...");
-    // an error case and fall through to MRAA_ERROR_PLATFORM_NOT_INITIALISED
+    /* If a usb platform lib is present, attempt to load and look for
-    if (plat != NULL) {
+     * necessary symbols for adding extended I/O */
-        mraa_platform_t usb_platform_type = mraa_usb_platform_extender(plat);
+    void* usblib = dlopen("libmraa-platform-ft4222.so", RTLD_LAZY);
-        // if we have no known platform just replace usb platform with platform
+    if (usblib)
-        if (plat->platform_type == MRAA_UNKNOWN_PLATFORM && usb_platform_type != MRAA_UNKNOWN_PLATFORM) {
+    {
-            plat->platform_type = usb_platform_type;
+        syslog(LOG_NOTICE, "Found USB platform extender library: libmraa-platform-ft4222.so");
-        }
+        syslog(LOG_NOTICE, "Detecting FT4222 subplatforms...");
-    }
+        fptr_add_platform_extender add_ft4222_platform =
-    if (plat == NULL) {
+            (fptr_add_platform_extender)dlsym(usblib, "mraa_usb_platform_extender");
-        printf("mraa: FATAL error, failed to initialise platform\n");
+
-        return MRAA_ERROR_PLATFORM_NOT_INITIALISED;
+        /* If this method exists, call it to add a subplatform */
+        syslog(LOG_NOTICE, "Detecting FT4222 subplatforms complete, found %i subplatform/s",
+                ((add_ft4222_platform != NULL) && (add_ft4222_platform(plat) == MRAA_SUCCESS))
+                ? 1 : 0);
     }
 #endif
 
@@ -273,7 +277,8 @@ mraa_deinit()
             free(plat->adv_func);
         }
         mraa_board_t* sub_plat = plat->sub_platform;
-        if (sub_plat != NULL) {
+        /* No alloc's in an FTDI_FT4222 platform structure */
+        if ((sub_plat != NULL) && (sub_plat->platform_type != MRAA_FTDI_FT4222)) {
             if (sub_plat->pins != NULL) {
                 free(sub_plat->pins);
             }

src/usb/CMakeLists.txt

@@ -1,12 +1 @@
-set (mraa_LIB_USB_SRCS_NOAUTO ${PROJECT_SOURCE_DIR}/src/usb/usb.c)
+add_subdirectory(ft4222)
-
-if (FTDI4222)
-  message (INFO " - FTDI4222")
-  set (mraa_LIB_USB_SRCS_NOAUTO ${mraa_LIB_USB_SRCS_NOAUTO}
-    ${PROJECT_SOURCE_DIR}/src/usb/ftdi_ft4222.c
-  )
-endif ()
-
-set (mraa_LIB_PLAT_SRCS_NOAUTO ${mraa_LIB_PLAT_SRCS_NOAUTO}
-     ${mraa_LIB_USB_SRCS_NOAUTO} PARENT_SCOPE)
-

src/usb/ft4222/CMakeLists.txt

@@ -0,0 +1,19 @@
+if (FTDI4222)
+    set(target_name mraa-platform-ft4222)
+    add_library(${target_name} SHARED ftdi_ft4222.cxx ftdi_ft4222.hpp)
+    target_link_libraries(${target_name} ${LIBFT4222_LIBRARIES})
+    install(TARGETS ${target_name} DESTINATION ${LIB_INSTALL_DIR})
+
+    set_target_properties(
+       ${target_name}
+       PROPERTIES
+       SOVERSION ${mraa_VERSION_MAJOR}
+       VERSION ${mraa_VERSION_STRING}
+    )
+    if (MSYS)
+      # Under MSYS we have to put our generated DLL into bin, otherwise it's not picked up
+      install(TARGETS ${target_name} DESTINATION ${CMAKE_INSTALL_BINDIR})
+    else ()
+      install(TARGETS ${target_name} DESTINATION ${LIB_INSTALL_DIR})
+    endif ()
+endif()

src/usb/ft4222/ftdi_ft4222.hpp

@@ -1,3 +1,6 @@
+#ifndef _EXPORT_USERS_NECK_MRAA_FT4222_SRC_USB_FT4222_FTDI_FT4222_HPP
+#define _EXPORT_USERS_NECK_MRAA_FT4222_SRC_USB_FT4222_FTDI_FT4222_HPP
+
 /*
  * Author: Henry Bruce <henry.bruce@intel.com>
  * Copyright (c) 2015 Intel Corporation.
@@ -26,16 +29,22 @@
 
 #ifdef __cplusplus
 extern "C" {
-#endif
 
 #include <dlfcn.h>
 #include "mraa_internal.h"
 
-mraa_result_t mraa_ftdi_ft4222_init();
+/**
-mraa_result_t mraa_ftdi_ft4222_get_version(unsigned int* versionChip, unsigned int* versionLib);
+ * Attempt to initialize a mraa_board_t for a UMFT4222EV module
-mraa_board_t* mraa_ftdi_ft4222();
+ *
-void *libft4222_lib;
+ * @param board Pointer to valid board structure.  If a mraa_board_t
+ * is initialized, it will be added to board->sub_platform
+ *
+ * @return MRAA_SUCCESS if a valid subplaform has been initialized,
+ * otherwise return MRAA_ERROR_PLATFORM_NOT_INITIALISED
+ */
+mraa_result_t mraa_usb_platform_extender(mraa_board_t* board);
 
-#ifdef __cplusplus
 }
 #endif
+
+#endif

src/usb/usb.c

@@ -1,70 +0,0 @@
-/*
- * Author: Henry Bruce <henry.bruce@intel.com>
- * Copyright (c) 2015 Intel Corporation.
- *
- * Permission is hereby granted, free of charge, to any person obtaining
- * a copy of this software and associated documentation files (the
- * "Software"), to deal in the Software without restriction, including
- * without limitation the rights to use, copy, modify, merge, publish,
- * distribute, sublicense, and/or sell copies of the Software, and to
- * permit persons to whom the Software is furnished to do so, subject to
- * the following conditions:
- *
- * The above copyright notice and this permission notice shall be
- * included in all copies or substantial portions of the Software.
- *
- * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
- * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
- * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
- * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
- * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
- * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
- * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
- */
-
-#include <stdlib.h>
-#include <string.h>
-
-#include "mraa_internal.h"
-#ifdef FTDI4222
-#include "usb/ftdi_ft4222.h"
-#endif
-
-
-mraa_platform_t
-mraa_usb_platform_extender(mraa_board_t* board)
-{
-    mraa_board_t* sub_plat = NULL;
-    mraa_platform_t platform_type = MRAA_UNKNOWN_PLATFORM;
-
-#ifdef FTDI4222
-    libft4222_lib = dlopen("libft4222.so", RTLD_LAZY);
-    if (!libft4222_lib) {
-        syslog(LOG_WARNING, "libft4222.so not found, skipping");
-    } else {
-        if (mraa_ftdi_ft4222_init() == MRAA_SUCCESS) {
-            unsigned int versionChip, versionLib;
-            if (mraa_ftdi_ft4222_get_version(&versionChip, &versionLib) == MRAA_SUCCESS) {
-                // TODO: Add ft4222 version checks
-                platform_type = MRAA_FTDI_FT4222;
-            }
-        }
-    }
-#endif
-    switch (platform_type) {
-#ifdef FTDI4222
-        case MRAA_FTDI_FT4222:
-            sub_plat = mraa_ftdi_ft4222();
-            break;
-#endif
-        default:
-            // this is not an error but more that we didn't find a USB platform extender we recognise
-            syslog(LOG_DEBUG, "Unknown USB Platform Extender, currently not supported by MRAA");
-    }
-
-    if (sub_plat != NULL) {
-        sub_plat->platform_type = platform_type;
-        board->sub_platform = sub_plat;
-    }
-    return platform_type;
-}

tests/unit/CMakeLists.txt

@@ -20,9 +20,20 @@ target_link_libraries(api_common_hpp_unit GTest::GTest GTest::Main mraa)
 target_include_directories(api_common_hpp_unit PRIVATE "${CMAKE_SOURCE_DIR}/api")
 gtest_add_tests(api_common_hpp_unit "" AUTO)
 
+if (FTDI4222)
+    # Unit tests - Test platform extenders (as much as possible)
+    add_executable(platform_extender platform_extender/platform_extender.cxx)
+    target_link_libraries(platform_extender GTest::GTest GTest::Main mraa-platform-ft4222 dl)
+    target_include_directories(platform_extender PRIVATE "${PROJECT_SOURCE_DIR}/api"
+        "${PROJECT_SOURCE_DIR}/api/mraa"
+        "${PROJECT_SOURCE_DIR}/include")
+    gtest_add_tests(platform_extender "" AUTO)
+endif ()
+
 # Add a custom target for unit tests
 add_custom_target(tests-unit ALL
     DEPENDS
         api_common_h_unit
         api_common_hpp_unit
-    COMMENT "mraa unit test collection")
+        platform_extender
+        COMMENT "mraa unit test collection")

tests/unit/platform_extender/platform_extender.cxx

@@ -0,0 +1,31 @@
+#include <dlfcn.h>
+
+#include "gtest/gtest.h"
+#include "mraa/common.h"
+#include "mraa_internal_types.h"
+
+/* MRAA test fixture */
+class plaform_extender : public ::testing::Test {};
+
+/* Not much to test w/o hw - test a no-init case.
+ * Note, w/o a link to libmraa, this binary does NOT initialize mraa
+ * with the loader constructor attribute.  This is intended. */
+TEST_F(plaform_extender, test_no_init)
+{
+    mraa_board_t plat = {};
+
+    void* usblib = dlopen("libmraa-platform-ft4222.so", RTLD_LAZY);
+    ASSERT_NE((void*)NULL, usblib)
+        << "Failed to load libmraa-platform-ft4222.so, reason (" << dlerror()
+        << ")";
+
+    fptr_add_platform_extender add_ft4222_platform =
+        (fptr_add_platform_extender)dlsym(usblib, "mraa_usb_platform_extender");
+
+    ASSERT_NE((void*)NULL, add_ft4222_platform) << "Symbol 'add_ft4222_platform' "
+        << "does not exist in libmraa-platform-ft4222.so";
+
+    ASSERT_EQ(MRAA_ERROR_PLATFORM_NOT_INITIALISED, add_ft4222_platform(&plat))
+        << "Initialization returned a valid platform.  Make sure no Ft4222 "
+        << "device is connected";
+}