Digit Mode for frequency field (#1298)

* Remove 'auto' step mode

* Support per-digit edits on the freq field.

* Swizzle instead of raw accessor

* Fix debug ui after swizzle

firmware/application/apps/analog_audio_app.hpp

@@ -48,9 +48,9 @@ class AMOptionsView : public View {
 
     OptionsField options_config{
         {3 * 8, 0 * 16},
-        6,  // number of blanking characters
+        6,  // Max option length
         {
-            // using  common messages from freqman.cpp
+            // Using common messages from freqman_ui.cpp
         }};
 };
 
@@ -65,9 +65,9 @@ class NBFMOptionsView : public View {
     };
     OptionsField options_config{
         {3 * 8, 0 * 16},
-        4,
+        3,  // Max option length
         {
-            // using  common messages from freqman.cpp
+            // Using common messages from freqman_ui.cpp
         }};
 
     Text text_squelch{
@@ -93,9 +93,9 @@ class WFMOptionsView : public View {
     };
     OptionsField options_config{
         {3 * 8, 0 * 16},
-        4,
+        4,  // Max option length
         {
-            // using  common messages from freqman.cpp
+            // Using common messages from freqman_ui.cpp
         }};
 };
 

firmware/application/apps/ui_debug.cpp

@@ -251,7 +251,7 @@ void ControlsSwitchesWidget::on_show() {
 
 bool ControlsSwitchesWidget::on_key(const KeyEvent key) {
     key_event_mask = 1 << toUType(key);
-    long_press_key_event_mask = switch_long_press_occurred((size_t)key) ? key_event_mask : 0;
+    long_press_key_event_mask = key_is_long_pressed(key) ? key_event_mask : 0;
     return true;
 }
 
@@ -264,9 +264,9 @@ void ControlsSwitchesWidget::paint(Painter& painter) {
         {32, 64, 16, 16},  // Down
         {32, 0, 16, 16},   // Up
         {32, 32, 16, 16},  // Select
+        {96, 0, 16, 16},   // Dfu
         {16, 96, 16, 16},  // Encoder phase 0
         {48, 96, 16, 16},  // Encoder phase 1
-        {96, 0, 16, 16},   // Dfu
         {96, 64, 16, 16},  // Touch
     }};
 
@@ -283,9 +283,9 @@ void ControlsSwitchesWidget::paint(Painter& painter) {
         {32 + 1, 64 + 1, 16 - 2, 16 - 2},  // Down
         {32 + 1, 0 + 1, 16 - 2, 16 - 2},   // Up
         {32 + 1, 32 + 1, 16 - 2, 16 - 2},  // Select
+        {96 + 1, 0 + 1, 16 - 2, 16 - 2},   // Dfu
         {16 + 1, 96 + 1, 16 - 2, 16 - 2},  // Encoder phase 0
         {48 + 1, 96 + 1, 16 - 2, 16 - 2},  // Encoder phase 1
-        {96 + 1, 0 + 1, 16 - 2, 16 - 2},   // Dfu
     }};
 
     auto switches_raw = control::debug::switches();
@@ -354,13 +354,13 @@ DebugControlsView::DebugControlsView(NavigationView& nav) {
     });
 
     button_done.on_select = [&nav](Button&) {
-        switches_long_press_enable(0);
+        set_switches_long_press_config(0);
         nav.pop();
     };
 
     options_switches_mode.on_change = [this](size_t, OptionsField::value_t v) {
         (void)v;
-        switches_long_press_enable(options_switches_mode.selected_index_value());
+        set_switches_long_press_config(options_switches_mode.selected_index_value());
     };
 }
 

firmware/application/hw/debounce.cpp

@@ -84,8 +84,8 @@ bool Debounce::feed(const uint8_t bit) {
                 // Button is being held down and long_press support is enabled for this key:
                 // if LONG_PRESS_DELAY is reached then finally report that switch is pressed and set flag
                 // indicating it was a LONG press
-                // (note that repease_support and long_press support are mutually exclusive)
+                // (note that repeat_support and long_press support are mutually exclusive)
-                if (held_time_ == LONG_PRESS_DELAY) {
+                if (held_time_ >= LONG_PRESS_DELAY) {
                     long_press_occurred_ = true;
                     pulse_upon_release_ = 0;
                     held_time_ = 0;

firmware/application/hw/debounce.hpp

@@ -31,7 +31,7 @@
 // # of timer0 ticks before a held button starts being counted as repeated presses
 #define REPEAT_INITIAL_DELAY 250
 #define REPEAT_SUBSEQUENT_DELAY 92
-#define LONG_PRESS_DELAY 1000
+#define LONG_PRESS_DELAY 800
 
 class Debounce {
    public:
@@ -45,7 +45,11 @@ class Debounce {
         repeat_enabled_ = true;
     }
 
-    void set_long_press_support(bool v) {
+    bool get_long_press_enabled() const {
+        return long_press_enabled_;
+    }
+
+    void set_long_press_enabled(bool v) {
         long_press_enabled_ = v;
     }
 

firmware/application/irq_controls.cpp

@@ -22,32 +22,33 @@
 #include "irq_controls.hpp"
 
 #include "ch.h"
-#include "hal.h"
+#include "debounce.hpp"
-
+#include "encoder.hpp"
 #include "event_m0.hpp"
-
+#include "hal.h"
 #include "touch.hpp"
 #include "touch_adc.hpp"
-#include "encoder.hpp"
-#include "debounce.hpp"
 #include "utility.hpp"
 
 #include <cstdint>
 #include <array>
 
 #include "portapack_io.hpp"
-
 #include "hackrf_hal.hpp"
+
 using namespace hackrf::one;
+using namespace portapack;
 
 static Thread* thread_controls_event = NULL;
 
-static std::array<Debounce, 8> switch_debounce;
+// Index with the Switch enum.
+static std::array<Debounce, 6> switch_debounce;
+static std::array<Debounce, 2> encoder_debounce;
+
+static_assert(std::size(switch_debounce) == toUType(Switch::Dfu) + 1);
 
 static Encoder encoder;
-
 static volatile uint32_t encoder_position{0};
-
 static volatile uint32_t touch_phase{0};
 
 /* TODO: Change how touch scanning works. It produces a decent amount of noise
@@ -60,11 +61,11 @@ static volatile uint32_t touch_phase{0};
  * Noise will only occur when the panel is being touched. Not ideal, but
  * an acceptable improvement.
  */
-static std::array<portapack::IO::TouchPinsConfig, 3> touch_pins_configs{
+static std::array<IO::TouchPinsConfig, 3> touch_pins_configs{
     /* State machine will pause here until touch is detected. */
-    portapack::IO::TouchPinsConfig::SensePressure,
+    IO::TouchPinsConfig::SensePressure,
-    portapack::IO::TouchPinsConfig::SenseX,
+    IO::TouchPinsConfig::SenseX,
-    portapack::IO::TouchPinsConfig::SenseY,
+    IO::TouchPinsConfig::SenseY,
 };
 
 static touch::Frame temp_frame;
@@ -80,7 +81,7 @@ static bool touch_update() {
     const auto current_phase = touch_pins_configs[touch_phase];
 
     switch (current_phase) {
-        case portapack::IO::TouchPinsConfig::SensePressure: {
+        case IO::TouchPinsConfig::SensePressure: {
             const auto z1 = samples.xp - samples.xn;
             const auto z2 = samples.yp - samples.yn;
             const auto touch_raw = (z1 > touch::touch_threshold) || (z2 > touch::touch_threshold);
@@ -94,11 +95,11 @@ static bool touch_update() {
             }
         } break;
 
-        case portapack::IO::TouchPinsConfig::SenseX:
+        case IO::TouchPinsConfig::SenseX:
             temp_frame.x += samples;
             break;
 
-        case portapack::IO::TouchPinsConfig::SenseY:
+        case IO::TouchPinsConfig::SenseY:
             temp_frame.y += samples;
             break;
 
@@ -122,25 +123,57 @@ static bool touch_update() {
 
 static uint8_t switches_raw = 0;
 
+/* The raw data is not packed in a way that makes looping over it easy.
+ * One option would be an accessor helper (RawSwitch). Another option
+ * is to swizzle the bits into a friendlier order. */
+// /* Type to access the bits in the raw switch data. */
+// struct RawSwitch {
+//     const uint8_t raw_{0};
+
+// uint8_t right() const { return (raw_ >> 0) & 1; }
+// uint8_t left() const { return (raw_ >> 1) & 1; }
+// uint8_t down() const { return (raw_ >> 2) & 1; }
+// uint8_t up() const { return (raw_ >> 3) & 1; }
+// uint8_t select() const { return (raw_ >> 4) & 1; }
+// uint8_t rot_a() const { return (raw_ >> 5) & 1; }
+// uint8_t rot_b() const { return (raw_ >> 6) & 1; }
+// uint8_t dfu() const { return (raw_ >> 7) & 1; }};
+
+static uint8_t swizzle_raw(uint8_t raw) {
+    return (raw & 0x1F) |         // Keep the bottom 5 bits the same.
+           ((raw >> 2) & 0x20) |  // Shift the DFU bit down to bit 6.
+           ((raw << 1) & 0xC0);   // Shift the encoder bits up to be 7 & 8.
+}
+
 static bool switches_update(const uint8_t raw) {
     // TODO: Only fire event on press, not release?
     bool switch_changed = false;
-    for (size_t i = 0; i < switch_debounce.size(); i++) {
+
-        switch_changed |= switch_debounce[i].feed((raw >> i) & 1);
+    for (size_t i = 0; i < switch_debounce.size(); ++i) {
+        uint8_t bit = (raw >> i) & 0x01;
+        switch_changed |= switch_debounce[i].feed(bit);
     }
 
     return switch_changed;
 }
 
+static bool encoder_update(const uint8_t raw) {
+    bool encoder_changed = false;
+
+    encoder_changed |= encoder_debounce[0].feed((raw >> 6) & 0x01);
+    encoder_changed |= encoder_debounce[1].feed((raw >> 7) & 0x01);
+
+    return encoder_changed;
+}
+
 static bool encoder_read() {
     const auto delta = encoder.update(
-        switch_debounce[5].state(),
+        encoder_debounce[0].state(),
-        switch_debounce[6].state());
+        encoder_debounce[1].state());
 
     if (delta != 0) {
         encoder_position += delta;
         return true;
-        ;
     } else {
         return false;
     }
@@ -149,11 +182,13 @@ static bool encoder_read() {
 void timer0_callback(GPTDriver* const) {
     eventmask_t event_mask = 0;
     if (touch_update()) event_mask |= EVT_MASK_TOUCH;
-    switches_raw = portapack::io.io_update(touch_pins_configs[touch_phase]);
+
-    if (switches_update(switches_raw)) {
+    switches_raw = swizzle_raw(io.io_update(touch_pins_configs[touch_phase]));
+    if (switches_update(switches_raw))
         event_mask |= EVT_MASK_SWITCHES;
-        if (encoder_read()) event_mask |= EVT_MASK_ENCODER;
+
-    }
+    if (encoder_update(switches_raw) && encoder_read())
+        event_mask |= EVT_MASK_ENCODER;
 
     /* Signal event loop */
     if (event_mask) {
@@ -189,37 +224,39 @@ void controls_init() {
     gptStartContinuous(&GPTD1, timer0_match_count);
 
     // Enable repeat for directional switches only
-    for (size_t i = 0; i < 4; i++)
+    for (auto i = Switch::Right; i <= Switch::Up; incr(i))
-        switch_debounce[i].enable_repeat();
+        switch_debounce[toUType(i)].enable_repeat();
 }
 
 SwitchesState get_switches_state() {
     SwitchesState result;
 
-    // Right, Left, Down, Up, & Select switches
+    // TODO: Ignore multiple keys at the same time?
-    for (size_t i = 0; i < result.size() - 1; i++) {
+    for (size_t i = 0; i < result.size(); i++)
-        // TODO: Ignore multiple keys at the same time?
         result[i] = switch_debounce[i].state();
-    }
 
-    // Grab Dfu switch from bit 7 and return in bit 5 so that all switches are grouped together in this 6-bit result
-    result[(size_t)Switch::Dfu] = switch_debounce[7].state();
     return result;
 }
 
-// Configure which switches support long press (note those switches will not support Repeat function)
+/* Gets the long press enabled state for all the switches. */
-void switches_long_press_enable(SwitchesState switches_long_press_enabled) {
+SwitchesState get_switches_long_press_config() {
-    // Right, Left, Down, Up, & Select switches
+    SwitchesState result;
-    for (size_t i = 0; i < switches_long_press_enabled.size() - 1; i++) {
-        switch_debounce[i].set_long_press_support(switches_long_press_enabled[i]);
-    }
 
-    // Dfu switch
+    for (size_t i = 0; i < result.size(); i++)
-    switch_debounce[7].set_long_press_support(switches_long_press_enabled[(size_t)Switch::Dfu]);
+        result[i] = switch_debounce[i].get_long_press_enabled();
+
+    return result;
 }
 
-bool switch_long_press_occurred(size_t v) {
+/* Configures which switches support long press.
-    return (v == (size_t)Switch::Dfu) ? switch_debounce[7].long_press_occurred() : switch_debounce[v].long_press_occurred();
+ * NB: those switches will not support Repeat function. */
+void set_switches_long_press_config(SwitchesState switch_config) {
+    for (size_t i = 0; i < switch_config.size(); i++)
+        switch_debounce[i].set_long_press_enabled(switch_config[i]);
+}
+
+bool switch_is_long_pressed(Switch s) {
+    return switch_debounce[toUType(s)].long_press_occurred();
 }
 
 EncoderPosition get_encoder_position() {

firmware/application/irq_controls.hpp

@@ -28,25 +28,28 @@
 #include "touch.hpp"
 
 // Must be same values as in ui::KeyEvent
-enum class Switch {
+// TODO: Why not just reuse one or the other?
+enum class Switch : uint8_t {
     Right = 0,
     Left = 1,
     Down = 2,
     Up = 3,
     Sel = 4,
-    Dfu = 5,
+    Dfu = 5
 };
 
+// Index with the Switch enum.
 using SwitchesState = std::bitset<6>;
-
 using EncoderPosition = uint32_t;
 
 void controls_init();
 SwitchesState get_switches_state();
 EncoderPosition get_encoder_position();
 touch::Frame get_touch_frame();
-void switches_long_press_enable(SwitchesState v);
+
-bool switch_long_press_occurred(size_t v);
+SwitchesState get_switches_long_press_config();
+void set_switches_long_press_config(SwitchesState switch_config);
+bool switch_is_long_pressed(Switch s);
 
 namespace control {
 namespace debug {

firmware/application/ui/ui_receiver.cpp

@@ -20,16 +20,15 @@
  * Boston, MA 02110-1301, USA.
  */
 
+#include "irq_controls.hpp"
+#include "max283x.hpp"
+#include "portapack.hpp"
+#include "string_format.hpp"
 #include "ui_receiver.hpp"
 #include "ui_freqman.hpp"
 
-#include "portapack.hpp"
 using namespace portapack;
 
-#include "string_format.hpp"
-
-#include "max283x.hpp"
-
 namespace ui {
 
 /* FrequencyField ********************************************************/
@@ -38,10 +37,15 @@ FrequencyField::FrequencyField(
     const Point parent_pos)
     : Widget{{parent_pos, {8 * 10, 16}}},
       length_{11},
-      range(rf::tuning_range) {
+      range_{rf::tuning_range} {
+    initial_switch_config_ = get_switches_long_press_config();
     set_focusable(true);
 }
 
+FrequencyField::~FrequencyField() {
+    set_switches_long_press_config(initial_switch_config_);
+}
+
 rf::Frequency FrequencyField::value() const {
     return value_;
 }
@@ -59,48 +63,75 @@ void FrequencyField::set_value(rf::Frequency new_value) {
 }
 
 void FrequencyField::set_step(rf::Frequency new_value) {
-    step = new_value;
     // TODO: Quantize current frequency to a step of the new size?
+    step_ = new_value;
 }
 
 void FrequencyField::paint(Painter& painter) {
-    const std::string str_value = to_string_short_freq(value_);
+    const auto str_value = to_string_short_freq(value_);
-
     const auto paint_style = has_focus() ? style().invert() : style();
 
     painter.draw_string(
         screen_pos(),
         paint_style,
         str_value);
+
+    // Highlight current digit in digit_mode.
+    if (digit_mode_) {
+        auto p = screen_pos();
+        p += {digit_ * char_width, 0};
+        painter.draw_char(p, Styles::bg_blue, str_value[digit_]);
+    }
 }
 
-bool FrequencyField::on_key(const ui::KeyEvent event) {
+bool FrequencyField::on_key(KeyEvent event) {
-    if (event == ui::KeyEvent::Select) {
+    if (event == KeyEvent::Select) {
+        // Toggle 'digit' mode with long-press.
+        if (digit_mode_ || key_is_long_pressed(event)) {
+            digit_mode_ = !digit_mode_;
+            set_dirty();
+            return true;
+        }
+
         if (on_edit) {
             on_edit();
             return true;
         }
     }
+
+    if (digit_mode_) {
+        switch (event) {
+            case KeyEvent::Up:
+                set_value(value_ + digit_step());
+                break;
+            case KeyEvent::Down:
+                set_value(value_ - digit_step());
+                break;
+            case KeyEvent::Left:
+                digit_--;
+                break;
+            case KeyEvent::Right:
+                digit_++;
+                break;
+            default:
+                return false;
+        }
+
+        // Clip value to the bounds of 'to_string_short_freq' result.
+        digit_ = clip<uint8_t>(digit_, 0, 8);
+        set_dirty();
+        return true;
+    }
+
     return false;
 }
 
 bool FrequencyField::on_encoder(const EncoderEvent delta) {
-    if (step == 0) {  // 'Auto' mode.'
+    if (digit_mode_)
-        auto ms = RTT2MS(halGetCounterValue());
+        set_value(value_ + (delta * digit_step()));
-        auto delta_ms = last_ms_ <= ms ? ms - last_ms_ : ms;
+    else
-        last_ms_ = ms;
+        set_value(value_ + (delta * step_));
 
-        // The goal is to map 'scale' to a range of about 10 to 10M.
-        // The faster the encoder is rotated, the larger the step.
-        // Linear doesn't feel right. Hyperbolic felt better.
-        // To get these magic numbers, I graphed the function until the
-        // curve shape seemed about right then tested on device.
-        delta_ms = std::min(145ull, delta_ms) + 5;  // Prevent DIV/0
-        int64_t scale = 200'000'000 / (0.001'55 * std::pow(delta_ms, 5.45)) + 8;
-        set_value(value() + (delta * scale));
-    } else {
-        set_value(value() + (delta * step));
-    }
     return true;
 }
 
@@ -115,10 +146,35 @@ void FrequencyField::on_focus() {
     if (on_show_options) {
         on_show_options();
     }
+
+    // Enable long press on "Select".
+    SwitchesState config;
+    config[toUType(Switch::Sel)] = true;
+    set_switches_long_press_config(config);
+}
+
+void FrequencyField::on_blur() {
+    set_switches_long_press_config(initial_switch_config_);
+}
+
+rf::Frequency FrequencyField::digit_step() const {
+    constexpr rf::Frequency steps[] = {
+        1'000'000'000,
+        100'000'000,
+        10'000'000,
+        1'000'000,
+        0,  // Decimal point position.
+        100'000,
+        10'000,
+        1'000,
+        100,
+    };
+
+    return digit_ < std::size(steps) ? steps[digit_] : 0;
 }
 
 rf::Frequency FrequencyField::clamp_value(rf::Frequency value) {
-    return range.clip(value);
+    return range_.clip(value);
 }
 
 /* FrequencyKeypadView ***************************************************/

firmware/application/ui/ui_receiver.hpp

@@ -27,6 +27,7 @@
 #include "ui_painter.hpp"
 #include "ui_widget.hpp"
 
+#include "irq_controls.hpp"
 #include "rf_path.hpp"
 
 #include <cstddef>
@@ -45,6 +46,7 @@ class FrequencyField : public Widget {
     using range_t = rf::FrequencyRange;
 
     FrequencyField(Point parent_pos);
+    ~FrequencyField();
 
     rf::Frequency value() const;
 
@@ -53,18 +55,26 @@ class FrequencyField : public Widget {
 
     void paint(Painter& painter) override;
 
-    bool on_key(ui::KeyEvent event) override;
+    bool on_key(KeyEvent event) override;
     bool on_encoder(EncoderEvent delta) override;
     bool on_touch(TouchEvent event) override;
     void on_focus() override;
+    void on_blur() override;
 
    private:
     const size_t length_;
-    const range_t range;
+    const range_t range_;
+
     rf::Frequency value_{0};
-    rf::Frequency step{25000};
+    rf::Frequency step_{25000};
     uint64_t last_ms_{0};
 
+    uint8_t digit_{3};
+    bool digit_mode_{false};
+    SwitchesState initial_switch_config_{};
+
+    /* Gets the step value for the given digit when in digit_mode. */
+    rf::Frequency digit_step() const;
     rf::Frequency clamp_value(rf::Frequency value);
 };
 
@@ -242,7 +252,6 @@ class FrequencyStepView : public OptionsField {
               parent_pos,
               5,
               {
-                  {" Auto", 0},  /* Faster == larger step. */
                   {"   10", 10}, /* Fine tuning SSB voice pitch,in HF and QO-100 sat #669 */
                   {"   50", 50}, /* added 50Hz/10Hz,but we do not increase GUI digit decimal */
                   {"  100", 100},

firmware/common/ui.cpp

@@ -20,6 +20,7 @@
  */
 
 #include "ui.hpp"
+#include "irq_controls.hpp"
 #include "sine_table.hpp"
 #include "utility.hpp"
 
@@ -102,4 +103,14 @@ Point fast_polar_to_point(int32_t angle, uint32_t distance) {
                  (int16_sin_s4(((1 << 16) * (-angle - 90)) / 360) * distance) / (1 << 16));
 }
 
+bool key_is_long_pressed(KeyEvent key) {
+    if (key < KeyEvent::Back) {
+        // TODO: this would make more sense as a flag on KeyEvent
+        // passed up to the UI via event dispatch.
+        return switch_is_long_pressed(static_cast<Switch>(key));
+    }
+
+    return false;
+}
+
 } /* namespace ui */

firmware/common/ui.hpp

@@ -34,6 +34,10 @@ using Dim = int16_t;
 constexpr uint16_t screen_width = 240;
 constexpr uint16_t screen_height = 320;
 
+/* Dimensions for the default font character. */
+constexpr uint16_t char_width = 8;
+constexpr uint16_t char_height = 16;
+
 struct Color {
     uint16_t v;  // rrrrrGGGGGGbbbbb
 
@@ -328,7 +332,7 @@ struct Bitmap {
     const uint8_t* const data;
 };
 
-enum class KeyEvent {
+enum class KeyEvent : uint8_t {
     /* Ordinals map to bit positions reported by CPLD */
     Right = 0,
     Left = 1,
@@ -356,6 +360,11 @@ Point polar_to_point(float angle, uint32_t distance);
 
 Point fast_polar_to_point(int32_t angle, uint32_t distance);
 
+/* Default font glyph size. */
+constexpr Size char_size{char_width, char_height};
+
+bool key_is_long_pressed(KeyEvent key);
+
 } /* namespace ui */
 
 #endif /*__UI_H__*/

firmware/common/utility.hpp

@@ -53,6 +53,12 @@ constexpr typename std::underlying_type<E>::type toUType(E enumerator) noexcept
     return static_cast<typename std::underlying_type<E>::type>(enumerator);
 }
 
+/* Increments an enum value. Enumerator values are assumed to be serial. */
+template <typename E>
+void incr(E& e) {
+    e = static_cast<E>(toUType(e) + 1);
+}
+
 inline uint32_t flp2(uint32_t v) {
     v |= v >> 1;
     v |= v >> 2;