From 60410fd6f82d18d60863f744adf52b908ab04c10 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 14 Feb 2021 18:21:59 +0100 Subject: [PATCH] some minor improvements --- linux/gpio/GpioCookie.cpp | 20 ++++++------- linux/gpio/GpioCookie.h | 31 ++++++++++---------- linux/gpio/GpioIF.h | 4 +-- linux/gpio/LinuxLibgpioIF.cpp | 55 ++++++++++++++++++----------------- linux/gpio/LinuxLibgpioIF.h | 10 +++---- 5 files changed, 59 insertions(+), 61 deletions(-) diff --git a/linux/gpio/GpioCookie.cpp b/linux/gpio/GpioCookie.cpp index 6355578c..7f80b550 100644 --- a/linux/gpio/GpioCookie.cpp +++ b/linux/gpio/GpioCookie.cpp @@ -1,25 +1,23 @@ #include "GpioCookie.h" -#include +#include GpioCookie::GpioCookie() { } void GpioCookie::addGpio(gpioId_t gpioId, GpioConfig_t gpioConfig){ - gpioMapIter = gpioMap.find(gpioId); + auto gpioMapIter = gpioMap.find(gpioId); if(gpioMapIter == gpioMap.end()) { - std::pair status = gpioMap.emplace(gpioId, gpioConfig); - if (status.second == false) { - sif::error << "GpioCookie::addGpio: Failed to add GPIO " - << gpioId << "to GPIO map" << std::endl; + auto statusPair = gpioMap.emplace(gpioId, gpioConfig); + if (statusPair.second == false) { + sif::error << "GpioCookie::addGpio: Failed to add GPIO " << gpioId << + "to GPIO map" << std::endl; } + return; } - else { - sif::error << "GpioCookie::addGpio: GPIO already exists in GPIO map " - << std::endl; - } + sif::error << "GpioCookie::addGpio: GPIO already exists in GPIO map " << std::endl; } -GpioMap& GpioCookie::getGpioMap() const { +GpioMap GpioCookie::getGpioMap() const { return gpioMap; } diff --git a/linux/gpio/GpioCookie.h b/linux/gpio/GpioCookie.h index 8ec30d79..5e4a7b1d 100644 --- a/linux/gpio/GpioCookie.h +++ b/linux/gpio/GpioCookie.h @@ -17,23 +17,22 @@ enum Direction { /** * @brief Struct containing information about the GPIO to use. This is * required by the libgpiod to access and drive a GPIO. - * @param chipname String of the chipname specifying the group which contains - * the GPIO to access. E.g. gpiochip0. To detect names of - * GPIO groups run gpiodetect on the linux command line. - * @param lineNum The offset of the GPIO within the GPIO group. - * @param consumer Name of the consumer. Simply a description of the GPIO configuration. - * @param direction Specifies whether the GPIO should be used as in- or output. - * @param initValue Defines the initial state of the GPIO when configured as output. Only required - * for output GPIOs. + * @param chipname String of the chipname specifying the group which contains the GPIO to + * access. E.g. gpiochip0. To detect names of GPIO groups run gpiodetect on + * the linux command line. + * @param lineNum The offset of the GPIO within the GPIO group. + * @param consumer Name of the consumer. Simply a description of the GPIO configuration. + * @param direction Specifies whether the GPIO should be used as in- or output. + * @param initValue Defines the initial state of the GPIO when configured as output. + * Only required for output GPIOs. * @param lineHandle The handle returned by gpiod_chip_get_line will be later written to this * pointer. */ typedef struct GpioConfig { GpioConfig(std::string chipname_, int lineNum_, std::string consumer_, - gpio::Direction direction_, int initValue_) : - chipname(chipname_), lineNum(lineNum_), consumer(consumer_), direction(direction_), - initValue(initValue_) { - } + gpio::Direction direction_, int initValue_): + chipname(chipname_), lineNum(lineNum_), consumer(consumer_), + direction(direction_), initValue(initValue_) {} std::string chipname; int lineNum; std::string consumer; @@ -41,6 +40,7 @@ typedef struct GpioConfig { int initValue; struct gpiod_line* lineHandle = nullptr; } GpioConfig_t; + using GpioMap = std::unordered_map; using GpioMapIter = GpioMap::iterator; @@ -65,12 +65,13 @@ public: /** * @brief Get map with registered GPIOs. */ - GpioMap& getGpioMap() const; + GpioMap getGpioMap() const; private: - + /** + * Returns a copy of the internal GPIO map. + */ GpioMap gpioMap; - GpioMapIter gpioMapIter; }; #endif diff --git a/linux/gpio/GpioIF.h b/linux/gpio/GpioIF.h index d1f02dd8..8d36a1b7 100644 --- a/linux/gpio/GpioIF.h +++ b/linux/gpio/GpioIF.h @@ -11,7 +11,7 @@ typedef uint16_t gpioId_t; * over GPIOs. * @author J. Meier */ -class GpioIF : public HasReturnvaluesIF{ +class GpioIF : public HasReturnvaluesIF { public: virtual ~GpioIF() {}; @@ -21,7 +21,7 @@ public: * @param cookie Cookie specifying informations of the GPIOs required * by a object. */ - virtual ReturnValue_t initialize(CookieIF * cookie) = 0; + virtual ReturnValue_t initialize(CookieIF* cookie) = 0; /** * @brief By implementing this function a child must provide the diff --git a/linux/gpio/LinuxLibgpioIF.cpp b/linux/gpio/LinuxLibgpioIF.cpp index e5db2934..da235716 100644 --- a/linux/gpio/LinuxLibgpioIF.cpp +++ b/linux/gpio/LinuxLibgpioIF.cpp @@ -33,7 +33,7 @@ ReturnValue_t LinuxLibgpioIF::initialize(CookieIF * cookie){ return result; } - result = configureGpios(&mapToAdd); + result = configureGpios(mapToAdd); if (result != RETURN_OK) { return RETURN_FAILED; } @@ -44,8 +44,8 @@ ReturnValue_t LinuxLibgpioIF::initialize(CookieIF * cookie){ return RETURN_OK; } -ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap* mapToAdd) { - GpioMapIter mapToAddIter; +ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap& mapToAdd) { + //GpioMapIter mapToAddIter; std::string chipname; unsigned int lineNum; struct gpiod_chip *chip; @@ -54,18 +54,18 @@ ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap* mapToAdd) { struct gpiod_line *lineHandle; int result; - mapToAddIter = mapToAdd->begin(); - for (; mapToAddIter != mapToAdd->end(); mapToAddIter++) { - - chipname = mapToAddIter->second.chipname; + //mapToAddIter = mapToAdd->begin(); + for(auto& gpioConfig: mapToAdd) { + //for (; mapToAddIter != mapToAdd->end(); mapToAddIter++) { + chipname = gpioConfig.second.chipname; chip = gpiod_chip_open_by_name(chipname.c_str()); if (!chip) { sif::error << "LinuxLibgpioIF::configureGpios: Failed to open chip " - << chipname << ". Gpio ID: " << mapToAddIter->first << std::endl; + << chipname << ". Gpio ID: " << gpioConfig.first << std::endl; return RETURN_FAILED; } - lineNum = mapToAddIter->second.lineNum; + lineNum = gpioConfig.second.lineNum; lineHandle = gpiod_chip_get_line(chip, lineNum); if (!lineHandle) { sif::error << "LinuxLibgpioIF::configureGpios: Failed to open line" << std::endl; @@ -73,16 +73,16 @@ ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap* mapToAdd) { return RETURN_FAILED; } - direction = mapToAddIter->second.direction; - consumer = mapToAddIter->second.consumer; + direction = gpioConfig.second.direction; + consumer = gpioConfig.second.consumer; /* Configure direction and add a description to the GPIO */ switch (direction) { case gpio::OUT: result = gpiod_line_request_output(lineHandle, consumer.c_str(), - mapToAddIter->second.initValue); + gpioConfig.second.initValue); if (result < 0) { sif::error << "LinuxLibgpioIF::configureGpios: Failed to request line " - << lineNum << " from GPIO instance with ID: " << mapToAddIter->first + << lineNum << " from GPIO instance with ID: " << gpioConfig.first << std::endl; gpiod_line_release(lineHandle); return RETURN_FAILED; @@ -92,7 +92,7 @@ ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap* mapToAdd) { result = gpiod_line_request_input(lineHandle, consumer.c_str()); if (result < 0) { sif::error << "LinuxLibgpioIF::configureGpios: Failed to request line " - << lineNum << " from GPIO instance with ID: " << mapToAddIter->first + << lineNum << " from GPIO instance with ID: " << gpioConfig.first << std::endl; gpiod_line_release(lineHandle); return RETURN_FAILED; @@ -107,7 +107,7 @@ ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap* mapToAdd) { * Write line handle to GPIO configuration instance so it can later be used to set or * read states of GPIOs. */ - mapToAddIter->second.lineHandle = lineHandle; + gpioConfig.second.lineHandle = lineHandle; } return RETURN_OK; } @@ -125,7 +125,7 @@ ReturnValue_t LinuxLibgpioIF::driveGpio(gpioId_t gpioId, int result; struct gpiod_line *lineHandle; - gpioMapIter = gpioMap.find(gpioId); + auto gpioMapIter = gpioMap.find(gpioId); if (gpioMapIter == gpioMap.end()){ sif::debug << "LinuxLibgpioIF::driveGpio: Unknown gpio id " << gpioId << std::endl; return RETURN_FAILED; @@ -145,7 +145,7 @@ ReturnValue_t LinuxLibgpioIF::driveGpio(gpioId_t gpioId, ReturnValue_t LinuxLibgpioIF::readGpio(gpioId_t gpioId, int* gpioState) { struct gpiod_line *lineHandle; - gpioMapIter = gpioMap.find(gpioId); + auto gpioMapIter = gpioMap.find(gpioId); if (gpioMapIter == gpioMap.end()){ sif::debug << "LinuxLibgpioIF::readGpio: Unknown gpio id " << gpioId << std::endl; return RETURN_FAILED; @@ -157,23 +157,24 @@ ReturnValue_t LinuxLibgpioIF::readGpio(gpioId_t gpioId, int* gpioState) { return RETURN_OK; } -ReturnValue_t LinuxLibgpioIF::checkForConflicts(GpioMap mapToAdd){ +ReturnValue_t LinuxLibgpioIF::checkForConflicts(GpioMap& mapToAdd){ gpioId_t gpioId; - GpioMapIter mapToAddIter = mapToAdd.begin(); - for(; mapToAddIter != mapToAdd.end(); mapToAddIter++){ - gpioId = mapToAddIter->first; + auto gpioMapIter = gpioMap.begin(); + for(auto& gpioConfig: mapToAdd) { + gpioId = gpioConfig.first; + /* Cross check with private map */ gpioMapIter = gpioMap.find(gpioId); - if(gpioMapIter != mapToAdd.end()){ + if(gpioMapIter != mapToAdd.end()) { /* An entry for this GPIO already exists. Check if configuration * of direction is equivalent */ - if (mapToAddIter->second.direction != gpioMapIter->second.direction){ - sif::error << "LinuxLibgpioIF::checkForConflicts: Detected conflict " - << "for GPIO " << mapToAddIter->first << std::endl; - return RETURN_OK; + if (gpioConfig.second.direction != gpioMapIter->second.direction){ + sif::error << "LinuxLibgpioIF::checkForConflicts: Detected conflict for GPIO " << + gpioConfig.first << std::endl; + return RETURN_FAILED; } /* Remove element from map to add because a entry for this GPIO * already exists */ - mapToAdd.erase(mapToAddIter); + mapToAdd.erase(gpioConfig.first); } } return RETURN_OK; diff --git a/linux/gpio/LinuxLibgpioIF.h b/linux/gpio/LinuxLibgpioIF.h index f91a35ca..6f210458 100644 --- a/linux/gpio/LinuxLibgpioIF.h +++ b/linux/gpio/LinuxLibgpioIF.h @@ -23,16 +23,14 @@ public: LinuxLibgpioIF(object_id_t objectId); virtual ~LinuxLibgpioIF(); - ReturnValue_t initialize(CookieIF * cookie) override; + ReturnValue_t initialize(CookieIF* gpioCookie) override; ReturnValue_t pullHigh(gpioId_t gpioId) override; ReturnValue_t pullLow(gpioId_t gpioId) override; ReturnValue_t readGpio(gpioId_t gpioId, int* gpioState) override; private: - - /*Holds the information and configuration of all used GPIOs */ + /* Holds the information and configuration of all used GPIOs */ GpioMap gpioMap; - GpioMapIter gpioMapIter; /** * @brief This functions drives line of a GPIO specified by the GPIO ID. @@ -52,12 +50,12 @@ private: * * @return RETURN_OK if successful, otherwise RETURN_FAILED */ - ReturnValue_t checkForConflicts(GpioMap mapToAdd); + ReturnValue_t checkForConflicts(GpioMap& mapToAdd); /** * @brief Performs the initial configuration of all GPIOs specified in the GpioMap mapToAdd. */ - ReturnValue_t configureGpios(GpioMap* mapToAdd); + ReturnValue_t configureGpios(GpioMap& mapToAdd); }; #endif /* BSP_Q7S_GPIO_LINUXLIBGPIOIF_H_ */