Control-Surface icon indicating copy to clipboard operation
Control-Surface copied to clipboard

Implement Velocity with NoteButton

Open frederico-betting opened this issue 3 years ago • 23 comments

Hi,

First of all congrats for the excellent work on this library!!! This is amazing...

Talking about my question: I want to implement velocity in project but I still don't have a clue how to do that. I've already tried to find an answer to my question in any old issue already submitted but I could only find this issue as something similar to what I need: https://github.com/tttapa/Control-Surface/issues/151. And I don't know exactly how to adapt to my project.

Currently, I'm using TCRT5000 Infrared Sensor Module to trigger each note. BTW, it works perfectly with the following code. In the analog pin (A0) I get a variation of 1023 ~ 0 based on the distance of any object and the sensor.

Could you help me with that? Do you have any example I could use to implement that using Bankable::NoteButton?

Thank you very much!

#include <Control_Surface.h>
 
// Instantiate a MIDI Interface to use
HairlessMIDI_Interface midi;

using namespace MIDI_Notes;

// Instantiate a Transposer that can transpose from one octave down to one
// octave up
Transposer<-12, +12> transposer;

// Instantiate a Selector to change the transposition
IncrementDecrementSelector<transposer.getNumberOfBanks()> selector = {
  transposer,
  {10, 11},
  Wrap::Wrap,
};

// Instantiate an analog multiplexer
CD74HC4067 mux0 = {
  A0,       // Analog input pin
  {2, 3, 4, 5} // Address pins S0, S1, S2, S3
};

// Instantiate an analog multiplexer
CD74HC4067 mux1 = {
  A1,       // Analog input pin
  {2, 3, 4, 5} // Address pins S0, S1, S2, S3
};

Bankable::NoteButton keys0[] = {
  {transposer, mux0.pin(0), {note(C, 2), CHANNEL_1}},
  {transposer, mux0.pin(1), {note(Db, 2), CHANNEL_1}},
  {transposer, mux0.pin(2), {note(D, 2), CHANNEL_1}},
  {transposer, mux0.pin(3), {note(Eb, 2), CHANNEL_1}},
  {transposer, mux0.pin(4), {note(E, 2), CHANNEL_1}},
  {transposer, mux0.pin(5), {note(F, 2), CHANNEL_1}},
  {transposer, mux0.pin(6), {note(Gb, 2), CHANNEL_1}},
  {transposer, mux0.pin(7), {note(G, 2), CHANNEL_1}},
  {transposer, mux0.pin(8), {note(Ab, 2), CHANNEL_1}},
  {transposer, mux0.pin(9), {note(A, 2), CHANNEL_1}},
  {transposer, mux0.pin(10), {note(Bb, 2), CHANNEL_1}},
  {transposer, mux0.pin(11), {note(B, 2), CHANNEL_1}},
  {transposer, mux0.pin(12), {note(C, 3), CHANNEL_1}},
  {transposer, mux0.pin(13), {note(Db, 3), CHANNEL_1}},
  {transposer, mux0.pin(14), {note(D, 3), CHANNEL_1}},
  {transposer, mux0.pin(15), {note(Eb, 3), CHANNEL_1}},
  {transposer, mux1.pin(0), {note(E, 3), CHANNEL_1}},
  {transposer, mux1.pin(1), {note(F, 3), CHANNEL_1}},
  {transposer, mux1.pin(2), {note(Gb, 3), CHANNEL_1}},
  {transposer, mux1.pin(3), {note(G, 3), CHANNEL_1}},
  {transposer, mux1.pin(4), {note(Ab, 3), CHANNEL_1}},
  {transposer, mux1.pin(5), {note(A, 3), CHANNEL_1}},
  {transposer, mux1.pin(6), {note(Bb, 3), CHANNEL_1}},
  {transposer, mux1.pin(7), {note(B, 3), CHANNEL_1}},
  {transposer, mux1.pin(8), {note(C, 4), CHANNEL_1}},
  {transposer, mux1.pin(9), {note(Db, 4), CHANNEL_1}},
  {transposer, mux1.pin(10), {note(D, 4), CHANNEL_1}},
  {transposer, mux1.pin(11), {note(Eb, 4), CHANNEL_1}},
  {transposer, mux1.pin(12), {note(E, 4), CHANNEL_1}},
  {transposer, mux1.pin(13), {note(F, 4), CHANNEL_1}},
  {transposer, mux1.pin(14), {note(Gb, 4), CHANNEL_1}},
  {transposer, mux1.pin(15), {note(G, 4), CHANNEL_1}},
};
(...)

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

frederico-betting avatar Sep 23 '20 23:09 frederico-betting

The NoteButton class has a setVelocity method that you could use. However, it's not ideal for large amounts of buttons. If the note numbers of subsequent buttons only differ by 1 semitone (which seems to be the case for your example code), you can use the NoteButtons (plural) class instead.

You could then do something like this:

#include <Control_Surface.h>

// Instantiate a MIDI Interface to use
HairlessMIDI_Interface midi;

// Instantiate a Transposer that can transpose from one octave down to one
// octave up
Transposer<-12, +12> transposer;

// Instantiate a Selector to change the transposition
IncrementDecrementSelector<transposer.getNumberOfBanks()> selector = {
  transposer,
  {10, 11},
  Wrap::Wrap,
};

// Instantiate some multiplexers
CD74HC4067 mux0 = {
  A0,          // Input pin
  {2, 3, 4, 5} // Address pins S0, S1, S2, S3
},
mux1 = {
  A1,          // Input pin
  {2, 3, 4, 5} // Address pins S0, S1, S2, S3
};

using namespace MIDI_Notes;

Bankable::NoteButtons<32> buttons = {
  transposer,                                    // Bank/Transposer configuration
  copyAs<Button>(cat(mux0.pins(), mux1.pins())), // Button pins
  {note(C, 2), CHANNEL_1},                       // Base address
  {1, 0},                                        // Increment note by 1 semitone, don't increment channel
};

// 7-bit filtered analog input for velocity control
FilteredAnalog<7> velocityInput = A2;

// Initialize the Control Surface
void setup() {
  Control_Surface.begin();
}

// Update the Control Surface
void loop() {
  if (velocityInput.update())
    buttons.setVelocity(velocityInput.getValue());
  Control_Surface.loop();
}

Instead of writing out all multiplexer pins, you can use:

cat(mux0.pins(), mux1.pins())

The pins method returns an array of all 16 pins of the multiplexer, and the cat function concatenates two arrays.
Since the NoteButtons expects an array of Buttons, not an array of pin numbers, you have to convert it to an array of buttons:

copyAs<Button>(...)

tttapa avatar Sep 24 '20 16:09 tttapa

Nice. Thanks! I already updated my code with your suggestion. I'm planning to have 4 octaves, so the code would become even bigger.

Regarding the velocity. I think it's a good start, but I'm not planning to have a separated input to define the velocity, but each pin (sensor) attached to multiplexers will define that.

I was thinking that I could calculate the time spent to run the interval from 1023 to 0 of each sensor. Then determine the velocity (0 ~ 127) based on a "magic" constant.

frederico-betting avatar Sep 24 '20 17:09 frederico-betting

Apologies, I misunderstood your question.

In that case, defining your own MIDI Output Element (like some of the code discussed in #151) would definitely be the best solution.

I can't really help you with the code that actually reads and interprets the measurements from the proximity sensors, but if you need help with the boilerplate code around it, to make it work with the library, sending MIDI, etc. feel free to ask questions about it here.

I think the best approach would be to start from the Custom-MIDI-Output-Element example. Once you understand how it works, you should be able to replace the digital Button input with your own logic for reading the analog inputs.

You'll probably have to keep a running average of the past velocities (the velocity as measured by the sensor, not MIDI velocity), wait for it to reach a certain threshold and then wait until the velocity decreases below some second threshold. Then you can send a MIDI note on message. You can then use the maximum velocity (the highest velocity before it started decreasing) to compute the MIDI velocity.
I don't know how to determine when to send a MIDI note off event, you could wait until the proximity is "far away" again, use a timeout, etc.
For the running average, you can use the EMA class.

Once you have something working, making your custom class "Bankable" isn't hard to do, I can help you with that once you get there.

tttapa avatar Sep 24 '20 18:09 tttapa

Don't worry...

You've provided me some documentation and now I know how to start. That's great.

I'll keep this issue opened. I'll contact you again if I have any question. Tks!

frederico-betting avatar Sep 24 '20 20:09 frederico-betting

Hi @tttapa

I think I already have a daft to perform what I want to implement.

This is what I have so far...

From NoteSensitiveKey.hpp

#include <Control_Surface.h>
 
BEGIN_CS_NAMESPACE
 
/*
 * @brief   A class for momentary buttons and switches that send MIDI events.
 *
 * The button is debounced, and the internal pull-up resistor is enabled.
 *
 * @see     NoteButton
 * @see     Button
 */
class NoteSensitiveKey : public MIDIOutputElement {
 public:
  /*
   * @brief   Create a new NoteSensitiveKey object on the
   *          given pin, with the given address and velocity.
   * 
   * @param   pin
   *          The digital input pin to read from.  
   *          The internal pull-up resistor will be enabled.
   * @param   address
   *          The MIDI address to send to.
   */
  NoteSensitiveKey(pin_t pin, const MIDIAddress &address)
    : analog{pin}, address{address} {}
 
  const int KEY_TRIGGER_MAX = 115;
  const int KEY_TRIGGER_MIN = 100;
  unsigned long startTriggerTime;

  uint8_t keyCurrentState = 0;
  uint8_t keyPreviousState = 0;
  
  // Indicates if the key is going down (>0) or up (<0).
  int stateVariation = 0;
  bool noteOnSent = false;

  // This method is called once by `Control_Surface.begin()`.
  void begin() final override {
  }
 
  // This method is called continuously by `Control_Surface.loop()`.
  void update() final override {
    if (analog.update()) {
      keyCurrentState = analog.getValue();
      keyCurrentState = 127 - keyCurrentState;
      stateVariation = keyCurrentState - keyPreviousState;
      keyPreviousState = keyCurrentState;

      if (stateVariation > 0) {
        if (keyCurrentState > KEY_TRIGGER_MIN && startTriggerTime == 0) {
          startTriggerTime = millis();
        }

        if (keyCurrentState >= KEY_TRIGGER_MAX && noteOnSent == false) {
          int endTriggerTime = millis() - startTriggerTime;
          int constrainedTriggerTime = constrain(endTriggerTime, 0, 127);
          noteOnSent = true;
          Control_Surface.sendNoteOn(address, 127-constrainedTriggerTime);
        }
      } else {
        if (noteOnSent == true) {
          Control_Surface.sendNoteOff(address, 127);
          startTriggerTime = 0;
          noteOnSent = false;
        }
      }
    }
  }
 
 private:
  FilteredAnalog<7> analog;
  const MIDIAddress address;
};
 
END_CS_NAMESPACE

From main.cpp, I'm calling this new class like this:

(...)
// Instantiate an analog multiplexer
CD74HC4067 mux[] = {
  {A0, {2, 3, 4, 5}}
};

NoteSensitiveKey keys[] = {
  {mux[0].pin(0), {note(C, 2), CHANNEL_1}},
  {mux[0].pin(1), {note(Db, 2), CHANNEL_1}},
  {mux[0].pin(2), {note(D, 2), CHANNEL_1}},
};
(...)

This implementation is really far from what I would expect to calculate the velocity but it's a good draft (I mean it minimally works). But before I think how I'll calculate that, I'm also thinking about 2 steps:

  1. Create a NoteSensitiveKeys (plural) to follow the same it exists to NoteButtons
  2. Make them Bankable

So, is there any template/example/documentation explaining how to make this kind of class Bankable?

frederico-betting avatar Sep 28 '20 19:09 frederico-betting

To make it bankable, you use the Bankable::SingleAddress class instead of the MIDIAddress class. Then use the Bankable::SingleAddress::getActiveAddress() method to get the address, determined by the selected bank.

I've added an example here: https://tttapa.github.io/Control-Surface-doc/Doxygen/d4/d6b/Custom-MIDI-Output-Element-Bankable_8ino-example.html

To support more than one button, you can have a look at the implementation of Bankable::MIDIButtons.

tttapa avatar Sep 29 '20 11:09 tttapa

@tttapa ,

I've tried so many different ways to implement the code but I don't know exactly how to implement it (some limitions on C++)...

I'm using this code as example to implement multiple sensors: https://github.com/tttapa/Control-Surface/blob/master/src/MIDI_Outputs/Abstract/MIDIButtons.hpp

As far as I understood, coinsidering that I'm using a sensor with analog ports, and not Button (switch) the correct data type to be used in this case should be FilteredAnalog. But how could I use it in my context?

template <uint8_t NumSensors>
class NoteSensitiveKeys : public MIDIOutputElement {
  public:
    NoteSensitiveKeys(const Array<AH::FilteredAnalog<>???, NumSensors> &sensors,
                      const MIDIAddress &baseAddress,
                      const RelativeMIDIAddress &incrementAddress)
        : sensors{sensors}, baseAddress(baseAddress),
          incrementAddress(incrementAddress) {}  

      (...)

  private:
    Array<AH::FilteredAnalog<7>???, NumSensors> sensors;
      const MIDIAddress baseAddress;
    const RelativeMIDIAddress incrementAddress;
};

I could not find any example on how to use multiple analog ports. That's why I'm asking you...

frederico-betting avatar Oct 16 '20 22:10 frederico-betting

It's not entirely clear to me what your question is. If you want to use the FilteredAnalog class, you have to specify the number of bits of resolution you want. The number of bits in the argument of your constructor has to be the same as the number of bits in the member variable in your class.

For example (using Control Change instead of notes so I don't have to worry about note on/off):

#include <Control_Surface.h>

template <uint8_t NumSensors>
class NoteSensitiveKeys : public MIDIOutputElement {
  public:
    NoteSensitiveKeys(const Array<AH::FilteredAnalog<7>, NumSensors> &sensors,
                      const MIDIAddress &baseAddress,
                      const RelativeMIDIAddress &incrementAddress)
        : sensors{sensors}, baseAddress(baseAddress),
          incrementAddress(incrementAddress) {}

    void begin() override {}
    void update() override {
      MIDIAddress address = baseAddress;
      for (auto &sensor : sensors) {
        if (sensor.update())
          Control_Surface.sendCC(address, sensor.getValue());
        address += incrementAddress;
      }
    }

  private:
    Array<AH::FilteredAnalog<7>, NumSensors> sensors;
      const MIDIAddress baseAddress;
    const RelativeMIDIAddress incrementAddress;
};

USBDebugMIDI_Interface midi;

NoteSensitiveKeys<4> keys = {
  {A0, A1, A2, A3},  // sensors
  {0x10, CHANNEL_1}, // base address
  {0, 1},            // increment address
};

void setup() {
  Control_Surface.begin();
}

void loop() {
  Control_Surface.loop();
}

tttapa avatar Oct 19 '20 14:10 tttapa

Indeed, I was not so clear in my question (my bad!). But you could respond part of it already.

I'm working with some multiplexers CD74HC4067 and I need to convert the list of pins of the multiplexers to an array of FilteredAnalog.

I've tried this approach:

#include <Arduino_Helpers.h>
#include <AH/Hardware/ExtendedInputOutput/AnalogMultiplex.hpp>
#include <AH/Hardware/FilteredAnalog.hpp>
#include <AH/STL/iterator> // std::back_inserter
#include <AH/STL/vector>   // std::vector

auto filteredAnalogs = [] {
  auto pins = mux.pins();
  std::vector<FilteredAnalog<>> v;
  v.reserve(pins.length);
  std::copy(std::begin(pins), std::end(pins), std::back_inserter(v));
  return v;
}(); // This is an immediately invoked lambda expression

from here: https://tttapa.github.io/Control-Surface-doc/Doxygen/d2/d96/3_8FilteredAnalogReadSerial_8ino-example.html But, I'm getting the following compilation error:

In file included from .pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/stl_iterator.h:65:0,
                 from .pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/iterator:63,
                 from .pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/iterator:8,
                 from .pio\libdeps\megaatmega2560\Control Surface\src/AH/Containers/Array.hpp:10,
                 from .pio\libdeps\megaatmega2560\Control Surface\src/AH/Hardware/Hardware-Types.hpp:8,
                 from .pio\libdeps\megaatmega2560\Control Surface\src/AH/Hardware/ExtendedInputOutput/ExtendedInputOutput.hpp:14,
                 from .pio\libdeps\megaatmega2560\Control Surface\src/AH/Hardware/ExtendedInputOutput/AnalogMultiplex.hpp:8,
                 from src\main.cpp:2:
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/../bits/move.h: In instantiation of 'constexpr _Tp* std::__addressof(_Tp&) [with _Tp = std::vector<AH::FilteredAnalog<> >]':
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/stl_iterator.h:468:35:   required from 'std::back_insert_iterator<_Container>::back_insert_iterator(_Container&) [with _Container = std::vector<AH::FilteredAnalog<> >]'
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/stl_iterator.h:534:50:   required from 'std::back_insert_iterator<_Container> std::back_inserter(_Container&) [with _Container = std::vector<AH::FilteredAnalog<> >]'
src\main.cpp:73:67:   required from here
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/../bits/move.h:48:33: error: '__builtin_addressof' was not declared in this scope
     { return __builtin_addressof(__r); }
                                 ^
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/../bits/move.h:48:40: error: body of constexpr function 'constexpr _Tp* std::__addressof(_Tp&) [with _Tp = std::vector<AH::FilteredAnalog<> >]'
not a return-statement
     { return __builtin_addressof(__r); }
                                        ^
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/../bits/move.h: In instantiation of 'constexpr _Tp* std::__addressof(_Tp&) [with _Tp = AH::FilteredAnalog<>]':
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/stl_uninitialized.h:83:35:   required from 'static _ForwardIterator std::__uninitialized_copy<_TrivialValueTypes>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::move_iterator<AH::FilteredAnalog<>*>; _ForwardIterator = AH::FilteredAnalog<>*; bool _TrivialValueTypes = false]'
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/stl_uninitialized.h:134:15:   required from '_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::move_iterator<AH::FilteredAnalog<>*>; _ForwardIterator = AH::FilteredAnalog<>*]'
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/stl_uninitialized.h:289:37:   required from '_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = std::move_iterator<AH::FilteredAnalog<>*>; _ForwardIterator = AH::FilteredAnalog<>*; _Tp = AH::FilteredAnalog<>]'
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/stl_vector.h:1269:35:   required from 'std::vector<_Tp, _Alloc>::pointer std::vector<_Tp, _Alloc>::_M_allocate_and_copy(std::vector<_Tp, _Alloc>::size_type, _ForwardIterator, _ForwardIterator) [with _ForwardIterator = std::move_iterator<AH::FilteredAnalog<>*>; _Tp = AH::FilteredAnalog<>; _Alloc = std::allocator<AH::FilteredAnalog<> >; std::vector<_Tp, _Alloc>::pointer = AH::FilteredAnalog<>*; std::vector<_Tp, _Alloc>::size_type = unsigned int]'
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/vector.tcc:73:40:   required from 'void std::vector<_Tp, _Alloc>::reserve(std::vector<_Tp, _Alloc>::size_type) [with _Tp = AH::FilteredAnalog<>; _Alloc = std::allocator<AH::FilteredAnalog<> >; std::vector<_Tp, _Alloc>::size_type = unsigned int]'
src\main.cpp:72:24:   required from here
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/../bits/move.h:48:33: error: '__builtin_addressof' was not declared in this scope
     { return __builtin_addressof(__r); }
                                 ^
.pio\libdeps\megaatmega2560\Control Surface\src/AH/STL/Fallback/bits/../bits/move.h:48:40: error: body of constexpr function 'constexpr _Tp* std::__addressof(_Tp&) [with _Tp = AH::FilteredAnalog<>]' not a return-statement
     { return __builtin_addressof(__r); }
                                        ^
*** [.pio\build\megaatmega2560\src\main.cpp.o] Error 1
============================================================================================ [FAILED] Took 2.77 seconds ============================================================================================

Environment     Status    Duration
--------------  --------  ------------
megaatmega2560  FAILED    00:00:02.770

So, how could I convert multiple multiplexers (and respective pins) to be used with this class NoteSensitiveKeys?

frederico-betting avatar Oct 19 '20 15:10 frederico-betting

You get this error because you're using an old compiler. Please upgrade the Arduino AVR Core to at least version 1.8.1. I've never really used PlatformIO, so I can't help you with that.

tttapa avatar Oct 19 '20 16:10 tttapa

By the way, filteredAnalogs is not an array of FilteredAnalog, it's an std::vector<FilteredAnalog>. You cannot easily create an array of FilteredAnalog because it's not default constructible. You could add a default constructor if you want, this is what Button does, for example: https://github.com/tttapa/Control-Surface/blob/21a09383bb3eaa5f1835184a63fdcdb4b8f5799b/src/AH/Hardware/Button.hpp#L18-L27

Either way, you cannot pass a vector to a function that expects an Array and vice versa, you have to pick one type of container and use that throughout your code.

tttapa avatar Oct 19 '20 16:10 tttapa

You get this error because you're using an old compiler. Please upgrade the Arduino AVR Core to at least version 1.8.1. I've never really used PlatformIO, so I can't help you with that.

I've got the anwswer for this issue. It builds now! (Just registering here in case someone uses PlatformIO and and needs to update the compiler version.)

To update the compiler version just add the configuration platform_packages with the proper version in the file platformio.ini. By default, I was using toolchain-atmelavr 1.50400.190710 (5.4.0). I updated to toolchain-atmelavr 1.70300.191015 (7.3.0) and the code built succesfully.

Using it in platformio.ini:

[env:megaatmega2560]
platform = atmelavr
board = megaatmega2560
framework = arduino
lib_deps = 
	tttapa/Control Surface@^1.2.0-4
	paulstoffregen/Encoder@^1.4.1
platform_packages = 
	toolchain-atmelavr@~1.70300.191015

More details about this configuration can be found here: https://docs.platformio.org/en/latest/projectconf/section_env_platform.html#platform-packages

frederico-betting avatar Oct 19 '20 20:10 frederico-betting

Thanks for sharing your solution!

tttapa avatar Oct 19 '20 20:10 tttapa

By the way, filteredAnalogs is not an array of FilteredAnalog, it's an std::vector<FilteredAnalog>. You cannot easily create an array of FilteredAnalog because it's not default constructible. You could add a default constructor if you want, this is what Button does, for example:

https://github.com/tttapa/Control-Surface/blob/21a09383bb3eaa5f1835184a63fdcdb4b8f5799b/src/AH/Hardware/Button.hpp#L18-L27

Either way, you cannot pass a vector to a function that expects an Array and vice versa, you have to pick one type of container and use that throughout your code.

I implemented this approach

class FilteredAnalog
    : public GenericFilteredAnalog<AnalogType (*)(AnalogType), Precision,
                                   FilterShiftFactor, FilterType, AnalogType,
                                   IncRes> {
  public:

    /** 
     * @brief   Construct a new Button object. 
     *  
     * **This constructor should not be used.**   
     * It is just a way to easily create arrays of buttons, and initializing 
     * them later. 
     */ 
    FilteredAnalog()
        : GenericFilteredAnalog<AnalogType (*)(AnalogType), Precision,
                                    FilterShiftFactor, FilterType, AnalogType,
                                    IncRes>(NO_PIN, nullptr, 0) {}

   (...)

and it seems to be working fine. I just need to make some changes in my initial code in order to handle multiple pins in the loop,

But I have questions... Is there any reason you didn't implement the constructor with no arguments for FilteredAnalog? Is there any concern on this approach?

frederico-betting avatar Oct 19 '20 20:10 frederico-betting

   FilteredAnalog()
       : GenericFilteredAnalog<AnalogType (*)(AnalogType), Precision,
                                   FilterShiftFactor, FilterType, AnalogType,
                                   IncRes>(NO_PIN, nullptr, 0) {}

You can do this without repetitions by using a delegating constructor:

    FilteredAnalog() : FilteredAnalog(NO_PIN) {}

But I have questions... Is there any reason you didn't implement the constructor with no arguments for FilteredAnalog? Is there any concern on this approach?

Because there was no need for it, and because it would be confusing: creating a FilteredAnalog object without specifying a pin is pretty useless in most cases. (The same goes for Buttons, but I did need arrays of buttons, so I added a default constructor.)
As long as you remember to initialize the FilteredAnalog later, there's no problem. If you do try to use it without initializing it to a valid pin number, you'll get a fatal runtime error.

tttapa avatar Oct 19 '20 22:10 tttapa

I updated the constructor. Makes sense. Tks

I got your point. In this case, I'll just fork Control-Surface, make my changes and reference the forked repo in my project.

frederico-betting avatar Oct 20 '20 13:10 frederico-betting

I'll add a default constructor later today, it's not too big of a change, and having arrays of FilteredAnalog could be useful.

tttapa avatar Oct 20 '20 13:10 tttapa

Nice. If you want I can open a pull request of it.

frederico-betting avatar Oct 20 '20 13:10 frederico-betting

The Arduino Helpers library is a separate repository, so I'll have to sync these libraries myself.

tttapa avatar Oct 20 '20 14:10 tttapa

See f440780dae168f96eb8111c3441bf81763999c9d. (Not yet merged into master.)

tttapa avatar Oct 20 '20 23:10 tttapa

Sounds really great! Thanks for working on this. Do you have any expectation when you will merge it?

frederico-betting avatar Oct 21 '20 12:10 frederico-betting

Merged.

tttapa avatar Oct 21 '20 14:10 tttapa