meaning of "expression 'xxx' has side-effects" in a case statement?

Working on a complex telescope controller.

(Background, in case you need it:)

  • Timer 1 controls a precision near-60-Hz frequency for tracking stars/moon/sun with a telescope’s 60 Hz synchronous motor.
  • Timing of the precision frequency varies depending on tracking mode (sun, moon, star, calibrate)
  • Timer 2 produces a series of alternating half-sines using strings of 16 PWM pulses.
  • The widths of the 16 pulses vary based on both tracking mode and override input
    (none, fast or slow)
  • The half-sines are alternated between OC2A & OC2B outputs to drive a push-pull DC-to-AC inverter.

INTENDED FUNCTIONALITY OF ISRs:
OCR1A ISR simply alternates between switching on the OCR2A and OCR2B interrupts.

The OCR2A and OCR2B ISRs are identical:

  • Define which PWM sineTable will be used:
  • Outer-wrap is an override switch case (‘noveride’, slow, fast)
  • Inner wrap for ‘noveride’ case is an if-statement for mode (moon or other)
  • Each of those conditions calls a specific sine table, and selects a value using “pwmIndex”.
  • When 16 pulses have occurred,
  • Turn off Timer 2 interrupt enables, and reset pwmIndex.

QUESTION:

  • On the “case (noveride)” statement in line 111 below, the compiler returns:
  • “expression ‘noveride’ has side-effects”
  • I do not understand what that means, or what I need to fix.
  • I did a search in the playground and forum, but couldn’t find a thread that seemed relevant.
  • I found “Bit Math Tutorial by CosineKitty”, that addressed “side-effects”
  • …but don’t understand how that would apply to a case statement.

Here is an extract that I BELIEVE contains all the relevant code - my entire (but still incomplete) code is too big for the 9000 character limit.

Extract:

// Global variable definition - 

// Define working value constants for MODE variable
const byte moon = 0;
const byte sun = 1; 
const byte star = 2;
const byte calibrate = 3;

// Define working constants for Override management
volatile const byte noveride = 0;
volatile const byte slow  = -1;
volatile const byte fast  = 1;

//Define variables for Override management (working value constants above)
volatile byte overide = noveride;    //Current state of override switch
volatile byte prevoride = noveride;    //Last loop’s state of the override switch
volatile byte speedSwitch;


// Define tracking mode (see working values as constants above)
byte    trackMode   = star;   //Current state of mode switch
byte    prevMode  = star;     //Last loop’s state of the mode switch
byte    ModeSwitch  = 2;
boolean   switchOpen  = false;

////////////////////////////////////////////////////////////

// Define hard-coded override slewing speeds
const unsigned int slowSpeed = 12000;    // ~3/4 starSpeed - slews FOV eastward
const unsigned int fastSpeed = 20000;   // ~4/3 starSpeed - slews FOV westward

////////////////////////////////////////////////////////////

// Telescope Control Output control variables
boolean          inverting = false;  // flag to swap motor drive outputs OCR1A <--> OCR1B

////////////////////////////////////////////////////////////

//Volatile variables that are shared between between ISRs and setup or loop

// Define PWM pulse definition constants
       // The pwm is zero-relative, so values are (interval count - 1).
volatile byte sineTableSlow[] = {18, 56, 91, 123, 150, 171, 186, 193, 193, 186, 171, 150, 123, 91, 56, 18};
volatile byte sineTableStarSun[] = {12, 36, 60, 81, 99, 113, 122, 127, 127, 122, 113, 99, 81, 60, 36, 12};
volatile byte sineTableMoon[] = {12, 38, 63, 85, 103, 118, 128, 133, 133, 128, 118, 103, 85, 63, 38, 12};
volatile byte sineTableFast[] = {9, 27, 45, 61, 74, 85, 92, 96, 96, 92, 85, 74, 61, 45, 27, 9};

volatile byte     pwmIndex    = 0;        // sequence index inside sineTable

//\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
////////////////////////////////////////////////////////////

void setup()
{
  // ...
}

//\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
////////////////////////////////////////////////////////////

ISR (TIMER1_COMPA_vect) // Updates backlight duty cycle if required
  {
  inverting != inverting;   // toggle to alternate between positive & negative half-sines
  
  if (inverting == true)
    {
    TIMSK2 = 4; // Enable Timer 2 Compare Match B interrupt;
    }
    else
    {
    TIMSK2 = 2; // Enables Timer 2 Compare Match A interrupt;
    }
  }

//\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
////////////////////////////////////////////////////////////

ISR (TIMER2_COMPA_vect)           // ISR for Timer 2’s OCR2A match
  {
    switch (overide)    // increment OCR2A to the next of 16 values in the current sine table
    {
      case (noveride)   // use a regular mode-driven PWM table when not overriding
        if (trackmode == moon) 
          { OCR2A = sineTableMoon[pwmIndex++]; } // moon mode uses a special sine table table
        else
          { OCR2A = sineTableStarSun[pwmIndex++]; } // sun, star and calibrate modes all use the same table
          
      case (slow)   // use an override (slow or fast) PWM table if currently overriding
        OCR2A = sineTableSlow[pwmIndex++];
        
      case (fast)
        OCR2A = sineTableFast[pwmIndex++];
        
      break;
     }
   
  if pwmIndex == 16               // After 16 pulses (0-->15), terminate the PWM activity
    {
    TIMSK2 = 0;                   // Disable both Timer 2 Compare Match A&B interrupts;
    pwmIndex = 0;                 // Reset pwm index
    }
  }

//\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
////////////////////////////////////////////////////////////

ISR (TIMER2_COMPB_vect)           // ISR for Timer 2’s OCR2B match
  {
    switch (overide)    // increment OCR2A to the next of 16 values in the current sine table
    {
      case (noveride)   // use a regular mode-driven PWM table when not overriding
        if (trackmode == moon) 
          { OCR2B = sineTableMoon[pwmIndex++]; } // moon mode uses a special sine table table
        else
          { OCR2B = sineTableStarSun[pwmIndex++]; } // sun, star and calibrate modes all use the same table
          
      case (slow)   // use an override (slow or fast) PWM table if currently overriding
        OCR2B = sineTableSlow[pwmIndex++];
        
      case (fast)
        OCR2B = sineTableFast[pwmIndex++];
        
      break;
     }
   
  if pwmIndex == 16               // After 16 pulses (0-->15), terminate the PWM activity
    {
    TIMSK2 = 0;                   // Disable both Timer 2 Compare Match A&B interrupts;
    pwmIndex = 0;                 // Reset pwm index
    }
  }
//\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\
////////////////////////////////////////////////////////////

void loop()
  {
    // ...
  }  // end of loop processing
 
  //////////////////////////////////////////////////////////

Thanks for any help folks can provide!

I don't follow the logic, but would guess that you need more study on the rules on switch case constructs.

You almost always need "break" statements between individual case entries, otherwise you will get fall through and totally unexpected results.

I think you are missing the colon after (noveride) which leads the compiler to think that the whole mess until the next case is part of the noveride case.

I think this is a gcc extension getting twisted in its shorts. Generally cases are supposed to be pretty simple, like what you have.

volatile const byte noveride = 0;
volatile const byte slow  = -1;
volatile const byte fast  = 1;

These seem to contradict themselves. A const by definition can't change so what does "volatile const" mean?

Pete

Thanks all, for the quick responses! I'll work on al three code malfs!

volatile const byte slow  = -1;

How do you really intend to store -1 in a variable that can hold values from 0 to 255?

PaulS - Thanks - really good point! Gotta fix that, too. Doing this for my son's and grandson's telescope (inherited from other grampa).

I can handle the mechanical and electrical design OK, but this is my first time writing code in several decades (formerly Fortran and 6802 assembler), and the program is massive, so I'm taking it in chunks, building onto a functioning and ever-expanding core. Slow row to hoe!

Slow row to hoe!

I haven't heard that phrase since I stopped doing Fortran coding, years ago. Thanks for the memory jog. 8)