fixed the gpio code
diff --git a/bbb_cape/src/bbb/gpi.cc b/bbb_cape/src/bbb/gpi.cc
index 90c46d5..5bb4902 100644
--- a/bbb_cape/src/bbb/gpi.cc
+++ b/bbb_cape/src/bbb/gpi.cc
@@ -1,48 +1,28 @@
-#include "gpi.h"
+#include "bbb/gpi.h"
#include <stdio.h>
+#include <errno.h>
+#include <string.h>
#include "aos/common/logging/logging.h"
namespace bbb {
-Gpi::Gpi(int bank, int pin) {
- // All the failures here are fatal, seeing as
- // if we can't initialize the pin, we're
- // probably screwed anyway.
- if (!InitPin(bank, pin)) {
- LOG(FATAL, "Failed to export pin.\n");
- }
- if (!DoPinDirSet(1)) {
- LOG(FATAL, "Failed to set pin as an input.\n");
- }
+Gpi::Gpi(int bank, int pin) : GpioPin(bank, pin, true) {
}
-int Gpi::Read() {
- // NOTE: I can't find any docs confirming that one can indeed
- // poll a pin's value using this method, but it seems that it
- // should work. Really, the "right" (interrupt driven) way to
- // do this is to set an event, but that involves messing with
- // dev tree crap, which I'm not willing to do unless we need
- // this functionality.
- char buf, val_path[64];
- snprintf(val_path, sizeof(val_path), "/sys/class/gpio/gpio%d/value",
- kernel_pin_);
-
- if ((handle_ = fopen(val_path, "rb")) == NULL) {
- LOG(ERROR, "Unable to open file for pin value reading.\n");
- fclose(handle_);
- return -1;
+bool Gpi::Read() {
+ int value = fgetc(value_handle_);
+ if (value < 0) {
+ LOG(FATAL, "fgetc(%p) for pin (%d,%d) failed with %d: %s\n",
+ value_handle_, bank_, pin_, errno, strerror(errno));
}
-
- if (fread(&buf, sizeof(char), 1, handle_) <= 1) {
- LOG(ERROR, "Reading from file failed with error %d\n",
- ferror(handle_));
- fclose(handle_);
- return -1;
+ switch (value - '0') {
+ case 0: return false;
+ case 1: return true;
+ default:
+ LOG(FATAL, "unknown pin value %c\n", value);
}
- fclose(handle_);
- return atoi(&buf);
}
} // namespace bbb
diff --git a/bbb_cape/src/bbb/gpi.h b/bbb_cape/src/bbb/gpi.h
index be41a11..0583f0b 100644
--- a/bbb_cape/src/bbb/gpi.h
+++ b/bbb_cape/src/bbb/gpi.h
@@ -1,21 +1,18 @@
#ifndef BBB_CAPE_SRC_BBB_GPI_H_
#define BBB_CAPE_SRC_BBB_GPI_H_
-#include "gpios.h"
-
-// Subclass of Pin for input pins.
+#include "bbb/gpios.h"
namespace bbb {
-class Gpi : public Pin {
+// Subclass of Pin for input pins.
+class Gpi : public GpioPin {
public:
- // See the base class for what these args mean.
Gpi(int bank, int pin);
// Read the current value of the pin.
- // Returns 1 if it reads high, 0 for low.
- // Returns -1 if fails to read the pin for some reason.
- int Read();
+ // Returns true if it's high and false if it's low.
+ bool Read();
};
} // namespace bbb
diff --git a/bbb_cape/src/bbb/gpios.cc b/bbb_cape/src/bbb/gpios.cc
index 8594ba4..79d38bc 100644
--- a/bbb_cape/src/bbb/gpios.cc
+++ b/bbb_cape/src/bbb/gpios.cc
@@ -2,67 +2,72 @@
#include <stdio.h>
#include <stdlib.h>
+#include <errno.h>
+#include <string.h>
#include "aos/common/logging/logging_impl.h"
namespace bbb {
-Pin::Pin() {}
-
-Pin::~Pin() {
- // Unexport the pin.
- if ((handle_ = fopen("/sys/class/gpio/unexport", "ab")) == NULL) {
- LOG(WARNING, "Could not open file to unexport pin.\n");
- // There's nothing intelligent we can really do here.
- return;
- }
-
- fprintf(handle_, "%d", kernel_pin_);
- fclose(handle_);
-}
-
-bool Pin::InitPin(int bank, int pin) {
- kernel_pin_ = bank * 32 + pin;
-
+GpioPin::GpioPin(int bank, int pin, bool input, bool initial_value)
+ : bank_(bank), pin_(pin), kernel_pin_(bank * 32 + pin) {
// Export the pin.
- if ((handle_ = fopen("/sys/class/gpio/export", "ab")) == NULL) {
- LOG(ERROR, "Could not open file for exporting pin.\n");
- return false;
+ FILE *export_handle = fopen("/sys/class/gpio/export", "a");
+ if (export_handle == NULL) {
+ LOG(WARNING,
+ "Could not open file to export pin (%d,%d) because of %d: %s.\n",
+ bank_, pin_, errno, strerror(errno));
+ } else {
+ if (fprintf(export_handle, "%d", kernel_pin_) < 0) {
+ LOG(WARNING, "Could not write to file %p to export pin (%d,%d) because "
+ "of %d: %s.\n",
+ export_handle, bank_, pin_, errno, strerror(errno));
+ }
+ if (fclose(export_handle) == -1) {
+ LOG(WARNING, "fclose(%p) failed with %d: %s\n", export_handle, errno,
+ strerror(errno));
+ }
}
- fprintf(handle_, "%d", kernel_pin_);
- fclose(handle_);
- return true;
+ char direction_path[64];
+ snprintf(direction_path, sizeof(direction_path),
+ "/sys/class/gpio/gpio%d/direction", kernel_pin_);
+
+ FILE *direction_handle = fopen(direction_path, "w");
+ if (direction_handle == NULL) {
+ LOG(FATAL, "fopen(%s, \"w+\") failed with %d: %s\n",
+ direction_path, errno, strerror(errno));
+ }
+
+ if (fputs(input ? "in" : (initial_value ? "high" : "low"),
+ direction_handle) < 0) {
+ LOG(FATAL, "setting direction for pin (%d,%d) failed with %d: %s\n",
+ bank_, pin_, errno, strerror(errno));
+ }
+ if (fclose(direction_handle) == -1) {
+ LOG(WARNING, "fclose(%p) failed with %d: %s\n", direction_handle, errno,
+ strerror(errno));
+ }
}
-bool Pin::DoPinDirSet(int direction) {
- char type_path[64];
- snprintf(type_path, sizeof(type_path), "/sys/class/gpio/gpio%d/direction",
- kernel_pin_);
-
- if ((handle_ = fopen(type_path, "rb+")) == NULL) {
- LOG(ERROR, "Unable open file for pin direction setting.\n");
- return false;
+GpioPin::~GpioPin() {
+ // Unexport the pin.
+ FILE *unexport_handle = fopen("/sys/class/gpio/unexport", "a");
+ if (unexport_handle == NULL) {
+ LOG(WARNING,
+ "Could not open file to unexport pin (%d,%d) because of %d: %s.\n",
+ bank_, pin_, errno, strerror(errno));
+ } else {
+ if (fprintf(unexport_handle, "%d", kernel_pin_) < 0) {
+ LOG(WARNING, "Could not write to file %p to unexport pin (%d,%d) because "
+ "of %d: %s.\n",
+ unexport_handle, bank_, pin_, errno, strerror(errno));
+ }
+ if (fclose(unexport_handle) == -1) {
+ LOG(WARNING, "fclose(%p) failed with %d: %s\n", unexport_handle, errno,
+ strerror(errno));
+ }
}
-
- switch (direction) {
- case 1:
- // Input.
- fprintf(handle_, "in");
- break;
- case 2:
- // Output.
- // If it's an output, we should specify an initial direction.
- fprintf(handle_, "low");
- break;
- default:
- LOG(ERROR, "Invalid direction identifier %d.\n", direction);
- fclose(handle_);
- return false;
- }
-
- fclose(handle_);
- return true;
}
} // namespace bbb
diff --git a/bbb_cape/src/bbb/gpios.h b/bbb_cape/src/bbb/gpios.h
index 4925f1b..1471b43 100644
--- a/bbb_cape/src/bbb/gpios.h
+++ b/bbb_cape/src/bbb/gpios.h
@@ -5,42 +5,28 @@
#include "aos/common/macros.h"
-// As it turns out, controlling the BBB's GPIO pins
-// from C++ is kind of a pain. The purpose of this
-// code is to provide a simple wrapper that masks
-// all the ugly stuff and exposes a nice API.
+// Controlling GPIO pins
+// from C++ is a pain. This code provides a simple wrapper that makes it easy to
+// use cleanly.
// Based on example from
// <http://learnbuildshare.wordpress.com/2013/05/29/beaglebone-black-digital-ouput/>
// This is a base class for all gpio related stuff.
-// Use either a gpi or gpo subclass if you want to do things.
+// Use either the Gpi or the Gpo subclass if you want to do things.
namespace bbb {
-class Pin {
- public:
- // Not strictly necessary for this to be virtual,
- // but it's a good idea.
- virtual ~Pin();
- // When you don't define this and use the
- // DISALLOW_COPY_AND_ASSIGN macro, the compiler
- // doesn't seem to create a default one.
- Pin();
-
+class GpioPin {
protected:
- // Set the pin direction.
- // 1 makes it an input.
- // 2 makes it an output and sets the initial state to low.
- bool DoPinDirSet(int direction);
- // Export the pin, so we can use it.
- // Here, for example, if you wanted to use
- // GPIO1_28, you would give it 1, 28 as arguments.
- bool InitPin(int bank, int pin);
+ // initial_value only matters if input is false.
+ GpioPin(int bank, int pin, bool input, bool initial_value = false);
+ virtual ~GpioPin();
- FILE *handle_ = NULL;
- int kernel_pin_ = -1;
+ FILE *value_handle_ = NULL;
+ const int bank_, pin_, kernel_pin_;
- DISALLOW_COPY_AND_ASSIGN(Pin);
+ private:
+ DISALLOW_COPY_AND_ASSIGN(GpioPin);
};
} // namespace bbb
diff --git a/bbb_cape/src/bbb/gpo.cc b/bbb_cape/src/bbb/gpo.cc
index 959b132..b43c4b0 100644
--- a/bbb_cape/src/bbb/gpo.cc
+++ b/bbb_cape/src/bbb/gpo.cc
@@ -1,37 +1,21 @@
-#include "gpo.h"
+#include "bbb/gpo.h"
#include <stdio.h>
+#include <errno.h>
+#include <string.h>
#include "aos/common/logging/logging.h"
namespace bbb {
-Gpo::Gpo(int bank, int pin) {
- // All the failures here are fatal, seeing as
- // failure to initialize the pin would at best
- // severely cripple the calling process.
- if (!InitPin(bank, pin)) {
- LOG(FATAL, "Failed to export the pin.\n");
+Gpo::Gpo(int bank, int pin, bool initial_value)
+ : GpioPin(bank, pin, false, initial_value) {}
+
+void Gpo::Set(bool high) {
+ if (fputc(high ? '1' : '0', value_handle_) < 0) {
+ LOG(FATAL, "fputc(%c, %p) for pin (%d,%d) failed with %d: %s\n",
+ high ? '1': '0', value_handle_, bank_, pin_, errno, strerror(errno));
}
- if (!DoPinDirSet(2)) {
- LOG(FATAL, "Failed to make the pin an output.\n");
- }
-}
-
-bool Gpo::Set(const bool high) {
- char val_path[64];
- snprintf(val_path, sizeof(val_path), "/sys/class/gpio/gpio%d/value",
- kernel_pin_);
-
- if ((handle_ = fopen(val_path, "rb+")) == NULL) {
- LOG(ERROR, "Unable open file for pin value setting.\n");
- return false;
- }
-
- fprintf(handle_, "%d", high);
- fclose(handle_);
-
- return true;
}
} // namespace bbb
diff --git a/bbb_cape/src/bbb/gpo.h b/bbb_cape/src/bbb/gpo.h
index 596daf5..893fe4a 100644
--- a/bbb_cape/src/bbb/gpo.h
+++ b/bbb_cape/src/bbb/gpo.h
@@ -6,15 +6,11 @@
namespace bbb {
// Gpios subclass for output pins.
-class Gpo : public Pin {
+class Gpo : public GpioPin {
public:
- // See the base class for what these args mean.
- Gpo(int bank, int pin);
- // Sets the pin to either high or low.
- // If the argument is true, is sets it high.
- // Otherwise, it sets it low.
- bool Set(const bool high);
-
+ Gpo(int bank, int pin, bool initial_value = false);
+ // Sets the pin to either high (true) or low (false).
+ void Set(bool high);
};
} // namespace bbb