namespace for utility drivers

[2bndy5]

My main concerns with the current implementations are

1. Naming conflicts for structures in global namespace. RF24_arduino_wrappers::SPI is better than ::SPI.
2. Indicate (in source code) that our Arduino wrappers are library-specific. I wouldn't want clever devs/users to assume they can use our Arduino-like wrappers for other Arduino libraries (specifically on Linux).

Proposal

Each utility driver that is not directly using the Arduino framework would be encapsulated in a namespace. For example purposes, I'll use RF24_arduino_wrappers as the namespace name.

// in utility/SPIDEV/spi.h
namespace RF24_arduino_wrappers {

class SPI { /* ... */ };

} // end namespace RF24_arduino_wrappers

Then declare a convenient macro to pull in the namespace

// in RF24_config.h
#if !defined(ARDUINO)
    #include "utility/includes.h"
    #define USE_RF24_ARDUINO_API using namespace RF24_arduino_wrappers;
#else
    // a dummy macro to avoid more ifdef conditions in RF24.cpp
    #define USE_RF24_ARDUINO_API
// ...

Finally, pull in the namespace in the RF24 functions' body that will be using the namespaced driver API.

// in RF24.cpp
void RF24::csn(bool mode)
{
    USE_RF24_ARDUINO_API

    // Now, digitalWrite() will resolve to RF24_arduino_wrappers::digitalWrite() 
    // for non-arduino platforms only.
    // Arduino platforms still use global digitalWrite()
    digitalWrite(csn_pin, mode);
} // namespace drops out of scope here

Warning
This idea is still a work in progress and would require a bit more work than proposed.

For function signatures that use types declared in the namespace, we could augment the definition for _SPI macro used in RF24::begin(_SPI*) and similarly for the various typedef rf24_gpio_pin_t declarations.

Alternative solutions

None that I'm aware of besides renaming the structures/macros (with more verbose names) to be more specific to RF24 library. I'm leaning towards namespaces because it allows compatibility with the original Arduino API; renaming macros isn't ideal and still pollutes the global namespace.

Additional context

Using namespaces might aid in other endeavors that want to customize how the Arduino API is used (#925), but this would be just a partial solution in that case.

I initially experimented with this idea when porting my CirquePinnacle Arduino library to Linux and PicoSDK. However, I was a bit more liberal with refactoring the drivers I initially copied from this repo; see 2bndy5/CirquePinnacle#2 (which also added python bindings).

[TMRh20]

I think this would be a good idea. Would like to see how to code all comes together though.

[2bndy5]

It might be considered a breaking change in the PicoSDK driver (or any non-Arduino platform) for those that are not using the default SPI bus... Linux drivers would have the most backward compatible changes (I think).

I'll try to get some coded in the next week or 2.

[2bndy5]

Just applied this idea to the SPIDEV driver... It was pretty easy and had no code changes to RF24.cpp (only 1 line changed in RF24.h). However, I neglected to wrap the interrupt stuff in the namespace because it would break with the examples_linux/interruptConfigure.cpp code.

experimental SPIDEV patch

--- a/RF24.h
+++ b/RF24.h
@@ -122,7 +122,7 @@ private:
 #endif
 
 #if defined(RF24_LINUX) || defined(XMEGA_D3) /* XMEGA can use SPI class */
-    SPI spi;
+    RF24_arduino_wrappers::SPI spi;
 #endif // defined (RF24_LINUX) || defined (XMEGA_D3)
 #if defined(RF24_SPI_PTR)
     _SPI* _spi;
diff --git a/utility/SPIDEV/RF24_arch_config.h b/utility/SPIDEV/RF24_arch_config.h
index fa457fd..9e79347 100644
--- a/utility/SPIDEV/RF24_arch_config.h
+++ b/utility/SPIDEV/RF24_arch_config.h
@@ -57,14 +57,14 @@ typedef uint16_t rf24_gpio_pin_t;
 #define pgm_read_ptr(p)  (*(void* const*)(p))
 
 // Function, constant map as a result of migrating from Arduino
-#define LOW                      GPIO::OUTPUT_LOW
-#define HIGH                     GPIO::OUTPUT_HIGH
-#define INPUT                    GPIO::DIRECTION_IN
-#define OUTPUT                   GPIO::DIRECTION_OUT
-#define digitalWrite(pin, value) GPIO::write(pin, value)
-#define pinMode(pin, direction)  GPIO::open(pin, direction)
-#define delay(milisec)           __msleep(milisec)
-#define delayMicroseconds(usec)  __usleep(usec)
-#define millis()                 __millis()
+#define LOW                      RF24_arduino_wrappers::GPIO::OUTPUT_LOW
+#define HIGH                     RF24_arduino_wrappers::GPIO::OUTPUT_HIGH
+#define INPUT                    RF24_arduino_wrappers::GPIO::DIRECTION_IN
+#define OUTPUT                   RF24_arduino_wrappers::GPIO::DIRECTION_OUT
+#define digitalWrite(pin, value) RF24_arduino_wrappers::GPIO::write(pin, value)
+#define pinMode(pin, direction)  RF24_arduino_wrappers::GPIO::open(pin, direction)
+#define delay(milisec)           RF24_arduino_wrappers::__msleep(milisec)
+#define delayMicroseconds(usec)  RF24_arduino_wrappers::__usleep(usec)
+#define millis()                 RF24_arduino_wrappers::__millis()
 
 #endif // RF24_UTILITY_SPIDEV_RF24_ARCH_CONFIG_H_
diff --git a/utility/SPIDEV/compatibility.cpp b/utility/SPIDEV/compatibility.cpp
index 6b96f65..23ad142 100644
--- a/utility/SPIDEV/compatibility.cpp
+++ b/utility/SPIDEV/compatibility.cpp
@@ -1,10 +1,12 @@
 #include "compatibility.h"
+#include <time.h>
+#include <chrono>
 
+namespace RF24_arduino_wrappers {
 long long mtime, seconds, useconds;
 //static struct timeval start, end;
 //struct timespec start, end;
-#include <time.h>
-#include <chrono>
+
 /**********************************************************************/
 /**
  * This function is added in order to simulate arduino delay() function
@@ -46,3 +48,5 @@ uint32_t __millis()
 
     return std::chrono::duration_cast<std::chrono::milliseconds>(end - start).count();
 }
+
+} // end namespace RF24_arduino_wrappers
diff --git a/utility/SPIDEV/compatibility.h b/utility/SPIDEV/compatibility.h
index 7015323..88628f4 100644
--- a/utility/SPIDEV/compatibility.h
+++ b/utility/SPIDEV/compatibility.h
@@ -18,6 +18,8 @@ extern "C" {
 #include <time.h>
 #include <sys/time.h>
 
+namespace RF24_arduino_wrappers{
+
 void __msleep(int milisec);
 
 void __usleep(int milisec);
@@ -26,6 +28,8 @@ void __start_timer();
 
 uint32_t __millis();
 
+} // end namespace RF24_arduino_wrappers
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/utility/SPIDEV/gpio.cpp b/utility/SPIDEV/gpio.cpp
index a898600..30f8919 100644
--- a/utility/SPIDEV/gpio.cpp
+++ b/utility/SPIDEV/gpio.cpp
@@ -16,7 +16,7 @@
 #include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-
+namespace RF24_arduino_wrappers {
 std::map<int, GPIOfdCache_t> GPIO::cache;
 
 GPIO::GPIO()
@@ -176,3 +176,5 @@ void GPIO::write(int port, int value)
     fclose(f);
     */
 }
+
+} // end namespace RF24_arduino_wrappers
diff --git a/utility/SPIDEV/gpio.h b/utility/SPIDEV/gpio.h
index c802ce6..7142d69 100644
--- a/utility/SPIDEV/gpio.h
+++ b/utility/SPIDEV/gpio.h
@@ -16,8 +16,8 @@
 #include <cstdio>
 #include <map>
 #include <stdexcept>
-
-/** Specific excpetion for SPI errors */
+namespace RF24_arduino_wrappers {
+/** Specific exception for SPI errors */
 class GPIOException : public std::runtime_error
 {
 public:
@@ -56,4 +56,6 @@ private:
     static std::map<int, GPIOfdCache_t> cache;
 };
 
+} // end namespace RF24_arduino_wrappers
+
 #endif // RF24_UTILITY_SPIDEV_GPIO_H_
diff --git a/utility/SPIDEV/interrupt.h b/utility/SPIDEV/interrupt.h
index f52117b..6e5c5e2 100644
--- a/utility/SPIDEV/interrupt.h
+++ b/utility/SPIDEV/interrupt.h
@@ -45,4 +45,5 @@ extern void rfInterrupts();
 #ifdef __cplusplus
 }
 #endif
-#endif
+
+#endif // __RF24_INTERRUPT_H__
diff --git a/utility/SPIDEV/spi.cpp b/utility/SPIDEV/spi.cpp
index 4fb0113..5c5d81d 100644
--- a/utility/SPIDEV/spi.cpp
+++ b/utility/SPIDEV/spi.cpp
@@ -21,7 +21,7 @@
 #include <unistd.h>
 
 #define RF24_SPIDEV_BITS 8
-
+namespace RF24_arduino_wrappers {
 SPI::SPI()
     : fd(-1), _spi_speed(RF24_SPI_SPEED)
 {
@@ -193,3 +193,5 @@ SPI::~SPI()
         close(this->fd);
     }
 }
+
+} // end namespace RF24_arduino_wrappers
\ No newline at end of file
diff --git a/utility/SPIDEV/spi.h b/utility/SPIDEV/spi.h
index 35efe2d..fc5ef2f 100644
--- a/utility/SPIDEV/spi.h
+++ b/utility/SPIDEV/spi.h
@@ -14,8 +14,8 @@
 #include <stdexcept>
 
 #include "../../RF24_config.h" // This is cyclical and should be fixed
-
-/** Specific excpetion for SPI errors */
+namespace RF24_arduino_wrappers {
+/** Specific exception for SPI errors */
 class SPIException : public std::runtime_error
 {
 public:
@@ -48,4 +48,6 @@ private:
     void init(uint32_t spi_speed = RF24_SPI_SPEED);
 };
 
+} // end namespace RF24_arduino_wrappers
+
 #endif // RF24_UTILITY_SPIDEV_SPI_H_

I'm starting to think this idea would better be addressed when refactoring the drivers. Since it would definitely cause breaking changes in PicoSDK (specifically for those using non-default SPI bus), I think a solution to this would be better for a v2.0 release.