Simpler implementation?
- getting rid of the if
int Pam8803::setVolumeInHalf()
{
while (_gainSetting <= MAX_GAIN_SETTING/2) volumeUp();
while (_gainSetting >= MAX_GAIN_SETTING/2) volumeDown();
return MAX_GAIN_SETTING/2;
}
why not a generic percentage function, ?
int Pam8803::setVolumePercentage(int percentage)
{
int newGain = (MAX_GAIN_SETTING * percentage)/100;
newGain = constrain(newGain , MIN_GAIN_SETTING, MAX_GAIN_SETTING); // to prevent 125% ;)
while (_gainSetting <= newGain ) volumeUp();
while (_gainSetting >= newGain ) volumeDown();
return newGain;
}
// the setVolumeInHalf() becomes then much simpler
int Pam8803::setVolumeInHalf()
{
return setVolumePercentage(50);
}
better is to return _gainsetting; in both cases as that is the inner state representing the volume.
I've thought for a while about this variation which removes (some of) the function calls, and unneeded tests,
int Pam8803::setVolumeInHalf()
{
while (_gainSetting <= MAX_GAIN_SETTING/2)
{
setGainSetting(_volumeUpPin);
_gainSetting++;
}
while (_gainSetting >= MAX_GAIN_SETTING/2)
{
setGainSetting(_volumeDownPin);
_gainSetting--;
}
return _gainSetting;
}
but most of the time is spent in setGainSetting(_volumeDownPin); so the above would not be a significant optimization @ cost of readability.
This line is trouble delay(30*3.5); == delay(105); but the compiler will optimize that. The real trouble is delay() blocks, and for 0.105 second.
If I want to put the volume to the max it blocks 64 x 0.105 sec == 6.5 seconds!! That is way to long imho, but for now this works.
idea for release 0.2
Imagine a timer interrupt that runs in the background and takes care of setting the volume.
It has the following pseudo code. (not engineered just a concept)
void TimerISR()
{
if (_gainSetting < desiredGain)
{
digitalWrite(volumePinUp, HIGH); // This takes care of the pulse run it with pen and paper
digitalWrite(volumePinUp, LOW);
rescheduleISR(105);
return;
}
if (_gainSetting > desiredGain)
{
digitalWrite(volumePinDown, HIGH);
digitalWrite(volumePinDown, LOW);
rescheduleISR(105);
return;
}
// reached the desired gain => pull pins HIGH and stop timer interrupt
digitalWrite(volumePinUp, HIGH);
digitalWrite(volumePinDown, HIGH);
stopISR();
}
The other functions only need to set the (volatile!!) desiredGain variable and start the ISR() routine [if not started] to adjust the volume in the background.
this gives the lib + responsiveness + allows the arduino to do other things simultaneously like leds and servo's and ...
This allows to set the volume to MAX or a certain percentage and change your mind directly and set a new desired gain value. ==> responsiveness
void setVolume(percentage)
{
desiredGain = percentage * max;
if (not started ISR) startISR();
}
Got the idea? (but then again for a next release)