Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SX126x in FSK mode always reports wrong CRC #1350

Closed
StevenCellist opened this issue Dec 15, 2024 · 15 comments
Closed

SX126x in FSK mode always reports wrong CRC #1350

StevenCellist opened this issue Dec 15, 2024 · 15 comments
Assignees
Labels
bug Something isn't working resolved Issue was resolved (e.g. bug fixed, or feature implemented)

Comments

@StevenCellist
Copy link
Collaborator

I've been trying to get LoRaWAN working on the FSK datarate, however unsuccessful. To that end, I fired up a second device that is configured as a listener for the FSK uplinks, but it turns out that I always get a CRC error (no address filtering).
As I don't have an SDR, I cannot do the fancy stuff in #1268 unfortunately. And whatever I tried, I cannot recover the original uplink content (it looks like the packet length & content is rather undefined when a CRC error occurs).

To Reproduce
Tested using Heltec Wireless Stick Lite V3, using the config.h from LoRaWAN examples. So be aware of radio (and SPI) setup.

FSK transmitter sketch

#include <Arduino.h>
#include <RadioLib.h>

#include "config.h"

// save transmission state between loops
int transmissionState = RADIOLIB_ERR_NONE;

// flag to indicate that a packet was received
volatile bool transmittedFlag  = false;

// this function is called when a complete packet
// is received by the module
ICACHE_RAM_ATTR void setFlag(void) {
  // we got a packet, set the flag
  transmittedFlag  = true;
}

const uint8_t len = 19;
uint8_t payload[len] = {0x40, 0x3c, 0x8a, 0xed, 0x00, 0x00, 0x05, 0x00, 0x01, 0x1c, 0x27, 0xcf, 0x26, 0x14, 0x71, 0x78, 0xae, 0xce, 0x8c};

void setup() {
  Serial.begin(115200);
  delay(5000);
  pinMode(0, INPUT);

  SPI.begin(9, 11, 10, 8);
  int state = radio.beginFSK();
  if(state == RADIOLIB_ERR_NONE) {
    Serial.println(F("success!"));
  } else {
    Serial.print(F("failed, code "));
    Serial.println(state);
    while(true);
  }

  state  = radio.setFrequency(868.8);
  state |= radio.setBitRate(50.0);
  state |= radio.setFrequencyDeviation(25.0);
  state |= radio.setPreambleLength(40);
  state |= radio.setDataShaping(RADIOLIB_SHAPING_1_0);
  state |= radio.setEncoding(RADIOLIB_ENCODING_WHITENING);
  uint8_t syncWord[] = {0xC1, 0x94, 0xC1};
  state |= radio.setSyncWord(syncWord, 3);
  state |= radio.setRxBandwidth(78.2);
  // state |= radio.setOutputPower(10.0);
  // state |= radio.setCurrentLimit(100.0);
  // state |= radio.setCRC(2); //2, 0x1D0F, 0x1021, true

  if (state != RADIOLIB_ERR_NONE) {
    Serial.print(F("Unable to set configuration, code "));
    Serial.println(state);
    while (true) { delay(10); }
  }

  // set the function that will be called
  // when packet transmission is finished
  radio.setPacketSentAction(setFlag);

  // start transmitting the first packet
  Serial.print(F("[SX1262] Sending first packet ... "));

  transmissionState = radio.startTransmit(payload, len);

}

void loop() {
  // check if the previous transmission finished
  if(transmittedFlag) {
    // reset flag
    transmittedFlag = false;

    if (transmissionState == RADIOLIB_ERR_NONE) {
      // packet was successfully sent
      Serial.println(F("transmission finished!"));

      // NOTE: when using interrupt-driven transmit method,
      //       it is not possible to automatically measure
      //       transmission data rate using getDataRate()

    } else {
      Serial.print(F("failed, code "));
      Serial.println(transmissionState);

    }

    // clean up after transmission is finished
    // this will ensure transmitter is disabled,
    // RF switch is powered down etc.
    radio.finishTransmit();

    // wait a second before transmitting again
    while(digitalRead(0));

    // send another one
    Serial.print(F("[SX1262] Sending another packet ... "));
    transmissionState = radio.startTransmit(payload, len);

  }
}

FSK receiver sketch

/* TS009 Protocol specification verification on FPort 224 */

#include <Arduino.h>
#include <RadioLib.h>

#include "config.h"
#include "lorawan.h"

// flag to indicate that a packet was received
volatile bool receivedFlag = false;

// this function is called when a complete packet
// is received by the module
ICACHE_RAM_ATTR void setFlag(void) {
  // we got a packet, set the flag
  receivedFlag = true;
}

void setup() {
  Serial.begin(115200);
  delay(5000);

  SPI.begin(9, 11, 10, 8);
  int state = radio.beginFSK();
  if(state == RADIOLIB_ERR_NONE) {
    Serial.println(F("success!"));
  } else {
    Serial.print(F("failed, code "));
    Serial.println(state);
    while(true);
  }

  state  = radio.setFrequency(868.8);
  state |= radio.setBitRate(50.0);
  state |= radio.setFrequencyDeviation(25.0);
  state |= radio.setPreambleLength(40);
  state |= radio.setDataShaping(RADIOLIB_SHAPING_1_0);
  state |= radio.setEncoding(RADIOLIB_ENCODING_WHITENING);
  uint8_t syncWord[] = {0xC1, 0x94, 0xC1};
  state |= radio.setSyncWord(syncWord, 3);
  state |= radio.setRxBandwidth(78.2);
  // state |= radio.setOutputPower(10.0);
  // state |= radio.setCurrentLimit(100.0);
  state |= radio.setCRC(0); //2, 0x1D0F, 0x1021, true

  if (state != RADIOLIB_ERR_NONE) {
    Serial.print(F("Unable to set configuration, code "));
    Serial.println(state);
    while (true) { delay(10); }
  }

  // set the function that will be called
  // when new packet is received
  radio.setPacketReceivedAction(setFlag);

  // start listening for LoRa packets
  Serial.print(F("[SX1262] Starting to listen ... "));
  state = radio.startReceive();
  if (state == RADIOLIB_ERR_NONE) {
    Serial.println(F("success!"));
  } else {
    Serial.print(F("failed, code "));
    Serial.println(state);
    while (true) { delay(10); }
  }

}

void loop() {
  // check if the flag is set
  if(receivedFlag) {
    // reset flag
    receivedFlag = false;

    uint8_t data[255] = { 0 };
    uint8_t len = radio.getPacketLength();
    int state = radio.readData(data, len);

    // you can also read received data as byte array
    /*
      byte byteArr[8];
      int numBytes = radio.getPacketLength();
      int state = radio.readData(byteArr, numBytes);
    */

    if (state == RADIOLIB_ERR_NONE || state == RADIOLIB_ERR_CRC_MISMATCH) {
      // packet was successfully received
      Serial.println(F("[SX1262] Received packet!"));

      // print data of the packet
      Serial.println(F("[SX1262] State:\t\t"));
      Serial.println(state);
      // Serial.println(str);
      RADIOLIB_DEBUG_PROTOCOL_HEXDUMP(data, len);

      // print RSSI (Received Signal Strength Indicator)
      Serial.print(F("[SX1262] RSSI:\t\t"));
      Serial.print(radio.getRSSI());
      Serial.println(F(" dBm"));

      // print SNR (Signal-to-Noise Ratio)
      Serial.print(F("[SX1262] SNR:\t\t"));
      Serial.print(radio.getSNR());
      Serial.println(F(" dB"));

      // print frequency error
      Serial.print(F("[SX1262] Frequency error:\t"));
      Serial.print(radio.getFrequencyError());
      Serial.println(F(" Hz"));

    } else if (state == RADIOLIB_ERR_CRC_MISMATCH) {
      // packet was received, but is malformed
      Serial.println(F("CRC error!"));

    } else {
      // some other error occurred
      Serial.print(F("failed, code "));
      Serial.println(state);

    }
  }
}

Additional info (please complete):

  • MCU: ESP32-S3 (Heltec WSL V3)
  • Radio: SX1262
  • Library version: 957a533
@StevenCellist StevenCellist added the more information needed Further investigation is needed, not necessarily by the author label Dec 15, 2024
@StevenCellist
Copy link
Collaborator Author

StevenCellist commented Dec 15, 2024

I hope to be able to investigate this a bit more, but in August, combined with Chirpstack 4.4.?, the gateway logs showed that FSK uplinks were being received (but dropped somewhere higher up) - in October, with v4.5.4, the uplinks weren't received and now on 4.6.2 still not received. This might indicate that CRC already fails on transmission (and started failing in between August's and October's tests).
If I have time, I will try and spin up a Chirpstack 4.4 install to see if it once again receives FSK uplinks - otherwise I'd be tempted to suggest there is actually a CRC problem on transmission. I now read that Chirpstack logs may show CRC failures, will check relatively soon if this shows up.

@StevenCellist
Copy link
Collaborator Author

I tried Chirpstack 4.4 again, but the FSK uplinks don't appear in the gateway logs now. As the changes to LoRaWAN & SX126x code since then are non-trivially convoluted, I cannot easily roll back to the code I had back then (I don't hoard copies like @HeadBoffin does - no, this is not a reason for me to buy a 14TB NAS).

Two more observations:

  1. leaving the full configuration on the default parameters (instead of configuring the frequency, preamble etc as in the original post), I do not get any output whatsoever (no messages received).
  2. leaving the full configuration on the default parameters but disabling CRC (setting to length 0), there is a reception, but still with ERR_CRC_MISMATCH, which looks pretty weird to me.

Either I'm doing stuff terribly wrong, or something very sketchy is happening. Note that I have no experience with FSK nor have an SDR in my possession, but this doesn't feel right.

@jgromes
Copy link
Owner

jgromes commented Dec 16, 2024

@StevenCellist I hope you didn't spend too much time on this, because it's really stupid mistake on my part - sorry!

In f23f798, configurable preamble detector length was added for SX126x, which changed the order of parameters for setPacketParamsFSK. Somehow I have failed to update all references to that method, and by sheer dumb luck, one of the two that I skipped was in startTransmit. The order of parameters no longer matched, causing such minor issues as FSK packet length always being 255 bytes and broken CRC config.

Pushed a commit that should fix this. I think this also manifests in other ways, such as in #1338 where FSK transmission is reported to be always timing out.

@StevenCellist
Copy link
Collaborator Author

LOL well I learned to speedrun Chirpstack by now so both sessions were just shy of a couple of hours. Not too bad to have it fixed. Let me give it a go :)

@StevenCellist
Copy link
Collaborator Author

In principle, the issue is fixed, until I alter the preamble length - then I get a CRC error again.

@jgromes
Copy link
Owner

jgromes commented Dec 16, 2024

That's odd, since changing preamble length should not (and does not) affect the CRC calculation, as seen below (top is 40 preamble bits, bottom is 160) - unless Chirpstack expects the CRC to include the preamble bits, which would be rather strange.

Screenshot_126

@StevenCellist
Copy link
Collaborator Author

StevenCellist commented Dec 16, 2024

I am simply going device-to-device, not device-to-gateway. Reception breaks for preamble length 32 bits and higher.
Edit: the preamble length for transmitter can be anything (e.g. 60), receiver works fine up until 31 bits, and breaks on the 32nd preamble bit.

@StevenCellist
Copy link
Collaborator Author

Now breaks my clog (gotta love Dutch sayings) - simply removing RADIOLIB_IRQ_CRC_ERR from the RADIOLIB_IRQ_RX_DEFAULT_FLAGS means I recover my package. So the CRC IRQ is incorrectly flagged and can be ignored in this case (as well as in #1268?)

@jgromes
Copy link
Owner

jgromes commented Dec 16, 2024

There's an interesting note in the SX126x datasheet:

Screenshot_127

Right now, since preamble is 40 bits, it will set the maximum preamble detector, which is 32 bits, and that's more than the 24 bits of the sync word. Lowering the detector length to 24 or 16 bits, reception starts working again.

@StevenCellist I can either expose the detector length to user, or we can add some logic to verify this condition is met. To be honest I'm more in favor of option 1 since the second would be kind of convoluted.

@StevenCellist
Copy link
Collaborator Author

Wow.. that looks like such a lazy shortcut by Semtech!
Would this also be affected by the address byte that comes after the sync word? 🤷‍♂️
The first solution is surely easier, the second one is safer. It may be simple enough to set detLength to 4 + min(3, min(syncWordBits, preLengthBits)/8) in both setPreambleLength() and setSyncWord()?

@StevenCellist
Copy link
Collaborator Author

Strictly speaking, you would yield better results with 32 bits detLength and 4-byte syncword, but IMO cutting down the 32-bit detLength to a 'best-effort' 24-bits detLength for the 3-byte syncWord is good enough.

@StevenCellist StevenCellist added bug Something isn't working and removed more information needed Further investigation is needed, not necessarily by the author labels Dec 16, 2024
@jgromes
Copy link
Owner

jgromes commented Dec 16, 2024

Wow.. that looks like such a lazy shortcut by Semtech!

Definitely smeels like it. I would need to see how the internals of the detector are implemented, but our preamble is for sure long enough to reliably detect 32 bits eve if the sync word is 24-bit.

Would this also be affected by the address byte that comes after the sync word?

Possibly since #1268 also reports CRC error. Seems like the CRC error flag is pretty ambiguous.

It may be simple enough to set detLength

What I don't like about that one is that there would be no easy way to allow user to force it. Though I guess we can always resolve that later e.g. with a dedicated method, and we can add comments to the docs explaining this automation.

@StevenCellist
Copy link
Collaborator Author

Well.. what use is there for the user in setting the preambleDetectorLen? Because the chip still uses both the detLen and preLen - the detLen is just an initial trigger to the packet engine. Maybe that engine is sometimes signaled falsely, but I assume it won't even trigger the preamble IRQ falsely because it must hit the preambleLen first anyway. So I think the impact is very very small.

@StevenCellist
Copy link
Collaborator Author

Seems like the CRC error flag is pretty ambiguous.

I agree, but with a bit more thought, it's not so bad of an idea. With the CRC disabled (#1353 and my own observation), it practically returns nothing, no error, just nothing. That is even worse than reporting some error. And there aren't many IRQs that signal an error - only Timeout (which makes no sense), Header (why though), and CRC (why though but OK). While it is very weird that this limitation is active in the first place, I see why they chose to resolve it in this way.

@jgromes
Copy link
Owner

jgromes commented Dec 16, 2024

Pushed a commit adding the automated configuration - with that the issue is resolved. I will keep digging further into #1268 since that very much feels related, but still present with this fix.

@jgromes jgromes closed this as completed Dec 16, 2024
@jgromes jgromes added the resolved Issue was resolved (e.g. bug fixed, or feature implemented) label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working resolved Issue was resolved (e.g. bug fixed, or feature implemented)
Projects
None yet
Development

No branches or pull requests

2 participants