PiInTheSky / pits

Pi In The Sky Telemetry Software
GNU General Public License v2.0
87 stars 49 forks source link

Small issue with FrequencyError Calculation #43

Closed KevWal closed 3 years ago

KevWal commented 3 years ago

The calculation of FrequencyError in Lora.c seems to be done based on TX bandwidth currently, rather than RX bandwidth. My code used to test, and then fix the issue below from Lora.c in double FrequencyError(int Channel):

        printf("FrequencyError Now using BandwidthInKHz = %d.\n", LoRaModes[Config.LoRaDevices[Channel].UplinkMode].Bandwidth);
        printf("FrequencyError Was using BandwidthInKHz = %f.\n", BandwidthInKHz(Channel));
        // return - ((double)Temp * (1<<24) / 32000000.0) * (BandwidthInKHz(Channel) / 500.0); // Was dividing by tx bandwidth not rx bandwidth?
        return - ((double)Temp * (1<<24) / 32000000.0) * (LoRaModes[Config.LoRaDevices[Channel].UplinkMode].Bandwidth / 500.0);
PiInTheSky commented 3 years ago

Thanks, will check this soon.

Dave

On Fri, 5 Mar 2021 at 16:48, Kevin Walton notifications@github.com wrote:

The calculation of FrequencyError in Lora.c seems to be done based on TX bandwidth currently, rather than RX bandwidth. My code used to test, and then fix the issue below from Lora.c in double FrequencyError(int Channel):

    printf("FrequencyError Now using BandwidthInKHz = %d.\n", LoRaModes[Config.LoRaDevices[Channel].UplinkMode].Bandwidth);
    printf("FrequencyError Was using BandwidthInKHz = %f.\n", BandwidthInKHz(Channel));
    // return - ((double)Temp * (1<<24) / 32000000.0) * (BandwidthInKHz(Channel) / 500.0); // Was dividing by tx bandwidth not rx bandwidth?
    return - ((double)Temp * (1<<24) / 32000000.0) * (LoRaModes[Config.LoRaDevices[Channel].UplinkMode].Bandwidth / 500.0);

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/PiInTheSky/pits/issues/43, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB4ERK7A2QTBEXGCYTAFLTTTCEDO5ANCNFSM4YVRFRGA .

daveake commented 3 years ago

Incorporated into latest version going out this week, thanks!