Tone and LED library and sensor reading causing glitches

Not sure if this is an audio, LED or sensor question, maybe a bit of all?

I am reading from an I2C distance sensor every 100ms (as per MaxBotix) timed with millis() and generate a tone() at an interval timed with millis(), depending on distance, while also changing the colour of an LED strip, depending on distance.

The responsiveness of everything depends on the 100ms sensor reading interval, and the blocking tone() function (fixed duration 50ms) sometimes, not always, causes glitches in responsiveness, if the distance changes rapidly, and the LED strip is updated only every 100ms, which is ok-ish, but not great.

Is there a better way to code this?

Thanks!

// RunningMedian library developed by Rob Tillaart
#include <Wire.h>
#include "RunningMedian.h"
#include <Adafruit_NeoPixel.h>

// Variables that remain constant
const byte pinData = 4;
const byte numLeds = 24;
const int colourMin = 0;
const int colourMax = 32767;
const byte speakerPin = 3;
const int toneFrequency = 880;
const byte toneDuration = 50;
const int toneIntervalMin = 100;
const int toneIntervalMax = 1000;
const int distanceMin = 20;
const int distanceMax = 200;
const byte sensorReadInterval = 100;

Adafruit_NeoPixel strip(numLeds, pinData, NEO_GRB + NEO_KHZ800);

RunningMedian samples = RunningMedian(7);

// Variables that change
int toneInterval = 0;
unsigned long timeNowTone = 0;
unsigned long timeNowSensor = 0;
word distance = 0;
int colourAlertHSV = 0;

void setup()
{
  pinMode(speakerPin, OUTPUT);

  strip.begin();
  strip.setBrightness(32);
  strip.show();

  Wire.begin();
  readDistance();
  delay(100);
}

void loop()
{
  if (millis() - timeNowSensor >= sensorReadInterval)
  {
    timeNowSensor = millis();

    distance = readDistance();
    samples.add(distance);
    float distanceMedian = samples.getMedian();
    distanceMedian = constrain(distanceMedian, distanceMin, distanceMax);

    toneInterval = (distanceMedian - distanceMin) * ((toneIntervalMax - toneIntervalMin) / (distanceMax - distanceMin)) + toneIntervalMin;
    colourAlertHSV = (distanceMedian - distanceMin) * ((colourMax - colourMin) / (distanceMax - distanceMin)) + colourMin;

    sendRangeCommand();
  }

  if (millis() - timeNowTone >= toneInterval)
  {
    timeNowTone = millis();

    if ((distance >= distanceMin) && (distance <= distanceMax))
    {
      tone(speakerPin, toneFrequency, toneDuration);

      uint32_t colourAlertRGB = strip.ColorHSV(colourAlertHSV);
      strip.fill(colourAlertRGB, 0);
      strip.show();
    }

    else
    {
      noTone(speakerPin);

      strip.clear();
      strip.show();
    }
  }
}

void sendRangeCommand()
{
  Wire.beginTransmission(0x70);
  Wire.write(0x51);
  Wire.endTransmission();
}

word readDistance()
{
  Wire.requestFrom(0x70, byte(2));
  if (Wire.available() >= 2)
  {
    byte high_byte = Wire.read();
    byte low_byte = Wire.read();
    word distance = word(high_byte, low_byte);
    return distance;
  }

  else
  {
    return word(0);
  }
}

tone() doesn't need to be blocking. You can call tone() without the duration argument, then use millis() to determine when the 50 ms has passed and call noTone() to stop the tone.

pert:
tone() doesn't need to be blocking. You can call tone() without the duration argument, then use millis() to determine when the 50 ms has passed and call noTone() to stop the tone.

Per, tone() with duration argument doesn't block

Ok, the Arduino documentation says tone() uses one of the internal timers and is non-blocking. I have the speaker on pin 3, maybe I should change that.

I have the speaker on pin 3, maybe I should change that.

No.
I would look at the Neopixels, when calling the show method all interrupts are turned off, that is probably what you are experiencing. No interrupts mean no millis update no I2C and probably messes up the tone duration as well.

Try running it without the LEDs being updated.

When I comment out strip.show(), the loudspeaker sounds the 50ms beeps according to distance alright, changing every 100ms, with each sensor reading, in case the distance sensed has changed.

So, I am calling strip.show() in the wrong way? Or in the wrong places? Or at the wrong time? Because of the NeoPixel library that "kills" interrupts? Adafruit says: "Normally one can use the millis() and micros() functions to provide a reasonable time base. But the NeoPixel library has to shut off interrupts while issuing data, and this throws off those functions."

Would using FastLED be better? The maintainer(s) say: "So what do I do? In an ideal world, you would move to a 4-wire led chipset, like the APA102 or LPD8806. Because these chipsets don't have the WS2812 timing requirements, they don't need to have interrupts disabled while writing data out, and this problem never happens."

So, no joy with FastLED either, as I have to use NeoPixel rings?

So, I am calling strip.show() in the wrong way? Or in the wrong places? Or at the wrong time?

No the problem is fundamentally the show method itself, because the interrupts have to be turned off. This is a requirement of the Neopixels and the way they work. So there will be no difference between any software that drives them.

You could use the Dot star type LED strips. These have a data and clock signal to drive the data into them. They do not need the exact timing of the Neopixels and the data transfer is interruptible. You can get rings using these sort of LEDs, how big is the ring you want to use?

In the past when I have had to use Neopixels with other strict timing requirements I have used an ATtiny chip. I send the data to that using an SPI like clock and data protocol that is interruptible and get that chip to actually drive the data out to the LEDs.

I need 24 LEDs per ring, so, yes, I could rip out the built-in NeoPixel ring and simply put in another type, just need to look up the right type then. You have a recommendation which type to buy? So far, I have only seen WS2812 (NeoPixel) type rings.

I also thought maybe take a 5V Trinket, a small "slave" MCU, and let that deal with the NeoPixel ring; the Metro Mini signalling the Trinket when to do something, master/slave style via TX/RX/GND common? Then I could use the ring already in place.

Well I would use just two pins and bit bang the protocol using the shift out function. If you want to get fancy and speed things up you could use the hardware SPI but you would have to do that at both ends. Of course a common ground but using serial could be tricky due to its other uses in programming the boards.

Yeah, I found this serial master slave way of one MCU triggering another, so that's probably the best solution, as I cannot find DotStar RGB LED rings.

I cannot find DotStar RGB LED rings.

Here they are:-