digitalWrite(_pin, _currentState); // handles the operation of the off(), on() and change() methods
Have you fixed the potential problem with the init() function being called from the constructor ?
Yes, thanks. This is all thats left in the constructor.
Led::Led(byte pin) {
// private variable _pin takes value from value passed to constructor
_pin = pin;
// init(); //NOTE: constructor runs before setup() so process all initialisation in setup routine.
}
void setup() {
Serial.begin(9600);
led1.init();
led2.init();
johnerrington:
digitalWrite(_pin, _currentState); // handles the operation of the off(), on() and change() methods
Sorry, I was not specific enough in my question. Why do it in the update() function ?
I WAS doing it in the on(), off(), & change() methods; and moved it to make update() relevant to all methods. And also to reduce duplication.
However since update() is using the change() method its becoming a circular argument.
Also with it in update() its getting called unnecessarily every time through the loop.
I've moved it back to the other methods.
Also fixed a glitch that was causing a momentary flash at the end of the cycle due to the off(); by
bool Led::update() {
if (_nTimes > 0) { // we need to flash
_timeNow = millis();
if (_timeNow - _timeWas >= _t[_currentState]) { //if _tOn expired turn led off
_nTimes--; //_nTimes is set to the number of phases ie twice the number of flashes.
if (_nTimes>0) change(); //if no further flashes are required LED should be in the off state
}
}
return (_currentState);
}
I'm returning _currentState from all my methods - but not using it in the program; is there any disadvantage to doing so?
I'm returning _currentState from all my methods - but not using it in the program; is there any disadvantage to doing so?
If you are doing nothing with the returned value then don't return it and define the function(s) as void. It is very likely that the compiler will optimise the code and produce the same machine code either way but why make it work harder than necessary ? It also means that when you look at the code in a few months you won't scratch your head wondering why you returned a value in the first place