pkg/u8g2: refactor the U8g2 package

- Passing RIOT-OS specific data as user_ptr
- Added necessary macros to Makefile.dep
- Added missing explicit dependency to xtimer module
- Renamed u8g2_riotos.c to more appropriate u8x8_riotos.c
- Added u8x8_riotos.h with the RIOT-OS specific functionality
- Removed the now-obsolete patch of U8g2
- Other minor fixes

pkg/u8g2/Makefile

@@ -1,14 +1,15 @@
 PKG_NAME=u8g2
 PKG_URL=https://github.com/olikraus/u8g2
-PKG_VERSION=f08ff974c03e5c848bc5d2ae3fddb6a97897881a
+PKG_VERSION=2e47ff05e7e7b38057fa875e14c4ecce2fb6a615
 PKG_LICENSE=BSD-2-Clause
 
 include $(RIOTBASE)/pkg/pkg.mk
 
 all:
-	cp -u src/Makefile $(PKG_BUILDDIR)/Makefile
+	cp src/Makefile $(PKG_BUILDDIR)/Makefile
-	cp -u src/csrc/Makefile $(PKG_BUILDDIR)/csrc/Makefile
+	cp src/csrc/Makefile $(PKG_BUILDDIR)/csrc/Makefile
-	cp -u src/csrc/u8g2_riotos.c $(PKG_BUILDDIR)/csrc/u8g2_riotos.c
+	cp src/csrc/u8x8_riotos.c $(PKG_BUILDDIR)/csrc/u8x8_riotos.c
-	cp -u src/sys/sdl/common/Makefile $(PKG_BUILDDIR)/sys/sdl/common/Makefile
+	cp src/csrc/u8x8_riotos.h $(PKG_BUILDDIR)/csrc/u8x8_riotos.h
-	cp -u src/sys/utf8/common/Makefile $(PKG_BUILDDIR)/sys/utf8/common/Makefile
+	cp src/sys/sdl/common/Makefile $(PKG_BUILDDIR)/sys/sdl/common/Makefile
+	cp src/sys/utf8/common/Makefile $(PKG_BUILDDIR)/sys/utf8/common/Makefile
 	"$(MAKE)" -C $(PKG_BUILDDIR)

pkg/u8g2/Makefile.dep

@@ -0,0 +1,3 @@
+USEMODULE += xtimer
+
+FEATURES_REQUIRED += periph_gpio

pkg/u8g2/Makefile.include

@@ -1,5 +1,8 @@
 INCLUDES += -I$(PKGDIRBASE)/u8g2/csrc
 
+# The RIOT-OS interface needs this to store peripheral information.
+CFLAGS += -DU8X8_WITH_USER_PTR
+
 # Link SDL if enabled.
 ifneq (,$(filter u8g2_sdl,$(USEMODULE)))
   LINKFLAGS += `sdl2-config --libs`

pkg/u8g2/README.md

@@ -6,47 +6,39 @@
 The library is originally written for Arduino boards, but it runs just fine on other platforms, as long as the right drivers are available.
 
 ## Usage
-Just put `USEPKG += u8g2` in your Makefile and `#include "u8g2.h"` to your code. Refer to the [U8g2 wiki](https://github.com/olikraus/u8g2/wiki) for more information on the API.
+Just put `USEPKG += u8g2` in your Makefile and `#include "u8g2.h"` into your code. Refer to the [U8g2 wiki](https://github.com/olikraus/u8g2/wiki) for more information on the API.
 
 ## RIOT-OS interface
-This package patches the original source to add an interface for RIOT-OS.
+This adds an interface for RIOT-OS.
 
 The following two callbacks add support for the included drivers via I2C and SPI peripherals:
 
-* `u8x8_byte_riotos_hw_spi`
+* `u8x8_byte_hw_spi_riotos`
-* `u8x8_byte_riotos_hw_i2c`
+* `u8x8_byte_hw_i2c_riotos`
 
 For timing and GPIO related operations, the following callback is available.
 
 * `u8x8_gpio_and_delay_riotos`
 
-U8g2 needs to map pin numbers to RIOT-OS pin numbers. It also needs to know which peripheral to use. The following two methods can be used to set this information.
+These methods require a structure containing peripheral information (`u8x8_riotos_t`), that is set using the `u8g2_SetUserPtr` function. This structure contains the peripheral and pin mapping.
 
-* `u8g2_SetPins(u8g2_dev, pins, bitmap)`
+If the above interface is not sufficient, it is still possible to write a dedicated interface by (re-)implementing the methods above. Refer to the [U8g2 wiki](https://github.com/olikraus/u8g2/wiki) for more information.
-* `u8g2_SetDevice(u8g2_dev, dev)`
-
-Note: `pins` should point to `gpio_t` array of U8g2 pin numbers to RIOT-OS pins. Due to this, `pins` can take up an additional 100 bytes, because it will use memory for the pins you do not map. You can overcome this limitation by implementing `u8x8_gpio_and_delay_riotos` yourself and hardcode the pins.
 
 ### Example
-```
+```c
 u8g2_t u8g2;
 
-gpio_t pins[] = {
+u8x8_riotos_t user_data =
-    [U8X8_PIN_CS] = GPIO(PA, 0),
+{
-    [U8X8_PIN_DC] = GPIO(PA, 1),
+    .device_index = SPI_DEV(0),
-    [U8X8_PIN_RESET] = GPIO(PA, 2)
+    .pin_cs = GPIO_PIN(PA, 0),
+    .pin_dc = GPIO_PIN(PA, 1),
+    .pin_reset = GPIO_PIN(PA, 2)
 };
 
-uint32_t bitmap = (
+u8g2_SetUserPtr(&u8g2, &user_data);
-    (1 << U8X8_PIN_CS) +
-    (1 << U8X8_PIN_DC) +
-    (1 << U8X8_PIN_RESET)
-);
 
 u8g2_Setup_ssd1306_128x64_noname_1(&u8g2, U8G2_R0, u8x8_byte_riotos_hw_spi, u8x8_gpio_and_delay_riotos);
-
-u8g2_SetPins(&u8g2, pins, bitmap);
-u8g2_SetDevice(&u8g2, SPI_DEV(0));
 ```
 
 ## Virtual displays
@@ -56,7 +48,7 @@ For targets without an I2C or SPI, virtual displays are available. These display
 * By adding `USEMODULE += u8g2_sdl`, a SDL virtual display will be used. This is only available on native targets that have SDL installed. It uses `sdl2-config` to find the headers and libraries. Note that RIOT-OS builds 32-bit binaries and requires 32-bit SDL libraries.
 
 ### Example
-```
+```c
 u8g2_t u8g2;
 
 u8g2_SetupBuffer_Utf8(&u8g2, U8G2_R0);

pkg/u8g2/patches/0001-add-RIOT-OS-interface.patch

@@ -1,83 +0,0 @@
-From a17e644521b91d5f5fc35e567375ac5998f24b66 Mon Sep 17 00:00:00 2001
-From: Bas Stottelaar <basstottelaar@gmail.com>
-Date: Sun, 11 Mar 2018 22:13:10 +0100
-Subject: [PATCH 1/1] add RIOT-OS interface.
-
----
- csrc/u8g2.h |  3 +++
- csrc/u8x8.h | 17 ++++++++++++++---
- 2 files changed, 17 insertions(+), 3 deletions(-)
-
-diff --git a/csrc/u8g2.h b/csrc/u8g2.h
-index 48239709..c8934cf7 100644
---- a/csrc/u8g2.h
-+++ b/csrc/u8g2.h
-@@ -388,6 +388,9 @@ void u8g2_ClearDisplay(u8g2_t *u8g2);
- #define u8g2_SetMenuDownPin(u8g2, val) u8x8_SetMenuDownPin(u8g2_GetU8x8(u8g2), (val))
- #endif
- 
-+#define u8g2_SetPins(u8x8,pins,pins_enabled) u8x8_SetPins(u8g2_GetU8x8(&u8g2), pins, pins_enabled)
-+#define u8g2_SetDevice(u8x8,device) u8x8_SetDevice(u8g2_GetU8x8(&u8g2), device)
-+
- /*==========================================*/
- /* u8g2_setup.c */
- 
-diff --git a/csrc/u8x8.h b/csrc/u8x8.h
-index bbeff59f..ae65bbb1 100644
---- a/csrc/u8x8.h
-+++ b/csrc/u8x8.h
-@@ -111,6 +111,8 @@
- #include <stdint.h>
- #include <stddef.h>
- 
-+#include "periph/gpio.h"
-+
- #if defined(__GNUC__) && defined(__AVR__)
- #include <avr/pgmspace.h>
- #endif 
-@@ -174,9 +176,9 @@ uint8_t u8x8_pgm_read_esp(const uint8_t * addr);   /* u8x8_8x8.c */
- #  define U8X8_PROGMEM
- #endif
- 
--#ifdef ARDUINO
--#define U8X8_USE_PINS
--#endif
-+//#ifdef ARDUINO
-+//#define U8X8_USE_PINS
-+//#endif
- 
- /*==========================================*/
- /* U8X8 typedefs and data structures */
-@@ -342,6 +344,10 @@ struct u8x8_struct
- #ifdef U8X8_USE_PINS 
-   uint8_t pins[U8X8_PIN_CNT];	/* defines a pinlist: Mainly a list of pins for the Arduino Envionment, use U8X8_PIN_xxx to access */
- #endif
-+
-+gpio_t* pins;
-+uint32_t pins_enabled;
-+uint32_t dev;
- };
- 
- #ifdef U8X8_WITH_USER_PTR
-@@ -371,6 +377,8 @@ struct u8x8_struct
- #define u8x8_SetMenuDownPin(u8x8, val) u8x8_SetPin((u8x8),U8X8_PIN_MENU_DOWN,(val))
- #endif
- 
-+#define u8x8_SetPins(u8x8,pins,pins_enabled) {(u8x8)->pins = (pins); (u8x8)->pins_enabled = (pins_enabled);}
-+#define u8x8_SetDevice(u8x8,device) ((u8x8)->dev = device)
- 
- /*==========================================*/
- 
-@@ -973,6 +981,9 @@ extern const uint8_t u8x8_font_pxplustandynewtv_u[] U8X8_FONT_SECTION("u8x8_font
- 
- /* end font list */
- 
-+extern uint8_t u8x8_byte_riotos_hw_spi(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr);
-+extern uint8_t u8x8_gpio_and_delay_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr);
-+extern uint8_t u8x8_byte_riotos_hw_i2c(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr);
- 
- #ifdef __cplusplus
- }
--- 
-2.14.2
-

pkg/u8g2/src/csrc/u8x8_riotos.c

@@ -11,63 +11,75 @@
  * @{
  *
  * @file
- * @brief       U8g2 driver for interacting with RIOT-OS drivers
+ * @brief       U8g2 driver for interacting with RIOT-OS peripherals
  *
  * @author      Bas Stottelaar <basstottelaar@gmail.com>
  *
  * @}
  */
 
+#include <assert.h>
 #include <stdio.h>
 #include <string.h>
 
-#include "u8g2.h"
+#include "u8x8_riotos.h"
 
 #include "xtimer.h"
-#include "periph/spi.h"
-#include "periph/i2c.h"
-#include "periph/gpio.h"
 
-#ifdef SPI_NUMOF
+#ifdef MODULE_PERIPH_SPI
+#include "periph/spi.h"
+#endif
+#ifdef MODULE_PERIPH_I2C
+#include "periph/i2c.h"
+#endif
+
+#ifdef MODULE_PERIPH_SPI
 static spi_clk_t u8x8_pulse_width_to_spi_speed(uint32_t pulse_width)
 {
-    uint32_t cycle_time = 2 * pulse_width;
+    const uint32_t cycle_time = 2 * pulse_width;
 
     if (cycle_time < 100) {
         return SPI_CLK_10MHZ;
-    } else if (cycle_time < 200) {
+    }
+    else if (cycle_time < 200) {
         return SPI_CLK_5MHZ;
-    } else if (cycle_time < 1000) {
+    }
+    else if (cycle_time < 1000) {
         return SPI_CLK_1MHZ;
-    } else if (cycle_time < 2500) {
+    }
+    else if (cycle_time < 2500) {
         return SPI_CLK_400KHZ;
     }
 
     return SPI_CLK_100KHZ;
 }
-#endif /* SPI_NUMOF */
 
-#ifdef SPI_NUMOF
 static spi_mode_t u8x8_spi_mode_to_spi_conf(uint32_t spi_mode)
 {
     return (spi_mode_t) spi_mode;
 }
-#endif /* SPI_NUMOF */
+#endif /* MODULE_PERIPH_SPI */
 
-static void u8x8_enable_pins(gpio_t* pins, uint32_t pins_enabled)
+/**
+ * @brief   Enable the selected pins in RIOT-OS.
+ */
+static void _enable_pins(const u8x8_riotos_t *u8x8_riot_ptr)
 {
-    uint8_t i;
+    /* no hardware peripheral is being used, nothing to be done */
+    if (u8x8_riot_ptr == NULL) {
+        return;
+    }
 
-    for (i = 0; i < 32; i++) {
+    if (u8x8_riot_ptr->pin_cs != GPIO_UNDEF) {
-        if (pins_enabled & ((uint32_t)1 << i)) {
+        gpio_init(u8x8_riot_ptr->pin_cs, GPIO_OUT);
-            if (pins[i] != GPIO_UNDEF) {
-                if (i < U8X8_PIN_OUTPUT_CNT) {
-                    gpio_init(pins[i], GPIO_OUT);
-                } else {
-                    gpio_init(pins[i], GPIO_IN);
-                }
     }
+
+    if (u8x8_riot_ptr->pin_dc != GPIO_UNDEF) {
+        gpio_init(u8x8_riot_ptr->pin_dc, GPIO_OUT);
     }
+
+    if (u8x8_riot_ptr->pin_reset != GPIO_UNDEF) {
+        gpio_init(u8x8_riot_ptr->pin_reset, GPIO_OUT);
     }
 }
 
@@ -75,9 +87,14 @@ uint8_t u8x8_gpio_and_delay_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, v
 {
     (void) arg_ptr;
 
+    const u8x8_riotos_t *u8x8_riot_ptr = u8x8_GetUserPtr(u8g2);
+
+    /* assert that user_ptr is correctly set */
+    assert(u8x8_riot_ptr != NULL);
+
     switch (msg) {
         case U8X8_MSG_GPIO_AND_DELAY_INIT:
-            u8x8_enable_pins(u8g2->pins, u8g2->pins_enabled);
+            _enable_pins(u8x8_riot_ptr);
             break;
         case U8X8_MSG_DELAY_MILLI:
             xtimer_usleep(arg_int * 1000);
@@ -89,18 +106,18 @@ uint8_t u8x8_gpio_and_delay_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, v
             xtimer_nanosleep(arg_int * 100);
             break;
         case U8X8_MSG_GPIO_CS:
-            if (u8g2->pins_enabled & (1 << U8X8_PIN_CS)) {
+            if (u8x8_riot_ptr != NULL && u8x8_riot_ptr->pin_cs != GPIO_UNDEF) {
-                gpio_write(u8g2->pins[U8X8_PIN_CS], arg_int);
+                gpio_write(u8x8_riot_ptr->pin_cs, arg_int);
             }
             break;
         case U8X8_MSG_GPIO_DC:
-            if (u8g2->pins_enabled & (1 << U8X8_PIN_DC)) {
+            if (u8x8_riot_ptr != NULL && u8x8_riot_ptr->pin_dc != GPIO_UNDEF) {
-                gpio_write(u8g2->pins[U8X8_PIN_DC], arg_int);
+                gpio_write(u8x8_riot_ptr->pin_dc, arg_int);
             }
             break;
         case U8X8_MSG_GPIO_RESET:
-            if (u8g2->pins_enabled & (1 << U8X8_PIN_RESET)) {
+            if (u8x8_riot_ptr != NULL &&  u8x8_riot_ptr->pin_reset != GPIO_UNDEF) {
-                gpio_write(u8g2->pins[U8X8_PIN_RESET], arg_int);
+                gpio_write(u8x8_riot_ptr->pin_reset, arg_int);
             }
             break;
         default:
@@ -110,10 +127,15 @@ uint8_t u8x8_gpio_and_delay_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, v
     return 1;
 }
 
-#ifdef SPI_NUMOF
+#ifdef MODULE_PERIPH_SPI
-uint8_t u8x8_byte_riotos_hw_spi(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr)
+uint8_t u8x8_byte_hw_spi_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr)
 {
-    spi_t dev = (spi_t) u8g2->dev;
+    const u8x8_riotos_t *u8x8_riot_ptr = u8x8_GetUserPtr(u8g2);
+
+    /* assert that user_ptr is correctly set */
+    assert(u8x8_riot_ptr != NULL);
+
+    spi_t dev = SPI_DEV(u8x8_riot_ptr->device_index);
 
     switch (msg) {
         case U8X8_MSG_BYTE_SEND:
@@ -121,7 +143,6 @@ uint8_t u8x8_byte_riotos_hw_spi(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void
                                arg_ptr, NULL, (size_t)arg_int);
             break;
         case U8X8_MSG_BYTE_INIT:
-            spi_init_pins(dev);
             break;
         case U8X8_MSG_BYTE_SET_DC:
             u8x8_gpio_SetDC(u8g2, arg_int);
@@ -146,20 +167,27 @@ uint8_t u8x8_byte_riotos_hw_spi(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void
 
     return 1;
 }
-#endif /* SPI_NUMOF */
+#endif /* MODULE_PERIPH_SPI */
 
-#ifdef I2C_NUMOF
+#ifdef MODULE_PERIPH_I2C
-uint8_t u8x8_byte_riotos_hw_i2c(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr)
+uint8_t u8x8_byte_hw_i2c_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr)
 {
-    static uint8_t buffer[255];
+    static uint8_t buffer[32]; /* u8x8 will never send more than 32 bytes
-    static uint8_t index;
+                                  between START_TRANSFER and END_TRANSFER */
+    static size_t index;
 
-    i2c_t dev = (i2c_t) u8g2->dev;
+    const u8x8_riotos_t *u8x8_riot_ptr = u8x8_GetUserPtr(u8g2);
+
+    /* assert that user_ptr is correctly set */
+    assert(u8x8_riot_ptr != NULL);
+
+    i2c_t dev = I2C_DEV(u8x8_riot_ptr->device_index);
 
     switch (msg) {
         case U8X8_MSG_BYTE_SEND:
             memcpy(&buffer[index], arg_ptr, arg_int);
             index += arg_int;
+            assert(index <= sizeof(buffer));
             break;
         case U8X8_MSG_BYTE_INIT:
             break;
@@ -179,4 +207,4 @@ uint8_t u8x8_byte_riotos_hw_i2c(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void
 
     return 1;
 }
-#endif /* I2C_NUMOF */
+#endif /* MODULE_PERIPH_I2C */

pkg/u8g2/src/csrc/u8x8_riotos.h

@@ -0,0 +1,70 @@
+/*
+ * Copyright (C) 2018 Petr Vyleta <vyleta.developer@gmail.com>
+ *
+ * This file is subject to the terms and conditions of the GNU Lesser
+ * General Public License v2.1. See the file LICENSE in the top level
+ * directory for more details.
+ */
+
+/**
+ * @ingroup     pkg_u8g2
+ * @{
+ *
+ * @file
+ * @brief       U8g2 driver for interacting with RIOT-OS peripherals
+ *
+ * @author      Bas Stottelaar <basstottelaar@gmail.com>
+ * @author      Petr Vyleta <vyleta.developer@gmail.com>
+ *
+ * @}
+ */
+
+#ifndef U8X8_RIOTOS_H
+#define U8X8_RIOTOS_H
+
+#include "u8x8.h"
+
+#include "periph/gpio.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * @brief   Holds RIOT-OS specific peripheral data.
+ *
+ * This structure has to be set via the U8g2 function @p u8g2_SetUserPtr, prior
+ * the call to @p u8x8_InitDisplay or @p u8g2_InitDisplay respectively.
+ *
+ * The structure can be easily extended with further required definitions (e.g
+ * other pins) if necessary, without breaking the RIOT-OS adaptation of U8g2.
+ */
+typedef struct {
+    void *user_ptr;             /**< Pointer to optionally store any additional user-data */
+
+    unsigned int device_index;  /**< Index of the SPI/I2C device */
+    gpio_t pin_cs;              /**< Pin for SPI CS, GPIO_UNDEF if not used */
+    gpio_t pin_dc;              /**< Pin for SPI DC, GPIO_UNDEF if not used */
+    gpio_t pin_reset;           /**< Pin for RESET, GPIO_UNDEF if not used */
+} u8x8_riotos_t;
+
+/**
+ * To be used as the u8x8_msg_cb as gpio_and_delay_cb in u8x8_Setup() for use with RIOT-OS
+ */
+uint8_t u8x8_gpio_and_delay_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr);
+
+/**
+ * To be used as the u8x8_msg_cb as gpio_and_delay_cb in u8x8_Setup() for use with RIOT-OS.
+ */
+uint8_t u8x8_byte_hw_spi_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr);
+
+/**
+ * To be used as the u8x8_msg_cb as gpio_and_delay_cb in u8x8_Setup() for use with RIOT-OS.
+ */
+uint8_t u8x8_byte_hw_i2c_riotos(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* U8X8_RIOTOS_H */