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

sx1262 radios were not being fully init if config.lora.use_preset is false #4167

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

geeksville
Copy link
Member

@geeksville geeksville commented Jun 23, 2024

I was testing a virgin board (just using default channels, flash etc...) and had set only lora.region.

I noticed warning messages about waiting for rx completion when those shouldn't be occurring. So I looked at the boot messages and saw:

INFO | ??:??:?? 3 SX126x init result -10

We weren't treating that as fatal (which we probably should). Later when the channel was setup many of the lora registers would then get properly set but unfortunately there is some init in the last half of SX126x::begin this is never run when this function bailed out.

The root cause seems to be:

  • config.lora.use_preset is false
  • therefore the second clause in RadioInterface::applyModemConfig is used
  • but config.lora.sf & coding_rate & spread_factor & bandwidth are zeros and nothing then provides sensible defaults.
  • Those zeros get passed in to sx1262::begin and it bails too soon

This fix is to provide sensible defaults. Later in the boot (once channel is selected) these values are reset correctly anyways.

Tested on a heltec tracker 1.1 but I suspect any sx1262 board should show it

I was testing a virgin board (just using default channels, flash etc...)
and had set only lora.region.

I noticed warning messages about waiting for rx completion when those
shouldn't be occurring.  So I looked at the boot messages and saw:

INFO  | ??:??:?? 3 SX126x init result -10

We weren't treating that as fatal (which we probably should).  Later when the
channel was setup many of the lora registers would then get properly set but
unfortunately there is some init in the last half of SX126x::begin this
is never run when this function bailed out.

The root cause seems to be:

* config.lora.use_preset is false
* therefore the second clause in RadioInterface::applyModemConfig is used
* but config.lora.sf & coding_rate & spread_factor & bandwidth are zeros and
nothing then provides sensible defaults.
* Those zeros get passed in to sx1262::begin and it bails too soon

This fix is to provide sensible defaults.  Later in the boot (once channel is selected)
these values are reset correctly anyways.

Tested on a heltec tracker 1.1 but I suspect any sx1262 board should show it
if (sf == 0)
sf = 11;
if (bw == 0)
bw = 250; // Just guess and use long fast settings
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also carry over the ternary logic for checking if we're on 2.4G lora?
image

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's super critical - because as long as these initial values are at least acceptable for the hw, later in the boot we'll be plugging in the 'real' settings based on the channel.

@geeksville
Copy link
Member Author

btw - I don't know if any release builds have been made that include this bug, but if they have then I think doing a release might be worth considering

@geeksville geeksville changed the title Fix an IMO serious problem: sx1262 radios were not being fully init sx1262 radios were not being fully init if config.lora.use_preset is false Jun 23, 2024
@geeksville
Copy link
Member Author

hmm @thebentern couldn't reproduce. And I went back to my board (been using it for other stuff) did factory reset and I couldn't either. From looking at the code I don't see any way for a factory reset to fail to set config.lora.use_preset true. But it definitely was on my board at somepoint.

I wonder if we have some sort of odd corruption bug and I was unlucky? oh well.

Changing this bug description to be non-urgent. but IMO still a good change because this !use_present clause was definitely wrong (and then much later in the boot leading to crummy rx behavior because DIO2 stuff had never been run.

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.

None yet

3 participants