You've already forked ivan-torvalds-GuitarPedal
forked from AllSpiceMirrors/torvalds-GuitarPedal
6e13afbfd6
The pedal has one rotary encoder - and most boards do not even populate that - one stomp switch, and one LED. The code still described a board with two of each, in a numbering that had never survived a generation: GPIO_SW1 was the first rotary's shaft, GPIO_SW3 was the first stomp, and SW2 and SW4 were a second rotary and second stomp that no longer exist. Nothing in those names said so, which is how a pile of bare numbers ended up spread across three files with nobody able to check them. So name pins for their job - LED_GPIO, ROTARY_A/B/SW_GPIO, STOMP_GPIO - and give the switches an enum instead of an index. The switch id is both the bit in 'switch_val' and the PIO state machine number, and switch_gpio[] is now the one place that ties an id to a pin, walked in order by init_sw_pins(), so a switch cannot quietly end up reading somebody else's pin. switch_pressed(2) becomes switch_pressed(STOMP_SWITCH), which can be read and, more to the point, can be got wrong visibly. That accounts for four bindings that turn out to have been aimed at hardware that is not there: - "hold both switches" tested SW1 and SW2 with gpio_get(). SW2 is unpopulated and pulled up, so it read high and the condition was never true. Nothing could reach the reset-everything path behind it, including the one that left the settings pseudo-effect unable to reinitialise itself. - switch_pressed(4) tested a bit that no state machine ever set. Bits are named now, so there is no fifth switch to name. - "save effect state to EEPROM" on a long press, and "enable/disable the current effect" on a press, were both bound to SW2/SW4. Neither has been reachable since those switches went. save_effect_state() and find_effect_slot() had no other callers and go too; saving a scene over SysEx uses save_scene(), which never touched them. - the second encoder picked which effect was being edited. That is done over MIDI now, so switch_effect() goes and update_ui() reads current_midi_effect_idx directly. What is left is what the hardware can actually do: turn the encoder to change a value, hold it and turn to pick a different pot, press it to step to the next pot, tap the stomp to bypass, hold the stomp for the tuner. The rotary loses its two-element arrays and its second state machine along the way, and gains an assert that the quadrature pair stays adjacent, which the PIO program has always required and nothing said. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
42 lines
1.4 KiB
C
42 lines
1.4 KiB
C
//
|
|
// The switches, as software sees them.
|
|
//
|
|
// One bit per switch in 'switch_val', set by the debounce PIO
|
|
// interrupt and cleared by whoever acts on it. A long press sets a
|
|
// second bit LONGPRESS_SHIFT higher instead of the short one.
|
|
//
|
|
// The switch id is the bit number *and* the PIO state machine index -
|
|
// switch_gpio[] below is what ties an id to a pin, and init_sw_pins()
|
|
// walks it in order, so a switch cannot end up reading the wrong pin
|
|
// without the table saying so. It used to be a set of bare numbers
|
|
// spread across three files, where switch_pressed(4) tested a bit that
|
|
// nothing ever set and nobody noticed.
|
|
//
|
|
enum switch_id {
|
|
ROTARY_SWITCH, // the rotary encoder's shaft, pressed down
|
|
STOMP_SWITCH, // the footswitch
|
|
NR_SWITCHES,
|
|
};
|
|
|
|
static const unsigned char switch_gpio[NR_SWITCHES] = {
|
|
[ROTARY_SWITCH] = ROTARY_SW_GPIO,
|
|
[STOMP_SWITCH] = STOMP_GPIO,
|
|
};
|
|
|
|
//
|
|
// Short presses live in the low bits, long presses the same distance
|
|
// up. Both have to fit in 'switch_val'.
|
|
//
|
|
#define LONGPRESS_SHIFT 16
|
|
#define LONGPRESS(sw) ((sw) + LONGPRESS_SHIFT)
|
|
|
|
_Static_assert(NR_SWITCHES <= LONGPRESS_SHIFT,
|
|
"switches and their long presses overlap");
|
|
_Static_assert(LONGPRESS(NR_SWITCHES) <= 32,
|
|
"switch_val is too narrow for this many switches");
|
|
|
|
static unsigned int switch_val;
|
|
|
|
#define switch_pressed(sw) (!!(switch_val & (1u << (sw))))
|
|
#define switch_clear(sw) __atomic_and_fetch(&switch_val, ~(1u << (sw)), __ATOMIC_RELAXED)
|