0

I am creating an Arduino library for use in a Sketch. It uses the Encoder Library, but when compiling I get an obtuse error:

/var/folders/jy/f8dvlhcd4vdcvtl49bk8ytwc0000gn/T/builda847c0675e0bee2f5f05581e35ae65fe.tmp/sketch/MIDIEncoder.cpp: In constructor 'MIDIEncoder::MIDIEncoder(uint8_t, uint8_t, byte, byte)': MIDIEncoder.cpp:8: error: no matching function for call to 'Encoder::Encoder()' MIDIEncoder::MIDIEncoder(uint8_t pinA, uint8_t pinB, byte midiChannel, byte midiCCNumber) ^ /var/folders/jy/f8dvlhcd4vdcvtl49bk8ytwc0000gn/T/builda847c0675e0bee2f5f05581e35ae65fe.tmp/sketch/MIDIEncoder.cpp:8:89: note: candidates are: In file included from /var/folders/jy/f8dvlhcd4vdcvtl49bk8ytwc0000gn/T/builda847c0675e0bee2f5f05581e35ae65fe.tmp/sketch/MIDIEncoder.h:17:0, from /var/folders/jy/f8dvlhcd4vdcvtl49bk8ytwc0000gn/T/builda847c0675e0bee2f5f05581e35ae65fe.tmp/sketch/MIDIEncoder.cpp:2: /Applications/Arduino.app/Contents/Java/hardware/teensy/avr/libraries/Encoder/Encoder.h:72:2: note: Encoder::Encoder(uint8_t, uint8_t) Encoder(uint8_t pin1, uint8_t pin2) { ^ /Applications/Arduino.app/Contents/Java/hardware/teensy/avr/libraries/Encoder/Encoder.h:72:2: note: candidate expects 2 arguments, 0 provided /Applications/Arduino.app/Contents/Java/hardware/teensy/avr/libraries/Encoder/Encoder.h:69:7: note: constexpr Encoder::Encoder(const Encoder&) class Encoder ^ /Applications/Arduino.app/Contents/Java/hardware/teensy/avr/libraries/Encoder/Encoder.h:69:7: note: candidate expects 1 argument, 0 provided /Applications/Arduino.app/Contents/Java/hardware/teensy/avr/libraries/Encoder/Encoder.h:69:7: note: constexpr Encoder::Encoder(Encoder&&) /Applications/Arduino.app/Contents/Java/hardware/teensy/avr/libraries/Encoder/Encoder.h:69:7: note: candidate expects 1 argument, 0 provided no matching function for call to 'Encoder::Encoder()'

Fundamentally, I believe the compiler is telling me it can't find a constructor for the Encoder library MIDIEncoder.cpp:8: error: no matching function for call to 'Encoder::Encoder()', but I'm stumped as to why. Below is the full source for my library.

MIDIEncoder.h

/*
  MIDIEncoder.h
  A library for creating relative MIDI CC messages from a rotary encoder.

  Created by Paul Williamson, 11 August 2016.

  Project source available at:
  http://github.com/squarefrog/teensy-midi-encoder-box

  Released into the public domain.
*/

#ifndef MIDIEncoder_h
#define MIDIEncoder_h

#include "Arduino.h"
#include <Encoder.h>

class MIDIEncoder
{
  public:    
  MIDIEncoder(uint8_t pin1, uint8_t pin2, byte midiChannel, byte midiCCNumber);
    byte channel;
    byte ccNumber;

    byte read();

  private:
    unsigned long _lastTurnedTime;
    long _oldPosition;
    Encoder _enc;
};

#endif

MIDIEncoder.cpp

#include "MIDIEncoder.h"
#include <Encoder.h>

const byte incrementValue = 66; // A constant for the start of increment values
const byte decrementValue = 2;  // A constant for the start of decrement values

MIDIEncoder::MIDIEncoder(uint8_t pin1, uint8_t pin2, byte midiChannel, byte midiCCNumber)
{
  channel = midiChannel;
  ccNumber = midiCCNumber;
  _enc = Encoder(pin1, pin2);
  _oldPosition = -999;
  _lastTurnedTime = millis();
}

byte MIDIEncoder::read()
{
  long newPosition = _enc.read();

  // If position hasn't changed, ignore.
  if (newPosition == _oldPosition) {
    return 0;
  }

  // If position is not divisible by 4, ignore.
  if (newPosition % 4 != 0) {
    return 0;
  }

  unsigned long delta = millis() - _lastTurnedTime;
  byte offset = 0;

  // Apply crude acceleration
  if (delta < 100) offset = 4;
  if (delta > 99 && delta < 180) offset = 2;
  if (delta > 179 && delta <= 250) offset = 1;

  _lastTurnedTime = millis();

  // Return MIDI CC value
  if (newPosition > _oldPosition) {
    return incrementValue + offset;
  } else {
    return decrementValue + offset;
  }
}

Some excellent answers below. Now that I know what to look for I found this resource which explains where I made my mistake. In particular look at number 3.

squarefrog
  • 4,750
  • 4
  • 35
  • 64

2 Answers2

2

Encoder doesn't have a default constructor, and it is implicitly call here

MIDIEncoder::MIDIEncoder(uint8_t pin1, uint8_t pin2, byte midiChannel, byte midiCCNumber) 
    /*Implicit constructor call*/ 
{ 
    //...
}

You tried to call it the the constructor body, but by then "it is too late" :) What I mean is, _enc has to be already initialized when the constructor body executes, but _enc can't be default initialized, so the compiler complains.

That's what the constructor initializer list is for:

MIDIEncoder::MIDIEncoder(uint8_t pin1, uint8_t pin2, byte midiChannel, byte midiCCNumber)
    : _enc(pin1, pin2) //Calls appropriate constructor for _enc, not default
{
    //...
}
Rakete1111
  • 47,013
  • 16
  • 123
  • 162
  • Oh snap. I thought the variable name in the header declaration was just a placeholder, for use in the implementation file. I never would have figured that out. – squarefrog Aug 11 '16 at 20:27
  • Actually, the declaration in the header doesn't call any constructor. Default constructor is implicitly called inside `MIDIEncoder` constructor, along with constructors for the other member variables (is not specified otherwise). – goldwin-es Aug 11 '16 at 20:36
  • I thought I was calling the constructor with `Encoder(pin1, pin2);`. I presume its just objects that need constructing in this manner? – squarefrog Aug 11 '16 at 20:39
  • 1
    @squarefrog No, `_enc = Encoder(pin1, pin2);` would - if your class was default-constructible and so you didn't just get a compile-time error - construct a _temporary_ `Encoder` and _move-assign_ that to the member. That's not what you want. Anything that can be initialised in the initialisation-list _should_ be, for both semantics and efficiency - and as you've just seen, some things can _only_ be set up there. – underscore_d Aug 11 '16 at 21:00
1

You're initializing your _enc object too late.

In the beginning of the MIDIEncoder constructor body _enc should already be created, so the compiler tries to instantiate it using the (missing) default constructor and fails.

Initialize it in the constructor initialization list instead:

MIDIEncoder::MIDIEncoder(uint8_t pin1, uint8_t pin2, ...) : _enc(pin1, pin2)
{

Actually, the same is true for other variables - there is no reason to initialize it inside constructor body.

goldwin-es
  • 956
  • 6
  • 11
  • Oh really? Thats what I'm used to in other languages, so it's easy to see where I've made that mistake. i was following along with the Arduino library tutorial https://www.arduino.cc/en/Hacking/LibraryTutorial which used the constructor. – squarefrog Aug 11 '16 at 20:29
  • `there is no reason to initialize it inside constructor body.` I know what you mean but will be the token pedant! One simply **can't** initialise members within the body. One can only _assign_ to them. But yes, terminology aside, I agree that "there is no reason". Anything that _can_ be initialised in the initialisation-list _should_ be, even if one could instead assign it in the body. That's just semantically correct and, in many cases, more efficient. Plus, for object's like the OP's, or `const` members, or etc.... initialisation is the **only** possible option. – underscore_d Aug 11 '16 at 21:03
  • Pedantry is welcome when learning a new language! So objects, constants, anything else obvious that's should be instantiated there? – squarefrog Aug 11 '16 at 21:10
  • 1
    @squarefrog IMO anything for which there is an appropriate constructor to call, regardless of whether said ctor is completely necessary or just more semantic/performant than assignment/method calls. Course, you might find classes that only have basic or default constructors and require setup be done by assign or init() methods, in which case your hands are tied. Or they might let you set some properties by ctor but require others in the body with methods. But again, for any property that you can set, if you can do it using a constructor, that's usually best - assuming a sane class designer ;-) – underscore_d Aug 11 '16 at 23:40