Skip to content

Conversation

@Aerosnail
Copy link
Contributor

@Aerosnail Aerosnail commented Apr 26, 2025

What's new

The external CC1101 plugin sets up TIM17 to tick once every 2us, but does so without triggering an update with LL_TIM_GenerateEvent_UPDATE(). This means that the prescaler isn't actually applied until the counter register overflows. Until then, since the default prescaler value is 0, bits are reported as being 128x the duration that they actually are.

If the GDO0 pin of the external CC1101 toggles fast enough (at least once every 0xFFFF/64MHz = ~1ms minus ISR overhead), subghz_device_cc1101_ext_capture_ISR() will keep resetting the counter before it has a chance to overflow, preventing the correct prescaler from being applied for an arbitrary amount of time.

This PR uses LL_TIM_Init() to initialize the timer instead, which calls LL_TIM_GenerateEvent_UPDATE() to immediately apply the config.

Verification

To see the issue with the old code:

  1. Connect an external CC1101 module to the Flipper
  2. Open the Sub-GHz app, make sure that the external module is selected as the data source
  3. Select "Read RAW"
  4. Choose the FM476 modulation, select a frequency with just noise (or a signal with a high enough bitrate)
  5. Record around a second worth of samples
  6. Save the file. Notice how the first few hundred samples are significantly longer than the rest

You can repeat the same steps with the new code, and verify that the issue is no longer present.

Checklist (For Reviewer)

  • PR has description of feature/bug or link to Confluence/Jira task
  • Description contains actions to verify feature/bugfix
  • I've built this code, uploaded it to the device and verified feature/bugfix

@hedger hedger added the Sub-GHz Sub-GHz-related label Apr 26, 2025
@Skorpionm
Copy link
Member

what's the point of rewriting the code to use a structure? why not just add LL_TIM_GenerateEvent_UPDATE(TIM17)?

@Aerosnail
Copy link
Contributor Author

I found out about GenerateEvent by looking at how the internal CC1101 driver initialized the timer:

// Timer: base
LL_TIM_InitTypeDef TIM_InitStruct = {0};
TIM_InitStruct.Prescaler = 64 - 1;
TIM_InitStruct.CounterMode = LL_TIM_COUNTERMODE_UP;
TIM_InitStruct.Autoreload = 0x7FFFFFFE;
TIM_InitStruct.ClockDivision = LL_TIM_CLOCKDIVISION_DIV4; // Clock division for capture filter
LL_TIM_Init(TIM2, &TIM_InitStruct);

I thought it would be safer/clearer to use the same init sequence in both places. For instance, TIM17 doesn't actually have a "counter mode" bit (bits 4:6 of CR1 are marked as "Reserved, must be kept at reset value", they're only documented for TIM1 and TIM2). Probably not a big deal, but LL_TIM_Init() accounts for that too.

@hedger hedger added the Bug label Sep 25, 2025
@hedger hedger merged commit d17fd8e into flipperdevices:dev Sep 25, 2025
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Sub-GHz Sub-GHz-related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants