feat: support non-PWM tacho pins

refactor: handle all tacho pins w/ 1 task

src/config/pins.cpp

@@ -1,15 +1,11 @@
 #include "pins.hpp"
-#include "config.hpp"
 #include "hardware/gpio.h"
 #include "hardware/i2c.h"
 #include "hardware/spi.h"
+#include "utility/square_wave.hpp"
 #include <cassert>
 #include <cstdint>
 
-#ifndef NDEBUG
-#include "utility/square_wave.hpp"
-#endif
-
 namespace nevermore {
 
 namespace {
@@ -111,13 +107,26 @@ bool Pins::apply() const {
     bind_bus(i2c, bind_i2c_hw, bind_i2c_pio);
     bind_bus(spi, bind_spi_hw, bind_spi_pio);
 
-    foreach_pwm_function([](auto&& pins, bool) {
-        for (auto&& pin : pins)
-            if (pin) gpio_set_function(pin, GPIO_FUNC_PWM);
+    uint32_t pwm_slice_claimed = 0;
+    foreach_pwm_function([&](auto&& pins, bool) {
+        for (auto&& pin : pins) {
+            if (!pin) continue;
+
+            gpio_set_function(pin, GPIO_FUNC_PWM);
+            pwm_slice_claimed |= 1u << pwm_gpio_to_slice_num_(pin);
+        }
     });
 
-    for (auto&& pin : fan_tachometer)
-        if (pin) gpio_pull_up(pin);
+    for (auto&& pin : fan_tachometer) {
+        if (!pin) continue;
+
+        auto slice = pwm_gpio_to_channel(pin) == PWM_CHAN_B ? 1u << pwm_gpio_to_slice_num_(pin) : 0;
+        // FUTURE WORK: remove dbg msg once this feature matures
+        printf("tacho pin=%d mode=%s\n", (int)pin, !slice || pwm_slice_claimed & slice ? "POLL" : "PWM");
+        gpio_set_function(pin, !slice || pwm_slice_claimed & slice ? GPIO_FUNC_SIO : GPIO_FUNC_PWM);
+        gpio_pull_up(pin);
+        pwm_slice_claimed |= slice;
+    }
 
     // we're setting up the WS2812 controller on PIO0
     for (auto&& pin : neopixel_data)

src/config/pins.hpp

@@ -175,7 +175,6 @@ struct [[gnu::packed]] Pins {
     constexpr void validate_or_throw() const;
 
     constexpr void foreach_pwm_function(auto&& go) const {
-        go(fan_tachometer, false);
         go(fan_pwm, true);
         go(photocatalytic_pwm, true);
         // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
@@ -346,10 +345,6 @@ inline constexpr void nevermore::Pins::validate_or_throw() const {
         display_buses += bus && bus.kind == BusSPI::Kind::display;
     if (1 < display_buses) throw "Config uses multiple display SPI buses.";
 
-    for (auto&& pin : fan_tachometer)
-        if (pin && pwm_gpio_to_channel_(pin) != PWM_CHAN_B)
-            throw "Config uses fan tachometer on slice A GPIO. Put it on a B GPIO (i.e odd GPIO).";
-
     uint32_t pwm_slice_claimed = 0;
     foreach_pwm_function([&](std::span<GPIO const> pins, bool allow_sharing) {
         for (auto&& pin : pins) {

src/gatt/fan.cpp

@@ -44,7 +44,7 @@ constexpr uint32_t FAN_PWN_HZ = 25'000;
 
 BLE::Percentage8 g_fan_power = 0;
 BLE::Percentage8 g_fan_power_override;  // not-known -> automatic control
-array<sensors::Tachometer, Pins{}.fan_tachometer.size()> g_tachometers;
+sensors::Tachometer g_tachometer;
 
 struct [[gnu::packed]] FanPowerTachoAggregate {
     BLE::Percentage8 power = g_fan_power;
@@ -93,11 +93,7 @@ void fan_power_set(BLE::Percentage8 power, sensors::Sensors const& sensors = sen
 }  // namespace
 
 double fan_rpm() {
-    double total = 0;
-    for (auto const& t : g_tachometers)
-        total += t.revolutions_per_second() * 60;
-
-    return total;
+    return g_tachometer.revolutions_per_second() * 60;
 }
 
 double fan_power() {
@@ -129,24 +125,12 @@ bool init() {
         pwm_init(pwm_gpio_to_slice_num_(pin), &cfg, true);
     }
 
-    auto* it_tacho = begin(g_tachometers);
-    for (auto&& pin : Pins::active().fan_tachometer) {
-        if (!pin) continue;
-        assert(it_tacho != end(g_tachometers));
-        it_tacho->setup(pin, TACHOMETER_PULSE_PER_REVOLUTION);
-        it_tacho++;  // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic)
-
-        auto cfg = pwm_get_default_config();
-        pwm_config_set_clkdiv_mode(&cfg, PWM_DIV_B_FALLING);
-        pwm_init(pwm_gpio_to_slice_num_(pin), &cfg, false);
-    }
+    g_tachometer.setup(Pins::active().fan_tachometer, TACHOMETER_PULSE_PER_REVOLUTION);
+    g_tachometer.start();
 
     // set fan PWM level
     fan_power_set(g_fan_power);
 
-    for (auto& t : g_tachometers)
-        if (t.pin()) t.start();
-
     // HACK:  We'd like to notify on write to tachometer changes, but the code base isn't setup
     //        for that yet. Internally poll and update based on diffs for now.
     mk_timer("gatt-fan-tachometer-notify", SENSOR_UPDATE_PERIOD)([](auto*) {

src/sensors/tachometer.hpp

@@ -2,34 +2,50 @@
 
 #include "async_sensor.hpp"
 #include "config/pins.hpp"
+#include "hardware/gpio.h"
 #include "sdk/pwm.hpp"
+#include <algorithm>
+#include <bitset>
 #include <chrono>
 #include <cstdint>
 
 namespace nevermore::sensors {
 
 using namespace std::literals::chrono_literals;
 
+// 'Low' speed tachometer, intended for < 1000 pulses/sec.
+// DELTA BFB0712H spec sheet says an RPM of 2900 -> ~49 rev/s
+// PWM counter wraps at 2^16-1 -> if a fan is spinning that fast you've a problem.
 struct Tachometer final : SensorPeriodic {
-    // DELTA BFB0712H spec sheet says an RPM of 2900 -> ~49 rev/s
-    // Honestly at this low a rate it might be worth just using
-    // an interrupt instead of screwing with a PWM slice...
-    constexpr static auto TACHOMETER_READ_PERIOD = SENSOR_UPDATE_PERIOD;
-    static_assert(100ms <= TACHOMETER_READ_PERIOD && "need at least 100ms to get a good sampling");
-
-    Tachometer(GPIO pin = GPIO::none(), uint32_t pulses_per_revolution = 1) {
-        setup(pin, pulses_per_revolution);
-    }
+    // need at least 100ms for a reasonable read and no point sampling longer than 1s
+    constexpr static auto TACHOMETER_READ_PERIOD =
+            std::clamp<std::chrono::milliseconds>(SENSOR_UPDATE_PERIOD, 100ms, 1s);
+
+    // hz_max_pulse = hz_sample / 2
+    // sample at 10 kHz, that'll support up to 150'000 RPM w/ 2 pulses per rev
+    constexpr static auto PIN_SAMPLING_PERIOD = 0.1ms;
+
+    Tachometer() = default;
 
-    void setup(GPIO pin, uint32_t pulses_per_revolution = 1) {
-        assert((!pin || pwm_gpio_to_channel(pin) == PWM_CHAN_B) && "tachometers must run on a B channel");
+    void setup(Pins::GPIOs const& pins, uint32_t pulses_per_revolution = 1) {
         assert(0 < pulses_per_revolution);
-        this->pin_ = pin;
-        this->pulses_per_revolution = pulses_per_revolution;
-    }
 
-    [[nodiscard]] GPIO pin() const {
-        return pin_;
+        for (auto&& pin : pins) {
+            if (!pin) continue;
+
+            switch (gpio_get_function(pin)) {
+            default: assert(false); break;
+            case GPIO_FUNC_SIO: break;
+            case GPIO_FUNC_PWM: {
+                auto cfg = pwm_get_default_config();
+                pwm_config_set_clkdiv_mode(&cfg, PWM_DIV_B_FALLING);
+                pwm_init(pwm_gpio_to_slice_num_(pin), &cfg, false);
+            } break;
+            }
+        }
+
+        std::copy(std::begin(pins), std::end(pins), this->pins);
+        this->pulses_per_revolution = pulses_per_revolution;
     }
 
     [[nodiscard]] double revolutions_per_second() const {
@@ -42,29 +58,93 @@ struct Tachometer final : SensorPeriodic {
 
 protected:
     void read() override {
-        if (!pin_) return;
-        auto slice_num = pwm_gpio_to_slice_num_(pin_);
-
-        pwm_set_counter(slice_num, 0);
+        // Since we're targetting relatively low pulse hz don't bother about
+        // keeping code in RAM to avoid flash penalty or function overhead;
+        // we're running at 125 MHz & reading <= 1 kHz pulse signals,
+        // we've plenty of room for sloppiness.
         auto begin = std::chrono::steady_clock::now();
-        pwm_set_enabled(slice_num, true);
-
-        task_delay(TACHOMETER_READ_PERIOD);
-
-        pwm_set_enabled(slice_num, false);
+        uint32_t pulses = pulse_count(begin);
         auto end = std::chrono::steady_clock::now();
 
         auto duration_sec =
                 std::chrono::duration_cast<std::chrono::duration<double, std::ratio<1>>>(end - begin);
-        auto count = pwm_get_counter(slice_num);
-        revolutions_per_second_ = count / duration_sec.count() / pulses_per_revolution;
+        revolutions_per_second_ = pulses / duration_sec.count() / pulses_per_revolution;
 
-        // printf("tachometer_measure dur=%f s cnt=%d rev-per-sec=%f rpm=%f\n",
-        //    duration_sec.count(), int(count), revolutions_per_second_, revolutions_per_second_ * 60);
+        // printf("tachometer_measure dur=%f s cnt=%u rev-per-sec=%f rpm=%f\n", duration_sec.count(),
+        //         unsigned(pulses), revolutions_per_second_, revolutions_per_second_ * 60);
     }
 
 private:
-    GPIO pin_;
+    // we're nowhere near high precision stuff
+    uint32_t pulse_count(std::chrono::steady_clock::time_point const begin) {
+        uint32_t pulses = 0;
+        if (pulse_start()) {  // polling required, we've non-PWM tacho pins
+            for (auto now = begin; (now - begin) < TACHOMETER_READ_PERIOD;
+                    now = std::chrono::steady_clock::now()) {
+                pulses += pulse_poll();
+                task_delay(PIN_SAMPLING_PERIOD);
+            }
+        } else  // everything is handled by PWM slices, just nap for a bit
+            task_delay(TACHOMETER_READ_PERIOD);
+
+        pulses += pulse_end();  // add pulses from PWM counters
+        return pulses;
+    }
+
+    bool pulse_start() {
+        bool do_polling = false;
+
+        for (auto&& pin : pins) {
+            if (!pin) continue;
+
+            switch (gpio_get_function(pin)) {
+            default: break;
+            case GPIO_FUNC_SIO: {
+                do_polling = true;
+                state.set(&pin - pins, gpio_get(pin));
+            } break;
+            case GPIO_FUNC_PWM: {
+                auto slice = pwm_gpio_to_slice_num_(pin);
+                pwm_set_counter(slice, 0);
+                pwm_set_enabled(slice, true);
+            } break;
+            }
+        }
+
+        return do_polling;
+    }
+
+    uint32_t pulse_poll() {
+        uint32_t pulses = 0;
+
+        for (auto&& pin : pins) {
+            if (pin && gpio_get_function(pin) == GPIO_FUNC_SIO) {
+                auto curr = gpio_get(pin);
+                auto prev = state.test(&pin - pins);
+                pulses += (prev != curr) && curr;  // count rising edges
+                state.set(&pin - pins, curr);
+            }
+        }
+
+        return pulses;
+    }
+
+    uint32_t pulse_end() {
+        uint32_t pulses = 0;
+
+        for (auto&& pin : pins) {
+            if (pin && gpio_get_function(pin) == GPIO_FUNC_PWM) {
+                auto slice = pwm_gpio_to_slice_num_(pin);
+                pwm_set_enabled(slice, false);
+                pulses += pwm_get_counter(slice);
+            }
+        }
+
+        return pulses;
+    }
+
+    Pins::GPIOs pins;
+    std::bitset<Pins::ALTERNATIVES_MAX> state;
     uint pulses_per_revolution = 1;
     double revolutions_per_second_ = 0;
 };