CODEWITHSUNDEEP
c++arduinovolatileteensy
All-in-one Mighty
All-in-one Mighty

Reputation: 39

Do I need to mark my getter/setter and/or class volatile if one of its members is volatile?

I run a complex code on my Teensy board which does beat detection with a microphone and displays some cool LED effects on a bicycle by reacting to them, all in C++ code. But I currently experience a weird behavior where depending on the LED effect that I run, my beat detection does not work as well, and it's consistent for each effect: some work well, some others slow down the detection.

I have a suspicion, although no confirmation, that this comes from how I use my interrupt method to analyze sound.

I have a global variable that I use both in the loop() method and in an interrupt method.

The global variable is defined as:

ApplicationEnvironment env;

The ApplicationEnvironment class is defined as follows (truncated for readability):

class ApplicationEnvironment final : public EffectEnvironment {
 public:
  // Always returns adc0, since we're never using adc1.
  ADC_Module* const GetAdc() { return adc_->adc0; }

  void SetFilteredSignal(int16_t filtered_signal) {
    filtered_signal_ = filtered_signal;
  }
  void SetPeakSignalAmplitude(uint16_t peak_signal_amplitude) {
    unfiltered_signal_peak_amplitude_ = peak_signal_amplitude;
  }
  void SetPeakSignalFilteredAmplitude(uint16_t peak_signal_filtered_amplitude) {
    filtered_signal_peak_amplitude_ = peak_signal_filtered_amplitude;
  }
  uint16_t GetFilteredSignalPeakAmplitude() const override {
    return filtered_signal_peak_amplitude_;
  }

  // Sets whether the beat is currently detected or not.
  void SetBeatActive(bool beat_active) { beat_active_ = beat_active; }

 private:
  // ADC module, to handle ADC properties.
  // Read from ICRSoundProcessing() but does not seem to need 'volatile'.
  ADC* adc_;

  volatile int16_t filtered_signal_;
  // Values below represent the peak seen since their last respective update.
  volatile uint16_t unfiltered_signal_peak_amplitude_;
  // Values below represent the peak seen since their last respective update.
  volatile uint16_t filtered_signal_peak_amplitude_;

  volatile bool beat_active_;
};

In my interrupt method, I call the following methods:

env.GetAdc();
env.GetFilteredSignal()
env.GetFilteredSignalPeakAmplitude();

env.SetFilteredSignal(...);
env.SetPeakSignalAmplitude(...);
env.SetPeakSignalFilteredAmplitude(...);
env.SetBeatActive(...);

I call the setter methods only in the interrupt. I call the getters in both the interrupt and loop().

My questions are as follows:

  1. Do I need to set the env variable as volatile?
  2. More importantly, do I need to set the getter methods as volatile, knowing that they are called within loop()?
  3. Do I need to set the setter methods as volatile, although they are only called from the interrupt?

I believe the adc_ variable does not need to be volatile as it is never changed by the interrupt method.

Upvotes: 0

Views: 92

Answers (1)

All-in-one Mighty
All-in-one Mighty

Reputation: 39

@TedLyngmo answered this:

(...) "no" to all questions :)

In more details (see responses above):

Yes. The read/write from/to the volatile variable is guaranteed to be "non-cached" as you say. It must read/write and not reuse a previously stored value in some register. So, a function that makes a read from a volatile type and returns type will have to perform the read every time the function is called. The env variable shouldn't be volatile. Its adc_ member will not be changed outside the programs control. Making env volatile could possibly make the performance take a hit.

Upvotes: 2

Related Questions