Merge pull request #13097 from basilfx/feature/u8g2_refactoring_and_fixes

pkg/u8g2: refactoring and 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 -u src/csrc/Makefile $(PKG_BUILDDIR)/csrc/Makefile
-	cp -u src/csrc/u8g2_riotos.c $(PKG_BUILDDIR)/csrc/u8g2_riotos.c
-	cp -u src/sys/sdl/common/Makefile $(PKG_BUILDDIR)/sys/sdl/common/Makefile
-	cp -u src/sys/utf8/common/Makefile $(PKG_BUILDDIR)/sys/utf8/common/Makefile
+	cp src/Makefile $(PKG_BUILDDIR)/Makefile
+	cp src/csrc/Makefile $(PKG_BUILDDIR)/csrc/Makefile
+	cp src/csrc/u8x8_riotos.c $(PKG_BUILDDIR)/csrc/u8x8_riotos.c
+	cp src/csrc/u8x8_riotos.h $(PKG_BUILDDIR)/csrc/u8x8_riotos.h
+	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_riotos_hw_i2c`
+* `u8x8_byte_hw_spi_riotos`
+* `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)`
-* `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.
+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.
 
 ### Example
-```
+```c
 u8g2_t u8g2;
 
-gpio_t pins[] = {
-    [U8X8_PIN_CS] = GPIO(PA, 0),
-    [U8X8_PIN_DC] = GPIO(PA, 1),
-    [U8X8_PIN_RESET] = GPIO(PA, 2)
+u8x8_riotos_t user_data =
+{
+    .device_index = SPI_DEV(0),
+    .pin_cs = GPIO_PIN(PA, 0),
+    .pin_dc = GPIO_PIN(PA, 1),
+    .pin_reset = GPIO_PIN(PA, 2)
 };
 
-uint32_t bitmap = (
-    (1 << U8X8_PIN_CS) +
-    (1 << U8X8_PIN_DC) +
-    (1 << U8X8_PIN_RESET)
-);
+u8g2_SetUserPtr(&u8g2, &user_data);
 
 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/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 (pins_enabled & ((uint32_t)1 << i)) {
-            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_cs != GPIO_UNDEF) {
+        gpio_init(u8x8_riot_ptr->pin_cs, GPIO_OUT);
+    }
+
+    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)) {
-                gpio_write(u8g2->pins[U8X8_PIN_CS], arg_int);
+            if (u8x8_riot_ptr != NULL && u8x8_riot_ptr->pin_cs != GPIO_UNDEF) {
+                gpio_write(u8x8_riot_ptr->pin_cs, arg_int);
             }
             break;
         case U8X8_MSG_GPIO_DC:
-            if (u8g2->pins_enabled & (1 << U8X8_PIN_DC)) {
-                gpio_write(u8g2->pins[U8X8_PIN_DC], arg_int);
+            if (u8x8_riot_ptr != NULL && u8x8_riot_ptr->pin_dc != GPIO_UNDEF) {
+                gpio_write(u8x8_riot_ptr->pin_dc, arg_int);
             }
             break;
         case U8X8_MSG_GPIO_RESET:
-            if (u8g2->pins_enabled & (1 << U8X8_PIN_RESET)) {
-                gpio_write(u8g2->pins[U8X8_PIN_RESET], arg_int);
+            if (u8x8_riot_ptr != NULL &&  u8x8_riot_ptr->pin_reset != GPIO_UNDEF) {
+                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
-uint8_t u8x8_byte_riotos_hw_spi(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr)
+#ifdef MODULE_PERIPH_SPI
+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
-uint8_t u8x8_byte_riotos_hw_i2c(u8x8_t *u8g2, uint8_t msg, uint8_t arg_int, void *arg_ptr)
+#ifdef MODULE_PERIPH_I2C
+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 index;
+    static uint8_t buffer[32]; /* u8x8 will never send more than 32 bytes
+                                  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 */

tests/pkg_u8g2/Makefile

@@ -12,33 +12,33 @@ TEST_SPI ?= 0
 
 TEST_ADDR ?= 0x3c
 
-TEST_PIN_CS ?= GPIO_PIN\(0,0\)
-TEST_PIN_DC ?= GPIO_PIN\(0,0\)
-TEST_PIN_RESET ?= GPIO_PIN\(0,0\)
+TEST_PIN_CS ?= GPIO_UNDEF
+TEST_PIN_DC ?= GPIO_UNDEF
+TEST_PIN_RESET ?= GPIO_UNDEF
 
 ifeq ($(TEST_OUTPUT),3)
-TEST_DISPLAY ?= u8g2_Setup_ssd1306_128x64_noname_1
+  TEST_DISPLAY ?= u8g2_Setup_ssd1306_128x64_noname_1
 endif
 
 ifeq ($(TEST_OUTPUT),4)
-TEST_DISPLAY ?= u8g2_Setup_ssd1306_i2c_128x64_noname_1
+  TEST_DISPLAY ?= u8g2_Setup_ssd1306_i2c_128x64_noname_1
 endif
 
 # features depend on output type
 ifeq ($(TEST_OUTPUT),1)
-USEMODULE += u8g2_utf8
+  USEMODULE += u8g2_utf8
 endif
 
 ifeq ($(TEST_OUTPUT),2)
-USEMODULE += u8g2_sdl
+  USEMODULE += u8g2_sdl
 endif
 
 ifeq ($(TEST_OUTPUT),3)
-FEATURES_REQUIRED += periph_gpio periph_spi
+  FEATURES_REQUIRED += periph_gpio periph_spi
 endif
 
 ifeq ($(TEST_OUTPUT),4)
-FEATURES_REQUIRED += periph_gpio periph_i2c
+  FEATURES_REQUIRED += periph_gpio periph_i2c
 endif
 
 # export parameters

tests/pkg_u8g2/README.md

@@ -11,7 +11,7 @@ This test application will initialize the U8g2 to output on one of the following
 * output to I2C graphics screen.
 * output to SPI graphics screen.
 
-Note: you may have to run `make clean` between different output modes.
+Note: you may have to run `make clean` after you changed to a different output mode.
 
 ### Output to terminal
 To output to this virtual screen, supply `TEST_OUTPUT=1` to the `make` command.
@@ -26,7 +26,7 @@ To output to screen, supply `TEST_OUTPUT=3` to the `make` command.
 
 * `TEST_SPI` — The SPI device.
 * `TEST_PIN_CS` — If applicable, the CS pin.
-* `TEST_PIN_DC` — If applicable, the Command/Data pin.
+* `TEST_PIN_DC` — If applicable, the Data/Command pin.
 * `TEST_PIN_RESET` — If applicable, the reset pin.
 * `TEST_DISPLAY` — The used display driver (see https://github.com/olikraus/u8g2/wiki/u8g2setupc). Make sure you select a SPI compatible display.
 

tests/pkg_u8g2/main.c

@@ -71,7 +71,9 @@
 #endif
 
 #include "xtimer.h"
+
 #include "u8g2.h"
+#include "u8x8_riotos.h"
 
 /**
  * @brief   RIOT-OS logo, 64x32 pixels at 8 pixels per byte.
@@ -101,28 +103,6 @@ static const uint8_t logo[] = {
     0x00, 0x00, 0x00, 0x00
 };
 
-#if (TEST_OUTPUT == TEST_OUTPUT_I2C) || (TEST_OUTPUT == TEST_OUTPUT_SPI)
-/**
- * @brief   RIOT-OS pin mapping of U8g2 pin numbers to RIOT-OS GPIO pins.
- * @note    To minimize the overhead, you can implement an alternative for
- *          u8x8_gpio_and_delay_riotos.
- */
-static gpio_t pins[] = {
-    [U8X8_PIN_CS] = TEST_PIN_CS,
-    [U8X8_PIN_DC] = TEST_PIN_DC,
-    [U8X8_PIN_RESET] = TEST_PIN_RESET
-};
-
-/**
- * @brief   Bit mapping to indicate which pins are set.
- */
-static uint32_t pins_enabled = (
-    (1 << U8X8_PIN_CS) +
-    (1 << U8X8_PIN_DC) +
-    (1 << U8X8_PIN_RESET)
-);
-#endif
-
 int main(void)
 {
     uint32_t screen = 0;
@@ -146,20 +126,34 @@ int main(void)
 #if TEST_OUTPUT == TEST_OUTPUT_SPI
     puts("Initializing to SPI.");
 
-    TEST_DISPLAY(&u8g2, U8G2_R0, u8x8_byte_riotos_hw_spi, u8x8_gpio_and_delay_riotos);
+    TEST_DISPLAY(&u8g2, U8G2_R0, u8x8_byte_hw_spi_riotos, u8x8_gpio_and_delay_riotos);
 
-    u8g2_SetPins(&u8g2, pins, pins_enabled);
-    u8g2_SetDevice(&u8g2, SPI_DEV(TEST_SPI));
+    u8x8_riotos_t user_data =
+    {
+        .device_index = TEST_SPI,
+        .pin_cs = TEST_PIN_CS,
+        .pin_dc = TEST_PIN_DC,
+        .pin_reset = TEST_PIN_RESET,
+    };
+
+    u8g2_SetUserPtr(&u8g2, &user_data);
 #endif
 
     /* initialize to I2C */
 #if TEST_OUTPUT == TEST_OUTPUT_I2C
     puts("Initializing to I2C.");
 
-    TEST_DISPLAY(&u8g2, U8G2_R0, u8x8_byte_riotos_hw_i2c, u8x8_gpio_and_delay_riotos);
+    TEST_DISPLAY(&u8g2, U8G2_R0, u8x8_byte_hw_i2c_riotos, u8x8_gpio_and_delay_riotos);
 
-    u8g2_SetPins(&u8g2, pins, pins_enabled);
-    u8g2_SetDevice(&u8g2, I2C_DEV(TEST_I2C));
+    u8x8_riotos_t user_data =
+    {
+        .device_index = TEST_I2C,
+        .pin_cs = TEST_PIN_CS,
+        .pin_dc = TEST_PIN_DC,
+        .pin_reset = TEST_PIN_RESET,
+    };
+
+    u8g2_SetUserPtr(&u8g2, &user_data);
     u8g2_SetI2CAddress(&u8g2, TEST_ADDR);
 #endif