Weird sound when modulating the pulse width on Arduino

Question

I'm trying to generate a pulse width modulated sound using Arduino Nano.

But it generates the weird sound unlike my expectation.

Here's my code:

class Phasor
{
public:
 Phasor(const float frequency)
 :frequency(frequency)
 ,phase(1.0f)
 ,lastTimeMicros(0){};
 virtual ~Phasor(){};
 void setFrequency(const float frequency)
 {
 this->frequency = frequency;
 }
 float process()
 {
 const unsigned long currentTimeMicros = micros();
 if (phase == 1.0f) lastTimeMicros = currentTimeMicros;
 const unsigned long elapsedTimeMicros = currentTimeMicros - lastTimeMicros;
 const unsigned long cycleDurationMicros = static_cast<unsigned long>(1000.0f / frequency) * 1000;
 if (elapsedTimeMicros < cycleDurationMicros)
 phase = static_cast<float>(elapsedTimeMicros) / static_cast<float>(cycleDurationMicros);
 else
 phase = 1.0f;
 return phase;
 }
private:
 float frequency;
 float phase;
 unsigned long lastTimeMicros;
};
class PulseOsc
{
public:
 PulseOsc(const float frequency, const float pulseWidth)
 :phasor(frequency)
 ,pulseWidth(pulseWidth){};
 virtual ~PulseOsc(){};
 void setFrequency(const float frequency)
 {
 this->phasor.setFrequency(frequency);
 }
 void setPulseWidth(const float pulseWidth)
 {
 this->pulseWidth = pulseWidth;
 }
 float process()
 {
 const float phase = phasor.process();
 if (phase <= pulseWidth)
 return 1.0f;
 else
 return -1.0f;
 }
private:
 Phasor phasor;
 float pulseWidth;
};
Phasor *phasorLFO;
PulseOsc *pulseOsc;
void setup()
{
 Serial.begin(9600);
 pinMode(9, OUTPUT);
 phasorLFO = new Phasor(0.1f);
 pulseOsc = new PulseOsc(440.0f, 0.5f);
}
void loop()
{
 pulseOsc->setPulseWidth(phasorLFO->process());
 int outVal = LOW;
 if (pulseOsc->process() > 0.0f) outVal = HIGH;
 digitalWrite(9, outVal);
}

The resulting sound is very weird and glitchy. It works fine when I fix the pulse width value. (without LFO)

What is wrong with my code? How do I fix this? I would appreciate any advice.

• Majenko: Probably the Nano isn't powerful enough to do all those floating point calculations in a timely manner.

Answer 1

I see two issues with this program. The first one has already been pointed out by Majenko in a comment: you are doing too much floating point calculations. The second has to do with the way your Phasor manages the time.

If you take a look at the Blink Without Delay Arduino tutorial, you will see something along these lines:

if (currentMillis - previousMillis >= interval) {
 previousMillis = currentMillis;
 toggle_the_LED();
}

In this code, interval is the minimum time between toggles. Whenever the program gets a little bit late and currentMillis - previousMillis is strictly larger than interval, this extra time is lost and the program will be late for all the subsequent LED toggles. If instead of the minimum you are interested in the average time between toggles, then you should write:

if (currentMillis - previousMillis >= interval) {
 previousMillis += interval; // <- advance by exactly one period
 toggle_the_LED();
}

Now, previousMillis is no more the time when the last toggle was performed, it is the time when the last toggle was scheduled. With this version you can still have some jitter, but at least the average frequency should be right. If the program is so busy that it can get late by a full period, then it would be better to replace the if by a while, in order to catch up all the missed periods.

Your Phasor::process() method is a little bit more involved than the tutorial above, yet the line

if (phase == 1.0f) lastTimeMicros = currentTimeMicros;

is going to get you out of schedule for exactly the same reason.

That being said, here is my take at your phasor idea, completely untested:

class Phasor()
{
public:
 Phasor(float frequency_Hz)
 {
 period = round(1e6 / frequency_Hz);
 frequency = 0x10000 / period;
 }
 uint16_t process()
 {
 uint16_t now = micros();
 while (now - last_time >= period)
 last_time += period;
 return (now - last_time) * frequency;
 }
private:
 uint16_t period; // unit = 1 us
 uint16_t frequency; // unit = 1/(2^16 us) ~ 15.3 Hz
 uint16_t last_time = 0;
};

I changed a few things from your implementation:

• removed the setFrequency() method because YAGNI
• no floating point except in the constructor, for performance reasons
• only 16-bit arithmetics, for the same reason
• no division in process(), because division is expensive
• the phase is not stored, because there is no use to storing it
• the phase is an integer in units of 2⁻¹⁶ periods

Note that the unit chosen for the phase is the most natural one when dealing with 16-bit arithmetics. Note also that storing microseconds in 16-bit variables only works for frequencies higher than 15.3 Hz. A very low frequency phasor (your "LFO" for instance) would need 32-bit arithmetics.

Answer 2

Seems like you are using the system clock micros() to calculate elapsedTimeMicros, so probably the time spent on the process of calculation and sending signal to the pin may affect it, which may lead to inconsistent duty cycle.