setFrequencyRaw(float freq) may work incorrect due float limitation

[amurchick]

The frequency may not be set quite correctly if we fall under the float (32bit) limitation

Function SX126x::setFrequencyRaw(float freq)

int16_t SX126x::setFrequencyRaw(float freq) {
  // calculate raw value
  this->freqMHz = freq;
  uint32_t frf = (this->freqMHz * (uint32_t(1) << RADIOLIB_SX126X_DIV_EXPONENT)) / RADIOLIB_SX126X_CRYSTAL_FREQ;
  return(setRfFrequency(frf));
}

may not correctly calc divider due float (32 bit) limitation.

For example, I am want set frequency 868.825MHz, but with float, 868.825 is stored only approximately (as 868.825012).
A float has about 7 decimal digits of precision, while double has about 15–16 decimal digits:

float(868.825)  ≈ 868.82501220703125
double(868.825) ≈ 868.825000000000045...

So results of next two lines - differed:

// == 911029056
uint32_t((float(868.825) * (uint32_t(1) << RADIOLIB_SX126X_DIV_EXPONENT)) / RADIOLIB_SX126X_CRYSTAL_FREQ)

// == 911029043
uint32_t((double(868.825) * (uint32_t(1) << RADIOLIB_SX126X_DIV_EXPONENT)) / RADIOLIB_SX126X_CRYSTAL_FREQ)

Therefore, two solutions seem possible:

1. Convert these functions to double
2. Convert these functions to uint32 and specify the frequency in hertz.

Additional info (please complete):
Heltec V4 (ESP-32-S3/SX1262)
Library version: 7.6.0

[lyusupov]

How much sense (?) does this negligible 12 Hz deviation have when

1. a typical 10 PPM XTAL @ 868.825 MHz carrier frequency gives an error / deviation value of 8688 Hz or
2. a typical 1 PPM TCXO gives 868 Hz of error / deviation value...

[jgromes]

I agree with @lyusupov that this is indeed a negligible difference in most real world use cases. The two proposed solutions also have significant downsides:

1. Changing the interface to double will cause a heavy code size penalty on devices without double-precision FPU.
2. Changing the interface to int in Hz is a large breaking change, because existing code would be treated as 868 Hz instead of 868 MHz. It is something that I am considering doing, especially for configuration like the bandwith where continuous configuration with float does not really make much sense. And if we break the public API by implementing Rework of begin methods #1620, we might as well do this too to avoid doing a big change twice. But this is definitely for the future and would need to be rolled out gradually.

[lyusupov]

@jgromes

I believe that the topic is a good candidate for this action