Make the library event driven with function callbacks

[codebeat-nl]

Because it is now a more advanced product and you can use more than one, make it event driven. Hopefully you understand the idea and why this would be nice. It makes it easier to handle more objects at once and avoids a if then else mess (structorize the code).

for example:

#define MAX_ENCODERS 5
#define ENC_ID_CHANNEL1 0x30
#define ENC_ID_CHANNEL2 0x31
#define ENC_ID_CHANNEL3 0x32
#define ENC_ID_CHANNEL4 0x33
#define ENC_ID_CHANNEL5 0x34

i2cEncoderLibV2* encoders[MAX_ENCODERS];

// Just an example event handler: 
void myRotaryEncoderChangeEvent(  i2cEncoderLibV2* opSender, int8_t iValue )
{
  Serial.print( opSender->addres );
  Serial.print( "has been changed to " );
  Serial.println( iValue );

 if( opSender->addres == ENC_ID_CHANNEL3 )
 {
   // do something nice 
 }
}

void setup()
{ 
  uint8_t i = MAX_ENCODERS;
  while( i-- )
  {
    encoders[i]= new i2cEncoderLibV2(ENC_ID_CHANNEL1+i);
    encoders[i]->onEncoderChange = myRotaryEncoderChangeEvent;
  }  
}

void loop()
{
  uint8_t i = MAX_ENCODERS;
  while( i-- )
  {
     // if something has been changed, it triggers an event if assigned
     encoders[i]->update();
  }  
}

--------------------

[Fattoresaimon]

Hi, Can be a nice idea, When i will have more free time, i would definitely try. Thank you Simone

[Fattoresaimon]

Hi, I have followed your suggestion. Here the result: https://github.com/Fattoresaimon/ArduinoDuPPaLib

[codebeat-nl]

Hi, Nice, great, however, why you call the event and handlers at some points interrupts? In your code of attachInterrupt() you assign a function to an event array (it is called Events). On update, you poll the encoder and fire events if assigned. Why is it not called attachEvent() ( and detachEvent() )? An interrupt is really something else, this is confusing.

Another thing is the amount of events and the type of events. For example ENCODER_INCREMENT and ENCODER_DECREMENT could be one event type, for example onChange. Now you have to specify two eventhandlers for practically the same - the encoder 'wiper' changed. This could be something like onChange( obj, direction ), for example negative (steps) when turned left and positive (steps) when turned right.

Also the assignment of events could be more user friendly and less prone to errors (for example using the wrong constants, there is also no range check in the attachInterruptfunction!). For example each encoder provides these simple event properties:

- onChange
- onButtonClick
- onButtonDown / onButtonPress 
- onButtonUp / onButtonRelease 
- onButtonDoubleClick
- onButtonHold
- onMinMaxRange   // Do not know why you want to know this.

So you can do, to assign an event handler:

encoder.onChange = evMyEncoderChange;
encoder.onButtonClick = evMyEncoderButtonClick;

This naming convention is more clear, nicer, easier and also very similar to other languages and you don't need to call a function to assign a handler. To remove the handler, simply assign NULL.

[Fattoresaimon]

Thank you a lot for the suggestion!

there is also no range check in the attachInterrupt function!

Yeah i have noticed that huge mistake, i will fix. I think i will follow (again 😅) your suggestions!

[Fattoresaimon]

Hello, I have made an update following your suggestions. Thank you so much!