Eventually icon indicating copy to clipboard operation
Eventually copied to clipboard

Bug in EvtContext::addListener ?

Open whaou opened this issue 6 years ago • 1 comments

Hi,

And many thanks for this nice library! I intend to use in for a project with my students.

I have two questions and one suggestion about the EvtContext::addListener method.

Question 1 :

I think that there is a bug in EvtContext::addListener. In the first for loop, I think you should use i as the index of listeners[] instead of listenerCount.

In other words, you should replace

for(int i = 0; i < listenerCount; i++) { // Try to add in empty slot
  if(listeners[listenerCount] == 0) {
    listeners[listenerCount] = lstn;
    return;
  }
}

by

for(int i = 0; i < listenerCount; i++) { // Try to add in empty slot
  if(listeners[i] == 0) {
    listeners[i] = lstn;
    return;
  }
}

Question 2 :

I'm not sure of that, but I think you should also add lstn->setupListener(); in this loop. The result would then be:

for(int i = 0; i < listenerCount; i++) { // Try to add in empty slot
  if(listeners[i] == 0) {
    listeners[i] = lstn;
    lstn->setupListener();
    return;
  }
}

Suggestion :

This method could return a pointer to the EvtListener * that it got in parameter. Thus the prototype would become EvtListener * EvtContext::addListener(EvtListener *lstn); and the method return lstn.

This would enable to make calls like:

EvtListener *pLstn;
pLstn = EvtContext::addListener(new EvtPinListener(SOME_PIN, (EvtAction)some_action));

and then use pLstn as an id to remove this event:

mgr.removeListener(pLstn);

whaou avatar Oct 04 '18 20:10 whaou

I just stumbled across the same issue when I tried adding two listeners to the same context. I ended up just copy/pasting the two files into my project and ditch the downloaded library. This is how I fixed the addListener method. It doesn't protect against too many listeners yet but

void EvtContext::addListener(EvtListener *lstn) {
  Serial.println(String("Adding listener ") + lstn->instanceNumber);

  int i;
  for(i = 0; i < listenerCount; i++) { // Try to add in empty slot
    if(listeners[i] == 0) { 
      break;
    }
  }

  listeners[i] = lstn;
  Serial.println(String("Added in slot ") + listenerCount);
  lstn->setupListener();
  listenerCount++;
}

cwienands1 avatar Jan 28 '21 04:01 cwienands1