Bug found in Si570 code for Minima

A problem has been discovered in my Si570 code that implements 1Hz tuning.  The code was unnecessarily resetting the DCO on every update when tuning down in frequency.  While setting the frequency correctly, the error was causing the DCO to restart on every increment in frequency producing an audible click in the Minima.  The frequency was being set correctly, just clicking the receiver when it was not necessary.

The error was two-fold.  Firstly, I failed to reserve sufficient bits for the 3500 ppm calculation.  Secondarily, the Arduino absolute value function (abs()) is very quirky and it was just simpler to remove the function from my code.

Lastly, I modified the Si570 code to only calculate the 3500 ppm offset whenever the DCO centre frequency was changed rather than on every frequency change in order to gain a slight performance improvement.

The changes will be posted to my GitHub Si570 repository as soon as I am able.  Meanwhile, here are the changes for the adventurous among us that want to take on the change manually.

In Si570.h, I added a member variable to the Si570 class to hold the calculated 3500 ppm value for the current center frequency.

class Si570
{
public:
  Si570(uint8_t i2c_address, uint32_t calibration_frequency);
  Si570_Status setFrequency(uint32_t newfreq);
  void debugSi570();

  Si570_Status status;

private:
  uint8_t i2c_address;
  uint8_t dco_reg[13];
  uint32_t f_center;
  uint32_t frequency;
  uint16_t hs, n1;
  uint32_t freq_xtal;
  uint64_t fdco;
  uint64_t rfreq;
  uint32_t max_delta;

  uint8_t i2c_read(uint8_t reg_address);
  int i2c_read(uint8_t reg_address, uint8_t *output, uint8_t length);

  void i2c_write(uint8_t reg_address, uint8_t data);
  int i2c_write(uint8_t reg_address, uint8_t *data, uint8_t length);

  bool read_si570();
  void write_si570();
  void qwrite_si570();

  uint8_t getHSDIV();
  uint8_t getN1();
  uint64_t getRFREQ();

  void setRFREQ(uint32_t fnew);
  int findDivisors(uint32_t f);

};

Secondarily, I modified Si570.cpp to initialize max_delta when the Si570 object is constructed in Si570::Si570.

  // We are about the reset the Si570, so set the current and center frequency to the calibration frequency.
  f_center = frequency = calibration_frequency;

  max_delta = ((uint64_t) f_center * 10035LL / 10000LL) - f_center;

Lastly,  I modified the setFrequency function to remove the quirky Arduino abs() function and to set the max_delta value only when it changes.

// Set the Si570 frequency
Si570_Status Si570::setFrequency(uint32_t newfreq) 
{
  // If the current frequency has not changed, we are done
  if (frequency == newfreq)
    return status;

  // Check how far we have moved the frequency
  uint32_t delta_freq = newfreq < f_center ? f_center - newfreq : newfreq - f_center;

  // If the jump is small enough, we don't have to fiddle with the dividers
  if (delta_freq < max_delta) 
  {
    setRFREQ(newfreq);
    frequency = newfreq;
    qwrite_si570();
  }
  else 
  {
    // otherwise it is a big jump and we need a new set of divisors and reset center frequency
    int err = findDivisors(newfreq);
    setRFREQ(newfreq);
    // Set the new center frequency
    f_center = frequency = newfreq;
    // Calculate the new 3500 ppm delta
    max_delta = ((uint64_t) f_center * 10035LL / 10000LL) - f_center;
    write_si570();
  }
  
  return status;

}

My apologies for not catching this bug before the code was made available.  Many thanks to one of the many Minima users "John - MI0DFG" for finding and reporting this issue so that I can get it fixed.