tttapa / Control-Surface

Arduino library for creating MIDI controllers and other MIDI devices.
GNU General Public License v3.0
1.23k stars 137 forks source link

GenericIncrementSelector with Callback #334

Closed marrold closed 3 years ago

marrold commented 3 years ago

Hi first of all thanks for this great library. I'd like to use a bank selector with a single button, however when I pass the LED pin (Like in the GenericIncrementDecrementSelector example ) to the callback I get errors when compiling:

Code:

GenericIncrementSelector<3, MySelectorCallback> selector = {
    bank,
    LED_BUILTIN,
    4,
};

I get the following error when compiling

sdr_console_v0.1:54:1: error: could not convert '{bank, 13, 4}' from '<brace-enclosed initializer list>' to 'CS::GenericIncrementSelector<3, MySelectorCallback>'
 };
could not convert '{bank, 13, 4}' from '<brace-enclosed initializer list>' to 'CS::GenericIncrementSelector<3, MySelectorCallback>'

Is it possible to pass the LED pin to the GenericIncrementSelector? Or otherwise use GenericIncrementDecrementSelector with a single pin?

Thanks

tttapa commented 3 years ago

Please post your full code.

As you can see from the documentation, the second argument has to be convertible to your custom callback type, so the MySelectorCallback class should have a constructor with a single integer argument.

marrold commented 3 years ago

Full code below. It works without modification for GenericIncrementDecrementSelector, but fails to compile with GenericIncrementSelector :

#include <Encoder.h> // Include the Encoder library.
#include <Control_Surface.h> // Include the Control Surface library
#include <MIDI_Interfaces/SerialMIDI_Interface.hpp>

#include <jled.h>

// Instantiate a MIDI over USB interface.
HairlessMIDI_Interface midi;

class MySelectorCallback {
  public:
    // Constructor
    MySelectorCallback(pin_t led_pins) : led_pins(led_pins) {}

    void begin() {
      led.On();
    }

    void update() {
        led.Update();
    }

    void update(setting_t oldSetting, setting_t newSetting) {

      switch (newSetting) {
        case 0: led.On(); break;
        case 1: led.Blink(500, 500).Forever(); break;
        case 2: led.Blink(100, 100).Forever(); break;
        default: led.On(); break;
      }
    }

  private:
    pin_t led_pins;
    JLed led = JLed(led_pins);
};

// A bank with 3 channels and a single encoder, for Fast, Normal and Slow Tuning
Bank<3> bank(1); 

// Assign an encoder to the bank
Bankable::CCRotaryEncoder enc1 = { {bank, BankType::CHANGE_CHANNEL}, {2, 3}, MCU::V_POT_1,  1, };

//// This works, passing the LED pin to the callback
//GenericIncrementDecrementSelector<3, MySelectorCallback> selector = {
//  bank,         // bank to manage
//  LED_BUILTIN,
//  {4, 5},       // incr/decr button pins
//  Wrap::Wrap,   // wrap around when reaching setting 6
//};

// This fails to compile
GenericIncrementSelector<3, MySelectorCallback> selector = {
    bank,       // Bank to manage
    LED_BUILTIN,
    4,     // push button pin
};

void setup() {
  RelativeCCSender::setMode(relativeCCmode::MACKIE_CONTROL_RELATIVE);
  Control_Surface.begin(); // Initialize Control Surface
}

void loop() {
  Control_Surface.loop(); // Update the Control Surface
}

Thanks

tttapa commented 3 years ago

You need an extra pair of braces, because the argument is of type AH::IncrementButton, not pin_t.

GenericIncrementSelector<3, MySelectorCallback> selector = {
    bank,       // Bank to manage
    LED_BUILTIN,
    {4},     // push button pin
};
marrold commented 3 years ago

That cured the compilation problem but now the update() function isn't being called. Apologies if this is a trivial mistake, it just seems odd it works fine for a GenericIncrementDecrementSelector. Thanks again.

class MySelectorCallback {
  public:
    // Constructor
    MySelectorCallback(pin_t led_pins) : led_pins(led_pins) {}

    void begin() {
      led.On();
    }

    void update() {
        led.Update();
    }

    void update(setting_t oldSetting, setting_t newSetting) {

      switch (newSetting) {
        case 0: led.On(); break;
        case 1: led.Blink(500, 500).Forever(); break;
        case 2: led.Blink(100, 100).Forever(); break;
        default: led.On(); break;
      }
    }

  private:
    pin_t led_pins;
    JLed led = JLed(led_pins);
};
tttapa commented 3 years ago

That does indeed seem to be a bug. I'll push a patch in a moment.

marrold commented 3 years ago

The latest master works as expected, thanks for your help and such a speedy fix. Let me know if you have a donation link, I owe you a beer / coffee 😀