Problem with structs in program memory

I have built a compass using an Arduino Pro Mini, a GY511 magnetometer and a GPS module. The bdisplay is a Nokia 5110 LCD.

I wanted to free some RAM, so I moved the constants for the compass low pass filtering into a struct in program memory. And it returns zero.

The same code with the struct in RAM works fine.

I copied the filter function into a short sketch which puts the struct in program memory and it works ok.

The problematic sketch (with some hacks to run without inputs) and the short test sketch are appended. Can anyone suggest why the filter code misbehaves in the problematic sketch?

Pogo.

/* A rewrite of Compass2LineEllipse_A.ino to use a Nokia 5110 lcd.
 * The Nokia display requires a 504 byte buffer in RAM. Moving the 
 * constants to program space with PROGMEM should free enough RAM
 * for this.
 * The display shows a compass needle on the right side of the screen
 * with the heading and a GPS update indicator on the left.
 * The GPS data is used to correct the compass for the local declination.
 * The display backlight is now controlled externally.
*/
#include <avr/pgmspace.h> // Store constants in program space
#include <Wire.h> // To talk to compass
#include <LSM303.h> // LSM303 Compass Library
#include <Adafruit_PCD8544.h> // To talk to display
#include <SoftwareSerial.h> // To read GPS

struct Samples // Past values for low-pass filter
{
  int x1;
  int x2;
  int x3;
  int x4;
  float y1;
  float y2;
  float y3;
  float y4;
};

Samples magX, magY, magZ;

struct Coeff // Coefficients for low pass filter
{
  float b0;
  float b1;
  float b2;
  float b3;
  float b4;
  float a1;
  float a2;
  float a3;
  float a4;  
};

// Filter coefficients. 4 pole Butterworth with a 10:1 frequency ratio
const Coeff Coeffs PROGMEM = {0.0048243,
                              0.0192974,
                              0.0289461,
                              0.0192974,
                              0.0048243,
                              2.3695159,
                              -2.3139877,
                              1.0546647,
                              -0.1873776};

// Matrix of coefficients to correct soft iron ellipse
const float Correction [3][3] PROGMEM = {{0.929821, 0.045086, 0.018747},
                                         {0.045086, 0.831375, 0.046055},
                                         {0.018747, 0.046055, 1.245971}}; 

float Latitude = -33.865143, Longitude = 151.209900, Declination = 12.67; // Sydney
int Reading; // Calculated heading
unsigned long LastTime; // millis() count for measurement and display timing
bool Indicator = true, GPSvalid = false;
// I/O pins
// LDR voltage divider for light level (Large LDR + 47k resistor)
// const byte LightPin = A0; Not used
// const byte BacklightPin = 8; Not used
const byte ssRXpin = 9, ssTXpin = 10; // RX and TX for GPS Note: DO NOT CONNECT ssTX
// I2C pins for magnetometer
// SDA A4, SCL A5

// Display interface pins. Keystudio device pinout
// 3V3 - VCC (from reg) (1)
// GND - GND (2)
// D 8 - Backlight (8) // Not used
// D 7 - Serial clock out (SCLK) (7)
// D 6 - Serial data out (DIN) (6)
// D 5 - Data/Command select (D/C) (5)
// D 4 - LCD chip select (CS) (3) <--- Note these are crossed over 
// D 3 - LCD reset (RST) (4) <--- between Arduino and display

Adafruit_PCD8544 display = Adafruit_PCD8544(7, 6, 5, 4, 3); // Check these!
LSM303 compass;
SoftwareSerial ss(ssRXpin,ssTXpin);

void setup()
{
  // pinMode(BacklightPin, OUTPUT); // Not used
  Serial.begin(9600);
  Serial.println();
  Serial.println(F("CompassNokiaEllipse_A.ino"));
  ss.begin(9600);
  Wire.begin();
  // Initialise the display
  if(!display.begin())
  {
    Serial.println(F("Display failed to initialise."));
    //ErrorLed(3); // Won't return
  }
  display.setContrast(75);  // A guess at contrast.
  display.setTextWrap(false);
  display.setTextColor(BLACK);
  display.setTextSize(2);
  display.clearDisplay();  // Clears the screen buffer
  // Initialise the compass
  if(!compass.init())
  {
    Serial.println(F("Compass failed to initialise."));
    display.clearDisplay();
    display.setCursor(0, 8);
    display.println(F("Compass failed"));
    display.print(F("to initialise."));
    display.display();
    //ErrorLed(2); // Won't return
  }
  pinMode(13, OUTPUT);
  compass.setTimeout(250); // Longer than the update rate
  compass.enableDefault();
  // Calibration values obtained using calibration sketch in library
  compass.m_min = (LSM303::vector<int16_t>){-110, -255, -604}; // Magnetometer
  compass.m_max = (LSM303::vector<int16_t>){+568, +505, -501}; // on the bike
  // compass.m_min = (LSM303::vector<int16_t>){-590, -642, -459}; // Yellow cable
  // compass.m_max = (LSM303::vector<int16_t>){+657, +669, +839};
  
  MyReadMag(); // Take one reading
  // Make all filter values equal to initial value. Looks like a DC signal.
  compass.m.x = 490; compass.m.y = -91; compass.m.z = -513;
  magX.x4 = compass.m.x; magX.x3 = magX.x4; magX.x2 = magX.x4; magX.x1 = magX.x4;
  magY.x4 = compass.m.y; magY.x3 = magY.x4; magY.x2 = magY.x4; magY.x1 = magY.x4;
  magZ.x4 = compass.m.z; magZ.x3 = magZ.x4; magZ.x2 = magZ.x4; magZ.x1 = magZ.x4;
  magX.y4 = magX.x4; magX.y3 = magX.x4; magX.y2 = magX.x4; magX.y1 = magX.x4;
  magY.y4 = magY.x4; magY.y3 = magY.x4; magY.y2 = magY.x4; magY.y1 = magY.x4;
  magZ.y4 = magZ.x4; magZ.y3 = magZ.x4; magZ.y2 = magZ.x4; magZ.y1 = magZ.x4;

  Reading = MyHeading(compass.m.x, compass.m.y); //No point doing this here
  LastTime = millis(); // Start the update timer
}

void loop() // Read the magnetometer and calculate a heading to display
{
  const byte ReadPeriod = 200; // Five times a second
  unsigned long Now;
  char c;
  int magValue;

 if(ss.available()) // Check for new character from GPS
 {
   c = ss.read();
   getLatLon(c); // Pass it to the parsing state machine
 }
 
  Now = millis();
  if(Now - LastTime > ReadPeriod)
  {
    LastTime = Now;
    if(GPSvalid == true) // Updates available once per second
      getDeclination(Latitude, Longitude); 
    MyReadMag(); // Read only the magnetometer
    // Low pass filter the reading. 4 x 200 = 800 ms lag
  // Serial.println("Before LPF");
  // Serial.print(compass.m.x);
  // Serial.print(", ");
  // Serial.println(compass.m.y);
    compass.m.x = 490;
    magValue = compass.m.x; 
    compass.m.x = (int32_t)LPF(magValue, &magX, Coeffs);
    compass.m.y = -91;
    magValue = compass.m.y;
    compass.m.y = (int32_t)LPF(magValue, &magY, Coeffs);
    compass.m.z = -513;
    magValue = compass.m.z;
    compass.m.z = (int32_t)LPF(magValue, &magZ, Coeffs);
  // Serial.println("After LPF");
  // Serial.print(compass.m.x);
  // Serial.print(", ");
  // Serial.println(compass.m.y);
    
//    // Check light level and set display mode accordingly. 200 count hysteresis
// Now using independent transistor
//    int Light = analogRead(LightPin);
//    if(Light > 600) // It's daytime
//    {
//      display.invertDisplay(false); // Black on white
//      digitalWrite(BacklightPin,LOW);
//    }
//    else if(Light < 400) // It's night time
//    {
//      display.invertDisplay(true); // White on black
//      digitalWrite(BacklightPin,HIGH); // Turn on backlight
//    }
//
    Reading = MyHeading(compass.m.x, compass.m.y);
    // Serial.println(Reading);
    Redraw(Reading);
  }
}

void MyReadMag() // Get the raw magnetic vector
{
  compass.readMag();
  if(compass.timeoutOccurred() == true)
  {
    Serial.println(F("Timeout"));
    display.clearDisplay();
    display.setCursor(0, 12);
    display.print(F("Timeout"));
    display.display();
    delay(1000);
    // do
    // {
    //   compass.init();
    //   compass.readMag(); // May hang forever
    // }
    // while(compass.timeoutOccurred() == true);
  }
  // Subtract offset (average of min and max) from magnetometer readings
  // Copied from lsm303.h heading(vector<T> from)
  // Needed because I don't call compass.heading() where the correction
  // is normally made but do need the offsets corrected
  // Now scaled to reduce soft iron effect
  compass.m.x -= (int32_t)(compass.m_min.x + compass.m_max.x) / 2;
  compass.m.y -= (int32_t)(compass.m_min.y + compass.m_max.y) / 2;
  compass.m.z -= (int32_t)(compass.m_min.z + compass.m_max.z) / 2; // Used only for ellipse correction
  // Soft iron ellipse correction
  float Mx = compass.m.x * pgm_read_float(&Correction[0][0]) + 
             compass.m.y * pgm_read_float(&Correction[0][1]) + 
             compass.m.z * pgm_read_float(&Correction[0][2]);  
  float My = compass.m.x * pgm_read_float(&Correction[1][0]) + 
             compass.m.y * pgm_read_float(&Correction[1][1]) + 
             compass.m.z * pgm_read_float(&Correction[1][2]);  
  float Mz = compass.m.x * pgm_read_float(&Correction[2][0]) + 
             compass.m.y * pgm_read_float(&Correction[2][1]) + 
             compass.m.z * pgm_read_float(&Correction[2][2]);
  // Serial.print(F("Mx ")); Serial.println(Mx); // Debugs to check calculation
  // Serial.print(F("My ")); Serial.println(My);
  // Serial.print(F("Mz ")); Serial.println(Mz);
  compass.m.x = (int32_t)Mx;
  compass.m.y = (int32_t)My;
  compass.m.z = (int32_t)Mz;
  // Serial.print(compass.m.x);
  // Serial.print(", ");
  // Serial.print(compass.m.y);
  // Serial.print(", ");
  // Serial.print(compass.m.z);
  // Serial.print(", ");
}

float MyHeading(int x, int y) // Calculate compass heading in degrees without tilt compensation
{
  // float H = atan2(-compass.m.x, -compass.m.y) * 180 / PI; // North-clockwise convention
  float H = atan2(-x, -y) * 180 / PI; // North-clockwise convention
  // Remove declination obtained from GPS and interpolation
  // Serial.print(compass.m.x);
  // Serial.print(", ");
  // Serial.print(compass.m.y);
  // Serial.print(", ");
  // Serial.print(compass.m.z);
  // Serial.print(", ");

  H -= Declination; // 12.67;
  if(H < 0) H += 360;
  else if(H >= 360) H -= 360;
  // Serial.println(H);
  return H;
}

void ErrorLed(byte Count)
{
  // Flash the display backlight count times pause then repeat forever
  // Now uses onboard LED as well
  const byte LedPin = 13;
  pinMode(LedPin, OUTPUT);
  do
  {
    for(int i = 0; i < Count; i++)
    {
      // digitalWrite(BacklightPin, HIGH); // Not used
      display.invertDisplay(true);
      digitalWrite(LedPin, HIGH);
      delay(500);
      // digitalWrite(BacklightPin, LOW);
      display.invertDisplay(false);
      digitalWrite(LedPin, LOW);
      delay(500);
    }
    delay(2000);
  }
  while(true);
}

void getLatLon(char c)
{
  // Reads and parses the serial stream to extract the latitude and longitude
  // from GLL and GGA GPS sentences and sets the Latitude and Longitude globals.
  enum States {WaitingForStart,
               WaitingForTime,
               WaitingForStype,
               WaitingForLat,
               WaitingForNS,
               WaitingForLon,
               WaitingForEW,
               WaitingForCsum,
               WaitingForEnd};
  static States State = WaitingForStart;
  enum SentenceType{GGA, GLL, Other}; // Used to steer state machine
  static SentenceType SType = Other;  // depending on sentence type
  static int CharCount = 0;
  static char Line[12];
  static float Lat, Lon;
  static char chk = 0; // Checksum

  float Minutes;
  int Degrees;
  char inLine[4] = {'\0', '\0', '\0', '\0'};
  // Accumulate checksum
  if(State > WaitingForStart && State < WaitingForEnd && c != '*') chk ^= c;

  switch(State)
  {
    case WaitingForStart:
      if(c != '$'); // Dump characters until "$" appears
      else
      {
        CharCount = 0;
        chk = 0;
        SType = Other;
        State = WaitingForStype;
      }
    break;
    case WaitingForStype:
      // Add characters to buffer until a comma comes in
      if(c != ',')
      {
        Line[CharCount++] = c;
        Line[CharCount] = '\0'; // Make it a string in case strstr() cares
        break;
      }
      else if(CharCount == 5) 
      {
          if(strstr_P(Line, PSTR("GGA"))) // Found a GGA sentence
          {
            CharCount = 0;
            SType = GGA;
            State = WaitingForTime;
            break;
          }
          else if(strstr_P(Line, PSTR("GLL"))) // Found a GLL sentence
          {
            CharCount = 0;
            SType = GLL;
            State = WaitingForLat;
            break;
          }
          else
          {
            SType = Other;
            State = WaitingForStart; // Not a GGA or GLL sentence
            break;
          }
      }
      else
        State = WaitingForStart; // Too few characters
    break;
    case WaitingForTime: // Only GGA does this
      // Add characters to buffer until a comma comes in
      if(c != ','); // Skip characters until a comma comes in
      // {
      //   Line[CharCount++] = c;
      //   Line[CharCount] = '\0'; // Make it a string 
      //   break;
      // }
      // else if(CharCount == 9) 
      // {
      //   CharCount = 0;
      //   State = WaitingForLat;
      //   break;
      // }
      // else
      //   State = WaitingForStart; // Too few characters
      else
      {
        CharCount = 0;
        State = WaitingForLat;
      }
    break;
    case WaitingForLat:
    // Add characters to buffer until a comma comes in
      if(c != ',')
      {
        Line[CharCount++] = c;
        Line[CharCount] = '\0'; // Make it a string in case atof() cares
        break;
      }
      else if(CharCount == 10) // Length of latitude field "DDMM.mmmmm"
      {
        Lat = atof(Line); // Convert string to float. Some round off here
        strncpy(inLine, Line, 2); // Avoid rounding errors in division
        Degrees = atoi(inLine); // Build decimal degrees
        Minutes = Lat - Degrees * 100;
        Lat = Degrees + Minutes / 60; // More round off here
        CharCount = 0;
        State = WaitingForNS;
        break;
      }
      else State = WaitingForStart; // Too few characters
    break;
    case WaitingForNS:
      if(c != ',')
      {
        Line[CharCount++] = toupper(c);
        Line[CharCount] = '\0'; // Make it a string;
        break;
      }
      else if(Line[0] != 'N' && Line[0] != 'S') // Not what we expected
      {
        State = WaitingForStart;
        break;
      }
      else if(Line[0] == 'S') Lat = -Lat;
      CharCount = 0;
      State = WaitingForLon;
    break;
    case WaitingForLon:
    // Add characters to buffer until a comma comes in
      if(c != ',')
      {
        Line[CharCount++] = c;
        Line[CharCount] = '\0'; // Make it a string in case atof() cares
        break;
      }
      else if(CharCount == 11) // Length of longitude field "DDDMM.mmmmm"
      {
        Lon = atof(Line); // Convert string to float. Some round off here
        strncpy(inLine, Line, 3); // Avoid rounding errors in division
        Degrees = atoi(inLine); // Build decimal degrees
        Minutes = Lon - Degrees * 100;
        Lon = Degrees + Minutes / 60; // More round off here
        CharCount = 0;
        State = WaitingForEW;
        break;
      }
    break;
    case WaitingForEW:
      if(c != ',')
      {
        Line[CharCount++] = toupper(c);
        Line[CharCount] = '\0'; // Make it a string;
        break;
      }
      else if(Line[0] != 'E' && Line[0] != 'W') // Not what we expected
      {
        State = WaitingForStart;
        break;
      }
      else if(Line[0] == 'W') Lon = -Lon;
      CharCount = 0;
      State = WaitingForCsum;
    break;
  // At this point we have two signed floats waiting to see if the checksum
  // is correct
    case WaitingForCsum:
      if(c != '*'); // Skip characters until checksum field starts
      else
      {
        CharCount = 0;
        State = WaitingForEnd;
      }
    break;
    case WaitingForEnd:
    // Add characters to buffer until carriage return comes in
      if(c != '\r')
      {
        Line[CharCount++] = c;
        Line[CharCount] = '\0'; // Make it a string
        break;
      }
      else if(CharCount == 2) // Two (presumably) hex characters are in Line
      {
        if(isxdigit(Line[0]) && isxdigit(Line[1]) && FromHex(Line) == chk)
        // Checksum is correct so overwrite globals with new values
        {
          Latitude = Lat;
          Longitude = Lon;
          GPSvalid = true;
          Indicator = !Indicator; // Show that something is happening
          // Serial.print(Lat); // These can be commented out to gain 12 bytes
          // Serial.print(F(", "));
          // Serial.println(Lon);
        }
        else 
        {
          GPSvalid = false;
        }
      }
      State = WaitingForStart; // Start again regardless
    break;
  }
}

byte FromHex(char *TwoChars) // Converts a two digit hex number to a byte
{
  byte a, b;
  char c;

  for(int i = 0; i < 2; i++)
  {
    c = toupper(TwoChars[i]); // Force upper case for hex evaluation
    if(isdigit(c)) a = c - '0';
    else if(c >= 'A' && c <= 'F') a = 10 + c - 'A';
    if(i == 0) b = a << 4; // Shift first character to top nybble
    else b += a;
  }
  return b;
}

float getDeclination(float lat, float lon)
{
  // Calculates the magnetic declination at lat, lon by a bilinear interpolation
  float Qxy;
  // Lat and lon 'corners' for Australia
  const float y2 = -15, y1 = -45, x2 = 150, x1 = 120;
  // Corresponding magnetic declinations from WMM2020
  const float Q22 = 7.5, Q21 = 18, Q12 = -1.5, Q11 = -6;
  // Bilinear interpolation
  Qxy = 1/((x2 - x1)*(y2 - y1)) * 
  (Q11*(x2 - lon)*(y2 - lat) + 
  Q21*(lon - x1)*(y2 - lat) + 
  Q12*(lon - x1)*(y2 - lat) + 
  Q22*(lon - x1)*(lat - y1));
  return Qxy;
}

int32_t LPF(int x0, Samples *S, Coeff C)
{
  // Calculates a four pole low pass filter on samples supplied in S
  // The newest sample is in x0. The coefficients in C determine the
  // type of filter. Currently a Butterworth filter with 10:1 frequency ratio.
  // Based on code in stackoverflow <https://stackoverflow.com/questions/20924868/
  // calculate-coefficients-of-2nd-order-butterworth-low-pass-filter>

  float y0, dummy;
  y0 = pgm_read_float(&C.b0) * x0 + pgm_read_float(&C.b1) * S->x1 + 
       pgm_read_float(&C.b2) * S->x2 + pgm_read_float(&C.b3) * S->x3 + 
       pgm_read_float(&C.b4) * S->x4 + pgm_read_float(&C.a1) * S->y1 + 
       pgm_read_float(&C.a2) * S->y2 + pgm_read_float(&C.a3) * S->y3 + 
       pgm_read_float(&C.a4) * S->y4;
dummy = pgm_read_float(&C.b0);
Serial.print(F("C.b0: "));
Serial.println(dummy,6);//*x0);// + pgm_read_float(&C.b1)*S->x1);
// Serial.println(y0);
Serial.print(F("S->x1: "));
Serial.println(S->x1,6);
  // Shuffle samples down
  S->y4 = S->y3;
  S->y3 = S->y2;
  S->y2 = S->y1;
  S->y1 = y0;
  S->x4 = S->x3;
  S->x3 = S->x2;
  S->x2 = S->x1;
  S->x1 = x0;

  return (int32_t)y0;
}

void Redraw(int Heading) // Draw the pointer and the heading
{
  byte x1, y1, x2, y2, x3, y3; // Vertices of triangle
  const int a1 = -12, b1 = 21, a2 = 0, b2 = -24, a3 = 12, b3 = 21; // Triangle pointing north 

  // Rotate needle away from north by Heading degrees
  float H = Heading * PI / 180; // Convert to radians for rotation
  float S = sin(H); 
  float C = cos(H);
  x1 = RotX(a1, b1, S, C); y1 = RotY(a1, b1, S, C);
  x2 = RotX(a2, b2, S, C); y2 = RotY(a2, b2, S, C);
  x3 = RotX(a3, b3, S, C); y3 = RotY(a3, b3, S, C);

  // Print the heading and the update indicator
  display.clearDisplay();
  display.setTextSize(2);
  display.setCursor(0,16);
  if(Heading < 100) display.print(0); // Add leading zeroes as required
  if(Heading < 10) display.print(0);
  display.print(Heading);
  display.setCursor(10, 32);
  if(GPSvalid == true)
  {
    // Toggle indicator to show GPS updated
    if(Indicator == true)
      display.print('\\'); // backslash
    else
      display.print('/');
  }
  else 
  {
    display.print('-'); // No update
  }

  // Draw the compass
  display.fillTriangle(x1, y1, x2, y2, x3, y3, BLACK);
  display.drawCircle(59, 23, 23, BLACK);
  display.setTextSize(1);
  display.setCursor(56, 0); // Cardinal points inside the circle
  display.print(F("N"));
  display.setCursor(78, 20);
  display.print(F("E"));
  display.setCursor(56, 40);
  display.print(F("S"));
  display.setCursor(38, 20);
  display.print(F("W"));
  display.display();
}

byte RotX(int a, int b, float S, float C) // Rotate and offset x coordinate
{
  return (byte)(a*C - b*S + 59);
}

byte RotY(int a, int b, float S, float C) // Rotate and offset y coordinate
{
  return (byte)(a*S + b*C + 24);
}

Please post a short sketch that demonstrates the problem, using code tags.

While you are at it, people would appreciate it if you fixed the OP, too (adding code tags where needed).

@jremington, I'm struggling to add the second piece of code. I use the tags, but it goes into body text.

Simply select the code and push the <code> editor button.

Like this?

/* Testpgmspace.ino Chunks cut out of CompassNokiaHeading.ino and 
 * CompassNokiaEllipse_A.ino to test putting structs and arrays of floats
   in program memory.
*/
#include <avr/pgmspace.h> // Store constants in program space

// Coefficients for low pass filter
struct Coeff
{
  float b0;
  float b1;
  float b2;
  float b3;
  float b4;
  float a1;
  float a2;
  float a3;
  float a4;  
};

// Coefficients for low pass filter
const Coeff C PROGMEM = {0.0048243,
                              0.0192974,
                              0.0289461,
                              0.0192974,
                              0.0048243,
                              2.3695159,
                              -2.3139877,
                              1.0546647,
                              -0.1873776};

const float Correction [3][3] PROGMEM = {{0.929821, 0.045086, 0.018747},
                                         {0.045086, 0.831375, 0.046055},
                                         {0.018747, 0.046055, 1.245971}}; 

struct Samples // Past values for low-pass filter
{
  int x1;
  int x2;
  int x3;
  int x4;
  float y1;
  float y2;
  float y3;
  float y4;
};

Samples Sn = {1, 2, 3, 4, 0.5, 0.6, 0.7, 0.8};
Samples *S = &Sn;

void setup() 
{
  float x;
  int x0 = 234;
  Serial.begin(9600);
  delay(1000);
  // Cut from LPF
  float y0, dummy;
  y0 = pgm_read_float(&C.b0) * x0 + pgm_read_float(&C.b1) * S->x1 + 
       pgm_read_float(&C.b2) * S->x2 + pgm_read_float(&C.b3) * S->x3 + 
       pgm_read_float(&C.b4) * S->x4 + pgm_read_float(&C.a1) * S->y1 + 
       pgm_read_float(&C.a2) * S->y2 + pgm_read_float(&C.a3) * S->y3 + 
       pgm_read_float(&C.a4) * S->y4;
dummy = pgm_read_float(&C.b0);
Serial.print(F("C.b0: "));
Serial.println(dummy,6);//*x0);// + pgm_read_float(&C.b1)*S->x1);
Serial.println(y0);
Serial.print(F("S->x1: "));
Serial.println(S->x1,6);
  // Shuffle samples down
  S->y4 = S->y3;
  S->y3 = S->y2;
  S->y2 = S->y1;
  S->y1 = y0;
  S->x4 = S->x3;
  S->x3 = S->x2;
  S->x2 = S->x1;
  S->x1 = x0;
}

void loop() 
{

}

Did that work?

Yes, can't you see the change?

int32_t LPF(int x0, Samples *S, Coeff C)

You need to pass Coeff by reference, it will not copy from PROGMEM to a temp variable in ram properly.

Yes, now, but in the original, It wouldn't work.

Anyway, I'd appreciate your thoughts on the behaviour of the code.

See post #7.

This works as expected;

/* Testpgmspace.ino Chunks cut out of CompassNokiaHeading.ino and
   CompassNokiaEllipse_A.ino to test putting structs and arrays of floats
   in program memory.
*/
#include <avr/pgmspace.h> // Store constants in program space

// Coefficients for low pass filter
struct Coeff
{
  float b0;
  float b1;
  float b2;
  float b3;
  float b4;
  float a1;
  float a2;
  float a3;
  float a4;
};

// Coefficients for low pass filter
const Coeff C PROGMEM = {0.0048243,
                         0.0192974,
                         0.0289461,
                         0.0192974,
                         0.0048243,
                         2.3695159,
                         -2.3139877,
                         1.0546647,
                         -0.1873776
                        };


struct Samples // Past values for low-pass filter
{
  int x1;
  int x2;
  int x3;
  int x4;
  float y1;
  float y2;
  float y3;
  float y4;
};

Samples Sn = {1, 2, 3, 4, 0.5, 0.6, 0.7, 0.8};
Samples *S = &Sn;

void setup()
{
  float x;
  int x0 = 234;
  Serial.begin(115200);
  delay(1000);

  float dummy = pgm_read_float(&C.b0);
  Serial.print(F("C.b0: "));
  Serial.println(dummy, 6);
  Serial.print(F("S->x1: "));
  Serial.println(S->x1, 6);

}

void loop()
{}

@david_2018, Half way there, David. The S struct still returns bad results, and it is global. I passed it by ref to not put the whole struct on the stack. I'll poke at it and see what works.

Yes, that was the confusing bit. The code was identical, but the behaviour was different. It seems that sometimes PROGMEM requires a struct to be passed by reference as @david_2018 pointed out, but sometimes not.

I would make everything in PROGMEM globals and use them that way.

I thought that Coeff was global. It's declared at the top of the program above any functions. That's right, isn't it?

Correct.

You also need to be careful of compiler optimizations. When you are directly referencing the struct in setup(), the compiler may be able to directly substitute the data from the struct into the code and never store the struct at all.

It is, but you are passing it as a function parameter, so you are not referencing the global struct, but a copy created on the stack.

In my experience that is in fact what happens, and the constants end up in program memory anyway. The only reason to pass the struct as a parameter would be if you needed different sets of coefficients for different applications of the same filter code.

The compiler is far too clever for me. :flushed: I think I'm on the right path. I'm getting sensible values with the changes suggested.

I've got a warning: "invalid conversion from 'const Coeff*' to 'Coeff*' [-fpermissive]
compass.m.x = (int32_t)LPF(magValue, &magX, &Coeffs);
^~~~~~~"
That's ignorable, isn't it?

As an example, the code from post #5 using PROGMEM for the struct, but leaving out all references to pgm_read_float(), still works, and uses noticeably less flash memory since the compiler sees no need for the actual struct.

Thanks to both of you. I now have working code, a good explanation of what was wrong and a little more understanding of what goes on under the hood while I'm trying to be clever.

The function parameter should be const, since you are intending to pass a const to the function.