How do I create an array of templated class as members of another class

[cmisip]

How do I iterate over rows and columns array to create Output objects in the class MyClass that I can refer to in pin array index? The pin array of Output objects must be MyClass members so I can use them in any MyClass method.

#include<iostream>

using namespace std;

template <uint8_t pin>
class Output {
 public:
 uint8_t Pin;
   Output():Pin(pin){};
   void print(){
       cout <<"Pin " <<(unsigned) Pin << endl;
   }
  
};


template< const uint8_t* row, const uint8_t* col>
class MyClass  {
  public:
  
  
  
  //How do I create an array of Output objects below as members of MyClass
  //Like:
  //Output pin[3];
  //
  //for (int i ; i< numrows ; i++) {
  //   pin[i] = Output<*row>;
  //   row++;
  //} 


  //This works but I want to use an array of Output objects especially since 
  //numcols and numrows will change for each object of MyClass
  Output<*row> pin1;
  Output<*(row+1)> pin2;

  MyClass(int numcols, int numrows){};
  void print(){
      pin1.print();
      pin2.print();
  }
  
};

constexpr uint8_t rowPins[3] = {5,6,7};   
constexpr uint8_t colPins[4] = {14,15,16,17}; 

int main()
{
    cout<<"Welcome to Online IDE!! Happy Coding :)" << endl;
    uint8_t x=5;
    
    MyClass<rowPins,colPins> object1(4,3);
    object1.print();
    
    return 0;
}

Thanks,
Chris

[mbozzi]

It's not possible, but please share what you hoped to accomplish with an answer.

A class template is a program which the compiler uses to write classes for you. They're instantiated by the compiler, at compile-time, to write the source code of actual classes. The resulting classes are then compiled into the executable.

Class templates are not types. It's impossible to create an array with element type Output because a class template like Output is not a type at all, but a program which produces them. Further, there is no relationship between specializations of the same class template. For example Output<a> and Output<b> are two unrelated classes with different names (assuming a != b).

Class templates can't depend on data that isn't known to the compiler. It's impossible to instantiate Output<*row> unless the compiler knows the value of the expression *row. For example the following program will not compile because the compiler must know the value of x when writing the class type Output<x>:

#include <iostream>
template <int X> struct Output {};
int main() {
  int x; 
  std::cin >> x;
  Output<x> o; // wrong: x is not usable in a constant expression
}

[helios]

It's possible to have a BaseOutput class from which all Output<pin> derive, but if you're going that route you may as well just skip the template:

class Output {
public:
    uint8_t Pin;
    Output() = default;
    Output(uint8_t pin): Pin(pin){}
    Output(const Output &) = default;
    Output &operator=(const Output &) = default;
    void print(){
        cout <<"Pin " <<(unsigned) Pin << endl;
    }
};

class MyClass{
public:
    Output pin[3];
    MyClass(const uint8_t *row, int numcols, int numrows){
        for (int i = 0; i < 3; i++)
            this->pin[i] = Output(row[i]);
        //...
    }
    void print(){
        for (auto &p : this->in)
            p.print();
    }
};

What you did there is pretty bad use of templates.

[cmisip]

I don't have control over Output. It's a library that I wish to use in my arduino code.

Here is the link to the class Output that is a template class:

https://github.com/mmarchetti/DirectIO/blob/master/DirectIO.h

It's on line 51.

Here is the relevant code:

template <u8 pin>
class Output {
    // An digital output with direct port I/O
    public:
        Output(boolean initial_value=LOW) {
            pinMode(pin, OUTPUT);

            // include a call to digitalWrite here which will
            // turn off PWM on this pin, if needed
            digitalWrite(pin, initial_value);
        }
        void write(boolean value) {
            _pins<pin>::output_write(value);
        }
        Output& operator =(boolean value) {
            write(value);
            return *this;
        }
        void toggle() {
            write(! read());
        }
        void pulse(boolean value=HIGH) {
            write(value);
            write(! value);
        }
        boolean read() {
            return _pins<pin>::output_read();
        }
        operator boolean() {
            return read();
        }
};

Basically, u8 pin needs to be a compile time constant. However, I am working with another class from another library to which I would like to integrate the features of DirectIO. That class is Keypad.

https://github.com/Chris--A/Keypad/blob/master/src/Keypad.h

I derived a class from Keypad called MyKeypad and I want to override the virtual functions on lines 81 and 82.

virtual void pin_write(byte pinNum, boolean level) { digitalWrite(pinNum, level); }
	virtual int  pin_read(byte pinNum) { return digitalRead(pinNum); }

So digitalWrite can be replaced by :

if (pinNum == 5)
  pin5=level;

However, for that to work, the class MyKeypad object must have this member:

Output<5> pin5;

I can just type each Output<u8> object as a member of MyKeypad. However, it would be a more elegant solution if MyKeypad could have a way of creating some sort of indexed array of Output objects based on any number of pins.

I would appreciate any solutions you can offer.

Thanks,
Chris

[helios]

In that case,

class DerivedOutput;

class DynamicOutput{
public:
    virtual ~DynamicOutput(){}
    virtual uint8_t get_pin() = 0 const;
    template <uint8_t N>
    static std::unique_ptr<DerivedOutput> create();
};

template <uint8_t pin>
class DerivedOutput : public DynamicOutput{
    Output<pin> out;
public:
    DerivedOutput(boolean v = LOW): out(v){}
    void write(boolean value){
        this->out.write(value);
    }
    DerivedOutput &operator=(boolean value){
        this->out = value;
        return *this;
    }
    void toggle(){
        this->out.toggle();
    }
    void pulse(boolean value = HIGH) {
        this->out.pulse(value);
    }
    boolean read(){
        return this->out.read();
    }
    operator boolean(){
        return (boolean)this->out;
    }
    uint8_t get_pin() override{
        return pin;
    }
};

template <uint8_t N>
std::unique_ptr<DerivedOutput> DynamicOutput::create(){
    return std::make_unique<DerivedOutput<pin>>();
}

//...

std::vector<std::unique_ptr<DerivedOutput>> v;
v.push_back(DynamicOutput::create<0>());
v.push_back(DynamicOutput::create<1>());
v.push_back(DynamicOutput::create<2>());

It's also possible to get the pin value from a run-time value. E.g.

for (int i = 0; i < 3; i++)
    v.push_back(DynamicOutput::create_from_variable(i));

You'd have to use template metaprogramming so it's lengthy and annoying, thus I left it out. Let me know if you're interested in doing this and I'll post the solution.

[cmisip]

Let me see if I understand this correctly.
1. Line 13 creates the Output object with uint8_t pin. So this would have called the Output class constructor and initialized the pin to LOW.

The constructor of DerivedOutput on line 15 , with "out(v)", does an assignment of LOW to the pin or "out=LOW". Why is it necessary to do this twice or I am misunderstanding?

2. Line 33 . Shouldn't this be "return (boolean)this->read();"

3. Line 35. What is "get_pin()" for?

Thanks,
Chris

[helios]

The constructor of DerivedOutput on line 15 , with "out(v)", does an assignment of LOW to the pin or "out=LOW". Why is it necessary to do this twice or I am misunderstanding?

The DerivedOutput constructor takes an optional boolean value. Like the Output constructor, the caller has the option to omit the parameter, in which case the value defaults to LOW. The point of doing it like this is that Output<n> pin/*whatever*/; translates directly to DerivedOutput<n> pin/*whatever*/;.

2. Line 33 . Shouldn't this be "return (boolean)this->read();"

I specifically wanted the wrapper to call the analogous function in Output. While it's true that Output::operator boolean() is implemented as you say, I didn't want the wrapper to concern itself with that implementation detail. How to implement operator boolean() is Output's problem.

3. Line 35. What is "get_pin()" for?

Nothing really. It's just a simple example of how to implement a polymorphic function in this class hierarchy, whose behavior depends on the value of pin.

[cmisip]

Line 41 seems to require <N>, otherwise won't compile
Line 42 should pin be N?

Line 47 vector won't compile also

error: '__alloc_traits' in namespace '__gnu_cxx' does not name a template type
       typedef typename __gnu_cxx::__alloc_traits<_Alloc>::template

I might be going down the rabbit hole here. Increasing code complexity and use of vectors might just offset the gain that I get from using Direct IO. Hmmm

Thanks,
Chris

[mbozzi]

I might be going down the rabbit hole here. Increasing code complexity and use of vectors might just offset the gain that I get from using Direct IO.

The state of the output pins can be controlled by writing a byte of memory to a particular memory address.

This process was too simple for the authors of DirectIO, so they programmed a template library to make this profoundly trivial operation slower, more complicated, and less flexible. Woe to the user, who must rely on memory allocation, runtime polymorphism, metacode, and type erasure in order to simply write a byte of memory, slowly, one single bit at a time.

There is no need for dynamic memory, type erasure, metaprogramming, or polymorphism to toggle pins.

Just do it by hand.

[cmisip]

This is my solution, unless anyone can propose a better one:

#include <DirectIO.h>
#include <Keypad.h>

template<  byte* row, int NUMROWS,  byte* col, int NUMCOLS>
class MyKeypad : public Keypad {
  public:

  Output<14> pin14;
  Output<15> pin15;
  Output<16> pin16;
  Output<17> pin17;

  Output<59> pin59;
  Output<60> pin60;
  Output<61> pin61;
  
   MyKeypad(char *buttons  ): Keypad( makeKeymap(buttons), row, col, NUMROWS, NUMCOLS){
   };

   virtual void pin_write(byte pinNum, boolean level)  //14,15,16,17
   { 
       //digitalWrite(pinNum, level);
       switch (pinNum) {
        case 14:
          pin14=level;
          break;
        case 15:
          pin15=level;
          break;
        case 16:
          pin16=level;
          break;
        case 17:
          pin17=level;
          break;
       }
    }

   virtual int  pin_read(byte pinNum)  //59 60 61
   { 
     switch (pinNum) {
        case 59:
          return pin59.read();
          break;
        case 60:
          return pin60.read();
          break;
        case 61:
          return pin61.read();
          break;
     }
   }

   virtual void pin_mode(byte pinNum, byte mode) { 
    //Serial.println(mode);
    pinMode(pinNum, mode); 
    }
    
      
   
};


//MATRIX
//A5 A6 A7 
//14 15 16 17

#define NUMROWS 3
#define NUMCOLS 4

const uint8_t buttons[NUMROWS][NUMCOLS] = {
  {'A', 'B', 'C', 'D'},    
  {'E', 'F', 'G', 'H'},    
  {'I', 'J', 'K', 'L'}   
};

  byte rowPins[NUMROWS] = {A5,A6,A7};  
  byte colPins[NUMCOLS] = {14,15,16,17}; 

 MyKeypad<rowPins, NUMROWS, colPins, NUMCOLS> buttbx( makeKeymap(buttons));

 void activate_keypad() {
  //buttbx.setHoldTime(500);               
  buttbx.setDebounceTime(100);
}

As you say, its by hand and hardcoded to the set of pins. It works though.
Didn't do pin_mode because its pretty convoluted: three modes possible for each of the 7 pins.

I think I know enough of the mechanics to be able to write a simpler Pin class. I will take a stab at it.

Chris

[cmisip]

I created this Pin class:

class Pin {
public:
  uint8_t pin;
  uint8_t bit;
  uint8_t port;
  volatile uint8_t *dir, *out, *in;
  uint8_t id;
  uint8_t mode; //0=INPUT, 1=OUTPUT, 2=INPUT_PULLUP
  
  static int idcounter;

  Pin(){};
  Pin(uint8_t ipin, uint8_t imode):pin(ipin),mode(imode){
       bit = digitalPinToBitMask(pin);
       port = digitalPinToPort(pin);
       dir = portModeRegister(port);
       out = portOutputRegister(port);
       in = portInputRegister(port);
       id=idcounter;
       idcounter++;

       pinMode(pin,mode);

       if (mode == OUTPUT) {
            // include a call to digitalWrite here which will
            // turn off PWM on this pin, if needed
            digitalWrite(pin, 0);
       }     
    };

   void write(boolean val){ //writing to input pin causes it to become pullup
      if (mode == OUTPUT) {
         if (val == LOW) {
            *out &= ~bit;
         } else {
            *out |= bit;
         }
      }
   }   

   boolean read(){       
      if (mode == OUTPUT)  {     //reading while in output mode, just read the out register
          return 1 == (( *out & bit )> 0);
      }    
      else {                     //reading while in input mode, read from the in register
          return 1 == ((*in & bit ) > 0);      
      }     
   }

   void toggle(){
      if (mode == OUTPUT)  { 
          write(! read());
      }
   }

   void pin_mode(uint8_t imode) { //pinMode without the safeties
       if (imode==INPUT) {
          *dir &= ~bit;  //set dir to 0
          *out &= ~bit;  //set out to 0
       }
       if (imode == INPUT_PULLUP) {//need to write 1 to out register
          *dir &= ~bit; //set dir to 0
          *out |= bit;  //set out to 1
       }
       if (imode == OUTPUT) {
          *dir |= bit;  //set dir to 1
       }
        mode=imode; 
   } 
};

int Pin::idcounter=0;

And this is my implementation of MyKeypad:

template<  byte* row, int NUMROWS,  byte* col, int NUMCOLS>
class MyKeypad : public Keypad {
  public:

  Pin* ColPin;
  Pin* RowPin;
  
   MyKeypad(char *buttons  ): Keypad( makeKeymap(buttons), row, col, NUMROWS, NUMCOLS){
     ColPin=new Pin[NUMCOLS];
     for ( int i=0; i< NUMCOLS ; i++) { //FINALLY!!!!
        ColPin[i]=Pin(col[i],OUTPUT);
     }
     RowPin=new Pin[NUMROWS];
     for ( int i=0; i< NUMROWS ; i++) {
        RowPin[i]=Pin(row[i],INPUT_PULLUP);
     }
   };

   ~MyKeypad(){    
        delete [] ColPin;
        delete [] RowPin;
   }
    
   void pin_mode_row(byte pinNum, byte mode) { 
    RowPin[pinNum].pin_mode(mode);
    }

   void pin_mode_col(byte pinNum, byte mode) { 
    ColPin[pinNum].pin_mode(mode);
   } 

   void pin_write_col(byte pinNum, boolean level){
    ColPin[pinNum].write(level);   
   }

   int pin_read_row(byte pinNum){
     return RowPin[pinNum].read();
   }

  virtual void scanKeys() {
  // Re-intialize the row pins. Allows sharing these pins with other hardware.
  for (byte r=0; r<sizeKpd.rows; r++) {
    pin_mode_row(r,INPUT_PULLUP);
  }

  // bitMap stores ALL the keys that are being pressed.
  for (byte c=0; c<sizeKpd.columns; c++) {
    pin_mode_col(c,OUTPUT);
    pin_write_col(c, LOW);  // Begin column pulse output.
    for (byte r=0; r<sizeKpd.rows; r++) {
      bitWrite(bitMap[r], c, !pin_read_row(r));  // keypress is active low so invert to high.
    }
    // Set pin to high impedance input. Effectively ends column pulse.
    pin_write_col(c,HIGH);
    pin_mode_col(c,INPUT);
  }
}
      
   
};

I had to cheat though. I changed Keypad.h to make scanKeys public and also changed it to virtual. I was hoping i could find a solution that allowed me to override Keypad::scanKeys() but it is private and can't be inherited.

For some reason, MyKeypad version of scanKeys() wasn't being used so I changed Keypad version to virtual.

I am using pointers here so hopefully there are no memory leaks.

Is there a faster way of doing things?

Is there a practical solution that does not involve making changes to the base class?

Does the use of pointers slow this down?

Thanks,
Chris

[cmisip]

I did some benchmarking. The values are in microseconds showing several iterations.

DirectIO class:

ORIG -> 2960
DIO -> 704
ORIG -> 2952
DIO -> 704
ORIG -> 2960
DIO -> 704
ORIG -> 2960
DIO -> 704
ORIG -> 2960
DIO -> 700
ORIG -> 2960

Pin.h with pointers

ORIG -> 2940
DIO -> 1332
ORIG -> 2944
DIO -> 1332
ORIG -> 2936
DIO -> 1332
ORIG -> 2944
DIO -> 1340
ORIG -> 2940
DIO -> 1332
ORIG -> 2944

Pin.h with no pointers


ORIG -> 2936
DIO -> 1280
ORIG -> 2940
DIO -> 1284
ORIG -> 2944
DIO -> 1276
ORIG -> 2944
DIO -> 1280
ORIG -> 2944
DIO -> 1276

So it seems DirectIO with its compile time constants is really faster than Pin.h.

I also found out that Pin.h performance is better if I don't use pointers.

Is there anything I can do to improve Pin.h's performance?

Thanks,
Chris

[mbozzi]

So it seems DirectIO with its compile time constants is really faster than Pin.h.

This should be unsurprising because the definition of Pin instructs the computer to do some amount of extra work.

The problem with these pin classes (DirectIO included) is that they require you to make N register accesses at minimum, not counting any indirections, calls to virtual functions, or other nonessential work that cannot be optimized. This scheme may also cause timing problems because each pin's state is sampled over a long period of time. The compiler optimizer is of limited utility because register accesses are optimization barriers.

Under the right assumptions, for example, that
- The keypad's data lines are connected to some PORTx as specified in the hardware schematic;
- The port is configured properly;
- The keypad has 16 buttons, etc.
A single access to PORTx would report the keypad state. Therefore ideally you could have a line of code that looks somewhat like
uint8_t const keypad_state = PORTx;
In isolation, you'll never do better than this.

Overly-general class types like Pin and Keypad do not exploit every useful assumption and their performance suffers as a result. DirectIO at least hard codes the register names and bit masks, but still forces you to work with one pin at a time.

You only have one keypad, right? So get rid of MyKeypad, copy the old Keypad into your project, and rewrite it in terms of your specific hardware. This way the machine doesn't need to compute stuff that is already known when the code is compiled.

There's certainly no need for virtual functions unless there's something you don't know about until run-time.

Avoid dynamic memory allocation, strongly prefer fixed-size arrays instead.

Instead of
return 1 == (( *out & bit )> 0)
just
return *out & bit
will suffice, assuming boolean is an alias for bool or _Bool but not some integer type.

[cmisip]

I think I understand now why the author of DirectIO hardcoded the registers in :

https://github.com/mmarchetti/DirectIO/blob/master/include/ports_avr.h

They needed to be available as compile time constants. In Pin.h I had to figure them out using the arduino functions at runtime:

dir = portModeRegister(port);
       out = portOutputRegister(port);
       in = portInputRegister(port);

Is there anyway that DirectIO's Output class can be recoded to allow some form of iteration of Output objects while preserving its speed by retaining compile time constants?

I know that the template structure made each Output class different from other Output classes because of the unique pin number.

What do you think about a pointer of Output<1>** objects for example, and then using reinterpret_cast to cheat, making each Output object with its specific pin and then copying the address over to each consecutive Output<1>**+ memory space?

Chris

[mbozzi]

Is there anyway that DirectIO's Output class can be recoded to allow some form of iteration of Output objects while preserving its speed by retaining compile time constants?

A metaprogram can be constructed to allow iteration. Something along the lines of the following:

template <u8 Pin> struct output { static constexpr u8 pin = Pin; };
template <u8 N> struct u8_c : std::integral_constant<u8, N> {};
template <u8 N> inline constexpr u8_c<N> u8_v;

template <u8 B, u8... Os> 
  constexpr auto make_output_tuple_impl(u8_c<B>, std::integer_sequence<u8, Os...>)
  { return std::make_tuple(output<B + Os>()...); }
  
template <u8 L, u8 U> 
  constexpr auto make_output_tuple(u8_c<L>, u8_c<U>) 
  { return make_output_tuple_impl(u8_v<L>, std::make_integer_sequence<u8, U - L>()); }

template <typename F, typename... Ts>
  constexpr auto for_each_parameter(F f, Ts&&... ts) 
  { return (((void)(f(std::forward<Ts>(ts)))), ...), f; }
  
template <typename F, typename Tuple>
  constexpr auto for_each_tuple_element(F f, Tuple&& t)
  { 
    return std::apply([f_(std::move(f))](auto&&... elts) mutable {
      return for_each_parameter(f_, std::forward<decltype(elts)>(elts)...); }, t); 
  }

inline constexpr auto print_pin = [](auto p) { std::printf("%d\n", static_cast<int>(p.pin)); };

int main()
{
  auto pins_4_thru_9 = make_output_tuple(u8_v<4>, u8_v<10>);
  for_each_tuple_element(print_pin, pins_4_thru_9);
}

https://godbolt.org/z/51rKx7b8c

But I already told you that this approach will not improve performance. All this does is write code for you, it cannot be substantially better than the hand-coded version because the code it writes is subject to the same problems I outlined in my prior post. Like all templates it is merely shorthand.
http://cplusplus.com/forum/general/278799/#msg1204592

You have to reduce the amount of redundant work that your software is doing. You can do this by combining writes and reads of pins within the same port.

What do you think about a pointer of Output<1>** objects for example, and then using reinterpret_cast to cheat, making each Output object with its specific pin and then copying the address over to each consecutive Output<1>**+ memory space?

It doesn't solve the problem because there is no way to go from a runtime loop index to a type that depends on the value of that index.

[cmisip]

Thanks a lot for all the pointers. I have to admit that at this time template metaprogramming is really just above my level. It is interesting though and thank you for the example. I do agree that hardcoding to the specific circumstance of the hardware really would provide the best performance. I just wanted code that I could reuse in other keypad configurations in future projects. MyKeypad offers flexibility without investing too much time in coding.

Writing to multiple pins of the same port is definitely an optimization avenue. I will include that in planning my wiring for future projects so that i use GPIO pins preferably connected to the same port. In the case of keypad, though, the output GPIO pins need to be fired in sequence but the input pins can be read all at once if they are in the same port. This is because, the key pressed is a combination of which output pin is active and which input pin detected the voltage. Anyway, I would have to decide if it would be a worthwhile venture to make further optimizations to this specific arduino box configuration. Output<pin> class has been used in writing to 8 7 segment 6 digit LED displays (only needs two pins per display and that made it easier to code using Output<pin> class) in the same arduino box and that has significantly increased the arduino box's button and encoder responsiveness.

https://happyscrollsflightsimconnector.godaddysites.com/

Here is the project I was working on. It's on hiatus right now as my interest seems to have jumped elsewhere as it usually does. However, the arduino box is in a working state and I think the software is pretty stable.

Thanks again for everyone's time and expertise.
Chris