Merge pull request #340 from soburi/improve_tone

cores: arduino: zephyrCommon: rework tone slot handling and timer lifecycle

Author: pennam

Kconfig

@@ -30,6 +30,14 @@ config ARDUINO_ENTRY
 	bool "Provide arduino setup and loop entry points"
 	default y
 
+config ARDUINO_MAX_TONES
+	int "Maximum number of tones that can be played simultaneously with tone()"
+	default -1
+	help
+	  Specify the maximum number of tones that can be played simultaneously with tone().
+	  If set to -1 (or any other negative value), the maximum number will be
+	  determined from the system's digital pin configuration.
+
 endif
 
 if USB_DEVICE_STACK_NEXT

cores/arduino/zephyrCommon.cpp

@@ -7,6 +7,8 @@
 #include <Arduino.h>
 #include "zephyrInternal.h"
 
+#include <zephyr/spinlock.h>
+
 // create an array of arduino_pins with functions to reinitialize pins if needed
 static const struct device *pinmux_array[DT_PROP_LEN(DT_PATH(zephyr_user), digital_pin_gpios)] = {
 	nullptr};
@@ -263,53 +265,172 @@ PinStatus digitalRead(pin_size_t pinNumber) {
 	return (gpio_pin_get_dt(&arduino_pins[pinNumber]) == 1) ? HIGH : LOW;
 }
 
-struct k_timer arduino_pin_timers[ARRAY_SIZE(arduino_pins)];
-struct k_timer arduino_pin_timers_timeout[ARRAY_SIZE(arduino_pins)];
+#if CONFIG_ARDUINO_MAX_TONES < 0
+#define MAX_TONE_PINS DT_PROP_LEN(DT_PATH(zephyr_user), digital_pin_gpios)
+#else
+#define MAX_TONE_PINS CONFIG_ARDUINO_MAX_TONES
+#endif
 
-void tone_expiry_cb(struct k_timer *timer) {
-	const struct gpio_dt_spec *spec = (gpio_dt_spec *)k_timer_user_data_get(timer);
-	gpio_pin_toggle_dt(spec);
+#define TOGGLES_PER_CYCLE 2ULL
+
+static struct pin_timer {
+	struct k_timer timer;
+	uint32_t count{0};
+	pin_size_t pin{pin_size_t(-1)};
+	bool infinity{false};
+	bool timer_initialized{false};
+	struct k_spinlock lock{};
+} arduino_pin_timers[MAX_TONE_PINS];
+
+K_MUTEX_DEFINE(timer_cfg_lock);
+
+void tone_expiry_cb(struct k_timer *timer);
+
+/* Callers must hold timer_cfg_lock while using this helper. */
+static struct pin_timer *find_pin_timer(pin_size_t pinNumber, bool active_only) {
+	for (size_t i = 0; i < ARRAY_SIZE(arduino_pin_timers); i++) {
+		k_spinlock_key_t key = k_spin_lock(&arduino_pin_timers[i].lock);
+
+		if (arduino_pin_timers[i].pin == pinNumber) {
+			k_spin_unlock(&arduino_pin_timers[i].lock, key);
+			return &arduino_pin_timers[i];
+		}
+
+		k_spin_unlock(&arduino_pin_timers[i].lock, key);
+	}
+
+	if (active_only) {
+		return nullptr;
+	}
+
+	for (size_t i = 0; i < ARRAY_SIZE(arduino_pin_timers); i++) {
+		k_spinlock_key_t key = k_spin_lock(&arduino_pin_timers[i].lock);
+
+		if (arduino_pin_timers[i].pin == pin_size_t(-1)) {
+			arduino_pin_timers[i].pin = pinNumber;
+			k_spin_unlock(&arduino_pin_timers[i].lock, key);
+			return &arduino_pin_timers[i];
+		}
+
+		k_spin_unlock(&arduino_pin_timers[i].lock, key);
+	}
+
+	return nullptr;
 }
 
-void tone_timeout_cb(struct k_timer *timer) {
-	pin_size_t pinNumber = (pin_size_t)(uintptr_t)k_timer_user_data_get(timer);
-	noTone(pinNumber);
+void tone_expiry_cb(struct k_timer *timer) {
+	struct pin_timer *pt = CONTAINER_OF(timer, struct pin_timer, timer);
+	k_spinlock_key_t key = k_spin_lock(&pt->lock);
+	pin_size_t pin = pt->pin;
+
+	if (pt->count == 0 && !pt->infinity) {
+		if (pin != pin_size_t(-1)) {
+			gpio_pin_set_dt(&arduino_pins[pin], 0);
+		}
+
+		k_timer_stop(timer);
+		pt->count = 0;
+		pt->infinity = false;
+		pt->pin = pin_size_t(-1);
+	} else {
+		if (pin != pin_size_t(-1)) {
+			gpio_pin_toggle_dt(&arduino_pins[pin]);
+		}
+		pt->count--;
+	}
+
+	k_spin_unlock(&pt->lock, key);
 }
 
 void tone(pin_size_t pinNumber, unsigned int frequency, unsigned long duration) {
 	RETURN_ON_INVALID_PIN(pinNumber);
 
-	struct k_timer *timer = &arduino_pin_timers[pinNumber];
-	const struct gpio_dt_spec *spec = &arduino_pins[pinNumber];
+	k_spinlock_key_t key;
+	uint64_t toggles_count;
+	struct pin_timer *pt;
 	k_timeout_t timeout;
 
-	pinMode(pinNumber, OUTPUT);
+	if (k_is_in_isr()) {
+		return;
+	}
+
+	k_mutex_lock(&timer_cfg_lock, K_FOREVER);
+
+	pt = find_pin_timer(pinNumber, false);
 
-	if (frequency == 0) {
-		gpio_pin_set_dt(spec, 0);
+	if (pt == nullptr) {
+		k_mutex_unlock(&timer_cfg_lock);
 		return;
 	}
 
-	timeout = K_NSEC(NSEC_PER_SEC / (2 * frequency));
+	if (!pt->timer_initialized) {
+		k_timer_init(&pt->timer, tone_expiry_cb, NULL);
+		pt->timer_initialized = true;
+	}
+
+	pinMode(pinNumber, OUTPUT);
+	k_timer_stop(&pt->timer);
+
+	toggles_count = ((uint64_t)duration * frequency / (MSEC_PER_SEC / TOGGLES_PER_CYCLE));
+	if (frequency == 0 || (toggles_count == 0 && duration != 0)) {
+		key = k_spin_lock(&pt->lock);
+		pt->count = 0;
+		pt->infinity = false;
+		pt->pin = pin_size_t(-1);
+		k_spin_unlock(&pt->lock, key);
 
-	k_timer_init(timer, tone_expiry_cb, NULL);
-	k_timer_user_data_set(timer, (void *)spec);
-	gpio_pin_set_dt(spec, 1);
-	k_timer_start(timer, timeout, timeout);
+		gpio_pin_set_dt(&arduino_pins[pinNumber], 0);
+
+		k_mutex_unlock(&timer_cfg_lock);
+		return;
+	}
 
-	if (duration > 0) {
-		timer = &arduino_pin_timers_timeout[pinNumber];
-		k_timer_init(timer, tone_timeout_cb, NULL);
-		k_timer_user_data_set(timer, (void *)(uintptr_t)pinNumber);
-		k_timer_start(timer, K_MSEC(duration), K_NO_WAIT);
+	timeout = K_NSEC(NSEC_PER_SEC / (TOGGLES_PER_CYCLE * frequency));
+	if (timeout.ticks == 0) {
+		timeout.ticks = 1;
 	}
+
+	key = k_spin_lock(&pt->lock);
+	pt->infinity = (duration == 0);
+	pt->count = min(toggles_count, UINT32_MAX);
+	pt->pin = pinNumber;
+	k_spin_unlock(&pt->lock, key);
+
+	gpio_pin_set_dt(&arduino_pins[pinNumber], 1);
+	k_timer_start(&pt->timer, timeout, timeout);
+
+	k_mutex_unlock(&timer_cfg_lock);
 }
 
 void noTone(pin_size_t pinNumber) {
 	RETURN_ON_INVALID_PIN(pinNumber);
 
-	k_timer_stop(&arduino_pin_timers[pinNumber]);
+	struct pin_timer *pt;
+	k_spinlock_key_t key;
+
+	if (k_is_in_isr()) {
+		return;
+	}
+
+	k_mutex_lock(&timer_cfg_lock, K_FOREVER);
+
+	pt = find_pin_timer(pinNumber, true);
+
+	if (pt == nullptr) {
+		k_mutex_unlock(&timer_cfg_lock);
+		return;
+	}
+
+	key = k_spin_lock(&pt->lock);
+	k_timer_stop(&pt->timer);
+	pt->count = 0;
+	pt->infinity = false;
+	pt->pin = pin_size_t(-1);
+	k_spin_unlock(&pt->lock, key);
+
 	gpio_pin_set_dt(&arduino_pins[pinNumber], 0);
+
+	k_mutex_unlock(&timer_cfg_lock);
 }
 
 unsigned long micros(void) {