GPIO包装类

Question

我已经收到了关于上一个版本的这里的评论，但是由于更新会使答案不再合适，我认为打开一个新的帖子是明智的。

我遵循了上一次评论的大部分提示，现在我想检查一下我是做了什么，写了什么，还是犯了新的错误。

如果您对完整的源代码感兴趣，请单击这里

gpio.h

#ifndef SRC_GPIO_H_
#define SRC_GPIO_H_

#include <string>
#include <fstream>
#include <iostream>

using std::string;
using std::fstream;

enum GPIODirection {
    GPIO_IN  = 0,
    GPIO_OUT = 1
};

enum GPIOValue {
    GPIO_LOW  = 0,
    GPIO_HIGH = 1
};

class GPIO {
    public:
        explicit GPIO(int id);
        ~GPIO();

        int  Value();
        void Value(int value);
        int  Direction();
        void Direction(int value);

    private:
        int id_;

        fstream value_;
        fstream direction_;

        bool Exists();
        void Export();
        void Unexport();

        static const string PATH_EXPORT;
        static const string PATH_UNEXPORT;
        static const string PREFIX;
        static const string POSTFIX_VALUE;
        static const string POSTFIX_DIRECTION;
};

#endif

gpio.cc

#include "gpio.h"

#include <string>
#include <sstream>
#include <fstream>
#include <iostream>
#include <exception>
#include <stdexcept>

using std::ios;
using std::endl;
using std::string;
using std::stringstream;
using std::logic_error;
using std::runtime_error;

const string GPIO::PATH_EXPORT       = "/sys/class/gpio/export";
const string GPIO::PATH_UNEXPORT     = "/sys/class/gpio/unexport";
const string GPIO::PREFIX            = "/sys/class/gpio/gpio";
const string GPIO::POSTFIX_VALUE     = "/value";
const string GPIO::POSTFIX_DIRECTION = "/direction";

GPIO::GPIO(int id) {
    id_ = id;

    Export();

    stringstream value_path;
    stringstream direction_path;

    value_path << PREFIX;
    value_path << id;
    value_path << POSTFIX_VALUE;

    direction_path << PREFIX;
    direction_path << id;
    direction_path << POSTFIX_DIRECTION;

    value_.open(value_path.str().c_str());
    direction_.open(direction_path.str().c_str());
}

GPIO::~GPIO() {
    value_.close();
    direction_.close();

    Unexport();
}

bool
GPIO::Exists() {
    stringstream path;

    path << PREFIX;
    path << id_;

    fstream gpio;

    gpio.open(path.str().c_str());

    bool result = gpio.good();

    gpio.close();

    return result;
}

void
GPIO::Export() {
    if (Exists()) return;

    fstream gpio_export;
    stringstream string_stream;

    string_stream << id_;

    gpio_export.open(PATH_EXPORT.c_str(), ios::out);
    gpio_export << string_stream.str();
    gpio_export.close();
}

void
GPIO::Unexport() {
    if (!Exists()) return;

    fstream gpio_unexport;
    stringstream string_stream;

    string_stream << id_;

    gpio_unexport.open(PATH_UNEXPORT.c_str(), ios::out);
    gpio_unexport << string_stream.str();
    gpio_unexport.close();
}

int
GPIO::Value() {
    string value;

    value_.seekp(0);
    value_ >> value;

    if (value == "0") return GPIO_LOW;
    if (value == "1") return GPIO_HIGH;

    throw logic_error("Invalid GPIO value.");
}

void
GPIO::Value(int value) {
    value_.seekp(0);

    switch (value) {
        case GPIO_LOW:
            value_ << "0" << endl; 

            if (!value_.good())
                throw runtime_error("Error writting to value file stream.");

            break;
        case GPIO_HIGH:
            value_ << "1" << endl; 

            if (!value_.good())
                throw runtime_error("Error writting to value file stream.");

            break;
        default:
            throw logic_error("Error cannot set invalid GPIO value.");
    }
}

int
GPIO::Direction() {
    string direction;

    direction_.seekp(0);

    direction_ >> direction;

    if (direction == "in") return GPIO_IN;
    if (direction == "out") return GPIO_OUT;

    throw logic_error("Invalid GPIO direction.");
}

void
GPIO::Direction(int value) {
    direction_.seekp(0);

    switch (value) {
        case GPIO_IN:
            direction_ << "in" << endl;

            if (!direction_.good())
                throw runtime_error("Error writting to direciton file stream.");

            break;
        case GPIO_OUT:
            direction_ << "out" << endl;

            if (!direction_.good())
                throw runtime_error("Error writting to direciton file stream.");

            break;
        default:
            throw logic_error("Error cannot set invalid GPIO direction.");
    }
}

我想可以做得更好：

• 类方法
• Exist()的实现
• 错误处理
• 字符串格式
• 路径常数
• int到string强制转换(std::to_string似乎不可用)

Answer 1

一些简短的评论：

• 对我来说，value和direction听起来不像流名。也许他们该叫别的什么？这取决于领域术语(我不熟悉)。
• 不要在标头中使用using来污染全局命名空间。编写完整的限定符。
• 您的stringstream操作看起来像是可以将它们分解成一个单独的函数。
• 注意，std::endl强制流刷新。如果您只想要换行符，请使用<< '\n'代替。
• 我觉得把#includes按字母排序是很好的。
• 在C++11中，fstream::open也采用std::string参数。
• 考虑更改Value和Direction的名称(如William在另一篇评论中所指出的)。如果一个函数改变了对象的状态，那么它的命名应该是不同的(可能是用动词命名)。
• 更喜欢使用构造函数中的初始化程序列表初始化变量。
• 您的Value和Direction函数几乎是相同的。

您可以重构如下内容：

std::string GPIO::ReadFromBeginningOfStream(std::fstream& stream)
{
    stream.seekg(0); // NOTE: seekg, *not* seekp
    std::string tmp;
    stream >> tmp;
    return tmp;
}

int
GPIO::Value() {
    std::string const& value = ReadFromBeginningOfStream(value_);

    if (value == "0") return GPIO_LOW;
    if (value == "1") return GPIO_HIGH;

    throw logic_error("Invalid GPIO value.");
}

然后是类似的，但是在Direction()中传递另一个流。

注意:通过调用seekg()在输入流中查找；通过调用seekp()在输出流中查找。

• Exists()应该是const，如：bool Exists() const;