From d196c7c4a6890d6911bcfd509400d932c78671ab Mon Sep 17 00:00:00 2001 From: chrysn Date: Tue, 13 Oct 2020 19:25:31 +0200 Subject: [PATCH 1/5] drivers/saul/auto_init: Add PWM for LEDs In analogy to the existing GPIO mappings, this provides (write-only) SAUL entries for PWM'd LEDs in a single-LED (as SAUL_ACT_DIMMER) and an RGB (as SAUL_ACT_RGB_LED) mode. Co-authored-by: Marian Buschsieweke --- doc/doxygen/src/porting-boards.md | 4 +- drivers/Makefile.dep | 4 + drivers/include/saul/periph.h | 113 ++++++++++++ drivers/saul/Makefile | 3 + drivers/saul/init_devs/auto_init_saul_pwm.c | 186 ++++++++++++++++++++ drivers/saul/init_devs/init.c | 4 + drivers/saul/pwm_saul.c | 61 +++++++ makefiles/pseudomodules.inc.mk | 1 + 8 files changed, 375 insertions(+), 1 deletion(-) create mode 100644 drivers/saul/init_devs/auto_init_saul_pwm.c create mode 100644 drivers/saul/pwm_saul.c diff --git a/doc/doxygen/src/porting-boards.md b/doc/doxygen/src/porting-boards.md index 5c81b7d83f..09613e1b79 100644 --- a/doc/doxygen/src/porting-boards.md +++ b/doc/doxygen/src/porting-boards.md @@ -49,7 +49,9 @@ configurations. e.g: specific pin connections to a LCD screen, radio, etc.). Some boards might also define optimized `XTIMER_%` values (e.g. @ref XTIMER_BACKOFF). - `gpio_params.h`: if the board supports @ref drivers_saul "SAUL" then its - @ref saul_gpio_params_t is defined here. + @ref saul_gpio_params_t is defined here. (Analogously, a `adc_params.h` can + contain a @ref saul_adc_params_t, and `pwm_params.h` a @ref + saul_pwm_rgb_params_t and a @ref saul_pwm_dimmer_params_t). - other: other specific headers needed by one `BOARD` @note Header files do not need to be defined in `include/`, but if defined diff --git a/drivers/Makefile.dep b/drivers/Makefile.dep index 0d8da2b51d..d0c97ea903 100644 --- a/drivers/Makefile.dep +++ b/drivers/Makefile.dep @@ -154,6 +154,10 @@ ifneq (,$(filter saul_gpio,$(USEMODULE))) FEATURES_REQUIRED += periph_gpio endif +ifneq (,$(filter saul_pwm,$(USEMODULE))) + FEATURES_REQUIRED += periph_pwm +endif + ifneq (,$(filter saul,$(USEMODULE))) USEMODULE += phydat endif diff --git a/drivers/include/saul/periph.h b/drivers/include/saul/periph.h index 74f24c68f1..51d7a34be0 100644 --- a/drivers/include/saul/periph.h +++ b/drivers/include/saul/periph.h @@ -27,6 +27,10 @@ #include "periph/adc.h" #endif /* MODULE_SAUL_ADC */ +#if MODULE_SAUL_PWM || DOXYGEN +#include "periph/pwm.h" +#endif /* MODULE_SAUL_PWM */ + #ifdef __cplusplus extern "C" { #endif @@ -63,6 +67,115 @@ typedef struct { } saul_adc_params_t; #endif /* MODULE_SAUL_ADC */ +#if MODULE_SAUL_PWM || DOXYGEN +/** + * @brief Resolution of SAUL mapped PWMs + */ +static const uint16_t saul_pwm_resolution = 255; + +/** + * @brief SAUL PWM parameters + */ +typedef enum { + SAUL_PWM_REGULAR = (0 << 0), /**< Physical values are proportional to + average voltage levels (ie. LEDs are in + active-high, anode driven) */ + SAUL_PWM_INVERTED = (1 << 0), /**< Physical values are inverted from + average voltage levels (ie. LEDs are in + active-low, cathode driven) */ +} saul_pwm_flags_t; + +/** + * @brief Single PWM channel exposed via SAUL + * + * This does never need to be declared on its own, but is used inisde @ref + * saul_pwm_dimmer_params_t and @ref saul_pwm_rgb_params_t structs. + */ +typedef struct { + pwm_t dev; /**< PWM device backing this entry */ + uint8_t channel; /**< Channel on the PWM device */ + saul_pwm_flags_t flags; /**< Configuration flags */ +} saul_pwm_channel_t; + +/** @brief Default value for @ref SAUL_PWM_FREQ */ +#define SAUL_PWM_FREQ_DEFAULT 1000 + +/** + * @brief Define the PWM frequency for LEDs + * + * This frequency is requested from the PWM driver. As the per @ref pwm_init, + * the actual frequency may be lower, and the SAUL wrapper does not place a + * limit there. + * + * Frequencies of above 200Hz usually give a smooth visual experience. The + * higher 1kHz is picked as a default as some devices can't go that low with + * their timer. + * + * This is typically set in the board's `pwm_params.h`. + */ +/* This is not applied here as it would later need to be undef'd; actual + * application of the default happens in auto_init_saul_pwm.c */ +#if DOXYGEN +#define SAUL_PWM_FREQ SAUL_PWM_FREQ_DEFAULT +#endif + +/** + * @brief Suppress saul_pwm's dimmer generation + * + * This can be defined in `pwm_params.h` if the saul_pwm module is used, but no + * dimmers (and only RGB LEDs) are in use. Then, no @ref saul_pwm_dimmer_params + * needs to be set. + */ +#if DOXYGEN +#define SAUL_PWM_NO_DIMMER +#endif + +/** + * @brief PWM channels mapped to dimmer-style registration entries + * + * This is used to define a `static const saul_pwm_dimmer_params_t + * saul_pwm_dimer_params[]` in a board's `pwm_params.h` for use by the saul_pwm + * module. If the module is used but only RGB LEDs are present, a @ref + * SAUL_PWM_NO_DIMMER define can be set instead. + */ +typedef struct { + const char *name; /**< Name of the device connected to this + channel */ + saul_pwm_channel_t channel; /**< Full channel description (device, channel) + along with flags that indicate whether high + PWM values are dark or bright*/ +} saul_pwm_dimmer_params_t; + +/** + * @brief Suppress saul_pwm's RGB LED generation + * + * This can be defined in `pwm_params.h` if the saul_pwm module is used, but no + * RGB LEDs (and only dimmers) are in use. Then, no @ref saul_pwm_rgb_params_t + * needs to be set. + */ +#if DOXYGEN +#define SAUL_PWM_NO_RGB +#endif + +/** + * @brief PWM channels mapped to RGB LED registration entries + * + * This is used to define a `static const saul_pwm_rgb_params_t + * saul_pwm_rgb_params[]` in a board's `pwm_params.h` for use by the saul_pwm + * module. If the module is used but only dimmers are present, a @ref + * SAUL_PWM_NO_RGB define can be set instead. + */ +typedef struct { + const char *name; /**< Name of the device connected to these + channels */ + saul_pwm_channel_t channels[3]; /**< Full channel description (device, channel) + along with flags that indicate whether high + PWM values are dark or bright*/ +} saul_pwm_rgb_params_t; + + +#endif /* MODULE_SAUL_PWM */ + #ifdef __cplusplus } #endif diff --git a/drivers/saul/Makefile b/drivers/saul/Makefile index 39dc655787..0e9946400c 100644 --- a/drivers/saul/Makefile +++ b/drivers/saul/Makefile @@ -6,5 +6,8 @@ endif ifneq (,$(filter saul_adc,$(USEMODULE))) SRC += adc_saul.c endif +ifneq (,$(filter saul_pwm,$(USEMODULE))) + SRC += pwm_saul.c +endif include $(RIOTBASE)/Makefile.base diff --git a/drivers/saul/init_devs/auto_init_saul_pwm.c b/drivers/saul/init_devs/auto_init_saul_pwm.c new file mode 100644 index 0000000000..1fe8b4a1c5 --- /dev/null +++ b/drivers/saul/init_devs/auto_init_saul_pwm.c @@ -0,0 +1,186 @@ +/* + * Copyright (C) 2015 Freie Universität Berlin + * + * 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 sys_auto_init_saul + * @{ + * + * @file + * @brief Auto initialization of PWM pins directly mapped to SAUL reg + * + * @author Christian Amsüss + * + * When this module is used, any PWM device assigned inside the configuration + * structs inside `pwm_params.h` in the @ref saul_pwm_dimmer_params_t and @ref + * saul_pwm_rgb_params_t is initialized at startup for 8-bit dimming at about + * 1kHz, and the indicated channels are exposed via SAUL. + * + * @} + */ + +#include "log.h" +#include "saul_reg.h" +#include "saul/periph.h" +#include "pwm_params.h" +#include "periph/pwm.h" + +#if !defined(SAUL_PWM_FREQ) +#define SAUL_PWM_FREQ SAUL_PWM_FREQ_DEFAULT +#endif + +/** + * @brief Define the number of configured dimmers + */ +#ifndef SAUL_PWM_NO_DIMMER +#define SAUL_PWM_DIMMER_NUMOF ARRAY_SIZE(saul_pwm_dimmer_params) +#else +#define SAUL_PWM_DIMMER_NUMOF 0 +static const saul_pwm_dimmer_params_t saul_pwm_dimmer_params[0]; +#endif + +/** + * @brief Define the number of configured RGB LEDs + */ +#ifndef SAUL_PWM_NO_RGB +#define SAUL_PWM_RGB_NUMOF ARRAY_SIZE(saul_pwm_rgb_params) +#else +#define SAUL_PWM_RGB_NUMOF 0 +static const saul_pwm_rgb_params_t saul_pwm_rgb_params[0]; +#endif + +/** + * @brief Memory for the registry RGB LED entries + */ +/* The static variable will be unused in the 0 case and thus not emitted. */ +static saul_reg_t saul_reg_entries_rgb[SAUL_PWM_RGB_NUMOF]; + +/** + * @brief Memory for the registry dimmer entries + */ +/* The static variable will be unused in the 0 case and thus not emitted. */ +static saul_reg_t saul_reg_entries_dimmer[SAUL_PWM_DIMMER_NUMOF]; + +/** + * @brief Reference to the driver for single-channel dimmers + */ +extern saul_driver_t dimmer_saul_driver; + +/** + * @brief Reference to the driver for RGB LEDs + */ +extern saul_driver_t rgb_saul_driver; + +/** + * Configure a PWM driver for LED output (1kHz, 8bit) + */ +static int configure(pwm_t dev) +{ + LOG_DEBUG("[auto_init_saul] initializing PWM %u for LED operation,", dev); + uint32_t freq = pwm_init(dev, PWM_LEFT, SAUL_PWM_FREQ, saul_pwm_resolution); + LOG_DEBUG(" actual frequency %lu,\n", freq); + return freq != 0 ? 0 : -ENOTSUP; +} + +/** + * Configure the PWM driver at the given index (inside saul_pwm_dimmer_params, + * overflowing into saul_pwm_rgb_params) unless that device came up previously, + * in which case the function returns without any action. + * */ +static int configure_on_first_use(pwm_t dev, unsigned index) +{ + /* Work around -Werror=type-limits that would otherwise trigger */ + unsigned dimmer_numof = SAUL_PWM_DIMMER_NUMOF; + for (unsigned i = 0; i < dimmer_numof; ++i) { + pwm_t currentdev = saul_pwm_dimmer_params[i].channel.dev; + if (currentdev == dev) { + if (i == index) { + return configure(dev); + } + return 0; + } + } + + /* Work around -Werror=type-limits that would otherwise trigger */ + unsigned rgb_numof = SAUL_PWM_RGB_NUMOF; + for (unsigned i = 0; i < rgb_numof; ++i) { + for (int j = 0; j < 3; ++j) { + unsigned index = SAUL_PWM_DIMMER_NUMOF + i * 3 + j; + pwm_t currentdev = saul_pwm_rgb_params[i].channels[j].dev; + if (currentdev == dev) { + if (i == index) { + return configure(dev); + } + return 0; + } + } + } + return -ENOENT; +} + +void auto_init_saul_pwm(void) +{ + /* Work around -Werror=type-limits that would otherwise trigger */ + unsigned dimmer_numof = SAUL_PWM_DIMMER_NUMOF; + for (unsigned i = 0; i < dimmer_numof; i++) { + const saul_pwm_dimmer_params_t *p = &saul_pwm_dimmer_params[i]; + + LOG_DEBUG("[auto_init_saul] initializing dimmer #%u\n", i); + + saul_reg_entries_dimmer[i].dev = (void*)p; + saul_reg_entries_dimmer[i].name = p->name; + saul_reg_entries_dimmer[i].driver = &dimmer_saul_driver; + + int err = configure_on_first_use(p->channel.dev, i); + if (err != 0) { + LOG_ERROR( + "[auto_init_saul] Error initializing device for dimmer #%u\n", + i); + /* not `continue`ing: we could run into this on a non-first use and + * then we couldn't break either */ + } + /* set initial dark state */ + phydat_t s; + s.val[0] = 0; + saul_reg_entries_dimmer[i].driver->write(p, &s); + /* add to registry */ + saul_reg_add(&(saul_reg_entries_dimmer[i])); + } + + /* Work around -Werror=type-limits that would otherwise trigger */ + unsigned rgb_numof = SAUL_PWM_RGB_NUMOF; + for (unsigned i = 0; i < rgb_numof; i++) { + const saul_pwm_rgb_params_t *p = &saul_pwm_rgb_params[i]; + + LOG_DEBUG("[auto_init_saul] initializing RGB #%u\n", i); + + saul_reg_entries_rgb[i].dev = (void*)p; + saul_reg_entries_rgb[i].name = p->name; + saul_reg_entries_rgb[i].driver = &rgb_saul_driver; + + for (int j = 0; j < 3; ++j) { + unsigned index = SAUL_PWM_DIMMER_NUMOF + i * 3 + j; + int err = configure_on_first_use(p->channels[j].dev, index); + if (err != 0) { + LOG_ERROR( + "[auto_init_saul] Error initializing device for RGB #%u/%u\n", + i, j); + /* not `continue`ing: we could run into this on a non-first use and + * then we couldn't break either */ + } + } + /* set initial dark state */ + phydat_t s; + s.val[0] = 0; + s.val[1] = 0; + s.val[2] = 0; + saul_reg_entries_rgb[i].driver->write(p, &s); + /* add to registry */ + saul_reg_add(&(saul_reg_entries_rgb[i])); + } +} diff --git a/drivers/saul/init_devs/init.c b/drivers/saul/init_devs/init.c index db50b81e85..f5aa152ca9 100644 --- a/drivers/saul/init_devs/init.c +++ b/drivers/saul/init_devs/init.c @@ -35,6 +35,10 @@ void saul_init_devs(void) extern void auto_init_gpio(void); auto_init_gpio(); } + if (IS_USED(MODULE_SAUL_PWM)) { + extern void auto_init_saul_pwm(void); + auto_init_saul_pwm(); + } if (IS_USED(MODULE_SAUL_NRF_TEMPERATURE)) { extern void auto_init_nrf_temperature(void); auto_init_nrf_temperature(); diff --git a/drivers/saul/pwm_saul.c b/drivers/saul/pwm_saul.c new file mode 100644 index 0000000000..ca2db36859 --- /dev/null +++ b/drivers/saul/pwm_saul.c @@ -0,0 +1,61 @@ +/* + * Copyright (C) 2015 Freie Universität Berlin + * + * 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 drivers_saul + * @{ + * + * @file + * @brief SAUL wrapper for PWM pins + * + * @author Christian Amsüss + * + * @} + */ + +#include "saul.h" +#include "phydat.h" +#include "periph/pwm.h" +#include "saul/periph.h" + +static inline void setchan(const saul_pwm_channel_t *chan, uint16_t value) +{ + pwm_set(chan->dev, + chan->channel, + (chan->flags & SAUL_PWM_INVERTED) ? saul_pwm_resolution - value : value); +} + +static int write_dimmer(const void *dev, phydat_t *state) +{ + const saul_pwm_dimmer_params_t *p = dev; + + setchan(&p->channel, state->val[0]); + return 3; +} + +const saul_driver_t dimmer_saul_driver = { + .read = saul_notsup, + .write = write_dimmer, + .type = SAUL_ACT_DIMMER +}; + +static int write_rgb(const void *dev, phydat_t *state) +{ + const saul_pwm_rgb_params_t *p = dev; + + for (int i = 0; i < 3; ++i) { + setchan(&p->channels[i], state->val[i]); + } + return 3; +} + +const saul_driver_t rgb_saul_driver = { + .read = saul_notsup, + .write = write_rgb, + .type = SAUL_ACT_LED_RGB +}; diff --git a/makefiles/pseudomodules.inc.mk b/makefiles/pseudomodules.inc.mk index 41295be5f5..9decda7511 100644 --- a/makefiles/pseudomodules.inc.mk +++ b/makefiles/pseudomodules.inc.mk @@ -104,6 +104,7 @@ PSEUDOMODULES += saul_adc PSEUDOMODULES += saul_default PSEUDOMODULES += saul_gpio PSEUDOMODULES += saul_nrf_temperature +PSEUDOMODULES += saul_pwm PSEUDOMODULES += scanf_float PSEUDOMODULES += sched_cb PSEUDOMODULES += semtech_loramac_rx From 84414e701fad76afd5d37211894261001a070576 Mon Sep 17 00:00:00 2001 From: chrysn Date: Tue, 29 Sep 2020 20:25:18 +0200 Subject: [PATCH 2/5] boards/nrf52840dongle: Use LEDs in SAUL as PWM rather than GPIO Co-authored-by: Marian Buschsieweke --- boards/nrf52840dongle/Makefile.dep | 1 + boards/nrf52840dongle/include/gpio_params.h | 24 --------- boards/nrf52840dongle/include/pwm_params.h | 57 +++++++++++++++++++++ 3 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 boards/nrf52840dongle/include/pwm_params.h diff --git a/boards/nrf52840dongle/Makefile.dep b/boards/nrf52840dongle/Makefile.dep index 6c6293050f..42485c2d5e 100644 --- a/boards/nrf52840dongle/Makefile.dep +++ b/boards/nrf52840dongle/Makefile.dep @@ -1,5 +1,6 @@ ifneq (,$(filter saul_default,$(USEMODULE))) USEMODULE += saul_gpio + USEMODULE += saul_pwm endif ifeq (,$(filter-out stdio_cdc_acm,$(filter stdio_% slipdev_stdio,$(USEMODULE)))) diff --git a/boards/nrf52840dongle/include/gpio_params.h b/boards/nrf52840dongle/include/gpio_params.h index d637dfa3c1..85930549c4 100644 --- a/boards/nrf52840dongle/include/gpio_params.h +++ b/boards/nrf52840dongle/include/gpio_params.h @@ -31,30 +31,6 @@ extern "C" { */ static const saul_gpio_params_t saul_gpio_params[] = { - { - .name = "LD 1", - .pin = LED0_PIN, - .mode = GPIO_OUT, - .flags = (SAUL_GPIO_INVERTED | SAUL_GPIO_INIT_CLEAR), - }, - { - .name = "LD 2 Red", - .pin = LED1_PIN, - .mode = GPIO_OUT, - .flags = (SAUL_GPIO_INVERTED | SAUL_GPIO_INIT_CLEAR), - }, - { - .name = "LD 2 Green", - .pin = LED2_PIN, - .mode = GPIO_OUT, - .flags = (SAUL_GPIO_INVERTED | SAUL_GPIO_INIT_CLEAR), - }, - { - .name = "LD 2 Blue", - .pin = LED3_PIN, - .mode = GPIO_OUT, - .flags = (SAUL_GPIO_INVERTED | SAUL_GPIO_INIT_CLEAR), - }, { .name = "SW 1", .pin = BTN0_PIN, diff --git a/boards/nrf52840dongle/include/pwm_params.h b/boards/nrf52840dongle/include/pwm_params.h new file mode 100644 index 0000000000..cc1890c6f4 --- /dev/null +++ b/boards/nrf52840dongle/include/pwm_params.h @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2020 Christian Amsüss + * + * 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 boards_nrf52840dongle + * @{ + * + * @file + * @brief Configuration of SAUL mapped PWM channels + * + * @author Christian Amsüss + */ + +#ifndef PWM_PARAMS_H +#define PWM_PARAMS_H + +#include "board.h" +#include "saul/periph.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/** + * @brief The single LED exposed via SAUL + */ +static const saul_pwm_dimmer_params_t saul_pwm_dimmer_params[] = +{ + { + .name = "LD 1", + .channel = { PWM_DEV(0), 0, SAUL_PWM_INVERTED }, + } +}; + +static const saul_pwm_rgb_params_t saul_pwm_rgb_params[] = +{ + { + .name = "LD 2", + .channels = { + { PWM_DEV(0), 1, SAUL_PWM_INVERTED }, + { PWM_DEV(0), 2, SAUL_PWM_INVERTED }, + { PWM_DEV(0), 3, SAUL_PWM_INVERTED } + } + } +}; + +#ifdef __cplusplus +} +#endif + +#endif /* PWM_PARAMS_H */ +/** @} */ From 46875049363ff26540ab0a9f7a1bbb2349b18f58 Mon Sep 17 00:00:00 2001 From: chrysn Date: Wed, 30 Sep 2020 23:55:29 +0200 Subject: [PATCH 3/5] drivers/saul/auto_init: Support bool and percent for dimmers Booleans make sense for dimmers when accessed from a Sith application that deals in absolutes. Percent values are also a widespread interpretation of brightness levels, and are thus supported as well. The bit arithmetic makes sure that the arithmetic operation value / 100 * saul_pwm_resolution is done efficiently (by expression as a multiplication followed by shifting) and accurately (by maximizing the number of usable bits) while still being flexible both with respect to integer sizes and to changes of saul_pwm_resolution. Co-authored-by: Marian Buschsieweke --- drivers/saul/pwm_saul.c | 54 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/drivers/saul/pwm_saul.c b/drivers/saul/pwm_saul.c index ca2db36859..0a84425e45 100644 --- a/drivers/saul/pwm_saul.c +++ b/drivers/saul/pwm_saul.c @@ -22,6 +22,42 @@ #include "phydat.h" #include "periph/pwm.h" #include "saul/periph.h" +#include "bitarithm.h" + +/** + * Find factor and shiftback such that for each value entry in the phydat, the + * resulting PWM duty cycle would be (value * factor) >> shiftback. + */ +static int extract_scaling(const phydat_t *state, int *factor, int *shiftback) +{ + if (state->scale != 0) { + return -ECANCELED; + } + + /** Number of bits i by which we can shift the calculation (value * (255 << i)/100) >> i + * to get a better result than value * 2 (which would otherwise happen in integers) */ + int shift100 = bitarithm_msb(INT_MAX) - bitarithm_msb(saul_pwm_resolution); + + switch (state->unit) { + case UNIT_UNDEF: + case UNIT_NONE: + *factor = 1; + *shiftback = 0; + break; + case UNIT_BOOL: + *factor = saul_pwm_resolution; + *shiftback = 0; + break; + case UNIT_PERCENT: + *factor = ((int)saul_pwm_resolution << shift100) / 100; + *shiftback = shift100; + break; + default: + return -ECANCELED; + } + + return 0; +} static inline void setchan(const saul_pwm_channel_t *chan, uint16_t value) { @@ -34,7 +70,14 @@ static int write_dimmer(const void *dev, phydat_t *state) { const saul_pwm_dimmer_params_t *p = dev; - setchan(&p->channel, state->val[0]); + int factor, shiftback; + + int err = extract_scaling(state, &factor, &shiftback); + if (err < 0) { + return err; + } + + setchan(&p->channel, (state->val[0] * factor) >> shiftback); return 3; } @@ -48,8 +91,15 @@ static int write_rgb(const void *dev, phydat_t *state) { const saul_pwm_rgb_params_t *p = dev; + int factor, shiftback; + + int err = extract_scaling(state, &factor, &shiftback); + if (err < 0) { + return err; + } + for (int i = 0; i < 3; ++i) { - setchan(&p->channels[i], state->val[i]); + setchan(&p->channels[i], (state->val[i] * factor) >> shiftback); } return 3; } From d8ff6cd7b1ce8485475b2376f6bdc5cdde0e2dfd Mon Sep 17 00:00:00 2001 From: chrysn Date: Wed, 30 Sep 2020 15:47:57 +0200 Subject: [PATCH 4/5] boards/c/particle-mesh: Use LEDs in SAUL as PWM rather than GPIO Co-authored-by: Marian Buschsieweke --- boards/common/particle-mesh/Makefile.dep | 1 + .../particle-mesh/include/gpio_params.h | 18 ------- .../common/particle-mesh/include/pwm_params.h | 48 +++++++++++++++++++ 3 files changed, 49 insertions(+), 18 deletions(-) create mode 100644 boards/common/particle-mesh/include/pwm_params.h diff --git a/boards/common/particle-mesh/Makefile.dep b/boards/common/particle-mesh/Makefile.dep index b68127eea4..17e773c0ef 100644 --- a/boards/common/particle-mesh/Makefile.dep +++ b/boards/common/particle-mesh/Makefile.dep @@ -1,5 +1,6 @@ ifneq (,$(filter saul_default,$(USEMODULE))) USEMODULE += saul_gpio + USEMODULE += saul_pwm endif # include common nrf52 dependencies diff --git a/boards/common/particle-mesh/include/gpio_params.h b/boards/common/particle-mesh/include/gpio_params.h index a9858f0acd..0e5fa3f26e 100644 --- a/boards/common/particle-mesh/include/gpio_params.h +++ b/boards/common/particle-mesh/include/gpio_params.h @@ -31,24 +31,6 @@ extern "C" { */ static const saul_gpio_params_t saul_gpio_params[] = { - { - .name = "Led Red", - .pin = LED0_PIN, - .mode = GPIO_OUT, - .flags = (SAUL_GPIO_INVERTED | SAUL_GPIO_INIT_CLEAR), - }, - { - .name = "Led Green", - .pin = LED1_PIN, - .mode = GPIO_OUT, - .flags = (SAUL_GPIO_INVERTED | SAUL_GPIO_INIT_CLEAR), - }, - { - .name = "Led Blue", - .pin = LED2_PIN, - .mode = GPIO_OUT, - .flags = (SAUL_GPIO_INVERTED | SAUL_GPIO_INIT_CLEAR), - }, { .name = "MODE Button", .pin = BTN0_PIN, diff --git a/boards/common/particle-mesh/include/pwm_params.h b/boards/common/particle-mesh/include/pwm_params.h new file mode 100644 index 0000000000..85d68fb4d7 --- /dev/null +++ b/boards/common/particle-mesh/include/pwm_params.h @@ -0,0 +1,48 @@ +/* + * Copyright (C) 2020 Christian Amsüss + * + * 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 boards_common_particle-mesh + * @{ + * + * @file + * @brief Configuration of SAUL mapped PWM channels + * + * @author Christian Amsüss + */ + +#ifndef PWM_PARAMS_H +#define PWM_PARAMS_H + +#include "board.h" +#include "saul/periph.h" + +#ifdef __cplusplus +extern "C" { +#endif + +#define SAUL_PWM_NO_DIMMER + +static const saul_pwm_rgb_params_t saul_pwm_rgb_params[] = +{ + { + .name = "LED", + .channels = { + { PWM_DEV(0), 0, SAUL_PWM_INVERTED }, + { PWM_DEV(0), 1, SAUL_PWM_INVERTED }, + { PWM_DEV(0), 2, SAUL_PWM_INVERTED } + } + } +}; + +#ifdef __cplusplus +} +#endif + +#endif /* PWM_PARAMS_H */ +/** @} */ From 1b7cd3d1424cfb416715f24d79096e91bff3a77c Mon Sep 17 00:00:00 2001 From: chrysn Date: Thu, 5 Nov 2020 19:01:17 +0100 Subject: [PATCH 5/5] drivers/saul/auto_init: Err on out-of-range input --- drivers/saul/pwm_saul.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/drivers/saul/pwm_saul.c b/drivers/saul/pwm_saul.c index 0a84425e45..c8d3c2c4ff 100644 --- a/drivers/saul/pwm_saul.c +++ b/drivers/saul/pwm_saul.c @@ -27,8 +27,13 @@ /** * Find factor and shiftback such that for each value entry in the phydat, the * resulting PWM duty cycle would be (value * factor) >> shiftback. + * + * This also makes the maximum legal input value for that input (which allows + * clipping the input to a value that doesn't wrap during that multiplication, + * or just to err out). If future versions of this take the scale into account, + * they will adjust the maximum accordingly. */ -static int extract_scaling(const phydat_t *state, int *factor, int *shiftback) +static int extract_scaling(const phydat_t *state, int *factor, int *shiftback, int *max) { if (state->scale != 0) { return -ECANCELED; @@ -43,14 +48,17 @@ static int extract_scaling(const phydat_t *state, int *factor, int *shiftback) case UNIT_NONE: *factor = 1; *shiftback = 0; + *max = saul_pwm_resolution; break; case UNIT_BOOL: *factor = saul_pwm_resolution; *shiftback = 0; + *max = 1; break; case UNIT_PERCENT: *factor = ((int)saul_pwm_resolution << shift100) / 100; *shiftback = shift100; + *max = 100; break; default: return -ECANCELED; @@ -70,13 +78,17 @@ static int write_dimmer(const void *dev, phydat_t *state) { const saul_pwm_dimmer_params_t *p = dev; - int factor, shiftback; + int factor, shiftback, max; - int err = extract_scaling(state, &factor, &shiftback); + int err = extract_scaling(state, &factor, &shiftback, &max); if (err < 0) { return err; } + if (state->val[0] < 0 || state->val[0] > max) { + return -ECANCELED; + } + setchan(&p->channel, (state->val[0] * factor) >> shiftback); return 3; } @@ -91,13 +103,19 @@ static int write_rgb(const void *dev, phydat_t *state) { const saul_pwm_rgb_params_t *p = dev; - int factor, shiftback; + int factor, shiftback, max; - int err = extract_scaling(state, &factor, &shiftback); + int err = extract_scaling(state, &factor, &shiftback, &max); if (err < 0) { return err; } + for (int i = 0; i < 3; ++i) { + if (state->val[i] < 0 || state->val[i] > max) { + return -ECANCELED; + } + } + for (int i = 0; i < 3; ++i) { setchan(&p->channels[i], (state->val[i] * factor) >> shiftback); }