Skip to content

Lora longer preamble#1954

Open
OverkillFPV wants to merge 4 commits intomeshcore-dev:devfrom
OverkillFPV:lora-longer-preamble
Open

Lora longer preamble#1954
OverkillFPV wants to merge 4 commits intomeshcore-dev:devfrom
OverkillFPV:lora-longer-preamble

Conversation

@OverkillFPV
Copy link

After testing i have found that for the narrow settings i am using (62.5khz, sf7) that there is a reliability issue of transmissions that did not exist on wide (250khz,sf11). I have come to the conclusion that the preamble is too short for the low spreading factor as it is 100ms shorter going from 131ms for wide, down to 32ms for narrow. the increase to a preamble length of 32 brings the preamble time up to 65ms (32ms extra). In my opinion this extra on air time is offset by the lower need to resend an entire packet. I have been running this longer preamble for a while with good results, and everyone who has tested the modification so far has either noticed a slight, or decent improvement in reliability.

This change changes the preamble length to 32 <sf9 and the standard 16 >sf8. It allows the sf to be changed through the cli and will adjust the preamble on the next tx

Copilot AI review requested due to automatic review settings March 7, 2026 02:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts LoRa preamble length dynamically based on the configured spreading factor (SF), aiming to improve reliability at low SF / narrow bandwidth settings by increasing preamble symbols.

Changes:

  • Add a getSpreadingFactor() hook to RadioLibWrapper and implement it across radio-specific wrappers.
  • Set preamble length to 32 symbols for SF ≤ 8, otherwise 16 symbols; update preamble automatically on the next TX when SF changes.
  • Extend CustomLR1110/wrapper to expose and use the current spreading factor for preamble decisions.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/helpers/radiolib/RadioLibWrappers.h Adds _preamble_sf state and a virtual getSpreadingFactor() API.
src/helpers/radiolib/RadioLibWrappers.cpp Applies preamble length policy at init and before TX when SF changes.
src/helpers/radiolib/CustomSX1276Wrapper.h Implements getSpreadingFactor() via radio’s spreadingFactor.
src/helpers/radiolib/CustomSX1268Wrapper.h Implements getSpreadingFactor() via radio’s spreadingFactor.
src/helpers/radiolib/CustomSX1262Wrapper.h Implements getSpreadingFactor() via radio’s spreadingFactor.
src/helpers/radiolib/CustomSTM32WLxWrapper.h Implements getSpreadingFactor() via radio’s spreadingFactor.
src/helpers/radiolib/CustomLLCC68Wrapper.h Implements getSpreadingFactor() via radio’s spreadingFactor.
src/helpers/radiolib/CustomLR1110Wrapper.h Updates preamble reset logic and implements getSpreadingFactor() via the radio.
src/helpers/radiolib/CustomLR1110.h Exposes getSpreadingFactor() accessor used by the wrapper.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

void RadioLibWrapper::begin() {
_radio->setPacketReceivedAction(setFlag); // this is also SentComplete interrupt
_preamble_sf = getSpreadingFactor();
_radio->setPreambleLength(_preamble_sf <= 8 ? 32 : 16); // longer preamble for lower SF improves reliability
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RadioLibWrapper::begin() now unconditionally overrides the preamble length to 16/32 based on SF. This can silently override board/variant-specific preamble lengths configured during radio initialization (e.g., variants/ebyte_eora_s3/target.cpp passes a preamble length of 8 to radio.begin). If that shorter preamble is intentional for some targets, consider preserving the configured default (or making the 16/32 policy conditional/configurable) rather than forcing it here.

Suggested change
_radio->setPreambleLength(_preamble_sf <= 8 ? 32 : 16); // longer preamble for lower SF improves reliability
// Only override preamble length if none has been explicitly configured.
// This avoids clobbering board/variant-specific settings provided via radio.begin().
int desiredPreamble = (_preamble_sf <= 8) ? 32 : 16; // longer preamble for lower SF improves reliability
int currentPreamble = _radio->getPreambleLength();
if (currentPreamble <= 0 || currentPreamble == desiredPreamble) {
_radio->setPreambleLength(desiredPreamble);
}

Copilot uses AI. Check for mistakes.
Comment on lines +149 to +150
_preamble_sf = sf;
_radio->setPreambleLength(sf <= 8 ? 32 : 16); // update preamble when SF has changed
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

startSendRaw() forces preamble length back to 16 whenever SF > 8. If some targets intentionally use a different default preamble (e.g., 8 symbols configured at radio.begin), switching from SF<=8 to SF>=9 will overwrite that default and keep it overwritten for subsequent packets. Consider tracking/restoring the configured default (or only changing preamble when entering/leaving the low-SF case) to avoid target-specific regressions.

Suggested change
_preamble_sf = sf;
_radio->setPreambleLength(sf <= 8 ? 32 : 16); // update preamble when SF has changed
bool wasLowSf = (_preamble_sf <= 8);
bool isLowSf = (sf <= 8);
// Only adjust preamble length when crossing between low-SF (<= 8) and high-SF (> 8) regimes.
if (wasLowSf != isLowSf) {
_radio->setPreambleLength(isLowSf ? 32 : 16);
}
_preamble_sf = sf;

Copilot uses AI. Check for mistakes.
@JDat
Copy link

JDat commented Mar 7, 2026

Is this compatible with existing MeshCore versions? By changing preamble will this code work with existing repeaters on network? Just asking, not dive deep into SX12xx datasheets yet.

@OverkillFPV
Copy link
Author

Yes, this affects the tx of the node it is on, and is completely interchangeable between nodes. Currently all my repeaters and companions are running 32 and are working perfectly with the rest of the mesh

Copy link
Contributor

@weebl2000 weebl2000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the preamble expression is repeated 4 times — a small helper would reduce duplication.

@@ -38,6 +39,7 @@ class RadioLibWrapper : public mesh::Radio {
}

virtual float getCurrentRSSI() =0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider extracting the repeated ternary to a helper to keep the 4 call sites DRY:

Suggested change
virtual float getCurrentRSSI() =0;
virtual uint8_t getSpreadingFactor() const { return LORA_SF; }
static uint16_t preambleLengthForSF(uint8_t sf) { return sf <= 8 ? 32 : 16; }

@weebl2000
Copy link
Contributor

@OverkillFPV what do you think about 24 instead of 32.

@jbrazio
Copy link
Contributor

jbrazio commented Mar 7, 2026

LGTM but why sf9 as the pivot point ?

@OverkillFPV
Copy link
Author

I tried 24, 32 and 64 and 32 had an improvement over 24 with only a slight improvement going up to 64. Sf9 was just a middle ground i picked. As it would require many many iterations to determine where the swing should be.

@OverkillFPV
Copy link
Author

Minor: the preamble expression is repeated 4 times — a small helper would reduce duplication.

Possibly I'll have a look at simplifying it

@OverkillFPV
Copy link
Author

I'm currently playing around with a companion of 64 and repeaters at 32. As I am seeing if it helps with the random dropped messages

@weebl2000
Copy link
Contributor

Interested to hear the results!

@OverkillFPV
Copy link
Author

OverkillFPV commented Mar 11, 2026

Early testing is looking to suggest the tag being a bit more reliable for tx and less rx biased. Only my first impressions though

Edit: hasn't actually had much of an affect when testing from known hard to reach spots. My early thoughts was most likely due to good conditions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants